bug#74537: [PATCH] An on-disk image modification does a cache miss

[Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: 74537@debbugs.gnu.org
>> Date: Tue, 26 Nov 2024 19:38:35 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> 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