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

[Eli Zaretskii]

> 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?