* unsolicited patch to image-mode.el -- "fit to window"
@ 2015-07-12 16:50 Greg Minshall
2015-07-12 16:56 ` Zack Piper
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Greg Minshall @ 2015-07-12 16:50 UTC (permalink / raw)
To: emacs-devel
hi. the following patch, from the emacs-24 branch, adds an
(image-transform-fit-to-window) to imagemagick mode, similar to
(image-transform-fit-to-width) and (image-transform-fit-to-height).
and, similar to those, a "Fit to Window" menu item. basically, you get
the smaller of the (image-transfer-fit-to-height) and
(image-transfer-fit-to-width) (which is typically what i want when using
either of those two functions).
(i also have a much smaller patch to image.el which increases the chance
of using imagemagick mode (as opposed to, e.g., jpeg mode) on an image
file. i'm happy to submit it if there's any interest.)
apologies if there is any protocol i'm missing; i'm a new kid in town.
cheers, Greg Minshall
ps -- details: possibly i went overboard in the lambda's below...
----
diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 2ed7fbe..8811c72 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -385,6 +385,9 @@ call."
["Fit to Window Width" image-transform-fit-to-width
:visible (eq image-type 'imagemagick)
:help "Resize image to match the window width"]
+ ["Fit to Window" image-transform-fit-to-window
+ :visible (eq image-type 'imagemagick)
+ :help "Resize image to fit in the window"]
["Rotate Image..." image-transform-set-rotation
:visible (eq image-type 'imagemagick)
:help "Rotate the image"]
@@ -895,6 +898,7 @@ Its value should be one of the following:
- nil, meaning no resizing.
- `fit-height', meaning to fit the image to the window height.
- `fit-width', meaning to fit the image to the window width.
+ - `fit-window', meaning to fit the image to the window.
- A number, which is a scale factor (the default size is 1).")
(defvar image-transform-scale 1.0
@@ -1007,17 +1011,18 @@ of a rotated image."
(when (and (not (numberp image-transform-resize))
(boundp 'image-type)
(eq image-type 'imagemagick))
- (let ((size (image-display-size (image-get-display-property) t)))
- (cond ((eq image-transform-resize 'fit-width)
- (cl-assert (= (car size)
- (- (nth 2 (window-inside-pixel-edges))
+ (let ((size (image-display-size (image-get-display-property) t))
+ (width (- (nth 2 (window-inside-pixel-edges))
(nth 0 (window-inside-pixel-edges))))
- t))
+ (height (- (nth 3 (window-inside-pixel-edges))
+ (nth 1 (window-inside-pixel-edges)))))
+ (cond ((eq image-transform-resize 'fit-width)
+ (cl-assert (= (car size) width) t))
((eq image-transform-resize 'fit-height)
- (cl-assert (= (cdr size)
- (- (nth 3 (window-inside-pixel-edges))
- (nth 1 (window-inside-pixel-edges))))
- t))))))
+ (cl-assert (= (cdr size) height) t) )
+ ((eq image-transform-resize 'fit-window)
+ (cl-assert (or (= (car size) width)
+ (= (cdr size) height)) t))))))
(defun image-transform-properties (spec)
"Return rescaling/rotation properties for image SPEC.
@@ -1032,7 +1037,26 @@ compiled with ImageMagick support."
(/= image-transform-rotation 0.0))
;; Note: `image-size' looks up and thus caches the untransformed
;; image. There's no easy way to prevent that.
- (let* ((size (image-size spec t))
+ (let* ((size (image-size spec t)) ; (car size) == width;
+ ; (cdr size) == height
+ (ifwidth (lambda ()
+ (image-transform-fit-width
+ (car size) (cdr size)
+ (- (nth 2 (window-inside-pixel-edges))
+ (nth 0 (window-inside-pixel-edges))))))
+ (ifheight (lambda ()
+ (let ((res (image-transform-fit-width
+ (cdr size) (car size)
+ (- (nth 3 (window-inside-pixel-edges))
+ (nth 1 (window-inside-pixel-edges))))))
+ (cons (cdr res) (car res)))))
+ (ifwindow (lambda ()
+ (let ((ifw (funcall ifwidth))
+ (ifh (funcall ifheight)))
+ (if (< (/ (float (car ifw)) (car size))
+ (/ (float (cdr ifh)) (cdr size)))
+ ifw
+ ifh))))
(resized
(cond
((numberp image-transform-resize)
@@ -1040,16 +1064,11 @@ compiled with ImageMagick support."
(setq image-transform-scale image-transform-resize)
(cons nil (floor (* image-transform-resize (cdr size))))))
((eq image-transform-resize 'fit-width)
- (image-transform-fit-width
- (car size) (cdr size)
- (- (nth 2 (window-inside-pixel-edges))
- (nth 0 (window-inside-pixel-edges)))))
+ (funcall ifwidth))
((eq image-transform-resize 'fit-height)
- (let ((res (image-transform-fit-width
- (cdr size) (car size)
- (- (nth 3 (window-inside-pixel-edges))
- (nth 1 (window-inside-pixel-edges))))))
- (cons (cdr res) (car res)))))))
+ (funcall ifheight))
+ ((eq image-transform-resize 'fit-window)
+ (funcall ifwindow)))))
`(,@(when (car resized)
(list :width (car resized)))
,@(when (cdr resized)
@@ -1081,6 +1100,14 @@ ImageMagick support."
(setq image-transform-resize 'fit-width)
(image-toggle-display-image))
+(defun image-transform-fit-to-window ()
+ "Fit the current image to the width of the current window.
+This command has no effect unless Emacs is compiled with
+ImageMagick support."
+ (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. This command has no effect unless
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-12 16:50 unsolicited patch to image-mode.el -- "fit to window" Greg Minshall
@ 2015-07-12 16:56 ` Zack Piper
2015-07-13 5:19 ` Greg Minshall
2015-08-06 22:28 ` Stefan Monnier
2 siblings, 0 replies; 13+ messages in thread
From: Zack Piper @ 2015-07-12 16:56 UTC (permalink / raw)
To: emacs-devel
> apologies if there is any protocol i'm missing; i'm a new kid in town.
No worries, heh.
You need to sign the FSF papers.
https://www.gnu.org/prep/maintain/html_node/Copyright-Papers.html
--
Zack Piper <zack@apertron.net> http://apertron.net
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-12 16:50 unsolicited patch to image-mode.el -- "fit to window" Greg Minshall
2015-07-12 16:56 ` Zack Piper
@ 2015-07-13 5:19 ` Greg Minshall
2015-07-14 23:23 ` Stefan Monnier
2015-08-06 22:28 ` Stefan Monnier
2 siblings, 1 reply; 13+ messages in thread
From: Greg Minshall @ 2015-07-13 5:19 UTC (permalink / raw)
Cc: emacs-devel
sadly, there's a bug with that patch (involving rotating images). i
will try to fix and re-submit.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-13 5:19 ` Greg Minshall
@ 2015-07-14 23:23 ` Stefan Monnier
2015-07-23 17:52 ` Greg Minshall
0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2015-07-14 23:23 UTC (permalink / raw)
To: Greg Minshall; +Cc: emacs-devel
> sadly, there's a bug with that patch (involving rotating images). i
> will try to fix and re-submit.
Thanks.
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-14 23:23 ` Stefan Monnier
@ 2015-07-23 17:52 ` Greg Minshall
2015-07-23 18:26 ` martin rudalics
2015-08-19 14:17 ` Ted Zlatanov
0 siblings, 2 replies; 13+ messages in thread
From: Greg Minshall @ 2015-07-23 17:52 UTC (permalink / raw)
To: emacs-devel
sorry again for the defective patch. below is a reformed patch that,
after more testing, appears to work. again, this patch adds a
'fit-window option to the Image menu when viewing images, which resizes
the image (maintaining its aspect ratio) to fit within both the height
and width of the containing window.
i've filled out and submitted GNU paperwork. please let me know if the
paperwork has, or if the patch retains, deficiencies.
cheers, Greg Minshall
----
diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 2ed7fbe..6377699 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -385,6 +385,9 @@ call."
["Fit to Window Width" image-transform-fit-to-width
:visible (eq image-type 'imagemagick)
:help "Resize image to match the window width"]
+ ["Fit to Window" image-transform-fit-to-window
+ :visible (eq image-type 'imagemagick)
+ :help "Resize image to fit in the window"]
["Rotate Image..." image-transform-set-rotation
:visible (eq image-type 'imagemagick)
:help "Rotate the image"]
@@ -895,6 +898,7 @@ Its value should be one of the following:
- nil, meaning no resizing.
- `fit-height', meaning to fit the image to the window height.
- `fit-width', meaning to fit the image to the window width.
+ - `fit-window', meaning to fit the image to the window.
- A number, which is a scale factor (the default size is 1).")
(defvar image-transform-scale 1.0
@@ -952,6 +956,9 @@ a slightly different angle. Currently this is done for values
close to a multiple of 90, see `image-transform-right-angle-fudge'."
(cond ((< (abs (- (mod (+ image-transform-rotation 90) 180) 90))
image-transform-right-angle-fudge)
+ ;; in this case, there is either no rotation (close to zero)
+ ;; or a 180 degree rotation. in either case, the width of
+ ;; the image's post-rotation bounding box doesn't change.
(cl-assert (not (zerop width)) t)
(setq image-transform-rotation
(float (round image-transform-rotation))
@@ -959,6 +966,9 @@ close to a multiple of 90, see `image-transform-right-angle-fudge'."
(cons length nil))
((< (abs (- (mod (+ image-transform-rotation 45) 90) 45))
image-transform-right-angle-fudge)
+ ;; in this case, the rotation is 90 or 270 degrees, i.e., the
+ ;; width and height of the image are flipped. so, we use the
+ ;; image height as the new image width.
(cl-assert (not (zerop height)) t)
(setq image-transform-rotation
(float (round image-transform-rotation))
@@ -1007,17 +1017,18 @@ of a rotated image."
(when (and (not (numberp image-transform-resize))
(boundp 'image-type)
(eq image-type 'imagemagick))
- (let ((size (image-display-size (image-get-display-property) t)))
- (cond ((eq image-transform-resize 'fit-width)
- (cl-assert (= (car size)
- (- (nth 2 (window-inside-pixel-edges))
+ (let ((size (image-display-size (image-get-display-property) t))
+ (width (- (nth 2 (window-inside-pixel-edges))
(nth 0 (window-inside-pixel-edges))))
- t))
+ (height (- (nth 3 (window-inside-pixel-edges))
+ (nth 1 (window-inside-pixel-edges)))))
+ (cond ((eq image-transform-resize 'fit-width)
+ (cl-assert (= (car size) width) t))
((eq image-transform-resize 'fit-height)
- (cl-assert (= (cdr size)
- (- (nth 3 (window-inside-pixel-edges))
- (nth 1 (window-inside-pixel-edges))))
- t))))))
+ (cl-assert (= (cdr size) height) t) )
+ ((eq image-transform-resize 'fit-window)
+ (cl-assert (or (= (car size) width)
+ (= (cdr size) height)) t))))))
(defun image-transform-properties (spec)
"Return rescaling/rotation properties for image SPEC.
@@ -1032,24 +1043,38 @@ compiled with ImageMagick support."
(/= image-transform-rotation 0.0))
;; Note: `image-size' looks up and thus caches the untransformed
;; image. There's no easy way to prevent that.
- (let* ((size (image-size spec t))
+ (let* ((isize (image-size spec t)) ; (car isize) == width;
+ ; (cdr isize) == height
+ (wpixels (window-inside-pixel-edges))
+ (wsize (cons (- (nth 2 wpixels) (nth 0 wpixels))
+ (- (nth 3 wpixels) (nth 1 wpixels)))) ; ditto
+ (ifwidth (lambda ()
+ (image-transform-fit-width
+ (car isize) (cdr isize) (car wsize))))
+ (ifheight (lambda ()
+ (let ((res (image-transform-fit-width
+ (cdr isize) (car isize) (cdr wsize))))
+ (cons (cdr res) (car res)))))
+ (ifwindow (lambda ()
+ ;; is the constraint the *width* or the *height* of the
+ ;; image? we can tell this by comparing the *aspect*
+ ;; ratio of the image with that of the window
+ (if (>= (/ (float (car isize)) (cdr isize))
+ (/ (float (car wsize)) (cdr wsize)))
+ (funcall ifwidth)
+ (funcall ifheight))))
(resized
(cond
((numberp image-transform-resize)
(unless (= image-transform-resize 1)
(setq image-transform-scale image-transform-resize)
- (cons nil (floor (* image-transform-resize (cdr size))))))
+ (cons nil (floor (* image-transform-resize (cdr isize))))))
((eq image-transform-resize 'fit-width)
- (image-transform-fit-width
- (car size) (cdr size)
- (- (nth 2 (window-inside-pixel-edges))
- (nth 0 (window-inside-pixel-edges)))))
+ (funcall ifwidth))
((eq image-transform-resize 'fit-height)
- (let ((res (image-transform-fit-width
- (cdr size) (car size)
- (- (nth 3 (window-inside-pixel-edges))
- (nth 1 (window-inside-pixel-edges))))))
- (cons (cdr res) (car res)))))))
+ (funcall ifheight))
+ ((eq image-transform-resize 'fit-window)
+ (funcall ifwindow)))))
`(,@(when (car resized)
(list :width (car resized)))
,@(when (cdr resized)
@@ -1081,6 +1106,14 @@ ImageMagick support."
(setq image-transform-resize 'fit-width)
(image-toggle-display-image))
+(defun image-transform-fit-to-window ()
+ "Fit the current image to the width of the current window.
+This command has no effect unless Emacs is compiled with
+ImageMagick support."
+ (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. This command has no effect unless
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-23 17:52 ` Greg Minshall
@ 2015-07-23 18:26 ` martin rudalics
2015-07-24 3:46 ` Greg Minshall
2015-08-19 14:17 ` Ted Zlatanov
1 sibling, 1 reply; 13+ messages in thread
From: martin rudalics @ 2015-07-23 18:26 UTC (permalink / raw)
To: Greg Minshall, emacs-devel
+ (wpixels (window-inside-pixel-edges))
+ (wsize (cons (- (nth 2 wpixels) (nth 0 wpixels))
+ (- (nth 3 wpixels) (nth 1 wpixels)))) ; ditto
Hopefully you should be able to replace this with
+ (wsize (cons (window-body-width nil t) (window-body-height nil t))
martin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-23 18:26 ` martin rudalics
@ 2015-07-24 3:46 ` Greg Minshall
2015-07-24 5:30 ` martin rudalics
0 siblings, 1 reply; 13+ messages in thread
From: Greg Minshall @ 2015-07-24 3:46 UTC (permalink / raw)
To: martin rudalics; +Cc: emacs-devel
Martin,
> + (wpixels (window-inside-pixel-edges))
> + (wsize (cons (- (nth 2 wpixels) (nth 0 wpixels))
> + (- (nth 3 wpixels) (nth 1 wpixels)))) ; ditto
>
> Hopefully you should be able to replace this with
>
> + (wsize (cons (window-body-width nil t) (window-body-height nil t))
thanks, that's much clearer.
a "coding philosophy" question, though. there are a dozen or so other
references to (window-size-pixel-edges) (and its associated (nth)'s) in
the code. my tendency would be to change none (in a patch) or all (in a
more substantive re-write). changing all -- while "trivial" -- is, of
course, more likely to introduce bug, and i don't know of any test suite
for image mode?
thoughts?
cheers, Greg Minshall
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-24 3:46 ` Greg Minshall
@ 2015-07-24 5:30 ` martin rudalics
2015-07-24 9:03 ` Greg Minshall
0 siblings, 1 reply; 13+ messages in thread
From: martin rudalics @ 2015-07-24 5:30 UTC (permalink / raw)
To: Greg Minshall; +Cc: emacs-devel
> a "coding philosophy" question, though. there are a dozen or so other
> references to (window-size-pixel-edges) (and its associated (nth)'s) in
> the code. my tendency would be to change none (in a patch) or all (in a
> more substantive re-write). changing all -- while "trivial" -- is, of
> course, more likely to introduce bug, and i don't know of any test suite
> for image mode?
If such code is not buggy, it should be changed only when and only by
the person(s) working on it. Otherwise, as you say, it's too easy to
introduce a bug in code that was correct before.
martin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-24 5:30 ` martin rudalics
@ 2015-07-24 9:03 ` Greg Minshall
2015-07-25 8:27 ` martin rudalics
0 siblings, 1 reply; 13+ messages in thread
From: Greg Minshall @ 2015-07-24 9:03 UTC (permalink / raw)
To: martin rudalics; +Cc: emacs-devel
> If such code is not buggy, it should be changed only when and only by
> the person(s) working on it. Otherwise, as you say, it's too easy to
> introduce a bug in code that was correct before.
right, that seems like a good rule to me. so, i'd tend to leave my code
with the same pattern used in the rest of the file (i.e., using
(window-size-pixel-edges)), just for consistency. (but, even a very
light breeze would be enough to cause me to change it to use the clearer
calls, so feel free to so breathe.)
cheers, Greg
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-24 9:03 ` Greg Minshall
@ 2015-07-25 8:27 ` martin rudalics
2015-07-25 8:31 ` martin rudalics
0 siblings, 1 reply; 13+ messages in thread
From: martin rudalics @ 2015-07-25 8:27 UTC (permalink / raw)
To: Greg Minshall; +Cc: emacs-devel
> right, that seems like a good rule to me. so, i'd tend to leave my code
> with the same pattern used in the rest of the file (i.e., using
> (window-size-pixel-edges)), just for consistency.
I don't understand the "so" ;-)
> (but, even a very
> light breeze would be enough to cause me to change it to use the clearer
> calls, so feel free to so breathe.)
I can't test your patch because I build without image support. But I
have two questions: (1) Why can't you use `fit-window-to-buffer'? (2)
If the image appears on a frame alone, can you fit the frame to the
image? Can `fit-frame-to-buffer' fit the frame to the image? Both of
these should work, in principle.
martin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-25 8:27 ` martin rudalics
@ 2015-07-25 8:31 ` martin rudalics
0 siblings, 0 replies; 13+ messages in thread
From: martin rudalics @ 2015-07-25 8:31 UTC (permalink / raw)
To: Greg Minshall; +Cc: emacs-devel
> I can't test your patch because I build without image support. But I
> have two questions: (1) Why can't you use `fit-window-to-buffer'? (2)
> If the image appears on a frame alone, can you fit the frame to the
> image? Can `fit-frame-to-buffer' fit the frame to the image? Both of
> these should work, in principle.
Disregard this. After re-reading your patch I noticed that you fit the
image to the window and not vice-versa.
martin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-23 17:52 ` Greg Minshall
2015-07-23 18:26 ` martin rudalics
@ 2015-08-19 14:17 ` Ted Zlatanov
1 sibling, 0 replies; 13+ messages in thread
From: Ted Zlatanov @ 2015-08-19 14:17 UTC (permalink / raw)
To: emacs-devel
On Thu, 23 Jul 2015 20:52:33 +0300 Greg Minshall <minshall@acm.org> wrote:
GM> this patch adds a 'fit-window option to the Image menu when viewing
GM> images, which resizes the image (maintaining its aspect ratio) to
GM> fit within both the height and width of the containing window.
This is useful, thank you for writing it. Do you think a default
keyboard shortcut might be a good idea for this command? I'd find it
useful; the shorter the better (but I don't know if it's possible to
make it a single character without breaking other tools).
Ted
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: unsolicited patch to image-mode.el -- "fit to window"
2015-07-12 16:50 unsolicited patch to image-mode.el -- "fit to window" Greg Minshall
2015-07-12 16:56 ` Zack Piper
2015-07-13 5:19 ` Greg Minshall
@ 2015-08-06 22:28 ` Stefan Monnier
2 siblings, 0 replies; 13+ messages in thread
From: Stefan Monnier @ 2015-08-06 22:28 UTC (permalink / raw)
To: Greg Minshall; +Cc: emacs-devel
> (i also have a much smaller patch to image.el which increases the chance
> of using imagemagick mode (as opposed to, e.g., jpeg mode) on an image
> file. i'm happy to submit it if there's any interest.)
I can't say whether there'll be interest, but you can send it via M-x
report-emacs-bug, explaining why it's a good idea.
> apologies if there is any protocol i'm missing; i'm a new kid in town.
Patches sent here a fine, tho we also like to see them in bug-gnu-emacs
(e.g. via M-x report-emacs-bug) so they get a tracking number.
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-08-19 14:17 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-12 16:50 unsolicited patch to image-mode.el -- "fit to window" Greg Minshall
2015-07-12 16:56 ` Zack Piper
2015-07-13 5:19 ` Greg Minshall
2015-07-14 23:23 ` Stefan Monnier
2015-07-23 17:52 ` Greg Minshall
2015-07-23 18:26 ` martin rudalics
2015-07-24 3:46 ` Greg Minshall
2015-07-24 5:30 ` martin rudalics
2015-07-24 9:03 ` Greg Minshall
2015-07-25 8:27 ` martin rudalics
2015-07-25 8:31 ` martin rudalics
2015-08-19 14:17 ` Ted Zlatanov
2015-08-06 22:28 ` Stefan Monnier
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).