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

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

On 2024-12-07 17:21, Eli Zaretskii wrote:
>> Date: Sat, 7 Dec 2024 15:49:47 +0000
>> From: Alan Third <alan@idiocy.org>
>> Cc: David Ponce <da_vid@orange.fr>, 74725@debbugs.gnu.org
>>
>> 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.
> 
> I think recording the scale in the cache, and rejecting the cached
> image if the scale doesn't match, will cause the least surprise.  But
> I'm open to other opinions and suggestions.
> 

Seems like a good idea.