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

[David Ponce]

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.


Not sure either what a solution could be.