image-rotate: Accept angle as an argument

image-rotate: Accept angle as an argument

[Tino Calancha]

Hello,
lisp/image.el defines 'image-increase-size' and 'image-decrease-size',
with an argument N.  We might allow for an argument in 'image-rotate'
as well.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
commit 27fb0aeb3b12256853c86d8494ff258c8926073e
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Mon Sep 5 14:44:18 2016 +0900

     image-rotate: Accept angle as an argument

     * lisp/image.el (image-rotate):
     Add argument ANGLE, the angle in degrees for the rotation.
     Add optional argument _ARG; in interactive calls, a non-nil
     value prompt for ANGLE.

diff --git a/lisp/image.el b/lisp/image.el
index e1f52de..cad8332 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -1008,12 +1008,21 @@ image--current-scaling
          (display-width (car (image-size image t))))
      (/ (float display-width) image-width)))

-(defun image-rotate ()
-  "Rotate the image under point by 90 degrees clockwise."
-  (interactive)
+(defun image-rotate (angle &optional _arg)
+  "Rotate the image under point by ANGLE degrees clockwise.
+If ANGLE is a negative number, then rotate counterclockwise.
+When called interactively with a prefix argument, prompt for ANGLE."
+  (interactive
+   (let* ((ask current-prefix-arg)
+          (default 90)
+          (prompt "Rotate image by ANGLE degrees: ")
+          (rotation (if ask
+                        (read-number prompt default)
+                      default)))
+     (list rotation ask)))
    (let ((image (image--get-imagemagick-and-warn)))
      (plist-put (cdr image) :rotation
-               (float (mod (+ (or (plist-get (cdr image) :rotation) 0) 
90)
+               (float (mod (+ (or (plist-get (cdr image) :rotation) 0) 
angle)
                             ;; We don't want to exceed 360 degrees
                             ;; rotation, because it's not seen as valid
                             ;; in exif data.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.20.9)
  of 2016-09-05 built on calancha-pc
Repository revision: 62e4dc4660cb3b29cfffcad0639e51c7f382ced8

Re: image-rotate: Accept angle as an argument

[Tino Calancha]

On Mon, 5 Sep 2016, Tino Calancha wrote:

>
> Hello,
> lisp/image.el defines 'image-increase-size' and 'image-decrease-size',
> with an argument N.  We might allow for an argument in 'image-rotate'
> as well.

I would like also to modify 'image-increase-size' and 
'image-decrease-size' as i did with 'image-rotate':
A prefix argument prompt user for the size factor.  Then you can perform 
arbitrary rotations and resize of an image using these commands as 
follows:
;; FILE is the file name of an image file:
emacs -Q FILE:
1 r 15 RET ; rotate image 15 degrees
1 r -15 RET ; rotate image -15 degrees
1 + 50 RET ; increase image size 50%
1 - 40 RET ; decrease image size 40%

So the proposal is 2 patches:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From aefda287d0d739c99f6e45f568a22b67f1eb19da Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Mon, 5 Sep 2016 16:26:03 +0900
Subject: [PATCH 1/2] image-rotate: Accept angle as an argument

* lisp/image.el (image-rotate):
Add argument ANGLE, the angle in degrees for the rotation.
Add optional argument _ARG; in interactive calls, a non-nil
value prompt for ANGLE.
---
  lisp/image.el | 17 +++++++++++++----
  1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/lisp/image.el b/lisp/image.el
index e1f52de..cad8332 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -1008,12 +1008,21 @@ image--current-scaling
          (display-width (car (image-size image t))))
      (/ (float display-width) image-width)))

-(defun image-rotate ()
-  "Rotate the image under point by 90 degrees clockwise."
-  (interactive)
+(defun image-rotate (angle &optional _arg)
+  "Rotate the image under point by ANGLE degrees clockwise.
+If ANGLE is a negative number, then rotate counterclockwise.
+When called interactively with a prefix argument, prompt for ANGLE."
+  (interactive
+   (let* ((ask current-prefix-arg)
+          (default 90)
+          (prompt "Rotate image by ANGLE degrees: ")
+          (rotation (if ask
+                        (read-number prompt default)
+                      default)))
+     (list rotation ask)))
    (let ((image (image--get-imagemagick-and-warn)))
      (plist-put (cdr image) :rotation
-               (float (mod (+ (or (plist-get (cdr image) :rotation) 0) 
90)
+               (float (mod (+ (or (plist-get (cdr image) :rotation) 0) 
angle)
                             ;; We don't want to exceed 360 degrees
                             ;; rotation, because it's not seen as valid
                             ;; in exif data.
-- 
2.9.3

From 7428267a2723315e9a8df3f7a687aeb66513dde4 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Mon, 5 Sep 2016 16:29:56 +0900
Subject: [PATCH 2/2] image-increase-size: Prompt for size factor in
  interactive calls

* lisp/image.el (image-increase-size, image-decrease-size):
Add optional argument _ARG; in interactive calls, a non-nil
value prompt for the size factor.
---
  lisp/image.el | 43 ++++++++++++++++++++++++++++---------------
  1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/lisp/image.el b/lisp/image.el
index cad8332..1bfa042 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -951,23 +951,36 @@ imagemagick-enabled-types

  (imagemagick-register-types)

-(defun image-increase-size (n)
-  "Increase the image size by a factor of N.
+(defun image-increase-size (n &optional _arg)
+  "Increase the image size.
  If N is 3, then the image size will be increased by 30%.  The
-default is 20%."
-  (interactive "P")
-  (image--change-size (if n
-                          (1+ (/ n 10.0))
-                        1.2)))
-
-(defun image-decrease-size (n)
-  "Decrease the image size by a factor of N.
+default is 20%.
+When called interactively with a prefix argument, prompt for N."
+  (interactive
+   (let* ((ask current-prefix-arg)
+          (default 20)
+          (num (if ask
+                   (read-number "Increase image size by X %: " default)
+                 default)))
+     (list (/ num 10.0) ask)))
+  (image--change-size (1+ (/ n 10.0))))
+
+(defun image-decrease-size (n &optional _arg)
+  "Decrease the image size.
  If N is 3, then the image size will be decreased by 30%.  The
-default is 20%."
-  (interactive "P")
-  (image--change-size (if n
-                          (- 1 (/ n 10.0))
-                        0.8)))
+default is 20%.
+When called interactively with a prefix argument, prompt for N."
+  (interactive
+   (let* ((ask current-prefix-arg)
+          (default 20)
+          (num (if ask
+                   (read-number "Decrease image size by X %: " default)
+                 default)))
+     (list (/ num 10.0) ask)))
+  (let ((decrease (/ n 10.0)))
+    (image--change-size (if (< decrease 1)
+                            (- 1 decrease)
+                          0))))

  (defun image--get-image ()
    (let ((image (get-text-property (point) 'display)))
-- 
2.9.3

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.20.9)
  of 2016-09-05
Repository revision: 62e4dc4660cb3b29cfffcad0639e51c7f382ced8

Re: image-rotate: Accept angle as an argument

[Lars Ingebrigtsen]

Tino Calancha <tino.calancha@gmail.com> writes:

> * lisp/image.el (image-rotate):
> Add argument ANGLE, the angle in degrees for the rotation.
> Add optional argument _ARG; in interactive calls, a non-nil
> value prompt for ANGLE.

[...]

> +(defun image-rotate (angle &optional _arg)
> +  "Rotate the image under point by ANGLE degrees clockwise.
> +If ANGLE is a negative number, then rotate counterclockwise.
> +When called interactively with a prefix argument, prompt for ANGLE."
> +  (interactive
> +   (let* ((ask current-prefix-arg)
> +          (default 90)
> +          (prompt "Rotate image by ANGLE degrees: ")
> +          (rotation (if ask
> +                        (read-number prompt default)
> +                      default)))
> +     (list rotation ask)))

I don't understand this code -- why are you binding the unused argument
_arg to the value of current-prefix-arg?

Anyway, I don't really see the use case here: The only reason somebody
(in real life) has for changing the displayed angle of an image is if
it's vertical instead of horizontal (and vice versa), so rotating by 90
degrees is the only thing that makes sense, I think.

Do you see a use case for changing the displayed angle of an image by 72
degrees?

> -(defun image-increase-size (n)
> -  "Increase the image size by a factor of N.

[...]

> +default is 20%.
> +When called interactively with a prefix argument, prompt for N."

I think this is a bad idea.  `+'/`-' are quick, simple commands for
changing the displayed size, and is not something that's done for
fine-tuning: It's not an editing command.

`5 +' increases the size by 50%, and that feels natural and pleasant.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: image-rotate: Accept angle as an argument

[Stefan Monnier]

> Do you see a use case for changing the displayed angle of an image by 72
> degrees?
[...]
> `5 +' increases the size by 50%, and that feels natural and pleasant.

Agreed.  If you want to prompt the user (for a fine-tuned parameter
value), do it when the user did just C-u (i.e. a non-numeric prefix).


        Stefan

Re: image-rotate: Accept angle as an argument

[Tino Calancha]

On Mon, 5 Sep 2016, Lars Ingebrigtsen wrote:

> Anyway, I don't really see the use case here: The only reason somebody
> (in real life) has for changing the displayed angle of an image is if
> it's vertical instead of horizontal (and vice versa), so rotating by 90
> degrees is the only thing that makes sense, I think.
>
> Do you see a use case for changing the displayed angle of an image by 72
> degrees?
I think not, but unfortunately i am not the right person to answer: i 
don't like photograpy.
You are right, these commands are fine.
OTOH, I read at the first line of the file that image.el is the image API: 
Why don't we provide a function 'image--change-orientation' and use it
in the 'image-rotate' implementation? (See patch below)
The new function playing similar role as 'image--change-size' in 
'image-increase-size'.
Then, users will keep using the same commands, but programs might
use 'image--change-orientation' if they need it.
This came to my mind after looking image-mode.el:
i guess the code in image-mode.el for the rotations/resizes would be
much simple using the API from image.el.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
commit e4621bb833ba0bf6b1d62800c1d03137458bc58e
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Tue Sep 6 01:24:05 2016 +0900

     Extract function from image-rotate accepting arbitrary angles

     * lisp/image.el (image--change-orientation): New defun.
     (image-rotate): Use it.

diff --git a/lisp/image.el b/lisp/image.el
index e1f52de..780305e 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -1011,9 +1011,12 @@ image--current-scaling
  (defun image-rotate ()
    "Rotate the image under point by 90 degrees clockwise."
    (interactive)
+  (image--change-orientation 90))
+
+(defun image--change-orientation (angle)
    (let ((image (image--get-imagemagick-and-warn)))
      (plist-put (cdr image) :rotation
-               (float (mod (+ (or (plist-get (cdr image) :rotation) 0) 90)
+               (float (mod (+ (or (plist-get (cdr image) :rotation) 0) angle)
                             ;; We don't want to exceed 360 degrees
                             ;; rotation, because it's not seen as valid
                             ;; in exif data.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

Repository revision: 62e4dc4660cb3b29cfffcad0639e51c7f382ced8

Re: image-rotate: Accept angle as an argument

[Lars Ingebrigtsen]

Tino Calancha <tino.calancha@gmail.com> writes:

> i guess the code in image-mode.el for the rotations/resizes would be
> much simple using the API from image.el.

Well, the image-mode rotation settings do a lot more, like keeping the
width/height from exceeding the frame width/height etc, so I don't think
there's much potential for reuse there.  Unless I'm misremembering what
all that stuff was about in image-mode; it's been a while since I looked
at it.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: image-rotate: Accept angle as an argument

[Tino Calancha]

On Mon, 5 Sep 2016, Lars Ingebrigtsen wrote:

> Tino Calancha <tino.calancha@gmail.com> writes:
>
>> i guess the code in image-mode.el for the rotations/resizes would be
>> much simple using the API from image.el.
>
> Well, the image-mode rotation settings do a lot more, like keeping the
> width/height from exceeding the frame width/height etc, so I don't think
> there's much potential for reuse there.  Unless I'm misremembering what
> all that stuff was about in image-mode; it's been a while since I looked
> at it.
I noticed a recent change in image.c broke something in image-mode.  While
trying to fix it i realized that the fix is much simple if image-mode use
more the API in image.el that it currently does.

Maybe it's a good time to look at these two files simultaneously.