GNU bug report logs - #74725
31.0.50; image-scaling-factor is ignored by create-image

Previous Next

Package: emacs;

Reported by: David Ponce <da_vid <at> orange.fr>

Date: Sat, 7 Dec 2024 12:15:02 UTC

Severity: normal

Found in version 31.0.50

Done: Alan Third <alan <at> idiocy.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 74725 in the body.
You can then email your comments to 74725 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sat, 07 Dec 2024 12:15:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to David Ponce <da_vid <at> orange.fr>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sat, 07 Dec 2024 12:15:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; image-scaling-factor is ignored by create-image
Date: Sat, 7 Dec 2024 13:13:58 +0100
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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sat, 07 Dec 2024 12:42:01 GMT) Full text and rfc822 format available.

Message #8 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: 74725 <at> debbugs.gnu.org
Subject: 31.0.50; image-scaling-factor is ignored by create-image
Date: Sat, 7 Dec 2024 13:41:37 +0100
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!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sat, 07 Dec 2024 14:50:01 GMT) Full text and rfc822 format available.

Message #11 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: David Ponce <da_vid <at> orange.fr>, Alan Third <alan <at> idiocy.org>
Cc: 74725 <at> debbugs.gnu.org
Subject: Re: bug#74725: 31.0.50;
 image-scaling-factor is ignored by create-image
Date: Sat, 07 Dec 2024 16:49:41 +0200
> 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 <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sat, 07 Dec 2024 15:50:02 GMT) Full text and rfc822 format available.

Message #14 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74725 <at> debbugs.gnu.org, David Ponce <da_vid <at> orange.fr>
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sat, 7 Dec 2024 15:49:47 +0000
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 <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sat, 07 Dec 2024 16:22:02 GMT) Full text and rfc822 format available.

Message #17 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Third <alan <at> idiocy.org>
Cc: 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sat, 07 Dec 2024 18:21:25 +0200
> Date: Sat, 7 Dec 2024 15:49:47 +0000
> From: Alan Third <alan <at> idiocy.org>
> Cc: David Ponce <da_vid <at> orange.fr>, 74725 <at> 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 <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sat, 07 Dec 2024 16:28:01 GMT) Full text and rfc822 format available.

Message #20 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: Alan Third <alan <at> idiocy.org>, Eli Zaretskii <eliz <at> gnu.org>,
 74725 <at> debbugs.gnu.org
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sat, 7 Dec 2024 17:27:13 +0100
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 <at> 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 <at> yahoo.com>	2024-06-03 16:34:51 +0800
committer	Po Lu <luangruo <at> 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.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sat, 07 Dec 2024 16:33:02 GMT) Full text and rfc822 format available.

Message #23 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: Eli Zaretskii <eliz <at> gnu.org>, Alan Third <alan <at> idiocy.org>
Cc: 74725 <at> debbugs.gnu.org
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sat, 7 Dec 2024 17:32:20 +0100
On 2024-12-07 17:21, Eli Zaretskii wrote:
>> Date: Sat, 7 Dec 2024 15:49:47 +0000
>> From: Alan Third <alan <at> idiocy.org>
>> Cc: David Ponce <da_vid <at> orange.fr>, 74725 <at> 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 <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sat, 07 Dec 2024 16:46:02 GMT) Full text and rfc822 format available.

Message #26 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: David Ponce <da_vid <at> orange.fr>, Po Lu <luangruo <at> yahoo.com>
Cc: 74725 <at> debbugs.gnu.org, alan <at> idiocy.org
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sat, 07 Dec 2024 18:45:37 +0200
> Date: Sat, 7 Dec 2024 17:27:13 +0100
> From: David Ponce <da_vid <at> 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 <at> 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 <at> yahoo.com>	2024-06-03 16:34:51 +0800
> committer	Po Lu <luangruo <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sun, 08 Dec 2024 00:02:02 GMT) Full text and rfc822 format available.

Message #29 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74725 <at> debbugs.gnu.org, David Ponce <da_vid <at> orange.fr>, alan <at> idiocy.org
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sun, 08 Dec 2024 08:01:39 +0800
Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sat, 7 Dec 2024 17:27:13 +0100
>> From: David Ponce <da_vid <at> 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 <at> 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 <at> yahoo.com>	2024-06-03 16:34:51 +0800
>> committer	Po Lu <luangruo <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sun, 08 Dec 2024 06:04:02 GMT) Full text and rfc822 format available.

Message #32 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr, alan <at> idiocy.org
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sun, 08 Dec 2024 08:03:47 +0200
> From: Po Lu <luangruo <at> yahoo.com>
> Cc: David Ponce <da_vid <at> orange.fr>,  alan <at> idiocy.org,  74725 <at> debbugs.gnu.org
> Date: Sun, 08 Dec 2024 08:01:39 +0800
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> author	Po Lu <luangruo <at> yahoo.com>	2024-06-03 16:34:51 +0800
> >> committer	Po Lu <luangruo <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sun, 08 Dec 2024 08:04:02 GMT) Full text and rfc822 format available.

Message #35 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr, alan <at> idiocy.org
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sun, 08 Dec 2024 16:03:28 +0800
Eli Zaretskii <eliz <at> gnu.org> writes:

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sun, 08 Dec 2024 12:16:02 GMT) Full text and rfc822 format available.

Message #38 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr, alan <at> idiocy.org
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sun, 08 Dec 2024 14:15:02 +0200
> From: Po Lu <luangruo <at> yahoo.com>
> Cc: da_vid <at> orange.fr,  alan <at> idiocy.org,  74725 <at> debbugs.gnu.org
> Date: Sun, 08 Dec 2024 16:03:28 +0800
> 
> Eli Zaretskii <eliz <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sat, 21 Dec 2024 09:16:02 GMT) Full text and rfc822 format available.

