Cleanups to the ImageMagick code and docstrings.

* lisp/image-mode.el (image-toggle-display-image): Signal an error if
not in Image mode.
(image-transform-mode, image-transform-resize)
(image-transform-set-rotation): Doc fix.
(image-transform-set-resize): Deleted.
(image-transform-set-scale, image-transform-fit-to-height)
(image-transform-fit-to-width): Handle image-toggle-display-image
and image-transform-resize directly.

* src/image.c (Fimagemagick_types): Doc fix, and comment cleanup.

4 files changed, 102 insertions, 90 deletions

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 8d705cdaed6..402b078be7b 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,14 @@
+2011-04-09  Chong Yidong  <cyd@stupidchicken.com>
+
+	* image-mode.el (image-toggle-display-image): Signal an error if
+	not in Image mode.
+	(image-transform-mode, image-transform-resize)
+	(image-transform-set-rotation): Doc fix.
+	(image-transform-set-resize): Deleted.
+	(image-transform-set-scale, image-transform-fit-to-height)
+	(image-transform-fit-to-width): Handle image-toggle-display-image
+	and image-transform-resize directly.
+
 2011-04-08  Sho Nakatani <lay.sakura@gmail.com>
 
 	* doc-view.el (doc-view-fit-width-to-window)
diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 64dcf9076ae..96d874dbec6 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -469,6 +469,8 @@ Remove text properties that display the image."
   "Show the image of the image file.
 Turn the image data into a real image, but only if the whole file
 was inserted."
