bug#51596: image-transform-resize has inconsistent semantics wrt scaling up/down

[Stefan Kangas <stefan@marxist.se>]

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

tags 51596 + patch
thanks

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> Right, and that's a valid use case of course.  I still find the
>> interface inconsistent, as the naming scheme suggests that these three
>> options should behave similarly.

I've attached a patch below which implements the behavior I propose.

> Yes.  I think that, basically, the fit to height/width commands are
> pretty useless -- nobody wants that, because it'll inevitably make some
> images impossible to view.
>
> The two cases that make sense are "scale down so I can see the images"
> and "both scale down and scale up, because I want to see as much detail
> as possible".

That's true, now that you mention it.  The first patch below therefore
obsoletes the old commands, mostly to free up the "s w" binding which
gives the new, and strictly better, behavior.

We could also leave them unobsoleted and still take over the key.  It
won't hurt anyone because, as you point out, the current command is only
different in the cases where its worse.

> Sure, or a "don't scale up more than 200%", perhaps.  That's less
> finicky, I think.

Yes, that's probably better.  For this part, see my second patch below,
which builds on the first one.

[-- Attachment #2: 0001-New-command-image-transform-fit-to-window.patch --]
[-- Type: text/x-diff, Size: 8636 bytes --]

From c5760898d1e396682c4388dc8b474e0830dd2b2e Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Fri, 5 Nov 2021 03:24:50 +0100
Subject: [PATCH 1/2] New command image-transform-fit-to-window

* lisp/image-mode.el (image-auto-resize, image-transform-resize):
Add new value 'fit-window', meaning to scale the image up or down
to fit the window.  (Bug#)
(image-transform-fit-to-window): New command.
(image-transform-fit-to-height, image-transform-fit-to-width):
Make obsolete in favor of above new command.
(image-mode-map): Bind "s w" to 'image-transform-fit-to-window'.
Move binding for obsolete command 'image-transform-fit-to-width'
to "s i".
(image-mode-map): Add menu entry for
'image-transform-fit-to-window'.  Remove menu entries for above
obsolete commands.
* doc/emacs/files.texi (Image Mode): Update documentation.
(image-transform-fit-both): Doc fix.
---
 doc/emacs/files.texi |  4 ++--
 etc/NEWS             | 18 ++++++++++++++
 lisp/image-mode.el   | 57 +++++++++++++++++++++++++++++++-------------
 3 files changed, 60 insertions(+), 19 deletions(-)

diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index 65a57ccd31..3e0788307a 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -2205,11 +2205,11 @@ Image Mode
 behavior by using the options @code{image-auto-resize} and
 @code{image-auto-resize-on-window-resize}.
 
-@findex image-transform-fit-both
+@findex image-transform-fit-to-window
 @findex image-transform-set-scale
 @findex image-transform-reset
 To resize the image manually you can use the command
-@code{image-transform-fit-both} bound to @kbd{s b}
+@code{image-transform-fit-to-window} bound to @kbd{s w}
 that fits the image to both the window height and width.
 To scale the image specifying a scale factor, use the command
 @code{image-transform-set-scale} bound to @kbd{s s}.
diff --git a/etc/NEWS b/etc/NEWS
index 899f3567e6..ffb3345a0c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -268,6 +268,24 @@ To improve security, if an sql product has ':password-in-comint' set
 to t, a password supplied via the minibuffer will be sent in-process,
 as opposed to via the command-line.
 
+** Image Mode
+
++++
+*** New command 'image-transform-fit-to-window'.
+This command fits the image to the current window by scaling down or
+up as necessary.  Unlike 'image-transform-fit-both', this does not
+only scale the image down, but up as well.  It is bound to "s w" in
+Image Mode by default.
+
++++
+*** 'image-transform-fit-to-(height|width)' are now obsolete.
+Use the new command 'image-transform-fit-to-window' instead.
+The keybinding for 'image-transform-fit-to-width' is now 's i'.
+
+---
+*** User option 'image-auto-resize' can now be set to 'fit-window'.
+This works like 'image-transform-fit-to-window'.
+
 ** Image-Dired
 
 +++
diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 4a326cdc69..a911027a9a 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -58,16 +58,20 @@ image-auto-resize
   "Non-nil to resize the image upon first display.
 Its value should be one of the following:
  - nil, meaning no resizing.
- - t, meaning to fit the image to the window height and width.
+ - t, meaning to scale the image down to fit in the window.
+ - `fit-window', meaning to fit the image to the window.
  - `fit-height', meaning to fit the image to the window height.
  - `fit-width', meaning to fit the image to the window width.
- - A number, which is a scale factor (the default size is 1)."
+ - A number, which is a scale factor (the default size is 1).
+
+Resizing will always preserve the aspect ratio of the image."
   :type '(choice (const :tag "No resizing" nil)
-                 (other :tag "Fit height and width" t)
-                 (const :tag "Fit height" fit-height)
-                 (const :tag "Fit width" fit-width)
+                 (const :tag "Fit to window" fit-window)
+                 (const :tag "Fit to window height" fit-height)
+                 (const :tag "Fit to window width" fit-width)
+                 (other :tag "Scale down to fit window" t)
                  (number :tag "Scale factor" 1))
-  :version "27.1"
+  :version "29.1"
   :group 'image)
 
 (defcustom image-auto-resize-on-window-resize 1
@@ -82,12 +86,16 @@ image-auto-resize-on-window-resize
 
 (defvar-local image-transform-resize nil
   "The image resize operation.
+Non-nil to resize the image upon first display.
 Its value should be one of the following:
  - nil, meaning no resizing.
- - t, meaning to fit the image to the window height and width.
+ - t, meaning to scale the image down to fit in the window.
+ - `fit-window', meaning to fit the image to the window.
  - `fit-height', meaning to fit the image to the window height.
  - `fit-width', meaning to fit the image to the window width.
- - A number, which is a scale factor (the default size is 1).")
+ - A number, which is a scale factor (the default size is 1).
+
+Resizing will always preserve the aspect ratio of the image.")
 
 (defvar-local image-transform-scale 1.0
   "The scale factor of the image being displayed.")
@@ -455,8 +463,9 @@ image-mode-map
 
     ;; Transformation keys
     (define-key map "sf" 'image-mode-fit-frame)
+    (define-key map "sw" 'image-transform-fit-to-window)
     (define-key map "sh" 'image-transform-fit-to-height)
-    (define-key map "sw" 'image-transform-fit-to-width)
+    (define-key map "si" 'image-transform-fit-to-width)
     (define-key map "sb" 'image-transform-fit-both)
     (define-key map "ss" 'image-transform-set-scale)
     (define-key map "sr" 'image-transform-set-rotation)
@@ -511,12 +520,10 @@ image-mode-map
 	"--"
 	["Fit Frame to Image" image-mode-fit-frame :active t
 	 :help "Resize frame to match image"]
-	["Fit Image to Window (Best Fit)" image-transform-fit-both
-	 :help "Resize image to match the window height and width"]
-	["Fit to Window Height" image-transform-fit-to-height
-	 :help "Resize image to match the window height"]
-	["Fit to Window Width" image-transform-fit-to-width
-	 :help "Resize image to match the window width"]
+        ["Fit Image to Window" image-transform-fit-to-window
+         :help "Resize image to match the window height and width"]
+        ["Fit Image to Window (Scale down only)" image-transform-fit-both
+         :help "Scale image down to match the window height and width"]
 	["Zoom In" image-increase-size
 	 :help "Enlarge the image"]
 	["Zoom Out" image-decrease-size
@@ -837,7 +844,8 @@ image-toggle-display-image
 	    filename))
 	 ;; If we have a `fit-width' or a `fit-height', don't limit
 	 ;; the size of the image to the window size.
-	 (edges (when (eq image-transform-resize t)
+         (edges (when (or (eq image-transform-resize t)
+                          (eq image-transform-resize 'fit-window))
 		  (window-inside-pixel-edges (get-buffer-window))))
 	 (max-width (when edges
 		      (- (nth 2 edges) (nth 0 edges))))
@@ -884,6 +892,13 @@ image-toggle-display-image
                                 ;; Type hint.
                                 :format (and filename data-p))))
 
+    ;; Handle `fit-window'.
+    (when (eq image-transform-resize 'fit-window)
+      (setq image
+            (cons (car image)
+                  (plist-put (cdr image) :width
+                             (plist-get (cdr image) :max-width)))))
+
     ;; Discard any stale image data before looking it up again.
     (image-flush image)
     (setq image (append image (image-transform-properties image)))
@@ -1494,21 +1509,29 @@ image-transform-set-scale
 (defun image-transform-fit-to-height ()
   "Fit the current image to the height of the current window."
   (interactive)
+  (declare (obsolete nil "29.1"))
   (setq image-transform-resize 'fit-height)
   (image-toggle-display-image))
 
 (defun image-transform-fit-to-width ()
   "Fit the current image to the width of the current window."
+  (declare (obsolete nil "29.1"))
   (interactive)
   (setq image-transform-resize 'fit-width)
   (image-toggle-display-image))
 
 (defun image-transform-fit-both ()
-  "Fit the current image both to the height and width of the current window."
+  "Scale the current image down to fit in the current window."
   (interactive)
   (setq image-transform-resize t)
   (image-toggle-display-image))
 
+(defun image-transform-fit-to-window ()
+  "Fit the current image to the height and width of the current window."
+  (interactive)
+  (setq image-transform-resize 'fit-window)
+  (image-toggle-display-image))
+
 (defun image-transform-set-rotation (rotation)
   "Prompt for an angle ROTATION, and rotate the image by that amount.
 ROTATION should be in degrees."
-- 
2.30.2


[-- Attachment #3: 0002-New-user-option-image-auto-resize-max-scale-percent.patch --]
[-- Type: text/x-diff, Size: 3218 bytes --]

From 85b8b99b3a14b62fe770670ba795bba40538447c Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Fri, 5 Nov 2021 04:22:12 +0100
Subject: [PATCH 2/2] New user option image-auto-resize-max-scale-percent

* lisp/image-mode.el (image-auto-resize-max-scale-percent): New
user option to limit how much 'fit-window' will scale up an image.
(image--scale-within-limits-p): New function.
(image-toggle-display-image): Respect above new user option.
---
 etc/NEWS           |  4 ++++
 lisp/image-mode.el | 27 ++++++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index ffb3345a0c..d2b348d5b1 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -286,6 +286,10 @@ The keybinding for 'image-transform-fit-to-width' is now 's i'.
 *** User option 'image-auto-resize' can now be set to 'fit-window'.
 This works like 'image-transform-fit-to-window'.
 
+*** New user option 'image-auto-resize-max-scale-percent'.
+The new 'fit-window' options will never scale an image more than this
+much (in percent).  It is nil by default.
+
 ** Image-Dired
 
 +++
diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index a911027a9a..624c852cb8 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -74,6 +74,15 @@ image-auto-resize
   :version "29.1"
   :group 'image)
 
+(defcustom image-auto-resize-max-scale-percent nil
+  "Max size (in percent) to scale up to when `image-auto-resize' is `fit-window'.
+Can be either a number larger than 100, or nil, which means no
+max size."
+  :type '(choice (const nil "No max")
+                 natnum)
+  :version "29.1"
+  :group 'image)
+
 (defcustom image-auto-resize-on-window-resize 1
   "Non-nil to resize the image whenever the window's dimensions change.
 This will always keep the image fit to the window.
@@ -810,6 +819,21 @@ archive-superior-buffer
 (defvar tar-superior-buffer)
 (declare-function image-flush "image.c" (spec &optional frame))
 
+(defun image--scale-within-limits-p (image)
+  "Return t if `fit-window' will scale image within the customized limits.
+The limits are given by the user option
+`image-auto-resize-max-scale-percent'."
+  (or (not image-auto-resize-max-scale-percent)
+      (let ((scale (/ image-auto-resize-max-scale-percent 100))
+            (mw (plist-get (cdr image) :max-width))
+            (mh (plist-get (cdr image) :max-height))
+            ;; Note: `image-size' looks up and thus caches the
+            ;; untransformed image.  There's no easy way to
+            ;; prevent that.
+            (size (image-size image t)))
+        (or (<= mw (* (car size) scale))
+            (<= mh (* (cdr size) scale))))))
+
 (defun image-toggle-display-image ()
   "Show the image of the image file.
 Turn the image data into a real image, but only if the whole file
@@ -893,7 +917,8 @@ image-toggle-display-image
                                 :format (and filename data-p))))
 
     ;; Handle `fit-window'.
-    (when (eq image-transform-resize 'fit-window)
+    (when (and (eq image-transform-resize 'fit-window)
+               (image--scale-within-limits-p image))
       (setq image
             (cons (car image)
                   (plist-put (cdr image) :width
-- 
2.30.2