Message #41 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: luangruo <at> yahoo.com
Cc: 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr, alan <at> idiocy.org
Subject: Re: bug#74725: 31.0.50;
 image-scaling-factor is ignored by create-image
Date: Sat, 21 Dec 2024 11:14:50 +0200
Ping!

> Cc: 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr, alan <at> idiocy.org
> Date: Sun, 08 Dec 2024 14:15:02 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > From: Po Lu <luangruo <at> yahoo.com>
> > Cc: da_vid <at> orange.fr,  alan <at> idiocy.org,  74725 <at> debbugs.gnu.org
> > Date: Sun, 08 Dec 2024 16:03:28 +0800
> > 
> > Eli Zaretskii <eliz <at> 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?
> 
> 
> 
> 




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sun, 22 Dec 2024 00:27:02 GMT) Full text and rfc822 format available.

Message #44 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr, alan <at> idiocy.org
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sun, 22 Dec 2024 08:26:25 +0800
Eli Zaretskii <eliz <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sun, 22 Dec 2024 06:49:02 GMT) Full text and rfc822 format available.

Message #47 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>, alan <at> idiocy.org
Cc: 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sun, 22 Dec 2024 08:48:04 +0200
> From: Po Lu <luangruo <at> yahoo.com>
> Cc: 74725 <at> debbugs.gnu.org,  da_vid <at> orange.fr,  alan <at> idiocy.org
> Date: Sun, 22 Dec 2024 08:26:25 +0800
> 
> Eli Zaretskii <eliz <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Fri, 27 Dec 2024 12:13:02 GMT) Full text and rfc822 format available.

Message #50 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Po Lu <luangruo <at> yahoo.com>, 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Fri, 27 Dec 2024 12:11:33 +0000
[Message part 1 (text/plain, inline)]
On Sun, Dec 22, 2024 at 08:48:04AM +0200, Eli Zaretskii wrote:
> 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?

Hi Eli, patch attached.

I named a new function image_compute_scale because most of the other
functions in that file start image_, however I wasn't sure if I should
name it compute_image_scale to mirror compute_image_size. Let me know
if you think I should change it.
-- 
Alan Third
[0001-Make-image-cache-aware-of-image-scaling-factor-bug-7.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Fri, 27 Dec 2024 16:12:01 GMT) Full text and rfc822 format available.

Message #53 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: Alan Third <alan <at> idiocy.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Po Lu <luangruo <at> yahoo.com>, 74725 <at> debbugs.gnu.org
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Fri, 27 Dec 2024 17:11:30 +0100
On 2024-12-27 13:11, Alan Third wrote:
> On Sun, Dec 22, 2024 at 08:48:04AM +0200, Eli Zaretskii wrote:
>> 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?
> 
> Hi Eli, patch attached.
> 
> I named a new function image_compute_scale because most of the other
> functions in that file start image_, however I wasn't sure if I should
> name it compute_image_scale to mirror compute_image_size. Let me know
> if you think I should change it.

Hi Alan,

FYI, I applied your patch and it fixes the issue for me with the initial
examples I provided.

Many thanks Alan, and to all of you!





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74725; Package emacs. (Sat, 28 Dec 2024 12:27:02 GMT) Full text and rfc822 format available.

Message #56 received at 74725 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Third <alan <at> idiocy.org>
Cc: luangruo <at> yahoo.com, 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sat, 28 Dec 2024 14:26:47 +0200
> Date: Fri, 27 Dec 2024 12:11:33 +0000
> From: Alan Third <alan <at> idiocy.org>
> Cc: Po Lu <luangruo <at> yahoo.com>, 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr
> 
> > 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?
> 
> Hi Eli, patch attached.

Thanks, LGTM.

> I named a new function image_compute_scale because most of the other
> functions in that file start image_, however I wasn't sure if I should
> name it compute_image_scale to mirror compute_image_size. Let me know
> if you think I should change it.

Static functions can have any names we see fit, so feel free to rename
if you think compute_image_scale would be better.

Feel free to install on master, when you are satisfied with the code.




Reply sent to Alan Third <alan <at> idiocy.org>:
You have taken responsibility. (Sat, 28 Dec 2024 12:39:02 GMT) Full text and rfc822 format available.

Notification sent to David Ponce <da_vid <at> orange.fr>:
bug acknowledged by developer. (Sat, 28 Dec 2024 12:39:02 GMT) Full text and rfc822 format available.

Message #61 received at 74725-done <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 74725-done <at> debbugs.gnu.org, da_vid <at> orange.fr
Subject: Re: bug#74725: 31.0.50; image-scaling-factor is ignored by
 create-image
Date: Sat, 28 Dec 2024 12:37:52 +0000
On Sat, Dec 28, 2024 at 02:26:47PM +0200, Eli Zaretskii wrote:
> > Date: Fri, 27 Dec 2024 12:11:33 +0000
> > From: Alan Third <alan <at> idiocy.org>
> > Cc: Po Lu <luangruo <at> yahoo.com>, 74725 <at> debbugs.gnu.org, da_vid <at> orange.fr
> > 
> > > 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?
> > 
> > Hi Eli, patch attached.
> 
> Thanks, LGTM.
> 
> > I named a new function image_compute_scale because most of the other
> > functions in that file start image_, however I wasn't sure if I should
> > name it compute_image_scale to mirror compute_image_size. Let me know
> > if you think I should change it.
> 
> Static functions can have any names we see fit, so feel free to rename
> if you think compute_image_scale would be better.
> 
> Feel free to install on master, when you are satisfied with the code.

Done. Thanks.
-- 
Alan Third




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 26 Jan 2025 12:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 146 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.