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

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]

Hello,

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)


Thanks!


In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
  3.24.43, cairo version 1.18.0) of 2024-12-02
Repository revision: 8cd4ab7abde87ac04e05442196b4646ab46df9a7
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 40 (KDE Plasma)

Configured using:
  'configure --with-native-compilation=no
  PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LC_TIME: fr_FR.utf8
   value of $LANG: fr_FR.UTF-8
   locale-coding-system: utf-8-unix

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]

Hello,

After more investigation I found the issue is not in image.el, but is due
to image caching.

If I flush the image cache when I change the value of `image-scaling-factor'
the result is as expected:

(let* ((image-scaling-factor 2.0)
        (image (find-image '((:file "icons/hicolor/scalable/apps/emacs.svg")))))
   (image-flush image t)
   (image-size
    (find-image '((:file "icons/hicolor/scalable/apps/emacs.svg")))
    t))
(96 . 96)

;; Image is used from cache, size is not updated.
(let* ((image-scaling-factor 3.0)
        (image (find-image '((:file "icons/hicolor/scalable/apps/emacs.svg")))))
   ;; (image-flush image t)
   (image-size
    (find-image '((:file "icons/hicolor/scalable/apps/emacs.svg")))
    t))
(96 . 96)

;; Image not in cache, size is updated.
(let* ((image-scaling-factor 3.0)
        (image (find-image '((:file "icons/hicolor/scalable/apps/emacs.svg")))))
   (image-flush image t)
   (image-size
    (find-image '((:file "icons/hicolor/scalable/apps/emacs.svg")))
    t))
(144 . 144)

Thanks!

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

[Eli Zaretskii]

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

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

[Alan Third]

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.
-- 
Alan Third

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

[Eli Zaretskii]

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

Thanks.

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]

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.

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]

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.

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?

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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.

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

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: David Ponce <da_vid@orange.fr>,  alan@idiocy.org,  74725@debbugs.gnu.org
> Date: Sun, 08 Dec 2024 08:01:39 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> 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

How so?  Please elaborate.

> and computing the default scale in Lisp would result in images
> being displayed with an incorrect scale in the presence of multiple
> displays.

How does the above changeset solve this problem, then?

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

Please describe the issue in more detail, as I don't think I follow
what you are saying here.  If we need to detect changes in variables,
we can use the add-variable-watcher technique, similar to what we do
in frame.el with variables that need to force redisplay (but maybe I
don't understand the problem you are describing).

In any case, I don't think changes in image-scaling-factor are
supposed to be immediately reflected on display, if that's what you
have in mind.  This is not the documented effect of this variable.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: David Ponce <da_vid@orange.fr>,  alan@idiocy.org,  74725@debbugs.gnu.org
>> Date: Sun, 08 Dec 2024 08:01:39 +0800
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> 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
>
> How so?  Please elaborate.

When it is set to `auto' (the default value), the scaling factor to be
applied is decided by the configuration of a frame, namely, its
FRAME_COLUMN_WIDTH.

>> and computing the default scale in Lisp would result in images
>> being displayed with an incorrect scale in the presence of multiple
>> displays.
>
> How does the above changeset solve this problem, then?

By moving its application to image.c, which knows where an image is
being displayed and can apply specific scales for each frame.

