Subject: Re: bug#33990: 27.0.50; image-transform-fit-to-height/-width violates assert

From: Benjamin Riefenstahl <[email-masked]>
Date: Tue, 15 Jan 2019 22:38:40 +0100

Eli Zaretskii writes:
> Can you propose a patch that would implement your suggestion?

It looks like my analysis was wrong.  The problem is rather that
create-image applies a default scaling, when :scale is not given.  This
depends on the font size (only if char-width > 10 pixels), which
explains why it happens on my laptop but not on my desktop machine.  I
guess create-image is targeting image resources like toolbar buttons,
where it may be a good idea to scale the images with the fonts for
so-called "hidpi" displays.  But for image-mode the situation is
different and I at least do not want that behaviour.

I attach three patches, the first is a minimal change that fixes the
problem for me.  The second adds to the doc string of create-image,
because I think that its behaviour is unexpected enough to warrant
highlighting.  The third makes the image-mode transform parameters
buffer-local, because that seems more appropriate and even complies with
the doc string form image-transform-rotation better.  This third patch
changes behaviour, though, so maybe a second opinion is in order.

The patches are against master, I haven't tested if they apply against
emacs-26.  Let me know if there is something that needs to be changed.

>From 951eadeb77c665c9d419e982b3651f401ad5240a Mon Sep 17 00:00:00 2001
From: Benjamin Riefenstahl <[email-masked]>
Date: Mon, 14 Jan 2019 23:37:03 +0100
Subject: [PATCH 1/4] image-mode: Do not use default scaling (bug#33990)

* lisp/image-mode.el (image-toggle-display-image): Set :scale == 1 so
that create-image does not apply additional scaling.
---
 lisp/image-mode.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 3570c7cba4..7f290719bb 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -741,9 +741,11 @@ image-toggle-display-image
 	 (type (if (image--imagemagick-wanted-p filename)
 		   'imagemagick
 		 (image-type file-or-data nil data-p)))
+         ;; :scale 1: If we do not set this, create-image will apply
+         ;; default scaling based on font size.
 	 (image (if (not edges)
-		    (create-image file-or-data type data-p)
-		  (create-image file-or-data type data-p
+		    (create-image file-or-data type data-p :scale 1)
+		  (create-image file-or-data type data-p :scale 1
 				:max-width (- (nth 2 edges) (nth 0 edges))
 				:max-height (- (nth 3 edges) (nth 1 edges)))))
 	 (inhibit-read-only t)
-- 
2.11.0

>From ab0bb7674a702a2d142c6e7790c26fb880484591 Mon Sep 17 00:00:00 2001
From: Benjamin Riefenstahl <[email-masked]>
Date: Tue, 15 Jan 2019 17:00:04 +0100
Subject: [PATCH 2/4] create-image: Expand documentation (bug#33990)

* lisp/image.el (create-image): Discuss default for :scale in the doc
string.
---
 lisp/image.el | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lisp/image.el b/lisp/image.el
index 2e84e47b5c..3aa3b0aa24 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -415,13 +415,20 @@ image-type-auto-detected-p
 (defun create-image (file-or-data &optional type data-p &rest props)
   "Create an image.
 FILE-OR-DATA is an image file name or image data.
+
 Optional TYPE is a symbol describing the image type.  If TYPE is omitted
 or nil, try to determine the image type from its first few bytes
 of image data.  If that doesn't work, and FILE-OR-DATA is a file name,
 use its file extension as image type.
+
 Optional DATA-P non-nil means FILE-OR-DATA is a string containing image data.
+
 Optional PROPS are additional image attributes to assign to the image,
-like, e.g. `:mask MASK'.
+like, e.g. `:mask MASK'.  If the property `:scale' is not given and the
+display has a high resolution (more exactly, when the average width of a
+character in the default font is more than 10 pixels), the image is
+automatically scaled up in proportion to the default font.
+
 Value is the image created, or nil if images of type TYPE are not supported.
 
 Images should not be larger than specified by `max-image-size'.
-- 
2.11.0

>From aa78a077b2eaf5d844f0c8e65f7ac3354c07cfb4 Mon Sep 17 00:00:00 2001
From: Benjamin Riefenstahl <[email-masked]>
Date: Sat, 12 Jan 2019 15:36:24 +0100
Subject: [PATCH 3/4] image-mode: Make parameters buffer-local

Image parameters were treated as image specific, but because they
actually were global variables, their behaviour transfered to new
images.

* lisp/image-mode.el (image-transform-resize, image-transform-scale)
(image-transform-rotation): Declare with defvar-local.
---
 lisp/image-mode.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 7f290719bb..fa1362c471 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -53,7 +53,7 @@ image-mode-new-window-functions
 It is called with one argument, the initial WINPROPS.")
 
 ;; FIXME this doesn't seem mature yet. Document in manual when it is.
-(defvar image-transform-resize nil
+(defvar-local image-transform-resize nil
   "The image resize operation.
 Its value should be one of the following:
  - nil, meaning no resizing.
@@ -61,10 +61,10 @@ image-transform-resize
  - `fit-width', meaning to fit the image to the window width.
  - A number, which is a scale factor (the default size is 1).")
 
-(defvar image-transform-scale 1.0
+(defvar-local image-transform-scale 1.0
   "The scale factor of the image being displayed.")
 
-(defvar image-transform-rotation 0.0
+(defvar-local image-transform-rotation 0.0
   "Rotation angle for the image in the current Image mode buffer.")
 
 (defvar image-transform-right-angle-fudge 0.0001
-- 
2.11.0