Received: (at 74537) by debbugs.gnu.org; 10 Dec 2024 16:43:02 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Tue Dec 10 11:43:02 2024 Received: from localhost ([127.0.0.1]:59275 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>) id 1tL3KA-00059Y-24 for submit <at> debbugs.gnu.org; Tue, 10 Dec 2024 11:43:02 -0500 Received: from ledu-giraud.fr ([51.159.28.247]:13280) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <manuel@HIDDEN>) id 1tL3K7-000593-Ci for 74537 <at> debbugs.gnu.org; Tue, 10 Dec 2024 11:43:01 -0500 DKIM-Signature: v=1; a=ed25519-sha256; c=simple/simple; s=ed25519; bh=FywSDf31 s+p/GhMKTPMp/mmIR6WSrD5ZaWYYn0rvwTc=; h=date:references:in-reply-to: subject:cc:to:from; d=ledu-giraud.fr; b=iImjsNloBx8OVuI1q01fZN+Bu1K4c9 6tzV4R2dpxTb4I3pXt7JkoAnekPc4O5nfzdo6mmcbDaWLzhLJB2SwdBw== DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; s=rsa; bh=FywSDf31s+p/GhMK TPMp/mmIR6WSrD5ZaWYYn0rvwTc=; h=date:references:in-reply-to:subject: cc:to:from; d=ledu-giraud.fr; b=ZI8SdG+egaXa4t3eNCBAumIu+eb9S8yzSP+J/z HAP7Nid5R+G+E+KO2cdcIqSSJ3hpMqaWgOqN7owQrrAJqKptr5rPD8FBdrtPQwqof1jeVr k6snYhr+S5w8Hd3Qmp4G7yx7fj7nR6EIk0j0GECYe443mm5A+2jbhnpIL6MlL63U6cZVv5 FdDBr8/BfC6qXKEfs2MkPV1GvwD/qWeQs2FqJqYWXq/0Ui9Fv18hMk2Vn+tSJR8hTJMMdQ Zf7c83xAqZs46yHgl6TK/GGipSChkNnTnVF6u2N5SE73bhl/jI+dkqO/JeM0sLvGrviLp+ 4Iagxt6B6xtzgpZUipz9alRw== Received: from computer (<unknown> [10.1.1.1]) by ledu-giraud.fr (OpenSMTPD) with ESMTPSA id b2aa17dd (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 10 Dec 2024 17:42:57 +0100 (CET) From: Manuel Giraud <manuel@HIDDEN> To: Eli Zaretskii <eliz@HIDDEN> Subject: Re: bug#74537: [PATCH] An on-disk image modification does a cache miss In-Reply-To: <86v7w8g3rb.fsf@HIDDEN> (Eli Zaretskii's message of "Wed, 27 Nov 2024 16:17:28 +0200") References: <87ed2z6m7q.fsf@HIDDEN> <86plmiglyx.fsf@HIDDEN> <87r06x4z84.fsf@HIDDEN> <86v7w8g3rb.fsf@HIDDEN> Date: Tue, 10 Dec 2024 17:42:56 +0100 Message-ID: <87msh3h4kv.fsf@HIDDEN> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 74537 Cc: 74537 <at> debbugs.gnu.org X-BeenThere: debbugs-submit <at> debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: <debbugs-submit.debbugs.gnu.org> List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe> List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/> List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org> List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help> List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe> Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org> X-Spam-Score: -1.0 (-) Eli Zaretskii <eliz@HIDDEN> writes: >> From: Manuel Giraud <manuel@HIDDEN> >> Cc: 74537 <at> debbugs.gnu.org >> Date: Tue, 26 Nov 2024 19:38:35 +0100 >> >> Eli Zaretskii <eliz@HIDDEN> writes: >> >> > So this change could have weird confusing effects, whereby the fact >> > that the image was modified on disk becomes apparent only after some >> > display-related actions, but not after others. For example, scrolling >> > a window by a small amount will not notice the change on disk, whereas >> > significant changes, especially when the image goes off the window and >> > back into it, will show the change. Don't you see such issues if you >> > install the change? >> >> I've tried a bit with image-mode and also with something like: >> >> (insert (propertize "f" 'display '(image :file "/tmp/foo.jpg" :type jpeg :width 100))) >> >> and, yes, I can see the behavior you describe. But I also can't really >> see why it is a problem: the image has changed! At one point it should >> be reflected in Emacs, no? > > First, that isn't always true. Consider a file visited in some Emacs > buffer -- we don't revert it automatically when the visited file on > disk changes, do we? We have commands and minor modes to do that, but > they are optional. Why should images be different? > > But yes, it would be good to have a mode and command(s) to update the > images when/if they were modified. We just must be sure we update > _all_ of them, so if an image is displayed in several buffers/windows, > the update affects all of them. Otherwise we will have weird bugs > like the one above. And since the display engine has its own ideas > for when to perform a thorough redisplay and when flush the image > cache, these bugs will be hard to reproduce and debug. > > My point is that, since the display engine goes out of its way to try > not to do stuff that isn't necessary, it will not "re-lookup_image" if > it has no reason to believe what's on display is outdated. The image > cache is there to make redisplay of windows showing images faster. > What you want here is to disable some of these optimizations under > certain circumstances, but that must be done correctly, to avoid > incorrect/inaccurate/confusing display. > >> This I've tried with: >> >> (insert (propertize "foo" 'display '(space :width (0.1 . (image :file "/tmp/foo.jpg" :type jpeg))))) >> >> and also the re-alignment occurs at /some/ point. But likewise, I fail >> to see why this is a problem. > > It is a problem because what the Lisp program which used such a > property wanted (alignment on display) will not happen, since the > image on display and the one you generate by calling lookup_image in > the above scenario might have different metrics. What we must somehow > do in this case (assuming the user indeed wants the images to > immediately reflect their disk file changes) is to update the image on > display as well. One way of doing that is to perform a complete > redisplay of each window which shows the image. I'm not sure, but it > could be the only way, which means such updates will be somewhat > expensive if one has many windows with the image. (One trivial case > where an image is displayed many times is the tool bar, when you have > many frames. Another case could be the mode line.) > >> After all, maybe I'm a bit partial to image-mode with this patch. I >> think my idea was to eventually get rid of the systematic `image-flush' >> call in "image-mode.el" to make it beneficiate from the image cache more >> and still be able to display the correct image. > > Sorry, I don't understand: the patch you posted effectively flushes > the cache more often, so how is it more beneficial to users of the > cache? It flushes the cache iff the on-disk image has changed (which is, of course, rare)... I think this all boils down to the fact that I don't really understand the purpose of the `image-flush' call in image-mode.el -- Manuel Giraud
bug-gnu-emacs@HIDDEN:bug#74537; Package emacs.
Full text available.Received: (at 74537) by debbugs.gnu.org; 27 Nov 2024 14:18:18 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Wed Nov 27 09:18:18 2024 Received: from localhost ([127.0.0.1]:32854 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>) id 1tGIrx-00088F-Ge for submit <at> debbugs.gnu.org; Wed, 27 Nov 2024 09:18:17 -0500 Received: from eggs.gnu.org ([209.51.188.92]:55802) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <eliz@HIDDEN>) id 1tGIrv-000881-2n for 74537 <at> debbugs.gnu.org; Wed, 27 Nov 2024 09:18:15 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from <eliz@HIDDEN>) id 1tGIrn-0006ND-5a; Wed, 27 Nov 2024 09:18:08 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=c1jXUeo+4onEvinF9YrP6FnFMtxj36rAk2YJJBmcIqI=; b=OWxJzyU31Um7 Us/RGEbo4c7DkuambwnL7YWw43vKxVIdeja7uXPwJCTPYmu/olNGgxJRUk+GVBaOUbRT96GG0Izpi PjP5VCxR85oVftxxBoyaNdXhgxAYAY+FLKRn2oFnjUEkskhF/T15Ux2k6Zy7J9f5YX1cFytNq8qXg oOYABt6LTGUCvjoz+weiI/2GpHQUagE24mABKVmN6Acghvm5qnNlkCM0r67wtpqhPv0e+HYEWM4LB WHx+DNhN35eDU2zLWvFQ0mfeRKixsPuD12R4VaIqUSiHVe7iaCtF6KOmc5MqNgF8fNIF4mA6HMt7g b3xCOxTvzN3uZccoO2FDSA==; Date: Wed, 27 Nov 2024 16:17:28 +0200 Message-Id: <86v7w8g3rb.fsf@HIDDEN> From: Eli Zaretskii <eliz@HIDDEN> To: Manuel Giraud <manuel@HIDDEN> In-Reply-To: <87r06x4z84.fsf@HIDDEN> (message from Manuel Giraud on Tue, 26 Nov 2024 19:38:35 +0100) Subject: Re: bug#74537: [PATCH] An on-disk image modification does a cache miss References: <87ed2z6m7q.fsf@HIDDEN> <86plmiglyx.fsf@HIDDEN> <87r06x4z84.fsf@HIDDEN> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 74537 Cc: 74537 <at> debbugs.gnu.org X-BeenThere: debbugs-submit <at> debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: <debbugs-submit.debbugs.gnu.org> List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe> List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/> List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org> List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help> List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe> Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org> X-Spam-Score: -3.3 (---) > From: Manuel Giraud <manuel@HIDDEN> > Cc: 74537 <at> debbugs.gnu.org > Date: Tue, 26 Nov 2024 19:38:35 +0100 > > Eli Zaretskii <eliz@HIDDEN> writes: > > > So this change could have weird confusing effects, whereby the fact > > that the image was modified on disk becomes apparent only after some > > display-related actions, but not after others. For example, scrolling > > a window by a small amount will not notice the change on disk, whereas > > significant changes, especially when the image goes off the window and > > back into it, will show the change. Don't you see such issues if you > > install the change? > > I've tried a bit with image-mode and also with something like: > > (insert (propertize "f" 'display '(image :file "/tmp/foo.jpg" :type jpeg :width 100))) > > and, yes, I can see the behavior you describe. But I also can't really > see why it is a problem: the image has changed! At one point it should > be reflected in Emacs, no? First, that isn't always true. Consider a file visited in some Emacs buffer -- we don't revert it automatically when the visited file on disk changes, do we? We have commands and minor modes to do that, but they are optional. Why should images be different? But yes, it would be good to have a mode and command(s) to update the images when/if they were modified. We just must be sure we update _all_ of them, so if an image is displayed in several buffers/windows, the update affects all of them. Otherwise we will have weird bugs like the one above. And since the display engine has its own ideas for when to perform a thorough redisplay and when flush the image cache, these bugs will be hard to reproduce and debug. My point is that, since the display engine goes out of its way to try not to do stuff that isn't necessary, it will not "re-lookup_image" if it has no reason to believe what's on display is outdated. The image cache is there to make redisplay of windows showing images faster. What you want here is to disable some of these optimizations under certain circumstances, but that must be done correctly, to avoid incorrect/inaccurate/confusing display. > This I've tried with: > > (insert (propertize "foo" 'display '(space :width (0.1 . (image :file "/tmp/foo.jpg" :type jpeg))))) > > and also the re-alignment occurs at /some/ point. But likewise, I fail > to see why this is a problem. It is a problem because what the Lisp program which used such a property wanted (alignment on display) will not happen, since the image on display and the one you generate by calling lookup_image in the above scenario might have different metrics. What we must somehow do in this case (assuming the user indeed wants the images to immediately reflect their disk file changes) is to update the image on display as well. One way of doing that is to perform a complete redisplay of each window which shows the image. I'm not sure, but it could be the only way, which means such updates will be somewhat expensive if one has many windows with the image. (One trivial case where an image is displayed many times is the tool bar, when you have many frames. Another case could be the mode line.) > After all, maybe I'm a bit partial to image-mode with this patch. I > think my idea was to eventually get rid of the systematic `image-flush' > call in "image-mode.el" to make it beneficiate from the image cache more > and still be able to display the correct image. Sorry, I don't understand: the patch you posted effectively flushes the cache more often, so how is it more beneficial to users of the cache? > It seems I also lack some imagination about how users could use images > in Emacs. For instance, with your example about alignment, I imagined > that it could be something that a user would do in its modeline (for > example) but with images that would never ever change. It will change if we completely redisplay the window (after removing the image from the cache, or otherwise rejecting the cached image).
bug-gnu-emacs@HIDDEN:bug#74537; Package emacs.
Full text available.Received: (at 74537) by debbugs.gnu.org; 26 Nov 2024 18:38:44 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Tue Nov 26 13:38:43 2024 Received: from localhost ([127.0.0.1]:52393 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>) id 1tG0SR-0001CV-DF for submit <at> debbugs.gnu.org; Tue, 26 Nov 2024 13:38:43 -0500 Received: from ledu-giraud.fr ([51.159.28.247]:13425) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <manuel@HIDDEN>) id 1tG0SO-0001CK-Cl for 74537 <at> debbugs.gnu.org; Tue, 26 Nov 2024 13:38:41 -0500 DKIM-Signature: v=1; a=ed25519-sha256; c=simple/simple; s=ed25519; bh=ZB7tCdgT 3UF0SUAvZYYo2RSWPIHbxJ5RDR0zXUSxyuw=; h=date:references:in-reply-to: subject:cc:to:from; d=ledu-giraud.fr; b=MJIumaSWk0wFcc6SsOuYigNsTmgl+X 1kGavzbhO3mFId1Zisg9jROtdB/fgBeHxrMuYmu4K9JBYeZJx3sFgYCA== DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; s=rsa; bh=ZB7tCdgT3UF0SUAv ZYYo2RSWPIHbxJ5RDR0zXUSxyuw=; h=date:references:in-reply-to:subject: cc:to:from; d=ledu-giraud.fr; b=yi7UTX7htGYk2lbZBTJJ35JfUdKO6qYg+njUj5 hTL+TTOCKXhJe4fHOEo6b7SSwPCp0TxyqaytV4M4JUwDLEo1+ToPhnhziDufcEgOm5uBev dGCrqvAhcbfk0hKRmz39tgzgxwRDIDcnd7P9TKtLhpR/AcBz228NwmRVm5PADWrq5KCjd6 xvDeZzfWIPDM1clNN1RYWOosQZr/eviAKAj5MtpdNwjGD8zFixJDzJFON11U5hHJ64X4rQ vnbKjo6UL+37jwNUzWmCgNas1WInlX1u1MYxUen9Turo6uz0E7IFmNtE8dYqCEleZma0GC 3+sj/9tjdcDzbzBYZaOgxuaw== Received: from computer (<unknown> [10.1.1.1]) by ledu-giraud.fr (OpenSMTPD) with ESMTPSA id 0e65ca65 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 26 Nov 2024 19:38:36 +0100 (CET) From: Manuel Giraud <manuel@HIDDEN> To: Eli Zaretskii <eliz@HIDDEN> Subject: Re: bug#74537: [PATCH] An on-disk image modification does a cache miss In-Reply-To: <86plmiglyx.fsf@HIDDEN> (Eli Zaretskii's message of "Tue, 26 Nov 2024 15:31:50 +0200") References: <87ed2z6m7q.fsf@HIDDEN> <86plmiglyx.fsf@HIDDEN> Date: Tue, 26 Nov 2024 19:38:35 +0100 Message-ID: <87r06x4z84.fsf@HIDDEN> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 74537 Cc: 74537 <at> debbugs.gnu.org X-BeenThere: debbugs-submit <at> debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: <debbugs-submit.debbugs.gnu.org> List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe> List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/> List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org> List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help> List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe> Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org> X-Spam-Score: -1.0 (-) Eli Zaretskii <eliz@HIDDEN> writes: >> Date: Mon, 25 Nov 2024 22:24:25 +0100 >> From: Manuel Giraud via "Bug reports for GNU Emacs, >> the Swiss army knife of text editors" <bug-gnu-emacs@HIDDEN> >> >> With this patch, when an image file is modified on disk, the associated >> cache image is invalidated and re-read from disk. > > Thanks, but I'm not sure it's a good idea to do that automatically > whenever lookup_image is called. > > First, lookup_image is called only when we need to create or access an > image with only its Lisp descriptor in hand; once the image has been > displayed, that should happen relatively rarely. For example, the > glyph matrices used to manipulate the display record only the image's > cached ID, and the code accesses the cached image without calling > lookup_image. Ok. > So this change could have weird confusing effects, whereby the fact > that the image was modified on disk becomes apparent only after some > display-related actions, but not after others. For example, scrolling > a window by a small amount will not notice the change on disk, whereas > significant changes, especially when the image goes off the window and > back into it, will show the change. Don't you see such issues if you > install the change? I've tried a bit with image-mode and also with something like: (insert (propertize "f" 'display '(image :file "/tmp/foo.jpg" :type jpeg :width 100))) and, yes, I can see the behavior you describe. But I also can't really see why it is a problem: the image has changed! At one point it should be reflected in Emacs, no? > Second, if the image is already on display, and the file changes on > disk, some layout calculations could see different dimensions than > what is actually on display, which will cause subtle bugs. For > example, what if there's a 'display' property such as this: > > '(space :width (0.5 (image PROPS))) > > and an image with those same PROPS is already shown on display? The > code which implements the above calls lookup_image, which will > simulate a cache miss if the file has changed, and will then load the > new file, which could return dimensions different from the image > already on display. So the width of the space glyph produced by the > above will be different from that of the displayed image, and we could > have misalignment until the next redisplay which redraws the image. This I've tried with: (insert (propertize "foo" 'display '(space :width (0.1 . (image :file "/tmp/foo.jpg" :type jpeg))))) and also the re-alignment occurs at /some/ point. But likewise, I fail to see why this is a problem. After all, maybe I'm a bit partial to image-mode with this patch. I think my idea was to eventually get rid of the systematic `image-flush' call in "image-mode.el" to make it beneficiate from the image cache more and still be able to display the correct image. It seems I also lack some imagination about how users could use images in Emacs. For instance, with your example about alignment, I imagined that it could be something that a user would do in its modeline (for example) but with images that would never ever change. -- Manuel Giraud
bug-gnu-emacs@HIDDEN:bug#74537; Package emacs.
Full text available.Received: (at 74537) by debbugs.gnu.org; 26 Nov 2024 13:32:02 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Tue Nov 26 08:32:02 2024 Received: from localhost ([127.0.0.1]:48173 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>) id 1tFvfd-0002QO-WD for submit <at> debbugs.gnu.org; Tue, 26 Nov 2024 08:32:02 -0500 Received: from eggs.gnu.org ([209.51.188.92]:35644) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <eliz@HIDDEN>) id 1tFvfc-0002Q4-5z for 74537 <at> debbugs.gnu.org; Tue, 26 Nov 2024 08:32:00 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from <eliz@HIDDEN>) id 1tFvfW-0007Tz-C0; Tue, 26 Nov 2024 08:31:54 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=fvogOwNDR2JJ40f26CAhsO7/hL39RZ0SnbYBOukyxP8=; b=IGAsH5YUsH3Y zGgcuQ1Zb9jVXrhFzb3ps/YTVxbTbInOL7sl1CyCOcFk2FQw1NhlkLxH35wbOzlwHBEsSOClM0t09 6LNSlf7T7iV71Ub+DjObrdF3FRW2Kb+TU2YSFj7CYVEW67vxM5ZWfaR2iSNeO+3xc95LB8xiaSZH/ T0OgR38JqalFBknj6haIDgvntiP7ixjySKY79wmZVXnqZQrqGRyJZOsjuYOJ7+WPrttAnEQz2eL5H oO6Ysu2y3q0BOr9vOtv1/Fa9wQshhrk3AS1AqB2DrKyVbz7kekXIiWaaYpmodDn9sJcU240pBfE7l mXJQJvzPTw9rzvmwnCE+7g==; Date: Tue, 26 Nov 2024 15:31:50 +0200 Message-Id: <86plmiglyx.fsf@HIDDEN> From: Eli Zaretskii <eliz@HIDDEN> To: Manuel Giraud <manuel@HIDDEN> In-Reply-To: <87ed2z6m7q.fsf@HIDDEN> (bug-gnu-emacs@HIDDEN) Subject: Re: bug#74537: [PATCH] An on-disk image modification does a cache miss References: <87ed2z6m7q.fsf@HIDDEN> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 74537 Cc: 74537 <at> debbugs.gnu.org X-BeenThere: debbugs-submit <at> debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: <debbugs-submit.debbugs.gnu.org> List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe> List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/> List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org> List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help> List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe> Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org> X-Spam-Score: -3.3 (---) > Date: Mon, 25 Nov 2024 22:24:25 +0100 > From: Manuel Giraud via "Bug reports for GNU Emacs, > the Swiss army knife of text editors" <bug-gnu-emacs@HIDDEN> > > With this patch, when an image file is modified on disk, the associated > cache image is invalidated and re-read from disk. Thanks, but I'm not sure it's a good idea to do that automatically whenever lookup_image is called. First, lookup_image is called only when we need to create or access an image with only its Lisp descriptor in hand; once the image has been displayed, that should happen relatively rarely. For example, the glyph matrices used to manipulate the display record only the image's cached ID, and the code accesses the cached image without calling lookup_image. So this change could have weird confusing effects, whereby the fact that the image was modified on disk becomes apparent only after some display-related actions, but not after others. For example, scrolling a window by a small amount will not notice the change on disk, whereas significant changes, especially when the image goes off the window and back into it, will show the change. Don't you see such issues if you install the change? Second, if the image is already on display, and the file changes on disk, some layout calculations could see different dimensions than what is actually on display, which will cause subtle bugs. For example, what if there's a 'display' property such as this: '(space :width (0.5 (image PROPS))) and an image with those same PROPS is already shown on display? The code which implements the above calls lookup_image, which will simulate a cache miss if the file has changed, and will then load the new file, which could return dimensions different from the image already on display. So the width of the space glyph produced by the above will be different from that of the displayed image, and we could have misalignment until the next redisplay which redraws the image. So I think this should probably be controlled by a variable visible from Lisp, by default off, and we might also have a function to invalidate all the cached images whose files have changed (which will then need to trigger a thorough redisplay, I think).
bug-gnu-emacs@HIDDEN:bug#74537; Package emacs.
Full text available.
Received: (at submit) by debbugs.gnu.org; 25 Nov 2024 21:27:04 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Mon Nov 25 16:27:04 2024
Received: from localhost ([127.0.0.1]:41841 helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1tFgbo-0005h2-34
for submit <at> debbugs.gnu.org; Mon, 25 Nov 2024 16:27:04 -0500
Received: from lists.gnu.org ([209.51.188.17]:54002)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <manuel@HIDDEN>) id 1tFgbj-0005gY-5M
for submit <at> debbugs.gnu.org; Mon, 25 Nov 2024 16:27:02 -0500
Received: from eggs.gnu.org ([2001:470:142:3::10])
by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.90_1) (envelope-from <manuel@HIDDEN>)
id 1tFgZW-00073t-O7
for bug-gnu-emacs@HIDDEN; Mon, 25 Nov 2024 16:24:44 -0500
Received: from ledu-giraud.fr ([51.159.28.247])
by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.90_1) (envelope-from <manuel@HIDDEN>)
id 1tFgZL-0001NY-54
for bug-gnu-emacs@HIDDEN; Mon, 25 Nov 2024 16:24:41 -0500
DKIM-Signature: v=1; a=ed25519-sha256; c=simple/simple; s=ed25519; bh=OT26E7+k
jsAInj8EuqPc0C9msPgEmq5/OtzKMIm25go=; h=date:subject:to:from;
d=ledu-giraud.fr;
b=uLk/jDyExFZhE4r1G8xygpsSUD+I4BrUhIujfAo1X8lq4NvD4v
BmNGp3hJhhX6Z+paaB4FivhC01xVMGxYz+Bg==
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; s=rsa; bh=OT26E7+kjsAInj8E
uqPc0C9msPgEmq5/OtzKMIm25go=; h=date:subject:to:from;
d=ledu-giraud.fr; b=Q3NSxiCoHHN9JutbE3Vmrzm2rg3R6Ua0gngS++hiUQWlqCY7Tt
uJl64OhWW7WKzYAoU97rb8RcialjwUqLd/mFQ0GC0qD2THcuc2BIOHExWrIYzXhufe/bDk
d23gPPNtDFJk6B8AILu0eYI+mIhrDHcp8peGF81x7+EfShi+IN8VR4bycTFrwMbmE8RBJP
dh284TPTujw+LMmuz5X/as1TZG26VbelmDlK8NdlJdN/SdbHYpEPZaVoVcDHKLcepD+wNL
wOgoljhaC5q+ICwTrBmsULqH44g+PEHsi1uqThy9frw1zzXO74Pg67pRDXwy/9LU7eLgnC
+w7gvRQauCMg==
Received: from computer (<unknown> [10.1.1.1])
by ledu-giraud.fr (OpenSMTPD) with ESMTPSA id 05b9ae39
(TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for <bug-gnu-emacs@HIDDEN>;
Mon, 25 Nov 2024 22:24:26 +0100 (CET)
From: Manuel Giraud <manuel@HIDDEN>
To: bug-gnu-emacs@HIDDEN
Subject: [PATCH] An on-disk image modification does a cache miss
X-Debbugs-Cc:
Date: Mon, 25 Nov 2024 22:24:25 +0100
Message-ID: <87ed2z6m7q.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Received-SPF: pass client-ip=51.159.28.247; envelope-from=manuel@HIDDEN;
helo=ledu-giraud.fr
X-Spam_score_int: -20
X-Spam_score: -2.1
X-Spam_bar: --
X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1,
RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001,
SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-Spam-Score: -1.4 (-)
X-Debbugs-Envelope-To: submit
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -2.4 (--)
--=-=-=
Content-Type: text/plain
Tags: patch
Hi,
With this patch, when an image file is modified on disk, the associated
cache image is invalidated and re-read from disk.
In GNU Emacs 31.0.50 (build 4, x86_64-unknown-openbsd7.6, X toolkit) of
2024-11-23 built on computer
Repository revision: 5072b6110b69a8ba0d42772b26533dcdc39ffdfc
Repository branch: mgi/jpeg
Windowing system distributor 'The X.Org Foundation', version 11.0.12101014
System Description: OpenBSD computer 7.6 GENERIC.MP#437 amd64
Configured using:
'configure CC=egcc CPPFLAGS=-I/usr/local/include
LDFLAGS=-L/usr/local/lib MAKEINFO=gmakeinfo --prefix=/home/manuel/emacs
--bindir=/home/manuel/bin --with-x-toolkit=lucid
--with-toolkit-scroll-bars=no --without-cairo
--without-compress-install'
--=-=-=
Content-Type: text/patch
Content-Disposition: attachment;
filename=0001-An-on-disk-image-modification-does-a-cache-miss.patch
From 0b0e0cbc947e357c8053c2837622094b00a422d5 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@HIDDEN>
Date: Mon, 25 Nov 2024 17:04:03 +0100
Subject: [PATCH] An on-disk image modification does a cache miss
* src/dispextern.h: Add modification time to an image.
* src/image.c (make_image): Get the modification time from the
image file.
(lookup_image): Make a modified image on disk results in a cache
miss.
* lisp/startup.el (fancy-splash-head): Remove a non full-path
image file spec (XXX is there more?).
---
lisp/startup.el | 2 +-
src/dispextern.h | 4 ++++
src/image.c | 26 ++++++++++++++++++++++++++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/lisp/startup.el b/lisp/startup.el
index 3436409a35e..42f6e70f05d 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -1957,7 +1957,7 @@ fancy-splash-image-file
(defun fancy-splash-head ()
"Insert the head part of the splash screen into the current buffer."
(let* ((image-file (fancy-splash-image-file))
- (img (create-image image-file))
+ (img (create-image (image-search-load-path image-file)))
(image-width (and img (car (image-size img))))
(window-width (window-width)))
(when img
diff --git a/src/dispextern.h b/src/dispextern.h
index 9df6eaf623a..0f91568db99 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3170,6 +3170,10 @@ reset_mouse_highlight (Mouse_HLInfo *hlinfo)
in prepare_image_for_display. */
struct timespec timestamp;
+ /* The modification time of the file of this image. Set in
+ make_image. */
+ struct timespec mtime;
+
/* Pixmaps of the image. */
Emacs_Pixmap pixmap, mask;
diff --git a/src/image.c b/src/image.c
index db7f6acd171..c5bddaf5985 100644
--- a/src/image.c
+++ b/src/image.c
@@ -37,6 +37,7 @@ Copyright (C) 1989-2024 Free Software Foundation, Inc.
#include <stdint.h>
#include <c-ctype.h>
#include <flexmember.h>
+#include <stat-time.h>
#include "lisp.h"
#include "frame.h"
@@ -1744,6 +1745,7 @@ make_image (Lisp_Object spec, EMACS_UINT hash)
{
struct image *img = xzalloc (sizeof *img);
Lisp_Object file = image_spec_value (spec, QCfile, NULL);
+ struct stat st;
eassert (valid_image_p (spec));
img->dependencies = NILP (file) ? Qnil : list1 (file);
@@ -1754,6 +1756,9 @@ make_image (Lisp_Object spec, EMACS_UINT hash)
img->ascent = DEFAULT_IMAGE_ASCENT;
img->hash = hash;
img->corners[BOT_CORNER] = -1; /* Full image */
+ /* Retrieve file mtime. */
+ if (! NILP (file) && (emacs_fstatat (AT_FDCWD, SSDATA (file), &st, 0) == 0))
+ img->mtime = get_stat_mtime (&st);
return img;
}
@@ -3513,6 +3518,27 @@ lookup_image (struct frame *f, Lisp_Object spec, int face_id)
img = NULL;
}
+ /* If the on-disk image file has changed, this is also a cache
+ miss. */
+ if (img)
+ {
+ Lisp_Object file;
+ bool found;
+
+ file = image_spec_value (spec, QCfile, &found);
+ if (found && STRINGP (file))
+ {
+ struct stat st;
+
+ if (emacs_fstatat (AT_FDCWD, SSDATA (file), &st, 0) != 0
+ || timespec_cmp (get_stat_mtime (&st), img->mtime) != 0)
+ {
+ free_image (f, img);
+ img = NULL;
+ }
+ }
+ }
+
/* If not found, create a new image and cache it. */
if (img == NULL)
{
--
2.47.0
--=-=-=
Content-Type: text/plain
--
Manuel Giraud
--=-=-=--
Manuel Giraud <manuel@HIDDEN>:bug-gnu-emacs@HIDDEN.
Full text available.bug-gnu-emacs@HIDDEN:bug#74537; Package emacs.
Full text available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997 nCipher Corporation Ltd,
1994-97 Ian Jackson.