Mercurial > emacs

--- a/README.imagemagick	Mon Jun 14 15:57:48 2010 +0200
+++ b/README.imagemagick	Mon Jun 14 23:27:03 2010 +0200
@@ -15,18 +15,45 @@


 * TODO
-** complete documentation drafts below
+#B _ complete documentation drafts below

-** fix inconsistencys with spelling of imagemagick in the src
-** report number of images in image bundle types somehow
-** probably add pdf to inhibited types
-** inhibit types is defconst should probably be defcustom
-** decide what to do with some uncommitted imagemagick support
+#B X fix inconsistencys with spelling of imagemagick in the src
+#B X report number of images in image bundle types somehow
+Works like for "gif" support. Thanks to Juri Linkov.
+#B X probably add pdf to inhibited types
+#B X inhibit types is defconst should probably be defcustom
+#B _ decide what to do with some uncommitted imagemagick support
   functions for image size etc.
-** Test with more systems
+#B _ Test with more systems
 Tested on Fedora 12 so far, and the libmagick that ships with it.
 Ubuntu 8.04 was also tested, but it seems it ships a broken ImageMagick.
-Also need some way to handle render methods that only work on newer ImageMagicks
+#B X Also need some way to handle render methods that only work on newer ImageMagicks
+Is handled by configure now
+
+* Some nits from Stefan Monnier
+I just took a quick look at the code and I see the following nits to fix:
+#B _ obviously a merge will have to come with a good ChangeLog.
+#B X also the merge will need to come with documentation.  Maybe not in the
+   Texinfo form yet, but at least in the etc/NEWS with enough info that
+   describes the `scale' and other such arguments that someone can start
+   using them.
+#B X the README talks about naming inconsistencies, I think these should be
+  fixed before a first commit (should be straightforward).
+
+#B X the "let" in image.el should not be followed by a line break and the while
+   should be replaced by a dolist.
+
+#B X the prototype of imagemagick_load_image has some odd indentation in ([[2010.06.14]])
+   its args, not sure what happened.
+#B X a few lines in the C code break the 80columns limit.
+#B X please use ANSI style function declarations rather than K&R for new code. ([[2010.06.14]])
+#B X you can get rid of the prototypes by reordering the code. ([[2010.06.14]])
+#B X the docstrings in DEFUN should not be indented (they'll display ([[2010.06.14]])
+   weirdly otherwise in C-h f).
+#B X Some "{" are at the end of a for/if rather than on their own line. ([[2010.06.14]])
+#B X why use "*( imtypes + i)" rather than "imtypes[i]"? ([[2010.06.14]])
+#B X some "," lack a space after them. ([[2010.06.14]])
+#B X several "=" and "==" lack spaces around them. ([[2010.06.14]])


 * NEWS entry
@@ -44,7 +71,7 @@
 support for the extensions in imagemagick-types minus the types listed
 in imagemagick-types-inhibit.

-imagemagick-types-inhibit has the value  '(C HTML HTM TXT) by default.
+imagemagick-types-inhibit has the value  '(C HTML HTM TXT PDF) by default.
 This means imagemagick will be used also to load jpeg files, if you
 have both jpeg and imagemagick libraries linked. Add 'JPG to
 imagemagick-types-inhibit if you do not want this.
@@ -52,22 +79,36 @@
 Images loaded with imagemagick will support a couple of new display
 specifications:

-- scale
-- rotation
-- index
+- :scale takes a geometry string as defined by ImageMagick:

-See the ImageMagick manual for information on how these specifications
-work.
+scale%
+scale-x%xscale-y%
+width
+xheight
+widthxheight
+widthxheight^
+widthxheight!
+widthxheight>
+widthxheight<
+area@
+{size}{offset}                                                                                                                                                               {size}{+-}x{+-}y
+
+See the ImageMagick manual for more information.
+
+- :rotation specifies a rotation angle in degrees.
+
+- :index specifies which image inside an image bundle file format, such
+as TIFF or DJVM, to view.

 The image-metadata function can be used to retrieve the total number
-of images in an image bundle, and the :index property can be used to
-show a particular image within the bundle. This is simmilar to how GIF
-files work.
-
+of images in an image bundle. This is simmilar to how GIF files work.


 * Changelog entry
-nothing yet
+2010-06-12 Joakim Verona <joakim@verona.se>
+           * image.c
+           * image.el
+

 * Manual entry
-nothing yet
\ No newline at end of file
+nothing yet, but the NEWS entry could be adapted.
\ No newline at end of file
--- a/lisp/image.el	Mon Jun 14 15:57:48 2010 +0200
+++ b/lisp/image.el	Mon Jun 14 23:27:03 2010 +0200
@@ -694,8 +694,8 @@
 	(cons images tmo))))))


-(defconst imagemagick-types-inhibit
-  '(C HTML HTM TXT)
+(defcustom imagemagick-types-inhibit
+  '(C HTML HTM TXT PDF)
   "Types the imagemagick loader should not try to handle.")

 ;;;###autoload
--- a/src/image.c	Mon Jun 14 15:57:48 2010 +0200
+++ b/src/image.c	Mon Jun 14 23:27:03 2010 +0200
@@ -7645,21 +7645,28 @@
 #define DrawRectangle DrawRectangleGif
 #include <wand/MagickWand.h>

-/* imagemagick_load_image is a helper function for imagemagick_load, which does the
-   actual loading given contents and size, apart from frame and image
-   structures, passed from imagemagick_load.
+/* imagemagick_load_image is a helper function for imagemagick_load,
+   which does the actual loading given contents and size, apart from
+   frame and image structures, passed from imagemagick_load.

    Uses librimagemagick to do most of the image processing.

-   Returns non-zero when successful.
+    non-zero when successful.
 */

 static int
-imagemagick_load_image (struct frame *f,        /* Pointer to emacs frame structure.  */
-                        struct image *img,      /* Pointer to emacs image structure.  */
-                        unsigned char *contents,/* String containing the IMAGEMAGICK data to be parsed.  */
-                        unsigned int size,      /* Size of data in bytes.  */
-                        unsigned char *filename)/* Filename, either pass filename or contents/size.  */
+imagemagick_load_image (/* Pointer to emacs frame structure.  */
+                        struct frame *f,
+                        /* Pointer to emacs image structure.  */
+                        struct image *img,
+                        /* String containing the IMAGEMAGICK data to
+                           be parsed.  */
+                        unsigned char *contents,
+                        /* Size of data in bytes.  */
+                        unsigned int size,
+                        /* Filename, either pass filename or
+                           contents/size.  */
+                        unsigned char *filename)
 {
   long unsigned int width;
   long unsigned int height;
@@ -7716,17 +7723,17 @@


   if (MagickGetNumberImages(image_wand) > 1)
-    img->data.lisp_val = Fcons (Qcount,
-                                Fcons (make_number (MagickGetNumberImages(image_wand)),
-                                       img->data.lisp_val));
+    img->data.lisp_val =
+      Fcons (Qcount,
+             Fcons (make_number (MagickGetNumberImages(image_wand)),
+                    img->data.lisp_val));
   if(ino == 0)
     MagickSetFirstIterator(image_wand);
   else
     MagickSetIteratorIndex(image_wand, ino);

-  /*
-    If width and/or height is set in the display spec
-    assume we want to scale to those.  */
+  /* If width and/or height is set in the display spec assume we want
+    to scale to those.  */

   value = image_spec_value (img->spec, QCwidth, NULL);
   desired_width = (INTEGERP (value)  ? XFASTINT (value) : -1);
@@ -7742,22 +7749,25 @@
       }
     }

-  /* Also support :geometry and :crop which are imagemagick specific descriptors.  */
+  /* Also support :geometry and :crop which are imagemagick specific
+     descriptors.  */

   crop     = image_spec_value (img->spec, QCcrop, NULL);
   geometry = image_spec_value (img->spec, QCgeometry, NULL);
   if (STRINGP (crop) && STRINGP (geometry))
     {
       printf("MagickTransformImage %s %s\n", SDATA(crop), SDATA(geometry));
-      image_wand = MagickTransformImage (image_wand, SDATA (crop), SDATA (geometry));
+      image_wand = MagickTransformImage (image_wand, SDATA (crop),
+                                         SDATA (geometry));
       /* TODO differ between image_wand and transform_wand. */
     }

-  /* Furthermore :rotation. we need background color and angle for rotation.  */
+  /* Furthermore :rotation. we need background color and angle for
+     rotation.  */
   /*
-    TODO background handling for rotation
-    specified_bg = image_spec_value (img->spec, QCbackground, NULL);
-    if (!STRINGP (specified_bg)
+    TODO background handling for rotation specified_bg =
+    image_spec_value (img->spec, QCbackground, NULL); if (!STRINGP
+    (specified_bg)
   */
   value = image_spec_value (img->spec, QCrotation, NULL);
   if (FLOATP (value))
@@ -7799,11 +7809,13 @@


   init_color_table ();
-  imagemagick_rendermethod = (INTEGERP (Vimagemagick_render_type) ? XFASTINT (Vimagemagick_render_type) : 0);
+  imagemagick_rendermethod = (INTEGERP (Vimagemagick_render_type)
+                              ? XFASTINT (Vimagemagick_render_type) : 0);
   if (imagemagick_rendermethod == 0)
     {
       /* Try to create a x pixmap to hold the imagemagick pixmap.  */
-      if (!x_create_x_image_and_pixmap (f, width, height, 0, &ximg, &img->pixmap))
+      if (!x_create_x_image_and_pixmap (f, width, height, 0,
+                                        &ximg, &img->pixmap))
         {
           image_error("Imagemagick X bitmap allocation failure", Qnil, Qnil);
           goto imagemagick_error;
@@ -7820,7 +7832,8 @@
       iterator = NewPixelIterator (image_wand);
       if ((iterator == (PixelIterator *) NULL))
         {
-          image_error ("Imagemagick pixel iterator creation failed", Qnil, Qnil);
+          image_error ("Imagemagick pixel iterator creation failed",
+                       Qnil, Qnil);
           goto imagemagick_error;
         }

@@ -7832,7 +7845,11 @@
           for (x = 0; x < (long) width; x++)
             {
               PixelGetMagickColor (pixels[x], &pixel);
-              XPutPixel (ximg, x, y, lookup_rgb_color (f, pixel.red, pixel.green, pixel.blue));
+              XPutPixel (ximg, x, y,
+                         lookup_rgb_color (f,
+                                           pixel.red,
+                                           pixel.green,
+                                           pixel.blue));
             }
         }
       DestroyPixelIterator (iterator);
@@ -7841,13 +7858,16 @@
   if (imagemagick_rendermethod == 1)
     {
       /* Try if magicexportimage is any faster than pixelpushing. */
-      /* printf("ximg: bitmap_unit:%d format:%d byte_order:%d depth:%d bits_per_pixel:%d\n", */
-      /*        ximg->bitmap_unit,ximg->format,ximg->byte_order,ximg->depth,ximg->bits_per_pixel); */
+      /* printf("ximg: bitmap_unit:%d format:%d byte_order:%d depth:%d
+         bits_per_pixel:%d\n", */
+      /*        ximg->bitmap_unit,ximg->format,ximg->byte_order,
+                ximg->depth,ximg->bits_per_pixel); */
       int imagedepth = 24;/*MagickGetImageDepth(image_wand);*/
       char* exportdepth = imagedepth <= 8 ? "I" : "BGRP";/*"RGBP";*/
       /* Try to create a x pixmap to hold the imagemagick pixmap.  */
       printf("imagedepth:%d exportdepth:%s\n", imagedepth, exportdepth);
-      if (!x_create_x_image_and_pixmap (f, width, height, imagedepth, &ximg, &img->pixmap)){
+      if (!x_create_x_image_and_pixmap (f, width, height, imagedepth,
+                                        &ximg, &img->pixmap)){
         image_error("Imagemagick X bitmap allocation failure", Qnil, Qnil);
         goto imagemagick_error;
       }
@@ -7884,7 +7904,7 @@
                               /*&(img->pixmap));*/
                               ximg->data);
 #else
-      image_error("You dont have MagickExportImagePixels, upgrade ImageMagick if you want to try it!",
+      image_error("You dont have MagickExportImagePixels, upgrade ImageMagick!",
                   Qnil, Qnil);
 #endif
     }
@@ -7957,7 +7977,8 @@
       /* contents = slurp_file (SDATA (file), &size); */
       /* if (contents == NULL) */
       /*   { */
-      /*     image_error ("Error loading IMAGEMAGICK image `%s'", img->spec, Qnil); */
+      /*     image_error ("Error loading IMAGEMAGICK image `%s'",
+             img->spec, Qnil); */
       /*     UNGCPRO; */
       /*     return 0; */
       /*   } */
