bug#74725: 31.0.50; image-scaling-factor is ignored by create-image

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 7 Dec 2024 17:27:13 +0100
>> From: David Ponce <da_vid@orange.fr>
>> 
>> On 2024-12-07 16:49, Alan Third wrote:
>> > On Sat, Dec 07, 2024 at 04:49:41PM +0200, Eli Zaretskii wrote:
>> >>> Date: Sat, 7 Dec 2024 13:13:58 +0100
>> >>> From:  David Ponce via "Bug reports for GNU Emacs,
>> >>>   the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> >>>
>> >>> While working with images, I found what seems an issue to me with
>> >>> `create-image' which unconditionally set the :scale image property to
>> >>> 'default' when not specified, ignoring the value of the option
>> >>> `image-scaling-factor'.
>> >>>
>> >>> Here is an illustration:
>> >>>
>> >>> (let ((image-scaling-factor 1.0))
>> >>>      (image-size
>> >>>       (find-image '((:file "icons/hicolor/scalable/apps/emacs.svg")))
>> >>>       t))
>> >>> => (63 . 63)
>> >>>
>> >>> (let ((image-scaling-factor 2.0))
>> >>>      (image-size
>> >>>       (find-image '((:file "icons/hicolor/scalable/apps/emacs.svg")))
>> >>>       t))
>> >>> => (63 . 63)
>> >>>
>> >>> (image-size
>> >>>     (find-image '((:file "icons/hicolor/scalable/apps/emacs.svg" :scale 
>> >>> 1)))
>> >>>     t)
>> >>> => (48 . 48)
>> >>>
>> >>> (image-size
>> >>>     (find-image '((:file "icons/hicolor/scalable/apps/emacs.svg" :scale 
>> >>> 2)))
>> >>>     t)
>> >>> => (96 . 96)
>> >>>
>> >>> You can replace `image-size' with `insert-image' and observe the same.
>> >>>
>> >>> Here is a simple patch which fix the issue for me:
>> >>>
>> >>> diff --git a/lisp/image.el b/lisp/image.el
>> >>> index ce97eeb3ca1..2c1e865c336 100644
>> >>> --- a/lisp/image.el
>> >>> +++ b/lisp/image.el
>> >>> @@ -536,7 +536,9 @@ create-image
>> >>>                             file-or-data)
>> >>>                       (and (not (plist-get props :scale))
>> >>>                            ;; Add default scaling.
>> >>> -                        (list :scale 'default))
>> >>> +                        (list :scale (if (numberp image-scaling-factor)
>> >>> +                                         image-scaling-factor
>> >>> +                                       'default)))
>> >>>                     props)))
>> >>>          ;; Add default smoothing.
>> >>>          (unless (plist-member props :transform-smoothing)
>> >>
>> >> AFAIU, this is supposed to be taken care of in image.c.
>> >>
>> >> Alan, any ideas why this doesn't seem to work?
>> > 
>> > It's because the image spec doesn't change so the image is pulled from
>> > the cache each time.
>> > 
>> > Flushing the image between calls to image-size fixes it:
>> > 
>> >      (image-flush (find-image '((:file 
>> > "icons/hicolor/scalable/apps/emacs.svg"))))
>> > 
>> > I'm not sure what the solution is here. My feeling is that
>> > image-scaling-factor isn't intended as something you set for each
>> > image as you load it, it's a set-and-forget setting, so perhaps we
>> > just need to document that in order for it to take effect the image
>> > cache needs to be flushed.
>> > 
>> > Alternatively we make the image cache aware of it.
>> > 
>> > Perhaps we can flush the cache automatically when it changes? That
>> > might give unexpected results too.
>> 
>> This is exactly what I also observe.
>> It seems due to this change:
>> 
>> author       Po Lu <luangruo@yahoo.com>      2024-06-03 16:34:51 +0800
>> committer    Po Lu <luangruo@yahoo.com>      2024-06-03 16:36:29 +0800
>> commit       56376585134d627f96c71b7b063ec51548d3ad3f (patch)
>> 
>> Which replaced
>> 
>> -                        (list :scale
>> -                              (image-compute-scaling-factor
>> -                               image-scaling-factor)))
>> 
>> By this
>> 
>> +                        (list :scale 'default))
>> 
>> In create-image.
>> 
>> With the side effect that the image spec don't change when the scaling
>> factor changes, so the same cached image in always used.
>
> Po Lu, what were the reasons for that particular part of the commit?

The scale applied by image-scaling-factor is liable to differ by
display, and computing the default scale in Lisp would result in images
being displayed with an incorrect scale in the presence of multiple
displays.

Image caches must be flushed when image-scaling-factor is modified,
unless it is set to `auto' and a display's scale changes, because
image.c has no means of detecting variable modifications and so only the
latter event can be automatically detected.