bug#47819: 8.0.50; When :height/:width image attribute is specified, :scale factor is not applied

bug#47819: 8.0.50; When :height/:width image attribute is specified, :scale factor is not applied

[David Ponce]

Hello,

For the image :scale attribute the Elisp reference manual say that:

      If both ‘:scale’ and ‘:height’/‘:width’ are
      specified, the height/width will be adjusted by the specified
      scaling factor.

It seems, however, that this is not true, when the :height or :width
or both attributes of an image are specified, the :scale factor is not
applied.  For ex., evaluating the below code in the *scratch* buffer
should produce images with same size, but in the 2nd case the scale
factor is not applied:

(progn
   (insert-image
    (find-image
     '((:file "/usr/share/icons/oxygen/base/22x22/places/folder.png"
              :type png
              :scale 2
              ))))
   (insert-image
    (find-image
     '((:file "/usr/share/icons/oxygen/base/22x22/places/folder.png"
              :type png
              :scale 2
              :height 22
              :width 22
              ))))
   )

So, either the manual is wrong, or (better IMO) the code should be
fixed.

Thanks & Regards,
David Ponce


In GNU Emacs 28.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version 3.24.28, cairo version 1.16.0)
  of 2021-04-15 built on kilauea
Repository revision: c2372096433e6a7ab173e5f1993f0c65b85f353e
Repository branch: master
Windowing system distributor 'Fedora Project', version 11.0.12011000
System Description: Fedora 33 (KDE Plasma)

Configured using:
  'configure --prefix=/home/dponce --with-cairo
  PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XDBE XIM 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

Major mode: Lisp Interaction

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   line-number-mode: t
   transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 50691 7522)
  (symbols 48 6501 1)
  (strings 32 18168 1362)
  (string-bytes 1 604792)
  (vectors 16 12723)
  (vector-slots 8 171820 12342)
  (floats 8 23 45)
  (intervals 56 197 0)
  (buffers 992 10))

bug#47819: 8.0.50; When :height/:width image attribute is specified, :scale factor is not applied

[Alan Third]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

On Fri, Apr 16, 2021 at 08:16:20AM +0200, David Ponce wrote:
> Hello,
> 
> For the image :scale attribute the Elisp reference manual say that:
> 
>      If both ‘:scale’ and ‘:height’/‘:width’ are
>      specified, the height/width will be adjusted by the specified
>      scaling factor.
> 
> It seems, however, that this is not true, when the :height or :width
> or both attributes of an image are specified, the :scale factor is not
> applied.

Hmm, I'm not sure which is the more desirable behaviour, but fixing it
to match the documentation appears to be trivial. See attached.
-- 
Alan Third

[-- Attachment #2: 0001-Make-scale-affect-width-and-height-bug-47819.patch --]
[-- Type: text/plain, Size: 1096 bytes --]

From 6ffcd0d934e028732484ac1f9085901d1515036a Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Fri, 16 Apr 2021 19:12:23 +0100
Subject: [PATCH] Make :scale affect :width and :height (bug#47819)

* src/image.c (compute_image_size): Multiply width and height values
by scale.
---
 src/image.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/image.c b/src/image.c
index 1619886f5a..ff4ef01804 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2040,7 +2040,7 @@ compute_image_size (size_t width, size_t height,
   int_value = image_get_dimension (img, QCwidth);
   if (int_value >= 0)
     {
-      desired_width = int_value;
+      desired_width = int_value * scale;
       /* :width overrides :max-width. */
       max_width = -1;
     }
@@ -2048,7 +2048,7 @@ compute_image_size (size_t width, size_t height,
   int_value = image_get_dimension (img, QCheight);
   if (int_value >= 0)
     {
-      desired_height = int_value;
+      desired_height = int_value * scale;
       /* :height overrides :max-height. */
       max_height = -1;
     }
-- 
2.29.2

bug#47819: 8.0.50; When :height/:width image attribute is specified, :scale factor is not applied

[David Ponce]

On 16/04/2021 20:14, Alan Third wrote:
> On Fri, Apr 16, 2021 at 08:16:20AM +0200, David Ponce wrote:
>> Hello,
>>
>> For the image :scale attribute the Elisp reference manual say that:
>>
>>       If both ‘:scale’ and ‘:height’/‘:width’ are
>>       specified, the height/width will be adjusted by the specified
>>       scaling factor.
>>
>> It seems, however, that this is not true, when the :height or :width
>> or both attributes of an image are specified, the :scale factor is not
>> applied.
> 
> Hmm, I'm not sure which is the more desirable behaviour, but fixing it
> to match the documentation appears to be trivial. See attached.
> 

Hi Alan,

Your patch fixed the issue for me, and now the specified :scale factor
is applied when the :height/:width attributes are specified.

IMHO, this is the right fix to be consistent with the
documentation. At least because create-image apply a default scale
factor, maybe for better rendering on high resolution
screens. For ex., a default scale factor of 1.2 is applied on my
laptop (1920x1080). Scaling could also make sense when height/width
are specified in 'em'?

Thanks!

bug#47819: 8.0.50; When :height/:width image attribute is specified, :scale factor is not applied

[Alan Third]

On Fri, Apr 16, 2021 at 08:50:58PM +0200, David Ponce wrote:
> On 16/04/2021 20:14, Alan Third wrote:
> > On Fri, Apr 16, 2021 at 08:16:20AM +0200, David Ponce wrote:
> > > Hello,
> > > 
> > > For the image :scale attribute the Elisp reference manual say that:
> > > 
> > >       If both ‘:scale’ and ‘:height’/‘:width’ are
> > >       specified, the height/width will be adjusted by the specified
> > >       scaling factor.
> > > 
> > > It seems, however, that this is not true, when the :height or :width
> > > or both attributes of an image are specified, the :scale factor is not
> > > applied.
> > 
> > Hmm, I'm not sure which is the more desirable behaviour, but fixing it
> > to match the documentation appears to be trivial. See attached.
> > 
> 
> Hi Alan,
> 
> Your patch fixed the issue for me, and now the specified :scale factor
> is applied when the :height/:width attributes are specified.
> 
> IMHO, this is the right fix to be consistent with the
> documentation. At least because create-image apply a default scale
> factor, maybe for better rendering on high resolution
> screens. For ex., a default scale factor of 1.2 is applied on my
> laptop (1920x1080). Scaling could also make sense when height/width
> are specified in 'em'?

I don't know why you would want to use scale with em dimensions, since
you can just set the em dimensions directly.

For example ":scale 2 :height '(1 . em)" is exactly the same as
":scale 1 :height '(2 . em)", so why bother?

Plus if you specify something to be 1em in height, then create-image
applies its magic scale factor, you end up with something that is not
1em in height.

So yeah, I'm not convinced that this is behaviour we want.
-- 
Alan Third

bug#47819: 8.0.50; When :height/:width image attribute is specified, :scale factor is not applied

[Alan Third]

On Sat, Apr 17, 2021 at 09:23:55AM +0100, Alan Third wrote:
> 
> I don't know why you would want to use scale with em dimensions, since
> you can just set the em dimensions directly.
> 
> For example ":scale 2 :height '(1 . em)" is exactly the same as
> ":scale 1 :height '(2 . em)", so why bother?
> 
> Plus if you specify something to be 1em in height, then create-image
> applies its magic scale factor, you end up with something that is not
> 1em in height.
> 
> So yeah, I'm not convinced that this is behaviour we want.

OK, I'm still convinced this isn't really the behaviour we want, but I
did some further investigation and realised this is a bug introduced
by me recently. I thought it had worked this way for a long time.

I'll just push the change up to fix it.
-- 
Alan Third