@@ -7972,21 +7993,24 @@
       Lisp_Object data;

       data = image_spec_value (img->spec, QCdata, NULL);
-      success_p = imagemagick_load_image (f, img, SDATA (data), SBYTES (data), NULL);
+      success_p = imagemagick_load_image (f, img, SDATA (data),
+                                          SBYTES (data), NULL);
     }

   return success_p;
 }

-/* Structure describing the image type `imagemagick'.  Its the same type of
-   structure defined for all image formats, handled by Emacs image
-   functions.  See struct image_type in dispextern.h.  */
+/* Structure describing the image type `imagemagick'.  Its the same
+   type of structure defined for all image formats, handled by Emacs
+   image functions.  See struct image_type in dispextern.h.  */

 static struct image_type imagemagick_type =
   {
-    /* An identifier showing that this is an image structure for the IMAGEMAGICK format.  */
+    /* An identifier showing that this is an image structure for the
+       IMAGEMAGICK format.  */
     &Qimagemagick,
-    /* Handle to a function that can be used to identify a IMAGEMAGICK file.  */
+    /* Handle to a function that can be used to identify a IMAGEMAGICK
+       file.  */
     imagemagick_image_p,
     /* Handle to function used to load a IMAGEMAGICK file.  */
     imagemagick_load,
@@ -8002,8 +8026,8 @@

 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.  */)
+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.  */)
   ()
 {
   Lisp_Object typelist = Qnil;
@@ -8824,7 +8848,8 @@
   if (EQ (type, Qimagemagick)){
     /* MagickWandGenesis() initalizes the imagemagick library.  */
     MagickWandGenesis();
-    return CHECK_LIB_AVAILABLE (&imagemagick_type, init_imagemagick_functions, libraries);
+    return CHECK_LIB_AVAILABLE (&imagemagick_type, init_imagemagick_functions,
+                                libraries);
   }
 #endif