+  (unless (derived-mode-p 'image-mode major-mode)
+    (error "The buffer is not in Image mode"))
   (let* ((filename (buffer-file-name))
 	 (data-p (not (and filename
 			   (file-readable-p filename)
@@ -485,8 +487,7 @@ was inserted."
 	 (type (image-type file-or-data nil data-p))
          (image0 (create-animated-image file-or-data type data-p))
 	 (image (append image0
-                        (image-transform-properties image0)
-                        ))
+                        (image-transform-properties image0)))
 	 (props
 	  `(display ,image
 		    intangible ,image
@@ -557,80 +558,86 @@ the image file and `image-mode' showing the image as an image."
 
 (defvar image-transform-minor-mode-map
   (let ((map (make-sparse-keymap)))
-;    (define-key map  [(control ?+)] 'image-scale-in)
-;    (define-key map  [(control ?-)] 'image-scale-out)
-;    (define-key map  [(control ?=)] 'image-scale-none)
-;;    (define-key map "c f h" 'image-scale-fit-height)
-;;    (define-key map "c ]" 'image-rotate-right)
+    ;; (define-key map  [(control ?+)] 'image-scale-in)
+    ;; (define-key map  [(control ?-)] 'image-scale-out)
+    ;; (define-key map  [(control ?=)] 'image-scale-none)
+    ;; (define-key map "c f h" 'image-scale-fit-height)
+    ;; (define-key map "c ]" 'image-rotate-right)
     map)
-  "Minor mode keymap for transforming the view of images Image mode.")
+  "Minor mode keymap `image-transform-mode'.")
 
 (define-minor-mode image-transform-mode
-  "minor mode for scaleing and rotation"
-  nil "image-transform"
-  image-transform-minor-mode-map)
-
-(defvar image-transform-resize   nil
-  "The image resize operation. See the command
-  `image-transform-set-scale' for more information." )
+  "Minor mode for scaling and rotating images.
+This minor mode has no effect unless Emacs is compiled with
+ImageMagick support."
+  nil "image-transform" image-transform-minor-mode-map)
+
+(defvar image-transform-resize nil
+  "The image resize operation.
+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.
+ - A number, which is a scale factor (the default size is 100).")
 
 (defvar image-transform-rotation 0.0)
 
-
 (defun image-transform-properties (display)
-  "Calculate the display properties for transformations; scaling
-and rotation. "
-  (let*
-      ((size (image-size display t))
-       (height
-        (cond
-         ((and (numberp image-transform-resize) (eq 100 image-transform-resize))
-          nil)
-         ((numberp image-transform-resize)
-          (* image-transform-resize (cdr size)))
-         ((eq image-transform-resize 'fit-height)
-          (- (nth 3 (window-inside-pixel-edges)) (nth 1 (window-inside-pixel-edges))))
-         (t nil)))
-       (width (if (eq image-transform-resize 'fit-width)
-                  (- (nth 2 (window-inside-pixel-edges)) (nth 0 (window-inside-pixel-edges))))))
-
+  "Rescale and/or rotate the current image.
+The scale factor and rotation angle are given by the variables
+`image-transform-resize' and `image-transform-rotation'.  This
+takes effect only if Emacs is compiled with ImageMagick support."
+  (let* ((size (image-size display t))
+	 (height
+	  (cond
+	   ((numberp image-transform-resize)
+	    (unless (= image-transform-resize 100)
+	      (* image-transform-resize (cdr size))))
+	   ((eq image-transform-resize 'fit-height)
+	    (- (nth 3 (window-inside-pixel-edges))
+	       (nth 1 (window-inside-pixel-edges))))))
+	 (width (if (eq image-transform-resize 'fit-width)
+		    (- (nth 2 (window-inside-pixel-edges))
+		       (nth 0 (window-inside-pixel-edges))))))
+    ;;TODO fit-to-* should consider the rotation angle
     `(,@(if height (list :height height))
       ,@(if width (list :width width))
       ,@(if (not (equal 0.0 image-transform-rotation))
-            (list :rotation image-transform-rotation))
-      ;;TODO fit-to-* should consider the rotation angle
-      )))
+            (list :rotation image-transform-rotation)))))
 
 (defun image-transform-set-scale (scale)
-  "SCALE sets the scaling for images. "
-  (interactive "nscale:")
-  (image-transform-set-resize (float scale)))
+  "Prompt for a number, and resize the current image by that amount.
+This command has no effect unless Emacs is compiled with
+ImageMagick support."
+  (interactive "nScale: ")
+  (setq image-transform-resize resize)
+  (image-toggle-display-image))
 
 (defun image-transform-fit-to-height ()
-  "Fit image height to window height. "
+  "Fit the current image to the height of the current window.
+This command has no effect unless Emacs is compiled with
+ImageMagick support."
   (interactive)
-  (image-transform-set-resize 'fit-height))
+  (setq image-transform-resize 'fit-height)
+  (image-toggle-display-image))
 
 (defun image-transform-fit-to-width ()
-  "Fit image width to window width. "
+  "Fit the current image to the width of the current window.
+This command has no effect unless Emacs is compiled with
+ImageMagick support."
   (interactive)
-  (image-transform-set-resize 'fit-width))
-
-(defun image-transform-set-resize (resize)
-  "Set the resize mode for images. The RESIZE value can be the
-symbol fit-height which fits the image to the window height. The
-symbol fit-width fits the image to the window width.  A number
-indicates a scaling factor. nil indicates scale to 100%. "
-  (setq image-transform-resize resize)
-  (if (eq 'image-mode major-mode) (image-toggle-display-image)))
+  (setq image-transform-resize 'fit-width)
+  (image-toggle-display-image))
 
 (defun image-transform-set-rotation (rotation)
-  "Set the image ROTATION angle. "
-  (interactive "nrotation:")
+  "Prompt for an angle ROTATION, and rotate the image by that amount.
+ROTATION should be in degrees.  This command has no effect unless
+Emacs is compiled with ImageMagick support."
+  (interactive "nRotation angle (in degrees): ")
   ;;TODO 0 90 180 270 degrees are the only reasonable angles here
   ;;otherwise combining with rescaling will get very awkward
   (setq image-transform-rotation (float rotation))
-  (if (eq major-mode 'image-mode) (image-toggle-display-image)))
+  (image-toggle-display-image))
 
 (provide 'image-mode)
 
diff --git a/src/ChangeLog b/src/ChangeLog
index e7ea07705af..a1a5abe87c3 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,9 @@
 2011-04-09  Chong Yidong  <cyd@stupidchicken.com>
 
+	* image.c (Fimagemagick_types): Doc fix, and comment cleanup.
+
+2011-04-09  Chong Yidong  <cyd@stupidchicken.com>
+
 	* ftfont.c (get_adstyle_property, ftfont_pattern_entity): Use
 	unsigned char, to match FcChar8 type definition.
 
diff --git a/src/image.c b/src/image.c
index 260bc6eb260..b3d2be88b96 100644
--- a/src/image.c
+++ b/src/image.c
@@ -7370,14 +7370,13 @@ gif_load (struct frame *f, struct image *img)
 
 
 /***********************************************************************
-				 imagemagick
+				 ImageMagick
 ***********************************************************************/
 #if defined (HAVE_IMAGEMAGICK)
 
-/* The symbol `imagemagick' identifying images of this type.  */
-
 Lisp_Object Qimagemagick;
-/* Indices of image specification fields in imagemagick_format, below.  */
+
+/* Indices of image specification fields in imagemagick_format.  */
 
 enum imagemagick_keyword_index
   {
@@ -7418,6 +7417,7 @@ static struct image_keyword imagemagick_format[IMAGEMAGICK_LAST] =
     {":rotation",	IMAGE_NUMBER_VALUE,     		0},
     {":crop",		IMAGE_DONT_CHECK_VALUE_TYPE,		0}
   };
+
 /* Free X resources of imagemagick image IMG which is used on frame F.  */
 
 static void
@@ -7427,8 +7427,6 @@ imagemagick_clear_image (struct frame *f,
   x_clear_image (f, img);
 }
 
-
-
 /* Return non-zero if OBJECT is a valid IMAGEMAGICK image specification.  Do
    this by calling parse_image_spec and supplying the keywords that
    identify the IMAGEMAGICK format.   */
@@ -7457,7 +7455,7 @@ imagemagick_image_p (Lisp_Object object)
 
    Uses librimagemagick to do most of the image processing.
 
-   non-zero when successful.
+   Return non-zero if successful.
 */
 
 static int
@@ -7504,12 +7502,12 @@ imagemagick_load_image (/* Pointer to emacs frame structure.  */
   Image * im_image;
 
 
-  /* Handle image index for image types who can contain more than one
-     image.  Interface :index is same as for GIF.  First we "ping" the
-     image to see how many sub-images it contains. Pinging is faster
-     than loading the image to find out things about it.  */
+  /* Handle image index for image types who can contain more than one image.
+     Interface :index is same as for GIF.  First we "ping" the image to see how
+     many sub-images it contains.  Pinging is faster than loading the image to
+     find out things about it.  */
 
-  /* `MagickWandGenesis' initializes the imagemagick environment.  */
+  /* Initialize the imagemagick environment.  */
   MagickWandGenesis ();
   image = image_spec_value (img->spec, QCindex, NULL);
   ino = INTEGERP (image) ? XFASTINT (image) : 0;
@@ -7541,7 +7539,7 @@ imagemagick_load_image (/* Pointer to emacs frame structure.  */
   DestroyMagickWand (ping_wand);
 
   /* Now, after pinging, we know how many images are inside the
-     file. If its not a bundle, just one.  */
+     file.  If it's not a bundle, the number is one.  */
 
   if (filename != NULL)
     {
@@ -7572,7 +7570,7 @@ imagemagick_load_image (/* Pointer to emacs frame structure.  */
   if (status == MagickFalse) goto imagemagick_error;
 
   /* If width and/or height is set in the display spec assume we want
-     to scale to those values.  if either h or w is unspecified, the
+     to scale to those values.  If either h or w is unspecified, the
      unspecified should be calculated from the specified to preserve
      aspect ratio.  */
 
@@ -7584,17 +7582,13 @@ imagemagick_load_image (/* Pointer to emacs frame structure.  */
   height = MagickGetImageHeight (image_wand);
   width = MagickGetImageWidth (image_wand);
 
-  if(desired_width != -1 && desired_height == -1)
-    {
-      /* w known, calculate h.  */
-      desired_height = (double) desired_width / width * height;
-    }
-  if(desired_width == -1 && desired_height != -1)
-    {
-      /* h known, calculate w.  */
-      desired_width = (double) desired_height / height * width;
-    }
-  if(desired_width != -1 && desired_height != -1)
+  if (desired_width != -1 && desired_height == -1)
+    /* w known, calculate h.  */
+    desired_height = (double) desired_width / width * height;
+  if (desired_width == -1 && desired_height != -1)
+    /* h known, calculate w.  */
+    desired_width = (double) desired_height / height * width;
+  if (desired_width != -1 && desired_height != -1)
     {
       status = MagickScaleImage (image_wand, desired_width, desired_height);
       if (status == MagickFalse)
@@ -7604,19 +7598,17 @@ imagemagick_load_image (/* Pointer to emacs frame structure.  */
 	}
     }
 
-
   /* crop behaves similar to image slicing in Emacs but is more memory
      efficient.  */
   crop = image_spec_value (img->spec, QCcrop, NULL);
 
   if (CONSP (crop) && INTEGERP (XCAR (crop)))
     {
-      /* After some testing, it seems MagickCropImage is the fastest
-         crop function in ImageMagick.  This crop function seems to do
-         less copying than the alternatives, but it still reads the
-         entire image into memory before croping, which is aparently
-         difficult to avoid when using imagemagick.  */
-
+      /* After some testing, it seems MagickCropImage is the fastest crop
+         function in ImageMagick.  This crop function seems to do less copying
+         than the alternatives, but it still reads the entire image into memory
+         before croping, which is aparently difficult to avoid when using
+         imagemagick.  */
       int w, h, x, y;
       w = XFASTINT (XCAR (crop));
       crop = XCDR (crop);
@@ -7877,12 +7869,10 @@ static struct image_type imagemagick_type =
   };
 
 
-
-
 DEFUN ("imagemagick-types", Fimagemagick_types, Simagemagick_types, 0, 0, 0,
-       doc: /* Return image file types supported by ImageMagick.
-Since ImageMagick recognizes a lot of file-types that clash with Emacs,
-such as .c, we want to be able to alter the list at the lisp level.  */)
+       doc: /* Return the image types supported by ImageMagick.
+Note that ImageMagick recognizes many file-types that Emacs does not recognize
+as images, such as .c.  */)
   (void)
 {
   Lisp_Object typelist = Qnil;