>> 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.
>
> Please describe the issue in more detail, as I don't think I follow
> what you are saying here.  If we need to detect changes in variables,
> we can use the add-variable-watcher technique, similar to what we do
> in frame.el with variables that need to force redisplay (but maybe I
> don't understand the problem you are describing).
>
> In any case, I don't think changes in image-scaling-factor are
> supposed to be immediately reflected on display, if that's what you
> have in mind.  This is not the documented effect of this variable.

What I am trying to communicate is that changes to
`image-scaling-factor' must be accompanied by flushing the image cache
if it is to take effect on all previously displayed images.  This isn't
a problem, and the OP should simply flush the image cache after
modifying image-scaling-factor, rather than rely on the erroneous
behavior of find-image which was removed.

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

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: da_vid@orange.fr,  alan@idiocy.org,  74725@debbugs.gnu.org
> Date: Sun, 08 Dec 2024 16:03:28 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> > 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
> >
> > How so?  Please elaborate.
> 
> When it is set to `auto' (the default value), the scaling factor to be
> applied is decided by the configuration of a frame, namely, its
> FRAME_COLUMN_WIDTH.

So when the default font changes, all the images are supposed to be
resized?  Does that really happen, and if so, is that a good idea in
all cases?

> >> and computing the default scale in Lisp would result in images
> >> being displayed with an incorrect scale in the presence of multiple
> >> displays.
> >
> > How does the above changeset solve this problem, then?
> 
> By moving its application to image.c, which knows where an image is
> being displayed and can apply specific scales for each frame.

But, as this bug seems to indicate, that solution doesn't always work?

> >> 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.
> >
> > Please describe the issue in more detail, as I don't think I follow
> > what you are saying here.  If we need to detect changes in variables,
> > we can use the add-variable-watcher technique, similar to what we do
> > in frame.el with variables that need to force redisplay (but maybe I
> > don't understand the problem you are describing).
> >
> > In any case, I don't think changes in image-scaling-factor are
> > supposed to be immediately reflected on display, if that's what you
> > have in mind.  This is not the documented effect of this variable.
> 
> What I am trying to communicate is that changes to
> `image-scaling-factor' must be accompanied by flushing the image cache
> if it is to take effect on all previously displayed images.  This isn't
> a problem, and the OP should simply flush the image cache after
> modifying image-scaling-factor, rather than rely on the erroneous
> behavior of find-image which was removed.

OK, so do you consider the solution of recording the scale factor in
the cache a reasonable one?

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

[Eli Zaretskii]

Ping!

> Cc: 74725@debbugs.gnu.org, da_vid@orange.fr, alan@idiocy.org
> Date: Sun, 08 Dec 2024 14:15:02 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Po Lu <luangruo@yahoo.com>
> > Cc: da_vid@orange.fr,  alan@idiocy.org,  74725@debbugs.gnu.org
> > Date: Sun, 08 Dec 2024 16:03:28 +0800
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > >> > 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
> > >
> > > How so?  Please elaborate.
> > 
> > When it is set to `auto' (the default value), the scaling factor to be
> > applied is decided by the configuration of a frame, namely, its
> > FRAME_COLUMN_WIDTH.
> 
> So when the default font changes, all the images are supposed to be
> resized?  Does that really happen, and if so, is that a good idea in
> all cases?
> 
> > >> and computing the default scale in Lisp would result in images
> > >> being displayed with an incorrect scale in the presence of multiple
> > >> displays.
> > >
> > > How does the above changeset solve this problem, then?
> > 
> > By moving its application to image.c, which knows where an image is
> > being displayed and can apply specific scales for each frame.
> 
> But, as this bug seems to indicate, that solution doesn't always work?
> 
> > >> 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.
> > >
> > > Please describe the issue in more detail, as I don't think I follow
> > > what you are saying here.  If we need to detect changes in variables,
> > > we can use the add-variable-watcher technique, similar to what we do
> > > in frame.el with variables that need to force redisplay (but maybe I
> > > don't understand the problem you are describing).
> > >
> > > In any case, I don't think changes in image-scaling-factor are
> > > supposed to be immediately reflected on display, if that's what you
> > > have in mind.  This is not the documented effect of this variable.
> > 
> > What I am trying to communicate is that changes to
> > `image-scaling-factor' must be accompanied by flushing the image cache
> > if it is to take effect on all previously displayed images.  This isn't
> > a problem, and the OP should simply flush the image cache after
> > modifying image-scaling-factor, rather than rely on the erroneous
> > behavior of find-image which was removed.
> 
> OK, so do you consider the solution of recording the scale factor in
> the cache a reasonable one?
> 
> 
> 
> 

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> So when the default font changes, all the images are supposed to be
>> resized?  Does that really happen, and if so, is that a good idea in
>> all cases?

IMHO, yes, considering that most images are scaled by default.

>> But, as this bug seems to indicate, that solution doesn't always work?

Not when image-scaling-factor is not configured to `auto', yes.

>> OK, so do you consider the solution of recording the scale factor in
>> the cache a reasonable one?

That is alright by me.

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

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: 74725@debbugs.gnu.org,  da_vid@orange.fr,  alan@idiocy.org
> Date: Sun, 22 Dec 2024 08:26:25 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> So when the default font changes, all the images are supposed to be
> >> resized?  Does that really happen, and if so, is that a good idea in
> >> all cases?
> 
> IMHO, yes, considering that most images are scaled by default.
> 
> >> But, as this bug seems to indicate, that solution doesn't always work?
> 
> Not when image-scaling-factor is not configured to `auto', yes.
> 
> >> OK, so do you consider the solution of recording the scale factor in
> >> the cache a reasonable one?
> 
> That is alright by me.

OK, thanks.

Alan, could you please prepare a patch (for the master branch) that
records the scaling factor in the image cache and rejects a cache hit
with a different scaling factor?