Native image rotation

Native image rotation

[Alan Third]

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

I’ve attached a couple of patches. The second renames image scaling to
image transforms, as it covers more than just scaling.

It should be trivial to add cropping now. I can’t think of anything
else we might want to support.

Again I’ve not handled Windows, but hopefully it’s quite
straight‐forward.

The image rotation code is very explicit in what it’s doing and could
be optimized, but I’m not sure it matters.
-- 
Alan Third

[-- Attachment #2: 0001-Add-native-image-rotation.patch --]
[-- Type: text/plain, Size: 17135 bytes --]

From 1b1dbf1d77f40f09756c4d677dfdc4bf3e44952d Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 23 Feb 2019 20:56:48 +0000
Subject: [PATCH 1/2] Add native image rotation

* src/dispextern.h (INIT_MATRIX, COPY_MATRIX, MULT_MATRICES): New
macros for matrix manipulation.
* src/image.c (x_set_image_rotation):
(x_set_transform): New functions.
(x_set_image_size): Use transform matrix for resizing under X and NS.
(lookup_image): Use the new transform functions.
* src/nsimage.m (ns_load_image): Remove rotation code.
(ns_image_set_transform): New function.
([EmacsImage dealloc]): Release the saved transform.
([EmacsImage rotate:]): Remove unneeded method.
([EmacsImage setTransform:]): New method.
* src/nsterm.h (EmacsImage): Add transform property and update method
definitions.
* src/nsterm.m (ns_dumpglyphs_image): Use the transform to draw the
image correctly.
* src/xterm.c (x_composite_image): Clear under an image before
compositing it.
* doc/lispref/display.texi (Image Descriptors): Add :rotation.
(ImageMagick Images): Remove :rotation.
---
 doc/lispref/display.texi |   6 +-
 etc/NEWS                 |   2 +-
 src/dispextern.h         |  18 +++++
 src/image.c              | 149 +++++++++++++++++++++++++++++++++------
 src/nsimage.m            |  64 +++++------------
 src/nsterm.h             |   5 +-
 src/nsterm.m             |  41 +++++++----
 src/xterm.c              |   5 ++
 8 files changed, 204 insertions(+), 86 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 95379b342b..b0f1fd2676 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5157,6 +5157,9 @@ Image Descriptors
 specified, the height/width will be adjusted by the specified scaling
 factor.
 
+@item :rotation @var{angle}
+Specifies a rotation angle in degrees.
+
 @item :index @var{frame}
 @xref{Multi-Frame Images}.
 
@@ -5450,9 +5453,6 @@ ImageMagick Images
 image data, as found in @code{image-format-suffixes}.  This is used
 when the image does not have an associated file name, to provide a
 hint to ImageMagick to help it detect the image type.
-
-@item :rotation @var{angle}
-Specifies a rotation angle in degrees.
 @end table
 
 @node SVG Images
diff --git a/etc/NEWS b/etc/NEWS
index 0cafbaae96..3a50a51637 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1569,7 +1569,7 @@ buffer's 'default-directory' and invoke that file name handler to make
 the process.  That way 'make-process' can start remote processes.
 
 +++
-** Emacs now supports resizing (scaling) of images without ImageMagick.
+** Emacs now supports resizing and rotating images without ImageMagick.
 All modern systems are supported by this feature.  (On GNU and Unix
 systems, the XRender extension to X11 is required for this to be
 available; the configure script will test for it and, if found, enable
diff --git a/src/dispextern.h b/src/dispextern.h
index 894753669d..d45bc03d09 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -2940,6 +2940,24 @@ struct redisplay_interface
 
 # if defined HAVE_XRENDER || defined HAVE_NS || defined HAVE_NTGUI
 #  define HAVE_NATIVE_SCALING
+
+#  define INIT_MATRIX(m)                          \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++)                 \
+      m[i][j] = (i == j) ? 1 : 0;
+
+#  define COPY_MATRIX(a, b)                       \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++)                 \
+      b[i][j] = a[i][j];
+
+#  define MULT_MATRICES(a, b, result)             \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++) {               \
+      double sum = 0;                             \
+      for (int k = 0 ; k < 3 ; k++)               \
+        sum += a[k][j] * b[i][k];                 \
+      result[i][j] = sum;}
 # endif
 
 /* Structure describing an image.  Specific image formats like XBM are
diff --git a/src/image.c b/src/image.c
index 642bf67152..f48db69a24 100644
--- a/src/image.c
+++ b/src/image.c
@@ -56,6 +56,10 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include <sys/types.h>
 #endif /* HAVE_SYS_TYPES_H */
 
+#ifdef HAVE_NATIVE_SCALING
+#include <math.h>
+#endif
+
 #ifdef HAVE_WINDOW_SYSTEM
 #include TERM_HEADER
 #endif /* HAVE_WINDOW_SYSTEM */
@@ -1863,43 +1867,111 @@ compute_image_size (size_t width, size_t height,
 #endif /* HAVE_IMAGEMAGICK || HAVE_NATIVE_SCALING */
 
 static void
-x_set_image_size (struct frame *f, struct image *img)
+x_set_image_rotation (struct image *img, double tm[3][3])
 {
 #ifdef HAVE_NATIVE_SCALING
 # ifdef HAVE_IMAGEMAGICK
-  /* ImageMagick images are already the correct size.  */
+  /* ImageMagick images are already rotated.  */
   if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
     return;
 # endif
 
+# ifdef HAVE_XRENDER
+  if (!img->picture)
+    return;
+# endif
+
+  Lisp_Object value;
+  double rotation = 0;
+  double t[3][3], rot[3][3], tmp[3][3], tmp2[3][3];
   int width, height;
-  compute_image_size (img->width, img->height, img->spec, &width, &height);
 
-# ifdef HAVE_NS
-  ns_image_set_size (img->pixmap, width, height);
+  value = image_spec_value (img->spec, QCrotation, NULL);
+  if (NUMBERP (value))
+    rotation = M_PI * XFLOATINT (value) / 180; /* radians */
+
+  if (rotation == 0)
+    return;
+
+  if (rotation > 3 * M_PI && rotation <= M_PI)
+    {
+      width = fabs (img->height * cos (rotation))
+        + fabs (img->width * sin (rotation));
+      height = fabs (img->height * sin (rotation))
+        + fabs (img->width * cos (rotation));
+    }
+  else
+    {
+      width = fabs (img->height * sin (rotation))
+        + fabs (img->width * cos (rotation));
+      height = fabs (img->height * cos (rotation))
+        + fabs (img->width * sin (rotation));
+    }
+
+  /* Translate so (0, 0) is in the centre of the image.  */
+  INIT_MATRIX (t);
+  t[2][0] = img->width/2;
+  t[2][1] = img->height/2;
+
+  MULT_MATRICES (tm, t, tmp);
+
+  /* Rotate.  */
+  INIT_MATRIX (rot);
+  rot[0][0] = cos (rotation);
+  rot[1][0] = sin (rotation);
+  rot[0][1] = - sin (rotation);
+  rot[1][1] = cos (rotation);
+
+  MULT_MATRICES (tmp, rot, tmp2);
+
+  /* Translate back.  */
+  INIT_MATRIX (t);
+  t[2][0] = - width/2;
+  t[2][1] = - height/2;
+
+  MULT_MATRICES (tmp2, t, tm);
+
   img->width = width;
   img->height = height;
+#endif
+}
+
+static void
+x_set_image_size (struct image *img, double tm[3][3])
+{
+#ifdef HAVE_NATIVE_SCALING
+# ifdef HAVE_IMAGEMAGICK
+  /* ImageMagick images are already the correct size.  */
+  if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
+    return;
 # endif
 
 # ifdef HAVE_XRENDER
-  if (img->picture)
-    {
-      double xscale = img->width / (double) width;
-      double yscale = img->height / (double) height;
+  if (!img->picture)
+    return;
+# endif
 
-      XTransform tmat
-	= {{{XDoubleToFixed (xscale), XDoubleToFixed (0), XDoubleToFixed (0)},
-	    {XDoubleToFixed (0), XDoubleToFixed (yscale), XDoubleToFixed (0)},
-	    {XDoubleToFixed (0), XDoubleToFixed (0), XDoubleToFixed (1)}}};
+  double rm[3][3], tmp[3][3];
+  double xscale, yscale;
+  int width, height;
 
-      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
-			       0, 0);
-      XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+  compute_image_size (img->width, img->height, img->spec, &width, &height);
 
-      img->width = width;
-      img->height = height;
-    }
+  xscale = img->width / (double) width;
+  yscale = img->height / (double) height;
+
+# if defined (HAVE_NS) || defined (HAVE_XRENDER)
+  INIT_MATRIX (rm);
+  rm[0][0] = xscale;
+  rm[1][1] = yscale;
+
+  MULT_MATRICES (tm, rm, tmp);
+  COPY_MATRIX (tmp, tm);
+
+  img->width = width;
+  img->height = height;
 # endif
+
 # ifdef HAVE_NTGUI
   /* Under HAVE_NTGUI, we will scale the image on the fly, when we
      draw it.  See w32term.c:x_draw_image_foreground.  */
@@ -1909,6 +1981,35 @@ x_set_image_size (struct frame *f, struct image *img)
 #endif
 }
 
+static void
+x_set_transform (struct frame *f, struct image *img, double matrix[3][3])
+{
+#ifdef HAVE_NATIVE_SCALING
+# if defined (HAVE_NS)
+  /* Under NS the transform is applied to the drawing surface at
+     drawing time, so store it for later.  */
+  ns_image_set_transform (img->pixmap, matrix);
+# elif defined (HAVE_XRENDER)
+  if (img->picture)
+    {
+      XTransform tmat
+	= {{{XDoubleToFixed (matrix[0][0]),
+             XDoubleToFixed (matrix[1][0]),
+             XDoubleToFixed (matrix[2][0])},
+	    {XDoubleToFixed (matrix[0][1]),
+             XDoubleToFixed (matrix[1][1]),
+             XDoubleToFixed (matrix[2][1])},
+	    {XDoubleToFixed (matrix[0][2]),
+             XDoubleToFixed (matrix[1][2]),
+             XDoubleToFixed (matrix[2][2])}}};
+
+      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
+			       0, 0);
+      XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+    }
+# endif
+#endif
+}
 
 /* Return the id of image with Lisp specification SPEC on frame F.
    SPEC must be a valid Lisp image specification (see valid_image_p).  */
@@ -1964,7 +2065,15 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	     `:background COLOR'.  */
 	  Lisp_Object ascent, margin, relief, bg;
 	  int relief_bound;
-          x_set_image_size (f, img);
+
+#ifdef HAVE_NATIVE_SCALING
+          double transform_matrix[3][3];
+
+          INIT_MATRIX (transform_matrix);
+          x_set_image_rotation (img, transform_matrix);
+          x_set_image_size (img, transform_matrix);
+          x_set_transform (f, img, transform_matrix);
+#endif
 
 	  ascent = image_spec_value (spec, QCascent, NULL);
 	  if (FIXNUMP (ascent))
diff --git a/src/nsimage.m b/src/nsimage.m
index f16910de08..3a5dd24185 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -76,9 +76,8 @@ Updated by Christian Limpach (chris@nice.ch)
 {
   EmacsImage *eImg = nil;
   NSSize size;
-  Lisp_Object lisp_index, lisp_rotation;
+  Lisp_Object lisp_index;
   unsigned int index;
-  double rotation;
 
   NSTRACE ("ns_load_image");
 
@@ -87,9 +86,6 @@ Updated by Christian Limpach (chris@nice.ch)
   lisp_index = Fplist_get (XCDR (img->spec), QCindex);
   index = FIXNUMP (lisp_index) ? XFIXNAT (lisp_index) : 0;
 
-  lisp_rotation = Fplist_get (XCDR (img->spec), QCrotation);
-  rotation = NUMBERP (lisp_rotation) ? XFLOATINT (lisp_rotation) : 0;
-
   if (STRINGP (spec_file))
     {
       eImg = [EmacsImage allocInitFromFile: spec_file];
@@ -119,13 +115,6 @@ Updated by Christian Limpach (chris@nice.ch)
 
   img->lisp_data = [eImg getMetadata];
 
-  if (rotation != 0)
-    {
-      EmacsImage *temp = [eImg rotate:rotation];
-      [eImg release];
-      eImg = temp;
-    }
-
   size = [eImg size];
   img->width = size.width;
   img->height = size.height;
@@ -155,6 +144,12 @@ Updated by Christian Limpach (chris@nice.ch)
   [(EmacsImage *)img setSize:NSMakeSize (width, height)];
 }
 
+void
+ns_image_set_transform (void *img, double m[3][3])
+{
+  [(EmacsImage *)img setTransform:m];
+}
+
 unsigned long
 ns_get_pixel (void *img, int x, int y)
 {
@@ -225,6 +220,7 @@ - (void)dealloc
 {
   [stippleMask release];
   [bmRep release];
+  [transform release];
   [super dealloc];
 }
 
@@ -528,42 +524,16 @@ - (BOOL)setFrame: (unsigned int) index
   return YES;
 }
 
-- (instancetype)rotate: (double)rotation
+- (void)setTransform: (double[3][3]) m
 {
-  EmacsImage *new_image;
-  NSPoint new_origin;
-  NSSize new_size, size = [self size];
-  NSRect rect = { NSZeroPoint, [self size] };
-
-  /* Create a bezier path of the outline of the image and do the
-   * rotation on it.  */
-  NSBezierPath *bounds_path = [NSBezierPath bezierPathWithRect:rect];
-  NSAffineTransform *transform = [NSAffineTransform transform];
-  [transform rotateByDegrees: rotation * -1];
-  [bounds_path transformUsingAffineTransform:transform];
-
-  /* Now we can find out how large the rotated image needs to be.  */
-  new_size = [bounds_path bounds].size;
-  new_image = [[EmacsImage alloc] initWithSize:new_size];
-
-  new_origin = NSMakePoint((new_size.width - size.width)/2,
-                           (new_size.height - size.height)/2);
-
-  [new_image lockFocus];
-
-  /* Create the final transform.  */
-  transform = [NSAffineTransform transform];
-  [transform translateXBy:new_size.width/2 yBy:new_size.height/2];
-  [transform rotateByDegrees: rotation * -1];
-  [transform translateXBy:-new_size.width/2 yBy:-new_size.height/2];
-
-  [transform concat];
-  [self drawAtPoint:new_origin fromRect:NSZeroRect
-          operation:NSCompositingOperationCopy fraction:1];
-
-  [new_image unlockFocus];
-
-  return new_image;
+  transform = [[NSAffineTransform transform] retain];
+  NSAffineTransformStruct tm
+    = { m[0][0], m[0][1], m[1][0], m[1][1], m[2][0], m[2][1]};
+  [transform setTransformStruct:tm];
+
+  /* Because the transform is applied to the drawing surface, and not
+     the image itself, we need to invert it.  */
+  [transform invert];
 }
 
 @end
diff --git a/src/nsterm.h b/src/nsterm.h
index 78ce608554..2541b672bb 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -632,6 +632,8 @@ typedef id instancetype;
   unsigned char *pixmapData[5]; /* shortcut to access pixel data */
   NSColor *stippleMask;
   unsigned long xbm_fg;
+@public
+  NSAffineTransform *transform;
 }
 + (instancetype)allocInitFromFile: (Lisp_Object)file;
 - (void)dealloc;
@@ -648,7 +650,7 @@ typedef id instancetype;
 - (NSColor *)stippleMask;
 - (Lisp_Object)getMetadata;
 - (BOOL)setFrame: (unsigned int) index;
-- (instancetype)rotate: (double)rotation;
+- (void)setTransform: (double[3][3]) m;
 @end
 
 
@@ -1197,6 +1199,7 @@ extern bool ns_load_image (struct frame *f, struct image *img,
 extern int ns_image_width (void *img);
 extern int ns_image_height (void *img);
 extern void ns_image_set_size (void *img, int width, int height);
+extern void ns_image_set_transform (void *img, double m[3][3]);
 extern unsigned long ns_get_pixel (void *img, int x, int y);
 extern void ns_put_pixel (void *img, int x, int y, unsigned long argb);
 extern void ns_set_alpha (void *img, int x, int y, unsigned char a);
diff --git a/src/nsterm.m b/src/nsterm.m
index 2bf3e00786..3f227f525a 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3868,21 +3868,34 @@ Function modeled after x_draw_glyph_string_box ().
   /* Draw the image... do we need to draw placeholder if img == nil?  */
   if (img != nil)
     {
-#ifdef NS_IMPL_COCOA
+      /* The idea here is that the clipped area is set in the normal
+         view coordinate system, then we transform the coordinate
+         system so that when we draw the image it is rotated, resized
+         or whatever as required.  This is kind of backwards, but
+         there's no way to apply the transform to the image without
+         creating a whole new bitmap.  */
       NSRect dr = NSMakeRect (x, y, s->slice.width, s->slice.height);
-      NSRect ir = NSMakeRect (s->slice.x,
-                              s->img->height - s->slice.y - s->slice.height,
-                              s->slice.width, s->slice.height);
-      [img drawInRect: dr
-             fromRect: ir
-             operation: NSCompositingOperationSourceOver
-              fraction: 1.0
-           respectFlipped: YES
-                hints: nil];
-#else
-      [img compositeToPoint: NSMakePoint (x, y + s->slice.height)
-                  operation: NSCompositingOperationSourceOver];
-#endif
+      NSRect ir = NSMakeRect (0, 0, [img size].width, [img size].height);
+
+      NSAffineTransform *setOrigin = [NSAffineTransform transform];
+
+      [[NSGraphicsContext currentContext] saveGraphicsState];
+
+      /* Because of the transforms it's far too difficult to work out
+         what portion of the original, untransformed, image will be
+         drawn, so the clipping area will ensure we draw only the
+         correct bit.  */
+      NSRectClip (dr);
+
+      [setOrigin translateXBy:x - s->slice.x yBy:y - s->slice.y];
+      [setOrigin concat];
+      [img->transform concat];
+
+      [img drawInRect:ir fromRect:ir
+            operation:NSCompositingOperationSourceOver
+             fraction:1.0 respectFlipped:YES hints:nil];
+
+      [[NSGraphicsContext currentContext] restoreGraphicsState];
     }
 
   if (s->hl == DRAW_CURSOR)
diff --git a/src/xterm.c b/src/xterm.c
index d8eb45a00c..a81efac5c8 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2984,6 +2984,11 @@ x_composite_image (struct glyph_string *s, Pixmap dest,
       XRenderPictFormat *default_format;
       XRenderPictureAttributes attr;
 
+      /* A rotated image has no background in the "new" sections of
+         the image, so XRenderComposite makes them transparent with
+         PictOpOver.  Fill in the background before compositing.  */
+      x_clear_area (s->f, dstX, dstY, width, height);
+
       /* FIXME: Should we do this each time or would it make sense to
          store destination in the frame struct?  */
       default_format = XRenderFindVisualFormat (s->display,
-- 
2.20.1


[-- Attachment #3: 0002-Rename-image-scaling-to-image-transforms.patch --]
[-- Type: text/plain, Size: 7415 bytes --]

From c9345f094179372b0b83c2b6bafaf9276fd59b29 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sun, 24 Feb 2019 11:16:42 +0000
Subject: [PATCH 2/2] Rename image scaling to image transforms

* src/dispextern.h (HAVE_NATIVE_SCALING, HAVE_NATIVE_TRANSFORMS):
Rename and change all relevant locations.
* src/image.c (Fimage_scaling_p, Fimage_transforms_p): Rename and
update all callers.
---
 doc/lispref/display.texi | 15 ++++++++-------
 etc/NEWS                 |  4 ++--
 lisp/image.el            |  2 +-
 src/dispextern.h         |  4 ++--
 src/image.c              | 30 ++++++++++++++++--------------
 5 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index b0f1fd2676..801ab9595a 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5302,14 +5302,15 @@ Image Descriptors
 (@pxref{Input Focus}).
 @end defun
 
-@defun image-scaling-p &optional frame
-This function returns @code{t} if @var{frame} supports image scaling.
-@var{frame} @code{nil} or omitted means to use the selected frame
-(@pxref{Input Focus}).
+@defun image-transforms-p &optional frame
+This function returns @code{t} if @var{frame} supports image scaling
+and rotation.  @var{frame} @code{nil} or omitted means to use the
+selected frame (@pxref{Input Focus}).
 
-If image scaling is not supported, @code{:width}, @code{:height},
-@code{:scale}, @code{:max-width} and @code{:max-height} will only be
-usable through ImageMagick, if available (@pxref{ImageMagick Images}).
+If image transforms are not supported, @code{:rotation},
+@code{:width}, @code{:height}, @code{:scale}, @code{:max-width} and
+@code{:max-height} will only be usable through ImageMagick, if
+available (@pxref{ImageMagick Images}).
 @end defun
 
 @node XBM Images
diff --git a/etc/NEWS b/etc/NEWS
index 3a50a51637..4cc0ccaf07 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1575,8 +1575,8 @@ systems, the XRender extension to X11 is required for this to be
 available; the configure script will test for it and, if found, enable
 scaling.)
 
-The new function 'image-scaling-p' can be used to test whether any
-given frame supports resizing.
+The new function 'image-transforms-p' can be used to test whether any
+given frame supports this capability.
 
 +++
 ** (locale-info 'paper) now returns the paper size on systems that support it.
diff --git a/lisp/image.el b/lisp/image.el
index 3aa3b0aa24..42e906b3ad 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -989,7 +989,7 @@ image--get-image
     image))
 
 (defun image--get-imagemagick-and-warn ()
-  (unless (or (fboundp 'imagemagick-types) (image-scaling-p))
+  (unless (or (fboundp 'imagemagick-types) (image-transforms-p))
     (error "Cannot rescale images on this terminal"))
   (let ((image (image--get-image)))
     (image-flush image)
diff --git a/src/dispextern.h b/src/dispextern.h
index d45bc03d09..0eb7776056 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -2939,7 +2939,7 @@ struct redisplay_interface
 #ifdef HAVE_WINDOW_SYSTEM
 
 # if defined HAVE_XRENDER || defined HAVE_NS || defined HAVE_NTGUI
-#  define HAVE_NATIVE_SCALING
+#  define HAVE_NATIVE_TRANSFORMS
 
 #  define INIT_MATRIX(m)                          \
   for (int i = 0 ; i < 3 ; i++)                   \
@@ -2984,7 +2984,7 @@ struct image
      synchronized to Pixmap.  */
   XImagePtr ximg, mask_img;
 
-# ifdef HAVE_NATIVE_SCALING
+# ifdef HAVE_NATIVE_TRANSFORMS
   /* Picture versions of pixmap and mask for compositing.  */
   Picture picture, mask_picture;
 # endif
diff --git a/src/image.c b/src/image.c
index f48db69a24..dcc8cd06b6 100644
--- a/src/image.c
+++ b/src/image.c
@@ -56,7 +56,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include <sys/types.h>
 #endif /* HAVE_SYS_TYPES_H */
 
-#ifdef HAVE_NATIVE_SCALING
+#ifdef HAVE_NATIVE_TRANSFORMS
 #include <math.h>
 #endif
 
@@ -1765,7 +1765,7 @@ postprocess_image (struct frame *f, struct image *img)
     }
 }
 
-#if defined (HAVE_IMAGEMAGICK) || defined (HAVE_NATIVE_SCALING)
+#if defined (HAVE_IMAGEMAGICK) || defined (HAVE_NATIVE_TRANSFORMS)
 /* Scale an image size by returning SIZE / DIVISOR * MULTIPLIER,
    safely rounded and clipped to int range.  */
 
@@ -1864,12 +1864,12 @@ compute_image_size (size_t width, size_t height,
   *d_width = desired_width;
   *d_height = desired_height;
 }
-#endif /* HAVE_IMAGEMAGICK || HAVE_NATIVE_SCALING */
+#endif /* HAVE_IMAGEMAGICK || HAVE_NATIVE_TRANSFORMS */
 
 static void
 x_set_image_rotation (struct image *img, double tm[3][3])
 {
-#ifdef HAVE_NATIVE_SCALING
+#ifdef HAVE_NATIVE_TRANSFORMS
 # ifdef HAVE_IMAGEMAGICK
   /* ImageMagick images are already rotated.  */
   if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
@@ -1939,7 +1939,7 @@ x_set_image_rotation (struct image *img, double tm[3][3])
 static void
 x_set_image_size (struct image *img, double tm[3][3])
 {
-#ifdef HAVE_NATIVE_SCALING
+#ifdef HAVE_NATIVE_TRANSFORMS
 # ifdef HAVE_IMAGEMAGICK
   /* ImageMagick images are already the correct size.  */
   if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
@@ -1951,16 +1951,17 @@ x_set_image_size (struct image *img, double tm[3][3])
     return;
 # endif
 
-  double rm[3][3], tmp[3][3];
-  double xscale, yscale;
   int width, height;
 
   compute_image_size (img->width, img->height, img->spec, &width, &height);
 
+# if defined (HAVE_NS) || defined (HAVE_XRENDER)
+  double rm[3][3], tmp[3][3];
+  double xscale, yscale;
+
   xscale = img->width / (double) width;
   yscale = img->height / (double) height;
 
-# if defined (HAVE_NS) || defined (HAVE_XRENDER)
   INIT_MATRIX (rm);
   rm[0][0] = xscale;
   rm[1][1] = yscale;
@@ -1984,7 +1985,8 @@ x_set_image_size (struct image *img, double tm[3][3])
 static void
 x_set_transform (struct frame *f, struct image *img, double matrix[3][3])
 {
-#ifdef HAVE_NATIVE_SCALING
+  /* TODO: Add MS Windows support.  */
+#ifdef HAVE_NATIVE_TRANSFORMS
 # if defined (HAVE_NS)
   /* Under NS the transform is applied to the drawing surface at
      drawing time, so store it for later.  */
@@ -2066,7 +2068,7 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	  Lisp_Object ascent, margin, relief, bg;
 	  int relief_bound;
 
-#ifdef HAVE_NATIVE_SCALING
+#ifdef HAVE_NATIVE_TRANSFORMS
           double transform_matrix[3][3];
 
           INIT_MATRIX (transform_matrix);
@@ -10025,9 +10027,9 @@ DEFUN ("lookup-image", Flookup_image, Slookup_image, 1, 1, 0,
 			    Initialization
  ***********************************************************************/
 
-DEFUN ("image-scaling-p", Fimage_scaling_p, Simage_scaling_p, 0, 1, 0,
-       doc: /* Test whether FRAME supports resizing images.
-Return t if FRAME supports native scaling, nil otherwise.  */)
+DEFUN ("image-transforms-p", Fimage_transforms_p, Simage_transforms_p, 0, 1, 0,
+       doc: /* Test whether FRAME supports image transformation.
+Return t if FRAME supports native transforms, nil otherwise.  */)
      (Lisp_Object frame)
 {
 #if defined (HAVE_NS) || defined (HAVE_NTGUI)
@@ -10288,7 +10290,7 @@ non-numeric, there is no explicit limit on the size of images.  */);
   defsubr (&Slookup_image);
 #endif
 
-  defsubr (&Simage_scaling_p);
+  defsubr (&Simage_transforms_p);
 
   DEFVAR_BOOL ("cross-disabled-images", cross_disabled_images,
     doc: /* Non-nil means always draw a cross over disabled images.
-- 
2.20.1

Re: Native image rotation

[Eli Zaretskii]

> Date: Sun, 24 Feb 2019 11:30:50 +0000
> From: Alan Third <alan@idiocy.org>
> 
> I’ve attached a couple of patches. The second renames image scaling to
> image transforms, as it covers more than just scaling.
> 
> It should be trivial to add cropping now. I can’t think of anything
> else we might want to support.
> 
> Again I’ve not handled Windows, but hopefully it’s quite
> straight‐forward.
> 
> The image rotation code is very explicit in what it’s doing and could
> be optimized, but I’m not sure it matters.

Thanks.

Though I wonder what would be the use cases for rotating images in
Emacs by angles other than integral multiples of 90 deg.

Re: Native image rotation

[Clément Pit-Claudel]

On 24/02/2019 11.14, Eli Zaretskii wrote:
>> Date: Sun, 24 Feb 2019 11:30:50 +0000
>> From: Alan Third <alan@idiocy.org>
>>
>> I’ve attached a couple of patches. The second renames image scaling to
>> image transforms, as it covers more than just scaling.
>>
>> It should be trivial to add cropping now. I can’t think of anything
>> else we might want to support.
>>
>> Again I’ve not handled Windows, but hopefully it’s quite
>> straight‐forward.
>>
>> The image rotation code is very explicit in what it’s doing and could
>> be optimized, but I’m not sure it matters.
> 
> Thanks.
> 
> Though I wonder what would be the use cases for rotating images in
> Emacs by angles other than integral multiples of 90 deg.

I've used rotated images to display a spinning 'busy' indicator in the modeline in the past.  The corresponding package currently ships with a set of pre-rotated images, but generating them in Emacs would be more convenient.

Re: Native image rotation

[Eli Zaretskii]

> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Sun, 24 Feb 2019 12:34:32 -0500
> 
> > Though I wonder what would be the use cases for rotating images in
> > Emacs by angles other than integral multiples of 90 deg.
> 
> I've used rotated images to display a spinning 'busy' indicator in the modeline in the past.  The corresponding package currently ships with a set of pre-rotated images, but generating them in Emacs would be more convenient.

Before you decide this is more convenient, I suggest to check
performance.

Re: Native image rotation

[Clément Pit-Claudel]

On 24/02/2019 12.49, Eli Zaretskii wrote:
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Sun, 24 Feb 2019 12:34:32 -0500
>>
>>> Though I wonder what would be the use cases for rotating images in
>>> Emacs by angles other than integral multiples of 90 deg.
>>
>> I've used rotated images to display a spinning 'busy' indicator in the modeline in the past.  The corresponding package currently ships with a set of pre-rotated images, but generating them in Emacs would be more convenient.
> 
> Before you decide this is more convenient, I suggest to check
> performance.

The idea was to generate them with Emacs once, and cache the rotated version.

Re: Native image rotation

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Sun, 24 Feb 2019 13:06:36 -0500
> 
> The idea was to generate them with Emacs once, and cache the rotated version.

AFAIK, that's not what the feature does.  It rotates the specified
image on the fly.  At least that's how image scaling works.

Re: Native image rotation

[Stefan Monnier]

>> The idea was to generate them with Emacs once, and cache the rotated version.
> AFAIK, that's not what the feature does.  It rotates the specified
> image on the fly.  At least that's how image scaling works.

If Emacs's image cache does the caching post-rotation, that should
work OK.


        Stefan

Re: Native image rotation

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun, 24 Feb 2019 13:51:17 -0500
> 
> >> The idea was to generate them with Emacs once, and cache the rotated version.
> > AFAIK, that's not what the feature does.  It rotates the specified
> > image on the fly.  At least that's how image scaling works.
> 
> If Emacs's image cache does the caching post-rotation, that should
> work OK.

AFAIR, it doesn't, at least not on all platforms.

Re: Native image rotation

[Eli Zaretskii]

> Date: Sun, 24 Feb 2019 21:13:15 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Date: Sun, 24 Feb 2019 13:51:17 -0500
> > 
> > >> The idea was to generate them with Emacs once, and cache the rotated version.
> > > AFAIK, that's not what the feature does.  It rotates the specified
> > > image on the fly.  At least that's how image scaling works.
> > 
> > If Emacs's image cache does the caching post-rotation, that should
> > work OK.
> 
> AFAIR, it doesn't, at least not on all platforms.

Actually, I'm not sure I understand the question.  What do you mean by
"cache does post-rotation"?  A cache -- any cache -- just stores
something computed elsewhere.  How can a cache rotate anything?

Re: Native image rotation

[Stefan Monnier]

> Actually, I'm not sure I understand the question.  What do you mean by
> "cache does post-rotation"?

I assume that the images go through various steps starting from the
Elisp image descriptor and finishing with the final on-screen pixels
being drawn.  My question was if, along this "pipeline", Emacs's image
cache is placed before or after the rotation operation.


        Stefan

Re: Native image rotation

[Alan Third]

On Sun, Feb 24, 2019 at 03:08:25PM -0500, Stefan Monnier wrote:
> > Actually, I'm not sure I understand the question.  What do you mean by
> > "cache does post-rotation"?
> 
> I assume that the images go through various steps starting from the
> Elisp image descriptor and finishing with the final on-screen pixels
> being drawn.  My question was if, along this "pipeline", Emacs's image
> cache is placed before or after the rotation operation.

The maths for the rotation happen before the cache, the actual
rotation happens after, but should be hardware accelerated. If
hardware acceleration isn’t available, we simply ignore the
transformations.
-- 
Alan Third

Re: Native image rotation

[Alan Third]

On Sun, Feb 24, 2019 at 06:14:30PM +0200, Eli Zaretskii wrote:
> 
> Though I wonder what would be the use cases for rotating images in
> Emacs by angles other than integral multiples of 90 deg.

I’ve no idea, but the maths required isn’t significantly easier as we
still need to calculate the two transforms and the rotation. The main
advantage would be that we could replace all sin(r) and cos(r)
instances with either 1 or 0, but I expect we would get a better
performance improvement from precalculating those values and the
matrix multiplications.

And I guess we maybe wouldn’t have to worry about clearing under the
image any more in X: rotating can leave transparent sections.

But the maths is performed only once, when the image is loaded, so I
doubt we’d find a significant improvement.

-- 
Alan Third

Re: Native image rotation

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: emacs-devel@gnu.org
> Date: Sun, 24 Feb 2019 15:08:25 -0500
> 
> > Actually, I'm not sure I understand the question.  What do you mean by
> > "cache does post-rotation"?
> 
> I assume that the images go through various steps starting from the
> Elisp image descriptor and finishing with the final on-screen pixels
> being drawn.  My question was if, along this "pipeline", Emacs's image
> cache is placed before or after the rotation operation.

After.  And, assuming you rotate the same image N times, there's only
one cached entry of that at any given time.

Re: Native image rotation

[Eli Zaretskii]

> Date: Sun, 24 Feb 2019 23:22:28 +0000
> From: Alan Third <alan@idiocy.org>
> Cc: emacs-devel@gnu.org
> 
> And I guess we maybe wouldn’t have to worry about clearing under the
> image any more in X: rotating can leave transparent sections.
> 
> But the maths is performed only once, when the image is loaded, so I
> doubt we’d find a significant improvement.

It's indeed the clearing that bothered me, and also the increase in
the screen estate taken by a rotated image.  I'm asking whether it's
worth it.

Re: Native image rotation

[Van L]

> the increase in the screen estate taken
> by a rotated image.  I'm asking whether
> it's worth it.

A use case is to compensate for an image
taken at, say, a 30-degree tilt, which was
done to lengthen the horizontal distance captured
corner to corner.

Re: Native image rotation

[Evgeny Zajcev]

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

> I've used rotated images to display a spinning 'busy' indicator in the modeline
> in the past.  The corresponding package currently ships with a set of
> pre-rotated images, but generating them in Emacs would be more convenient.


I used runtime svg generation for similar task, to display video notes (and
its progression) inside Emacs.

It worked pretty nice, see the demo -
http://lgarc.narod.ru/pics/IMG_3498.MOV

-- 
lg

[-- Attachment #2: Type: text/html, Size: 693 bytes --]

Re: Native image rotation

[Stefan Monnier]

> It's indeed the clearing that bothered me, and also the increase in
> the screen estate taken by a rotated image.

Ah, indeed, I think I'd usually want to couple rotation with clipping,
to control the overall size.


        Stefan

Re: Native image rotation

[Clément Pit-Claudel]

On 25/02/2019 07.48, Evgeny Zajcev wrote:
>> I've used rotated images to display a spinning 'busy' indicator in the modeline 
>> in the past.  The corresponding package currently ships with a set of 
>> pre-rotated images, but generating them in Emacs would be more convenient.
> 
> 
> I used runtime svg generation for similar task, to display video notes (and its progression) inside Emacs.
> 
> It worked pretty nice, see the demo - http://lgarc.narod.ru/pics/IMG_3498.MOV

Fancy.

Re: Native image rotation

[Alan Third]

On Mon, Feb 25, 2019 at 05:36:46AM +0200, Eli Zaretskii wrote:
> > Date: Sun, 24 Feb 2019 23:22:28 +0000
> > From: Alan Third <alan@idiocy.org>
> > Cc: emacs-devel@gnu.org
> > 
> > And I guess we maybe wouldn’t have to worry about clearing under the
> > image any more in X: rotating can leave transparent sections.
> > 
> > But the maths is performed only once, when the image is loaded, so I
> > doubt we’d find a significant improvement.
> 
> It's indeed the clearing that bothered me,

Would it be better if it was done by another XRender composite rather
than x_clear_area?

If this really is a problem then I think we have to allow only 90
degree multiples. I can’t see any other reasonable solution.

> and also the increase in the screen estate taken by a rotated image.

I think that would be less of an issue with the addition of cropping,
as an image could be rotated then cropped down to size. Besides, if
someone wants to rotate at 45 degrees they’ve got to expect the size
of the image to change, there’s no other reasonable option.

> I'm asking whether it's worth it.

Limiting to 90 degree increments feels like an arbitrary limitation,
but I don’t feel strongly about it either way.
-- 
Alan Third

Re: Native image rotation

[Daniel Pittman]

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

As others, I'd certainly like to be able to perform arbitrary rotations.
Working with photos to extract, eg, document contents from a scanned image,
and displaying a graphical "busy" spinner, are the use cases I have handled
before.

If performance was too poor, I'd be more inclined to look at how to improve
performance in Emacs than to abandon the approach in favor of a large
number of barely changed images.  That would, after all, make life better
for everyone, and I have confidence that it *would* be possible to make
that efficient enough to work.

After all, I don't need 60FPS rendering of this, and that is definitely
achievable for vastly more complex 2D and 3D layouts on the display layers
on all the platforms Emacs supports, thanks to browsers among other tools.

On Mon, Feb 25, 2019 at 2:21 PM Alan Third <alan@idiocy.org> wrote:

> On Mon, Feb 25, 2019 at 05:36:46AM +0200, Eli Zaretskii wrote:
> > > Date: Sun, 24 Feb 2019 23:22:28 +0000
> > > From: Alan Third <alan@idiocy.org>
> > > Cc: emacs-devel@gnu.org
> > >
> > > And I guess we maybe wouldn’t have to worry about clearing under the
> > > image any more in X: rotating can leave transparent sections.
> > >
> > > But the maths is performed only once, when the image is loaded, so I
> > > doubt we’d find a significant improvement.
> >
> > It's indeed the clearing that bothered me,
>
> Would it be better if it was done by another XRender composite rather
> than x_clear_area?
>
> If this really is a problem then I think we have to allow only 90
> degree multiples. I can’t see any other reasonable solution.
>
> > and also the increase in the screen estate taken by a rotated image.
>
> I think that would be less of an issue with the addition of cropping,
> as an image could be rotated then cropped down to size. Besides, if
> someone wants to rotate at 45 degrees they’ve got to expect the size
> of the image to change, there’s no other reasonable option.
>
> > I'm asking whether it's worth it.
>
> Limiting to 90 degree increments feels like an arbitrary limitation,
> but I don’t feel strongly about it either way.
> --
> Alan Third
>
>

[-- Attachment #2: Type: text/html, Size: 2786 bytes --]

Re: Native image rotation

[Alan Third]

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

On Mon, Feb 25, 2019 at 07:21:02PM +0000, Alan Third wrote:
> On Mon, Feb 25, 2019 at 05:36:46AM +0200, Eli Zaretskii wrote:
> > 
> > It's indeed the clearing that bothered me,
> 
> Would it be better if it was done by another XRender composite rather
> than x_clear_area?

I tried to replace this with XRenderFillRectangle, but realised I need
to construct an XRenderColor struct, and I’ve no idea how to get the
colour data held in img->background.

I’ve attached a patch containing the original two, and the ability to
crop.

I decided to use a different order of processing from the ImageMagick
code we have just now, as this new ordering seems more useful for the
use cases that were suggested here.

Imagemagick does resize ‐> crop ‐> rotate.

I’ve gone with rotate ‐> crop ‐> resize.

If that’s no good it’s simple enough to reorder the functions. The
main issue I can foresee is that some people may prefer resize to come
first, but I feel that reduces the usefulness of :max-width and
:max-height.

I’ve not yet documented crop, and it’s not already documented for
Imagemagick. But in short:

    :crop '(width height left top)
    
Where left and top can be negative to operate from the right and
bottom of the image. Crop cannot be used to enlarge the image. As
mentioned before, the crop occurs before the resize, so sizes are in
terms of the original image, not the final one.

-- 
Alan Third

[-- Attachment #2: 0001-Add-native-image-rotation-and-cropping.patch --]
[-- Type: text/plain, Size: 23388 bytes --]

From adbae7ca26484a1a617a7b3fe7d89bbb8ed07506 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 23 Feb 2019 20:56:48 +0000
Subject: [PATCH] Add native image rotation and cropping

* lisp/image.el (image--get-imagemagick-and-warn): Only fallback to
ImageMagick if native transforms aren't available.
* src/dispextern.h (INIT_MATRIX, COPY_MATRIX, MULT_MATRICES): New
macros for matrix manipulation.
(HAVE_NATIVE_SCALING, HAVE_NATIVE_TRANSFORMS): Rename and change all
relevant locations.
* src/image.c (x_set_image_rotation):
(x_set_transform): New functions.
(x_set_image_size): Use transform matrix for resizing under X and NS.
(x_set_image_crop): New function.
(lookup_image): Use the new transform functions.
(Fimage_scaling_p, Fimage_transforms_p): Rename and update all
callers.
* src/nsimage.m (ns_load_image): Remove rotation code.
(ns_image_set_transform): New function.
([EmacsImage dealloc]): Release the saved transform.
([EmacsImage rotate:]): Remove unneeded method.
([EmacsImage setTransform:]): New method.
* src/nsterm.h (EmacsImage): Add transform property and update method
definitions.
* src/nsterm.m (ns_dumpglyphs_image): Use the transform to draw the
image correctly.
* src/xterm.c (x_composite_image): Clear under an image before
compositing it.
* doc/lispref/display.texi (Image Descriptors): Add :rotation.
(ImageMagick Images): Remove :rotation.
---
 doc/lispref/display.texi |  21 +--
 etc/NEWS                 |   6 +-
 lisp/image.el            |   5 +-
 src/dispextern.h         |  22 +++-
 src/image.c              | 268 +++++++++++++++++++++++++++++++++++----
 src/nsimage.m            |  64 +++-------
 src/nsterm.h             |   5 +-
 src/nsterm.m             |  41 ++++--
 src/xterm.c              |   5 +
 9 files changed, 333 insertions(+), 104 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 95379b342b..801ab9595a 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5157,6 +5157,9 @@ Image Descriptors
 specified, the height/width will be adjusted by the specified scaling
 factor.
 
+@item :rotation @var{angle}
+Specifies a rotation angle in degrees.
+
 @item :index @var{frame}
 @xref{Multi-Frame Images}.
 
@@ -5299,14 +5302,15 @@ Image Descriptors
 (@pxref{Input Focus}).
 @end defun
 
-@defun image-scaling-p &optional frame
-This function returns @code{t} if @var{frame} supports image scaling.
-@var{frame} @code{nil} or omitted means to use the selected frame
-(@pxref{Input Focus}).
+@defun image-transforms-p &optional frame
+This function returns @code{t} if @var{frame} supports image scaling
+and rotation.  @var{frame} @code{nil} or omitted means to use the
+selected frame (@pxref{Input Focus}).
 
-If image scaling is not supported, @code{:width}, @code{:height},
-@code{:scale}, @code{:max-width} and @code{:max-height} will only be
-usable through ImageMagick, if available (@pxref{ImageMagick Images}).
+If image transforms are not supported, @code{:rotation},
+@code{:width}, @code{:height}, @code{:scale}, @code{:max-width} and
+@code{:max-height} will only be usable through ImageMagick, if
+available (@pxref{ImageMagick Images}).
 @end defun
 
 @node XBM Images
@@ -5450,9 +5454,6 @@ ImageMagick Images
 image data, as found in @code{image-format-suffixes}.  This is used
 when the image does not have an associated file name, to provide a
 hint to ImageMagick to help it detect the image type.
-
-@item :rotation @var{angle}
-Specifies a rotation angle in degrees.
 @end table
 
 @node SVG Images
diff --git a/etc/NEWS b/etc/NEWS
index 0cafbaae96..4cc0ccaf07 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1569,14 +1569,14 @@ buffer's 'default-directory' and invoke that file name handler to make
 the process.  That way 'make-process' can start remote processes.
 
 +++
-** Emacs now supports resizing (scaling) of images without ImageMagick.
+** Emacs now supports resizing and rotating images without ImageMagick.
 All modern systems are supported by this feature.  (On GNU and Unix
 systems, the XRender extension to X11 is required for this to be
 available; the configure script will test for it and, if found, enable
 scaling.)
 
-The new function 'image-scaling-p' can be used to test whether any
-given frame supports resizing.
+The new function 'image-transforms-p' can be used to test whether any
+given frame supports this capability.
 
 +++
 ** (locale-info 'paper) now returns the paper size on systems that support it.
diff --git a/lisp/image.el b/lisp/image.el
index 3aa3b0aa24..0139c5263a 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -989,11 +989,12 @@ image--get-image
     image))
 
 (defun image--get-imagemagick-and-warn ()
-  (unless (or (fboundp 'imagemagick-types) (image-scaling-p))
+  (unless (or (fboundp 'imagemagick-types) (image-transforms-p))
     (error "Cannot rescale images on this terminal"))
   (let ((image (image--get-image)))
     (image-flush image)
-    (when (fboundp 'imagemagick-types)
+    (when (and (fboundp 'imagemagick-types)
+               (not (image-transforms-p)))
       (plist-put (cdr image) :type 'imagemagick))
     image))
 
diff --git a/src/dispextern.h b/src/dispextern.h
index 894753669d..0eb7776056 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -2939,7 +2939,25 @@ struct redisplay_interface
 #ifdef HAVE_WINDOW_SYSTEM
 
 # if defined HAVE_XRENDER || defined HAVE_NS || defined HAVE_NTGUI
-#  define HAVE_NATIVE_SCALING
+#  define HAVE_NATIVE_TRANSFORMS
+
+#  define INIT_MATRIX(m)                          \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++)                 \
+      m[i][j] = (i == j) ? 1 : 0;
+
+#  define COPY_MATRIX(a, b)                       \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++)                 \
+      b[i][j] = a[i][j];
+
+#  define MULT_MATRICES(a, b, result)             \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++) {               \
+      double sum = 0;                             \
+      for (int k = 0 ; k < 3 ; k++)               \
+        sum += a[k][j] * b[i][k];                 \
+      result[i][j] = sum;}
 # endif
 
 /* Structure describing an image.  Specific image formats like XBM are
@@ -2966,7 +2984,7 @@ struct image
      synchronized to Pixmap.  */
   XImagePtr ximg, mask_img;
 
-# ifdef HAVE_NATIVE_SCALING
+# ifdef HAVE_NATIVE_TRANSFORMS
   /* Picture versions of pixmap and mask for compositing.  */
   Picture picture, mask_picture;
 # endif
diff --git a/src/image.c b/src/image.c
index 642bf67152..21db915e61 100644
--- a/src/image.c
+++ b/src/image.c
@@ -56,6 +56,10 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include <sys/types.h>
 #endif /* HAVE_SYS_TYPES_H */
 
+#ifdef HAVE_NATIVE_TRANSFORMS
+#include <math.h>
+#endif
+
 #ifdef HAVE_WINDOW_SYSTEM
 #include TERM_HEADER
 #endif /* HAVE_WINDOW_SYSTEM */
@@ -1761,7 +1765,7 @@ postprocess_image (struct frame *f, struct image *img)
     }
 }
 
-#if defined (HAVE_IMAGEMAGICK) || defined (HAVE_NATIVE_SCALING)
+#if defined (HAVE_IMAGEMAGICK) || defined (HAVE_NATIVE_TRANSFORMS)
 /* Scale an image size by returning SIZE / DIVISOR * MULTIPLIER,
    safely rounded and clipped to int range.  */
 
@@ -1860,46 +1864,221 @@ compute_image_size (size_t width, size_t height,
   *d_width = desired_width;
   *d_height = desired_height;
 }
-#endif /* HAVE_IMAGEMAGICK || HAVE_NATIVE_SCALING */
+#endif /* HAVE_IMAGEMAGICK || HAVE_NATIVE_TRANSFORMS */
 
 static void
-x_set_image_size (struct frame *f, struct image *img)
+x_set_image_rotation (struct image *img, double tm[3][3])
 {
-#ifdef HAVE_NATIVE_SCALING
+#ifdef HAVE_NATIVE_TRANSFORMS
 # ifdef HAVE_IMAGEMAGICK
-  /* ImageMagick images are already the correct size.  */
+  /* ImageMagick images are already rotated.  */
   if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
     return;
 # endif
 
+# ifdef HAVE_XRENDER
+  if (!img->picture)
+    return;
+# endif
+
+  Lisp_Object value;
+  double rotation = 0;
+  double cos_r, sin_r;
+  double t[3][3], rot[3][3], tmp[3][3], tmp2[3][3];
   int width, height;
-  compute_image_size (img->width, img->height, img->spec, &width, &height);
 
-# ifdef HAVE_NS
-  ns_image_set_size (img->pixmap, width, height);
+  value = image_spec_value (img->spec, QCrotation, NULL);
+  if (NUMBERP (value))
+    rotation = M_PI * XFLOATINT (value) / 180; /* radians */
+
+  if (rotation == 0)
+    return;
+
+  cos_r = cos (rotation);
+  sin_r = sin (rotation);
+
+  if (rotation > 3 * M_PI && rotation <= M_PI)
+    {
+      width = fabs (img->height * cos_r)
+        + fabs (img->width * sin_r);
+      height = fabs (img->height * sin_r)
+        + fabs (img->width * cos_r);
+    }
+  else
+    {
+      width = fabs (img->height * sin_r)
+        + fabs (img->width * cos_r);
+      height = fabs (img->height * cos_r)
+        + fabs (img->width * sin_r);
+    }
+
+  /* Translate so (0, 0) is in the centre of the image.  */
+  INIT_MATRIX (t);
+  t[2][0] = img->width/2;
+  t[2][1] = img->height/2;
+
+  MULT_MATRICES (tm, t, tmp);
+
+  /* Rotate.  */
+  INIT_MATRIX (rot);
+  rot[0][0] = cos_r;
+  rot[1][0] = sin_r;
+  rot[0][1] = - sin_r;
+  rot[1][1] = cos_r;
+
+  MULT_MATRICES (tmp, rot, tmp2);
+
+  /* Translate back.  */
+  INIT_MATRIX (t);
+  t[2][0] = - width/2;
+  t[2][1] = - height/2;
+
+  MULT_MATRICES (tmp2, t, tm);
+
   img->width = width;
   img->height = height;
+#endif
+}
+
+static void
+x_set_image_crop (struct image *img, double tm[3][3])
+{
+#ifdef HAVE_NATIVE_TRANSFORMS
+# ifdef HAVE_IMAGEMAGICK
+  /* ImageMagick images are already rotated.  */
+  if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
+    return;
 # endif
 
 # ifdef HAVE_XRENDER
-  if (img->picture)
+  if (!img->picture)
+    return;
+# endif
+
+  double m[3][3], tmp[3][3];
+  int left, top, width, height;
+  Lisp_Object x = Qnil;
+  Lisp_Object y = Qnil;
+  Lisp_Object w = Qnil;
+  Lisp_Object h = Qnil;
+  Lisp_Object crop = image_spec_value (img->spec, QCcrop, NULL);
+
+  if (!CONSP (crop))
+    return;
+  else
     {
-      double xscale = img->width / (double) width;
-      double yscale = img->height / (double) height;
+      w = XCAR (crop);
+      crop = XCDR (crop);
+      if (CONSP (crop))
+	{
+          h = XCAR (crop);
+	  crop = XCDR (crop);
+	  if (CONSP (crop))
+	    {
+              x = XCAR (crop);
+	      crop = XCDR (crop);
+	      if (CONSP (crop))
+                y = XCAR (crop);
+	    }
+	}
+    }
 
-      XTransform tmat
-	= {{{XDoubleToFixed (xscale), XDoubleToFixed (0), XDoubleToFixed (0)},
-	    {XDoubleToFixed (0), XDoubleToFixed (yscale), XDoubleToFixed (0)},
-	    {XDoubleToFixed (0), XDoubleToFixed (0), XDoubleToFixed (1)}}};
+  if (FIXNATP (w) && XFIXNAT (w) < img->width)
+    width = XFIXNAT (w);
+  else
+    width = img->width;
 
-      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
-			       0, 0);
-      XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+  if (TYPE_RANGED_FIXNUMP (int, x))
+    {
+      left = XFIXNUM (x);
+      if (left < 0)
+        left = img->width - width + left;
+    }
+  else
+    left = (img->width - width)/2;
 
-      img->width = width;
-      img->height = height;
+  if (FIXNATP (h) && XFIXNAT (h) < img->height)
+    height = XFIXNAT (h);
+  else
+    height = img->height;
+
+  if (TYPE_RANGED_FIXNUMP (int, y))
+    {
+      top = XFIXNUM (y);
+      if (top < 0)
+        top = img->height - height + top;
+    }
+  else
+    top = (img->height - height)/2;
+
+  /* Limit the output to the dimensions of the original image.  */
+  if (left < 0)
+    {
+      width = img->width + left;
+      left = 0;
     }
+
+  if (width + left > img->width)
+    width = img->width - left;
+
+  if (top < 0)
+    {
+      height = img->height + top;
+      top = 0;
+    }
+
+  if (height + top > img->height)
+    height = img->height - top;
+
+  INIT_MATRIX (m);
+  m[2][0] = left;
+  m[2][1] = top;
+
+  MULT_MATRICES (tm, m, tmp);
+  COPY_MATRIX (tmp, tm);
+
+  img->width = width;
+  img->height = height;
+#endif
+}
+
+static void
+x_set_image_size (struct image *img, double tm[3][3])
+{
+#ifdef HAVE_NATIVE_TRANSFORMS
+# ifdef HAVE_IMAGEMAGICK
+  /* ImageMagick images are already the correct size.  */
+  if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
+    return;
+# endif
+
+# ifdef HAVE_XRENDER
+  if (!img->picture)
+    return;
 # endif
+
+  int width, height;
+
+  compute_image_size (img->width, img->height, img->spec, &width, &height);
+
+# if defined (HAVE_NS) || defined (HAVE_XRENDER)
+  double rm[3][3], tmp[3][3];
+  double xscale, yscale;
+
+  xscale = img->width / (double) width;
+  yscale = img->height / (double) height;
+
+  INIT_MATRIX (rm);
+  rm[0][0] = xscale;
+  rm[1][1] = yscale;
+
+  MULT_MATRICES (tm, rm, tmp);
+  COPY_MATRIX (tmp, tm);
+
+  img->width = width;
+  img->height = height;
+# endif
+
 # ifdef HAVE_NTGUI
   /* Under HAVE_NTGUI, we will scale the image on the fly, when we
      draw it.  See w32term.c:x_draw_image_foreground.  */
@@ -1909,6 +2088,36 @@ x_set_image_size (struct frame *f, struct image *img)
 #endif
 }
 
+static void
+x_set_transform (struct frame *f, struct image *img, double matrix[3][3])
+{
+  /* TODO: Add MS Windows support.  */
+#ifdef HAVE_NATIVE_TRANSFORMS
+# if defined (HAVE_NS)
+  /* Under NS the transform is applied to the drawing surface at
+     drawing time, so store it for later.  */
+  ns_image_set_transform (img->pixmap, matrix);
+# elif defined (HAVE_XRENDER)
+  if (img->picture)
+    {
+      XTransform tmat
+	= {{{XDoubleToFixed (matrix[0][0]),
+             XDoubleToFixed (matrix[1][0]),
+             XDoubleToFixed (matrix[2][0])},
+	    {XDoubleToFixed (matrix[0][1]),
+             XDoubleToFixed (matrix[1][1]),
+             XDoubleToFixed (matrix[2][1])},
+	    {XDoubleToFixed (matrix[0][2]),
+             XDoubleToFixed (matrix[1][2]),
+             XDoubleToFixed (matrix[2][2])}}};
+
+      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
+			       0, 0);
+      XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+    }
+# endif
+#endif
+}
 
 /* Return the id of image with Lisp specification SPEC on frame F.
    SPEC must be a valid Lisp image specification (see valid_image_p).  */
@@ -1964,7 +2173,16 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	     `:background COLOR'.  */
 	  Lisp_Object ascent, margin, relief, bg;
 	  int relief_bound;
-          x_set_image_size (f, img);
+
+#ifdef HAVE_NATIVE_TRANSFORMS
+          double transform_matrix[3][3];
+
+          INIT_MATRIX (transform_matrix);
+          x_set_image_rotation (img, transform_matrix);
+          x_set_image_crop (img, transform_matrix);
+          x_set_image_size (img, transform_matrix);
+          x_set_transform (f, img, transform_matrix);
+#endif
 
 	  ascent = image_spec_value (spec, QCascent, NULL);
 	  if (FIXNUMP (ascent))
@@ -9916,9 +10134,9 @@ DEFUN ("lookup-image", Flookup_image, Slookup_image, 1, 1, 0,
 			    Initialization
  ***********************************************************************/
 
-DEFUN ("image-scaling-p", Fimage_scaling_p, Simage_scaling_p, 0, 1, 0,
-       doc: /* Test whether FRAME supports resizing images.
-Return t if FRAME supports native scaling, nil otherwise.  */)
+DEFUN ("image-transforms-p", Fimage_transforms_p, Simage_transforms_p, 0, 1, 0,
+       doc: /* Test whether FRAME supports image transformation.
+Return t if FRAME supports native transforms, nil otherwise.  */)
      (Lisp_Object frame)
 {
 #if defined (HAVE_NS) || defined (HAVE_NTGUI)
@@ -10179,7 +10397,7 @@ non-numeric, there is no explicit limit on the size of images.  */);
   defsubr (&Slookup_image);
 #endif
 
-  defsubr (&Simage_scaling_p);
+  defsubr (&Simage_transforms_p);
 
   DEFVAR_BOOL ("cross-disabled-images", cross_disabled_images,
     doc: /* Non-nil means always draw a cross over disabled images.
diff --git a/src/nsimage.m b/src/nsimage.m
index f16910de08..3a5dd24185 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -76,9 +76,8 @@ Updated by Christian Limpach (chris@nice.ch)
 {
   EmacsImage *eImg = nil;
   NSSize size;
-  Lisp_Object lisp_index, lisp_rotation;
+  Lisp_Object lisp_index;
   unsigned int index;
-  double rotation;
 
   NSTRACE ("ns_load_image");
 
@@ -87,9 +86,6 @@ Updated by Christian Limpach (chris@nice.ch)
   lisp_index = Fplist_get (XCDR (img->spec), QCindex);
   index = FIXNUMP (lisp_index) ? XFIXNAT (lisp_index) : 0;
 
-  lisp_rotation = Fplist_get (XCDR (img->spec), QCrotation);
-  rotation = NUMBERP (lisp_rotation) ? XFLOATINT (lisp_rotation) : 0;
-
   if (STRINGP (spec_file))
     {
       eImg = [EmacsImage allocInitFromFile: spec_file];
@@ -119,13 +115,6 @@ Updated by Christian Limpach (chris@nice.ch)
 
   img->lisp_data = [eImg getMetadata];
 
-  if (rotation != 0)
-    {
-      EmacsImage *temp = [eImg rotate:rotation];
-      [eImg release];
-      eImg = temp;
-    }
-
   size = [eImg size];
   img->width = size.width;
   img->height = size.height;
@@ -155,6 +144,12 @@ Updated by Christian Limpach (chris@nice.ch)
   [(EmacsImage *)img setSize:NSMakeSize (width, height)];
 }
 
+void
+ns_image_set_transform (void *img, double m[3][3])
+{
+  [(EmacsImage *)img setTransform:m];
+}
+
 unsigned long
 ns_get_pixel (void *img, int x, int y)
 {
@@ -225,6 +220,7 @@ - (void)dealloc
 {
   [stippleMask release];
   [bmRep release];
+  [transform release];
   [super dealloc];
 }
 
@@ -528,42 +524,16 @@ - (BOOL)setFrame: (unsigned int) index
   return YES;
 }
 
-- (instancetype)rotate: (double)rotation
+- (void)setTransform: (double[3][3]) m
 {
-  EmacsImage *new_image;
-  NSPoint new_origin;
-  NSSize new_size, size = [self size];
-  NSRect rect = { NSZeroPoint, [self size] };
-
-  /* Create a bezier path of the outline of the image and do the
-   * rotation on it.  */
-  NSBezierPath *bounds_path = [NSBezierPath bezierPathWithRect:rect];
-  NSAffineTransform *transform = [NSAffineTransform transform];
-  [transform rotateByDegrees: rotation * -1];
-  [bounds_path transformUsingAffineTransform:transform];
-
-  /* Now we can find out how large the rotated image needs to be.  */
-  new_size = [bounds_path bounds].size;
-  new_image = [[EmacsImage alloc] initWithSize:new_size];
-
-  new_origin = NSMakePoint((new_size.width - size.width)/2,
-                           (new_size.height - size.height)/2);
-
-  [new_image lockFocus];
-
-  /* Create the final transform.  */
-  transform = [NSAffineTransform transform];
-  [transform translateXBy:new_size.width/2 yBy:new_size.height/2];
-  [transform rotateByDegrees: rotation * -1];
-  [transform translateXBy:-new_size.width/2 yBy:-new_size.height/2];
-
-  [transform concat];
-  [self drawAtPoint:new_origin fromRect:NSZeroRect
-          operation:NSCompositingOperationCopy fraction:1];
-
-  [new_image unlockFocus];
-
-  return new_image;
+  transform = [[NSAffineTransform transform] retain];
+  NSAffineTransformStruct tm
+    = { m[0][0], m[0][1], m[1][0], m[1][1], m[2][0], m[2][1]};
+  [transform setTransformStruct:tm];
+
+  /* Because the transform is applied to the drawing surface, and not
+     the image itself, we need to invert it.  */
+  [transform invert];
 }
 
 @end
diff --git a/src/nsterm.h b/src/nsterm.h
index 78ce608554..2541b672bb 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -632,6 +632,8 @@ typedef id instancetype;
   unsigned char *pixmapData[5]; /* shortcut to access pixel data */
   NSColor *stippleMask;
   unsigned long xbm_fg;
+@public
+  NSAffineTransform *transform;
 }
 + (instancetype)allocInitFromFile: (Lisp_Object)file;
 - (void)dealloc;
@@ -648,7 +650,7 @@ typedef id instancetype;
 - (NSColor *)stippleMask;
 - (Lisp_Object)getMetadata;
 - (BOOL)setFrame: (unsigned int) index;
-- (instancetype)rotate: (double)rotation;
+- (void)setTransform: (double[3][3]) m;
 @end
 
 
@@ -1197,6 +1199,7 @@ extern bool ns_load_image (struct frame *f, struct image *img,
 extern int ns_image_width (void *img);
 extern int ns_image_height (void *img);
 extern void ns_image_set_size (void *img, int width, int height);
+extern void ns_image_set_transform (void *img, double m[3][3]);
 extern unsigned long ns_get_pixel (void *img, int x, int y);
 extern void ns_put_pixel (void *img, int x, int y, unsigned long argb);
 extern void ns_set_alpha (void *img, int x, int y, unsigned char a);
diff --git a/src/nsterm.m b/src/nsterm.m
index 2bf3e00786..3f227f525a 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3868,21 +3868,34 @@ Function modeled after x_draw_glyph_string_box ().
   /* Draw the image... do we need to draw placeholder if img == nil?  */
   if (img != nil)
     {
-#ifdef NS_IMPL_COCOA
+      /* The idea here is that the clipped area is set in the normal
+         view coordinate system, then we transform the coordinate
+         system so that when we draw the image it is rotated, resized
+         or whatever as required.  This is kind of backwards, but
+         there's no way to apply the transform to the image without
+         creating a whole new bitmap.  */
       NSRect dr = NSMakeRect (x, y, s->slice.width, s->slice.height);
-      NSRect ir = NSMakeRect (s->slice.x,
-                              s->img->height - s->slice.y - s->slice.height,
-                              s->slice.width, s->slice.height);
-      [img drawInRect: dr
-             fromRect: ir
-             operation: NSCompositingOperationSourceOver
-              fraction: 1.0
-           respectFlipped: YES
-                hints: nil];
-#else
-      [img compositeToPoint: NSMakePoint (x, y + s->slice.height)
-                  operation: NSCompositingOperationSourceOver];
-#endif
+      NSRect ir = NSMakeRect (0, 0, [img size].width, [img size].height);
+
+      NSAffineTransform *setOrigin = [NSAffineTransform transform];
+
+      [[NSGraphicsContext currentContext] saveGraphicsState];
+
+      /* Because of the transforms it's far too difficult to work out
+         what portion of the original, untransformed, image will be
+         drawn, so the clipping area will ensure we draw only the
+         correct bit.  */
+      NSRectClip (dr);
+
+      [setOrigin translateXBy:x - s->slice.x yBy:y - s->slice.y];
+      [setOrigin concat];
+      [img->transform concat];
+
+      [img drawInRect:ir fromRect:ir
+            operation:NSCompositingOperationSourceOver
+             fraction:1.0 respectFlipped:YES hints:nil];
+
+      [[NSGraphicsContext currentContext] restoreGraphicsState];
     }
 
   if (s->hl == DRAW_CURSOR)
diff --git a/src/xterm.c b/src/xterm.c
index d8eb45a00c..a81efac5c8 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2984,6 +2984,11 @@ x_composite_image (struct glyph_string *s, Pixmap dest,
       XRenderPictFormat *default_format;
       XRenderPictureAttributes attr;
 
+      /* A rotated image has no background in the "new" sections of
+         the image, so XRenderComposite makes them transparent with
+         PictOpOver.  Fill in the background before compositing.  */
+      x_clear_area (s->f, dstX, dstY, width, height);
+
       /* FIXME: Should we do this each time or would it make sense to
          store destination in the frame struct?  */
       default_format = XRenderFindVisualFormat (s->display,
-- 
2.20.1

Re: Native image rotation

[Basil L. Contovounesios]

Alan Third <alan@idiocy.org> writes:

> On Mon, Feb 25, 2019 at 07:21:02PM +0000, Alan Third wrote:
>> On Mon, Feb 25, 2019 at 05:36:46AM +0200, Eli Zaretskii wrote:
>> > 
>> > It's indeed the clearing that bothered me,
>> 
>> Would it be better if it was done by another XRender composite rather
>> than x_clear_area?
>
> I tried to replace this with XRenderFillRectangle, but realised I need
> to construct an XRenderColor struct, and I’ve no idea how to get the
> colour data held in img->background.
>
> I’ve attached a patch containing the original two, and the ability to
> crop.

Thanks for working on this; I'm curious to know its status.

BTW, I think native image rotation should support arbitrary angles (if
possible), if for no other reason than ImageMagick also supporting them.

Thanks,

-- 
Basil

Re: Native image rotation

[Alan Third]

On Sun, May 19, 2019 at 09:29:02PM +0100, Basil L. Contovounesios wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > On Mon, Feb 25, 2019 at 07:21:02PM +0000, Alan Third wrote:
> >> On Mon, Feb 25, 2019 at 05:36:46AM +0200, Eli Zaretskii wrote:
> >> > 
> >> > It's indeed the clearing that bothered me,
> >> 
> >> Would it be better if it was done by another XRender composite rather
> >> than x_clear_area?
> >
> > I tried to replace this with XRenderFillRectangle, but realised I need
> > to construct an XRenderColor struct, and I’ve no idea how to get the
> > colour data held in img->background.
> >
> > I’ve attached a patch containing the original two, and the ability to
> > crop.
> 
> Thanks for working on this; I'm curious to know its status.

I’ve sort of put Emacs development on a back burner for the moment.
I’m planning on returning to this at some point, though.

> BTW, I think native image rotation should support arbitrary angles (if
> possible), if for no other reason than ImageMagick also supporting them.

I do think there is an issue with masks and cursors that maybe
wouldn’t be straight forward. I’m still somewhat unclear on how
images, cursors and masks interact. If someone has a good solution I’d
be happy to hear it, but of course it has to be passed by Eli too and
I don’t know if his objections are the same as what I’m thinking of.

I plan to go back and rewrite it to only handle 90 degree increments
and submit that, at least for now.
-- 
Alan Third

Re: Native image rotation

[Alan Third]

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

On Mon, May 20, 2019 at 07:18:48PM +0100, Alan Third wrote:
> 
> I plan to go back and rewrite it to only handle 90 degree increments
> and submit that, at least for now.

Patch attached.

I left the rotation code largely untouched in case we want to allow
non‐90 degree increments in the future, but that might be a waste of
time.
-- 
Alan Third

[-- Attachment #2: v2-0001-Add-native-image-rotation-and-cropping.patch --]
[-- Type: text/plain, Size: 24114 bytes --]

From e0f6a9cc83d9df09a2c4aad0ea77138b6c055f00 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 23 Feb 2019 20:56:48 +0000
Subject: [PATCH v2] Add native image rotation and cropping

* lisp/image.el (image--get-imagemagick-and-warn): Only fallback to
ImageMagick if native transforms aren't available.
* src/dispextern.h (INIT_MATRIX, COPY_MATRIX, MULT_MATRICES): New
macros for matrix manipulation.
(HAVE_NATIVE_SCALING, HAVE_NATIVE_TRANSFORMS): Rename and change all
relevant locations.
* src/image.c (x_set_image_rotation):
(x_set_transform): New functions.
(x_set_image_size): Use transform matrix for resizing under X and NS.
(x_set_image_crop): New function.
(lookup_image): Use the new transform functions.
(Fimage_scaling_p, Fimage_transforms_p): Rename and update all
callers.
* src/nsimage.m (ns_load_image): Remove rotation code.
(ns_image_set_transform): New function.
([EmacsImage dealloc]): Release the saved transform.
([EmacsImage rotate:]): Remove unneeded method.
([EmacsImage setTransform:]): New method.
* src/nsterm.h (EmacsImage): Add transform property and update method
definitions.
* src/nsterm.m (ns_dumpglyphs_image): Use the transform to draw the
image correctly.
* src/xterm.c (x_composite_image): Use PictOpSrc as we don't care
about alpha values here.
* doc/lispref/display.texi (Image Descriptors): Add :rotation.
(ImageMagick Images): Remove :rotation.
---
 doc/lispref/display.texi |  21 +--
 etc/NEWS                 |   6 +-
 lisp/image.el            |   5 +-
 src/dispextern.h         |  22 ++-
 src/image.c              | 292 +++++++++++++++++++++++++++++++++++----
 src/nsimage.m            |  64 +++------
 src/nsterm.h             |   5 +-
 src/nsterm.m             |  41 ++++--
 src/xterm.c              |   5 +-
 9 files changed, 352 insertions(+), 109 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index a2ed4b3891..283928c41e 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5176,6 +5176,9 @@ Image Descriptors
 specified, the height/width will be adjusted by the specified scaling
 factor.
 
+@item :rotation @var{angle}
+Specifies a rotation angle in degrees.
+
 @item :index @var{frame}
 @xref{Multi-Frame Images}.
 
@@ -5318,14 +5321,15 @@ Image Descriptors
 (@pxref{Input Focus}).
 @end defun
 
-@defun image-scaling-p &optional frame
-This function returns @code{t} if @var{frame} supports image scaling.
-@var{frame} @code{nil} or omitted means to use the selected frame
-(@pxref{Input Focus}).
+@defun image-transforms-p &optional frame
+This function returns @code{t} if @var{frame} supports image scaling
+and rotation.  @var{frame} @code{nil} or omitted means to use the
+selected frame (@pxref{Input Focus}).
 
-If image scaling is not supported, @code{:width}, @code{:height},
-@code{:scale}, @code{:max-width} and @code{:max-height} will only be
-usable through ImageMagick, if available (@pxref{ImageMagick Images}).
+If image transforms are not supported, @code{:rotation},
+@code{:width}, @code{:height}, @code{:scale}, @code{:max-width} and
+@code{:max-height} will only be usable through ImageMagick, if
+available (@pxref{ImageMagick Images}).
 @end defun
 
 @node XBM Images
@@ -5469,9 +5473,6 @@ ImageMagick Images
 image data, as found in @code{image-format-suffixes}.  This is used
 when the image does not have an associated file name, to provide a
 hint to ImageMagick to help it detect the image type.
-
-@item :rotation @var{angle}
-Specifies a rotation angle in degrees.
 @end table
 
 @node SVG Images
diff --git a/etc/NEWS b/etc/NEWS
index 72702a9aaa..3aae88f4a8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2001,14 +2001,14 @@ buffer's 'default-directory' and invoke that file name handler to make
 the process.  That way 'make-process' can start remote processes.
 
 +++
-** Emacs now supports resizing (scaling) of images without ImageMagick.
+** Emacs now supports resizing and rotating images without ImageMagick.
 All modern systems are supported by this feature.  (On GNU and Unix
 systems, Cairo drawing or the XRender extension to X11 is required for
 this to be available; the configure script will test for it and, if
 found, enable scaling.)
 
-The new function 'image-scaling-p' can be used to test whether any
-given frame supports resizing.
+The new function 'image-transforms-p' can be used to test whether any
+given frame supports this capability.
 
 +++
 ** '(locale-info 'paper)' now returns the paper size on systems that support it.
diff --git a/lisp/image.el b/lisp/image.el
index 6cc2cc3902..e360ba8fa6 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -991,11 +991,12 @@ image--get-image
     image))
 
 (defun image--get-imagemagick-and-warn ()
-  (unless (or (fboundp 'imagemagick-types) (image-scaling-p))
+  (unless (or (fboundp 'imagemagick-types) (image-transforms-p))
     (error "Cannot rescale images on this terminal"))
   (let ((image (image--get-image)))
     (image-flush image)
-    (when (fboundp 'imagemagick-types)
+    (when (and (fboundp 'imagemagick-types)
+               (not (image-transforms-p)))
       (plist-put (cdr image) :type 'imagemagick))
     image))
 
diff --git a/src/dispextern.h b/src/dispextern.h
index ec1c9620be..95b6b48116 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -2972,7 +2972,25 @@ struct redisplay_interface
 #ifdef HAVE_WINDOW_SYSTEM
 
 # if defined USE_CAIRO || defined HAVE_XRENDER || defined HAVE_NS || defined HAVE_NTGUI
-#  define HAVE_NATIVE_SCALING
+#  define HAVE_NATIVE_TRANSFORMS
+
+#  define INIT_MATRIX(m)                          \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++)                 \
+      m[i][j] = (i == j) ? 1 : 0;
+
+#  define COPY_MATRIX(a, b)                       \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++)                 \
+      b[i][j] = a[i][j];
+
+#  define MULT_MATRICES(a, b, result)             \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++) {               \
+      double sum = 0;                             \
+      for (int k = 0 ; k < 3 ; k++)               \
+        sum += a[k][j] * b[i][k];                 \
+      result[i][j] = sum;}
 # endif
 
 /* Structure describing an image.  Specific image formats like XBM are
@@ -2998,7 +3016,7 @@ struct image
      synchronized to Pixmap.  */
   XImage *ximg, *mask_img;
 
-# ifdef HAVE_NATIVE_SCALING
+# ifdef HAVE_NATIVE_TRANSFORMS
   /* Picture versions of pixmap and mask for compositing.  */
   Picture picture, mask_picture;
 # endif
diff --git a/src/image.c b/src/image.c
index 57b405f6db..97a51b4465 100644
--- a/src/image.c
+++ b/src/image.c
@@ -56,6 +56,10 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include <sys/types.h>
 #endif /* HAVE_SYS_TYPES_H */
 
+#ifdef HAVE_NATIVE_TRANSFORMS
+#include <math.h>
+#endif
+
 #ifdef HAVE_WINDOW_SYSTEM
 #include TERM_HEADER
 #endif /* HAVE_WINDOW_SYSTEM */
@@ -1669,7 +1673,7 @@ postprocess_image (struct frame *f, struct image *img)
     }
 }
 
-#if defined (HAVE_IMAGEMAGICK) || defined (HAVE_NATIVE_SCALING)
+#if defined (HAVE_IMAGEMAGICK) || defined (HAVE_NATIVE_TRANSFORMS)
 /* Scale an image size by returning SIZE / DIVISOR * MULTIPLIER,
    safely rounded and clipped to int range.  */
 
@@ -1768,49 +1772,246 @@ compute_image_size (size_t width, size_t height,
   *d_width = desired_width;
   *d_height = desired_height;
 }
-#endif /* HAVE_IMAGEMAGICK || HAVE_NATIVE_SCALING */
+#endif /* HAVE_IMAGEMAGICK || HAVE_NATIVE_TRANSFORMS */
 
 static void
-image_set_image_size (struct frame *f, struct image *img)
+image_set_rotation (struct image *img, double tm[3][3])
 {
-#ifdef HAVE_NATIVE_SCALING
+#ifdef HAVE_NATIVE_TRANSFORMS
 # ifdef HAVE_IMAGEMAGICK
-  /* ImageMagick images are already the correct size.  */
+  /* ImageMagick images are already rotated.  */
   if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
     return;
 # endif
 
-  int width, height;
-  compute_image_size (img->width, img->height, img->spec, &width, &height);
+# ifdef HAVE_XRENDER
+  if (!img->picture)
+    return;
+# endif
+
+  Lisp_Object value;
+  double rotation = 0;
+  double cos_r, sin_r;
+  double t[3][3], rot[3][3], tmp[3][3], tmp2[3][3];
+  int rotation_d, width, height;
+
+  value = image_spec_value (img->spec, QCrotation, NULL);
+  if (! NUMBERP (value))
+    return;
+
+  rotation_d = XFLOATINT (value);
+
+  /* FIXME: If the rotation value is not a multiple of 90 degrees
+     display a warning to the user.  */
+  if (rotation_d%360 == 0 || rotation_d%90 != 0)
+    return;
+
+  if (rotation_d%180 != 0)
+    {
+      width = img->height;
+      height = img->width;
+    }
+  else
+    {
+      width = img->width;
+      height = img->height;
+    }
+
+  rotation = M_PI * rotation_d / 180; /* radians */
+
+  cos_r = cos (rotation);
+  sin_r = sin (rotation);
+
+# if 0
+  /* This calculates the correct width and height if the rotation is
+     not a multiple of 90 degrees.  We don't allow that at the
+     moment, though.  */
+  if (rotation > 3 * M_PI && rotation <= M_PI)
+    {
+      width = fabs (img->height * cos_r)
+        + fabs (img->width * sin_r);
+      height = fabs (img->height * sin_r)
+        + fabs (img->width * cos_r);
+    }
+  else
+    {
+      width = fabs (img->height * sin_r)
+        + fabs (img->width * cos_r);
+      height = fabs (img->height * cos_r)
+        + fabs (img->width * sin_r);
+    }
+# endif
+
+  /* Translate so (0, 0) is in the centre of the image.  */
+  INIT_MATRIX (t);
+  t[2][0] = img->width/2;
+  t[2][1] = img->height/2;
+
+  MULT_MATRICES (tm, t, tmp);
+
+  /* Rotate.  */
+  INIT_MATRIX (rot);
+  rot[0][0] = cos_r;
+  rot[1][0] = sin_r;
+  rot[0][1] = - sin_r;
+  rot[1][1] = cos_r;
+
+  MULT_MATRICES (tmp, rot, tmp2);
+
+  /* Translate back.  */
+  INIT_MATRIX (t);
+  t[2][0] = - width/2;
+  t[2][1] = - height/2;
+
+  MULT_MATRICES (tmp2, t, tm);
 
-# ifdef HAVE_NS
-  ns_image_set_size (img->pixmap, width, height);
   img->width = width;
   img->height = height;
+#endif
+}
+
+static void
+image_set_crop (struct image *img, double tm[3][3])
+{
+#ifdef HAVE_NATIVE_TRANSFORMS
+# ifdef HAVE_IMAGEMAGICK
+  /* ImageMagick images are already cropped.  */
+  if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
+    return;
 # endif
 
 # ifdef USE_CAIRO
   img->width = width;
   img->height = height;
 # elif defined HAVE_XRENDER
-  if (img->picture)
+  if (!img->picture)
+    return;
+# endif
+
+  double m[3][3], tmp[3][3];
+  int left, top, width, height;
+  Lisp_Object x = Qnil;
+  Lisp_Object y = Qnil;
+  Lisp_Object w = Qnil;
+  Lisp_Object h = Qnil;
+  Lisp_Object crop = image_spec_value (img->spec, QCcrop, NULL);
+
+  if (!CONSP (crop))
+    return;
+  else
     {
-      double xscale = img->width / (double) width;
-      double yscale = img->height / (double) height;
+      w = XCAR (crop);
+      crop = XCDR (crop);
+      if (CONSP (crop))
+	{
+          h = XCAR (crop);
+	  crop = XCDR (crop);
+	  if (CONSP (crop))
+	    {
+              x = XCAR (crop);
+	      crop = XCDR (crop);
+	      if (CONSP (crop))
+                y = XCAR (crop);
+	    }
+	}
+    }
 
-      XTransform tmat
-	= {{{XDoubleToFixed (xscale), XDoubleToFixed (0), XDoubleToFixed (0)},
-	    {XDoubleToFixed (0), XDoubleToFixed (yscale), XDoubleToFixed (0)},
-	    {XDoubleToFixed (0), XDoubleToFixed (0), XDoubleToFixed (1)}}};
+  if (FIXNATP (w) && XFIXNAT (w) < img->width)
+    width = XFIXNAT (w);
+  else
+    width = img->width;
 
-      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
-			       0, 0);
-      XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+  if (TYPE_RANGED_FIXNUMP (int, x))
+    {
+      left = XFIXNUM (x);
+      if (left < 0)
+        left = img->width - width + left;
+    }
+  else
+    left = (img->width - width)/2;
+
+  if (FIXNATP (h) && XFIXNAT (h) < img->height)
+    height = XFIXNAT (h);
+  else
+    height = img->height;
+
+  if (TYPE_RANGED_FIXNUMP (int, y))
+    {
+      top = XFIXNUM (y);
+      if (top < 0)
+        top = img->height - height + top;
+    }
+  else
+    top = (img->height - height)/2;
+
+  /* Limit the output to the dimensions of the original image.  */
+  if (left < 0)
+    {
+      width = img->width + left;
+      left = 0;
+    }
 
-      img->width = width;
-      img->height = height;
+  if (width + left > img->width)
+    width = img->width - left;
+
+  if (top < 0)
+    {
+      height = img->height + top;
+      top = 0;
     }
+
+  if (height + top > img->height)
+    height = img->height - top;
+
+  INIT_MATRIX (m);
+  m[2][0] = left;
+  m[2][1] = top;
+
+  MULT_MATRICES (tm, m, tmp);
+  COPY_MATRIX (tmp, tm);
+
+  img->width = width;
+  img->height = height;
+#endif
+}
+
+static void
+image_set_size (struct image *img, double tm[3][3])
+{
+#ifdef HAVE_NATIVE_TRANSFORMS
+# ifdef HAVE_IMAGEMAGICK
+  /* ImageMagick images are already the correct size.  */
+  if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
+    return;
 # endif
+
+# ifdef HAVE_XRENDER
+  if (!img->picture)
+    return;
+# endif
+
+  int width, height;
+
+  compute_image_size (img->width, img->height, img->spec, &width, &height);
+
+# if defined (HAVE_NS) || defined (HAVE_XRENDER)
+  double rm[3][3], tmp[3][3];
+  double xscale, yscale;
+
+  xscale = img->width / (double) width;
+  yscale = img->height / (double) height;
+
+  INIT_MATRIX (rm);
+  rm[0][0] = xscale;
+  rm[1][1] = yscale;
+
+  MULT_MATRICES (tm, rm, tmp);
+  COPY_MATRIX (tmp, tm);
+
+  img->width = width;
+  img->height = height;
+# endif
+
 # ifdef HAVE_NTGUI
   /* Under HAVE_NTGUI, we will scale the image on the fly, when we
      draw it.  See w32term.c:x_draw_image_foreground.  */
@@ -1820,6 +2021,36 @@ image_set_image_size (struct frame *f, struct image *img)
 #endif
 }
 
+static void
+image_set_transform (struct frame *f, struct image *img, double matrix[3][3])
+{
+  /* TODO: Add MS Windows support.  */
+#ifdef HAVE_NATIVE_TRANSFORMS
+# if defined (HAVE_NS)
+  /* Under NS the transform is applied to the drawing surface at
+     drawing time, so store it for later.  */
+  ns_image_set_transform (img->pixmap, matrix);
+# elif defined (HAVE_XRENDER)
+  if (img->picture)
+    {
+      XTransform tmat
+	= {{{XDoubleToFixed (matrix[0][0]),
+             XDoubleToFixed (matrix[1][0]),
+             XDoubleToFixed (matrix[2][0])},
+	    {XDoubleToFixed (matrix[0][1]),
+             XDoubleToFixed (matrix[1][1]),
+             XDoubleToFixed (matrix[2][1])},
+	    {XDoubleToFixed (matrix[0][2]),
+             XDoubleToFixed (matrix[1][2]),
+             XDoubleToFixed (matrix[2][2])}}};
+
+      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
+			       0, 0);
+      XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+    }
+# endif
+#endif
+}
 
 /* Return the id of image with Lisp specification SPEC on frame F.
    SPEC must be a valid Lisp image specification (see valid_image_p).  */
@@ -1875,7 +2106,16 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	     `:background COLOR'.  */
 	  Lisp_Object ascent, margin, relief, bg;
 	  int relief_bound;
-          image_set_image_size (f, img);
+
+#ifdef HAVE_NATIVE_TRANSFORMS
+          double transform_matrix[3][3];
+
+          INIT_MATRIX (transform_matrix);
+          image_set_rotation (img, transform_matrix);
+          image_set_crop (img, transform_matrix);
+          image_set_size (img, transform_matrix);
+          image_set_transform (f, img, transform_matrix);
+#endif
 
 	  ascent = image_spec_value (spec, QCascent, NULL);
 	  if (FIXNUMP (ascent))
@@ -9672,9 +9912,9 @@ DEFUN ("lookup-image", Flookup_image, Slookup_image, 1, 1, 0,
 			    Initialization
  ***********************************************************************/
 
-DEFUN ("image-scaling-p", Fimage_scaling_p, Simage_scaling_p, 0, 1, 0,
-       doc: /* Test whether FRAME supports resizing images.
-Return t if FRAME supports native scaling, nil otherwise.  */)
+DEFUN ("image-transforms-p", Fimage_transforms_p, Simage_transforms_p, 0, 1, 0,
+       doc: /* Test whether FRAME supports image transformation.
+Return t if FRAME supports native transforms, nil otherwise.  */)
      (Lisp_Object frame)
 {
 #if defined (USE_CAIRO) || defined (HAVE_NS) || defined (HAVE_NTGUI)
@@ -9934,7 +10174,7 @@ non-numeric, there is no explicit limit on the size of images.  */);
   defsubr (&Slookup_image);
 #endif
 
-  defsubr (&Simage_scaling_p);
+  defsubr (&Simage_transforms_p);
 
   DEFVAR_BOOL ("cross-disabled-images", cross_disabled_images,
     doc: /* Non-nil means always draw a cross over disabled images.
diff --git a/src/nsimage.m b/src/nsimage.m
index 0249d22aca..7268e66263 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -76,9 +76,8 @@ Updated by Christian Limpach (chris@nice.ch)
 {
   EmacsImage *eImg = nil;
   NSSize size;
-  Lisp_Object lisp_index, lisp_rotation;
+  Lisp_Object lisp_index;
   unsigned int index;
-  double rotation;
 
   NSTRACE ("ns_load_image");
 
@@ -87,9 +86,6 @@ Updated by Christian Limpach (chris@nice.ch)
   lisp_index = Fplist_get (XCDR (img->spec), QCindex);
   index = FIXNUMP (lisp_index) ? XFIXNAT (lisp_index) : 0;
 
-  lisp_rotation = Fplist_get (XCDR (img->spec), QCrotation);
-  rotation = NUMBERP (lisp_rotation) ? XFLOATINT (lisp_rotation) : 0;
-
   if (STRINGP (spec_file))
     {
       eImg = [EmacsImage allocInitFromFile: spec_file];
@@ -119,13 +115,6 @@ Updated by Christian Limpach (chris@nice.ch)
 
   img->lisp_data = [eImg getMetadata];
 
-  if (rotation != 0)
-    {
-      EmacsImage *temp = [eImg rotate:rotation];
-      [eImg release];
-      eImg = temp;
-    }
-
   size = [eImg size];
   img->width = size.width;
   img->height = size.height;
@@ -155,6 +144,12 @@ Updated by Christian Limpach (chris@nice.ch)
   [(EmacsImage *)img setSize:NSMakeSize (width, height)];
 }
 
+void
+ns_image_set_transform (void *img, double m[3][3])
+{
+  [(EmacsImage *)img setTransform:m];
+}
+
 unsigned long
 ns_get_pixel (void *img, int x, int y)
 {
@@ -225,6 +220,7 @@ - (void)dealloc
 {
   [stippleMask release];
   [bmRep release];
+  [transform release];
   [super dealloc];
 }
 
@@ -528,42 +524,16 @@ - (BOOL)setFrame: (unsigned int) index
   return YES;
 }
 
-- (instancetype)rotate: (double)rotation
+- (void)setTransform: (double[3][3]) m
 {
-  EmacsImage *new_image;
-  NSPoint new_origin;
-  NSSize new_size, size = [self size];
-  NSRect rect = { NSZeroPoint, [self size] };
-
-  /* Create a bezier path of the outline of the image and do the
-   * rotation on it.  */
-  NSBezierPath *bounds_path = [NSBezierPath bezierPathWithRect:rect];
-  NSAffineTransform *transform = [NSAffineTransform transform];
-  [transform rotateByDegrees: rotation * -1];
-  [bounds_path transformUsingAffineTransform:transform];
-
-  /* Now we can find out how large the rotated image needs to be.  */
-  new_size = [bounds_path bounds].size;
-  new_image = [[EmacsImage alloc] initWithSize:new_size];
-
-  new_origin = NSMakePoint((new_size.width - size.width)/2,
-                           (new_size.height - size.height)/2);
-
-  [new_image lockFocus];
-
-  /* Create the final transform.  */
-  transform = [NSAffineTransform transform];
-  [transform translateXBy:new_size.width/2 yBy:new_size.height/2];
-  [transform rotateByDegrees: rotation * -1];
-  [transform translateXBy:-new_size.width/2 yBy:-new_size.height/2];
-
-  [transform concat];
-  [self drawAtPoint:new_origin fromRect:NSZeroRect
-          operation:NSCompositingOperationCopy fraction:1];
-
-  [new_image unlockFocus];
-
-  return new_image;
+  transform = [[NSAffineTransform transform] retain];
+  NSAffineTransformStruct tm
+    = { m[0][0], m[0][1], m[1][0], m[1][1], m[2][0], m[2][1]};
+  [transform setTransformStruct:tm];
+
+  /* Because the transform is applied to the drawing surface, and not
+     the image itself, we need to invert it.  */
+  [transform invert];
 }
 
 @end
diff --git a/src/nsterm.h b/src/nsterm.h
index 1e56276ca3..567f462ec6 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -632,6 +632,8 @@ typedef id instancetype;
   unsigned char *pixmapData[5]; /* shortcut to access pixel data */
   NSColor *stippleMask;
   unsigned long xbm_fg;
+@public
+  NSAffineTransform *transform;
 }
 + (instancetype)allocInitFromFile: (Lisp_Object)file;
 - (void)dealloc;
@@ -648,7 +650,7 @@ typedef id instancetype;
 - (NSColor *)stippleMask;
 - (Lisp_Object)getMetadata;
 - (BOOL)setFrame: (unsigned int) index;
-- (instancetype)rotate: (double)rotation;
+- (void)setTransform: (double[3][3]) m;
 @end
 
 
@@ -1201,6 +1203,7 @@ extern bool ns_load_image (struct frame *f, struct image *img,
 extern int ns_image_width (void *img);
 extern int ns_image_height (void *img);
 extern void ns_image_set_size (void *img, int width, int height);
+extern void ns_image_set_transform (void *img, double m[3][3]);
 extern unsigned long ns_get_pixel (void *img, int x, int y);
 extern void ns_put_pixel (void *img, int x, int y, unsigned long argb);
 extern void ns_set_alpha (void *img, int x, int y, unsigned char a);
diff --git a/src/nsterm.m b/src/nsterm.m
index 0cae5e9d44..f12e98ebfd 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3813,21 +3813,34 @@ Function modeled after x_draw_glyph_string_box ().
   /* Draw the image... do we need to draw placeholder if img == nil?  */
   if (img != nil)
     {
-#ifdef NS_IMPL_COCOA
+      /* The idea here is that the clipped area is set in the normal
+         view coordinate system, then we transform the coordinate
+         system so that when we draw the image it is rotated, resized
+         or whatever as required.  This is kind of backwards, but
+         there's no way to apply the transform to the image without
+         creating a whole new bitmap.  */
       NSRect dr = NSMakeRect (x, y, s->slice.width, s->slice.height);
-      NSRect ir = NSMakeRect (s->slice.x,
-                              s->img->height - s->slice.y - s->slice.height,
-                              s->slice.width, s->slice.height);
-      [img drawInRect: dr
-             fromRect: ir
-             operation: NSCompositingOperationSourceOver
-              fraction: 1.0
-           respectFlipped: YES
-                hints: nil];
-#else
-      [img compositeToPoint: NSMakePoint (x, y + s->slice.height)
-                  operation: NSCompositingOperationSourceOver];
-#endif
+      NSRect ir = NSMakeRect (0, 0, [img size].width, [img size].height);
+
+      NSAffineTransform *setOrigin = [NSAffineTransform transform];
+
+      [[NSGraphicsContext currentContext] saveGraphicsState];
+
+      /* Because of the transforms it's far too difficult to work out
+         what portion of the original, untransformed, image will be
+         drawn, so the clipping area will ensure we draw only the
+         correct bit.  */
+      NSRectClip (dr);
+
+      [setOrigin translateXBy:x - s->slice.x yBy:y - s->slice.y];
+      [setOrigin concat];
+      [img->transform concat];
+
+      [img drawInRect:ir fromRect:ir
+            operation:NSCompositingOperationSourceOver
+             fraction:1.0 respectFlipped:YES hints:nil];
+
+      [[NSGraphicsContext currentContext] restoreGraphicsState];
     }
 
   if (s->hl == DRAW_CURSOR)
diff --git a/src/xterm.c b/src/xterm.c
index 559d1b4892..7b407d8971 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2856,10 +2856,7 @@ x_composite_image (struct glyph_string *s, Pixmap dest,
       destination = XRenderCreatePicture (display, dest,
                                           default_format, 0, &attr);
 
-      /* FIXME: It may make sense to use PictOpSrc instead of
-         PictOpOver, as I don't know if we care about alpha values too
-         much here.  */
-      XRenderComposite (display, PictOpOver,
+      XRenderComposite (display, PictOpSrc,
                         s->img->picture, s->img->mask_picture, destination,
                         srcX, srcY,
                         srcX, srcY,
-- 
2.21.0

Re: Native image rotation

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Sun, 19 May 2019 21:29:02 +0100
> 
> BTW, I think native image rotation should support arbitrary angles (if
> possible), if for no other reason than ImageMagick also supporting them.

As mentioned in past discussions, such rotations create conceptual
problems regarding the dimensions of the result and whether we should
or shouldn't clip the results.

Re: Native image rotation

[Alan Third]

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

On Tue, May 21, 2019 at 09:11:46PM +0100, Alan Third wrote:
> On Mon, May 20, 2019 at 07:18:48PM +0100, Alan Third wrote:
> > 
> > I plan to go back and rewrite it to only handle 90 degree increments
> > and submit that, at least for now.
> 
> Patch attached.
> 
> I left the rotation code largely untouched in case we want to allow
> non‐90 degree increments in the future, but that might be a waste of
> time.

I’ve decided it is a waste of time and have removed it. I’ve also
reverted the order of operations to match ImageMagick as the use cases
I’d imagined requiring a different order aren’t so useful with 90
degree rotations.

Aside from that it’s basically the same as the last patch. If nobody
complains I’ll push this in a few days.
-- 
Alan Third

[-- Attachment #2: v3-0001-Add-native-image-rotation-and-cropping.patch --]
[-- Type: text/plain, Size: 23421 bytes --]

From 05610b355f75d51299b93b731f1d14d7f66f0673 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 23 Feb 2019 20:56:48 +0000
Subject: [PATCH v3] Add native image rotation and cropping

* lisp/image.el (image--get-imagemagick-and-warn): Only fallback to
ImageMagick if native transforms aren't available.
* src/dispextern.h (INIT_MATRIX, COPY_MATRIX, MULT_MATRICES): New
macros for matrix manipulation.
(HAVE_NATIVE_SCALING, HAVE_NATIVE_TRANSFORMS): Rename and change all
relevant locations.
* src/image.c (x_set_image_rotation):
(x_set_transform): New functions.
(x_set_image_size): Use transform matrix for resizing under X and NS.
(x_set_image_crop): New function.
(lookup_image): Use the new transform functions.
(Fimage_scaling_p, Fimage_transforms_p): Rename and update all
callers.
* src/nsimage.m (ns_load_image): Remove rotation code.
(ns_image_set_transform): New function.
([EmacsImage dealloc]): Release the saved transform.
([EmacsImage rotate:]): Remove unneeded method.
([EmacsImage setTransform:]): New method.
* src/nsterm.h (EmacsImage): Add transform property and update method
definitions.
* src/nsterm.m (ns_dumpglyphs_image): Use the transform to draw the
image correctly.
* src/xterm.c (x_composite_image): Use PictOpSrc as we don't care
about alpha values here.
* doc/lispref/display.texi (Image Descriptors): Add :rotation.
(ImageMagick Images): Remove :rotation.
---
 doc/lispref/display.texi |  21 +--
 etc/NEWS                 |   6 +-
 lisp/image.el            |   5 +-
 src/dispextern.h         |  22 ++-
 src/image.c              | 283 +++++++++++++++++++++++++++++++++++----
 src/nsimage.m            |  64 +++------
 src/nsterm.h             |   5 +-
 src/nsterm.m             |  41 ++++--
 src/xterm.c              |   5 +-
 9 files changed, 343 insertions(+), 109 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index a2ed4b3891..283928c41e 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5176,6 +5176,9 @@ Image Descriptors
 specified, the height/width will be adjusted by the specified scaling
 factor.
 
+@item :rotation @var{angle}
+Specifies a rotation angle in degrees.
+
 @item :index @var{frame}
 @xref{Multi-Frame Images}.
 
@@ -5318,14 +5321,15 @@ Image Descriptors
 (@pxref{Input Focus}).
 @end defun
 
-@defun image-scaling-p &optional frame
-This function returns @code{t} if @var{frame} supports image scaling.
-@var{frame} @code{nil} or omitted means to use the selected frame
-(@pxref{Input Focus}).
+@defun image-transforms-p &optional frame
+This function returns @code{t} if @var{frame} supports image scaling
+and rotation.  @var{frame} @code{nil} or omitted means to use the
+selected frame (@pxref{Input Focus}).
 
-If image scaling is not supported, @code{:width}, @code{:height},
-@code{:scale}, @code{:max-width} and @code{:max-height} will only be
-usable through ImageMagick, if available (@pxref{ImageMagick Images}).
+If image transforms are not supported, @code{:rotation},
+@code{:width}, @code{:height}, @code{:scale}, @code{:max-width} and
+@code{:max-height} will only be usable through ImageMagick, if
+available (@pxref{ImageMagick Images}).
 @end defun
 
 @node XBM Images
@@ -5469,9 +5473,6 @@ ImageMagick Images
 image data, as found in @code{image-format-suffixes}.  This is used
 when the image does not have an associated file name, to provide a
 hint to ImageMagick to help it detect the image type.
-
-@item :rotation @var{angle}
-Specifies a rotation angle in degrees.
 @end table
 
 @node SVG Images
diff --git a/etc/NEWS b/etc/NEWS
index 72702a9aaa..3aae88f4a8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2001,14 +2001,14 @@ buffer's 'default-directory' and invoke that file name handler to make
 the process.  That way 'make-process' can start remote processes.
 
 +++
-** Emacs now supports resizing (scaling) of images without ImageMagick.
+** Emacs now supports resizing and rotating images without ImageMagick.
 All modern systems are supported by this feature.  (On GNU and Unix
 systems, Cairo drawing or the XRender extension to X11 is required for
 this to be available; the configure script will test for it and, if
 found, enable scaling.)
 
-The new function 'image-scaling-p' can be used to test whether any
-given frame supports resizing.
+The new function 'image-transforms-p' can be used to test whether any
+given frame supports this capability.
 
 +++
 ** '(locale-info 'paper)' now returns the paper size on systems that support it.
diff --git a/lisp/image.el b/lisp/image.el
index 6cc2cc3902..e360ba8fa6 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -991,11 +991,12 @@ image--get-image
     image))
 
 (defun image--get-imagemagick-and-warn ()
-  (unless (or (fboundp 'imagemagick-types) (image-scaling-p))
+  (unless (or (fboundp 'imagemagick-types) (image-transforms-p))
     (error "Cannot rescale images on this terminal"))
   (let ((image (image--get-image)))
     (image-flush image)
-    (when (fboundp 'imagemagick-types)
+    (when (and (fboundp 'imagemagick-types)
+               (not (image-transforms-p)))
       (plist-put (cdr image) :type 'imagemagick))
     image))
 
diff --git a/src/dispextern.h b/src/dispextern.h
index ec1c9620be..95b6b48116 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -2972,7 +2972,25 @@ struct redisplay_interface
 #ifdef HAVE_WINDOW_SYSTEM
 
 # if defined USE_CAIRO || defined HAVE_XRENDER || defined HAVE_NS || defined HAVE_NTGUI
-#  define HAVE_NATIVE_SCALING
+#  define HAVE_NATIVE_TRANSFORMS
+
+#  define INIT_MATRIX(m)                          \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++)                 \
+      m[i][j] = (i == j) ? 1 : 0;
+
+#  define COPY_MATRIX(a, b)                       \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++)                 \
+      b[i][j] = a[i][j];
+
+#  define MULT_MATRICES(a, b, result)             \
+  for (int i = 0 ; i < 3 ; i++)                   \
+    for (int j = 0 ; j < 3 ; j++) {               \
+      double sum = 0;                             \
+      for (int k = 0 ; k < 3 ; k++)               \
+        sum += a[k][j] * b[i][k];                 \
+      result[i][j] = sum;}
 # endif
 
 /* Structure describing an image.  Specific image formats like XBM are
@@ -2998,7 +3016,7 @@ struct image
      synchronized to Pixmap.  */
   XImage *ximg, *mask_img;
 
-# ifdef HAVE_NATIVE_SCALING
+# ifdef HAVE_NATIVE_TRANSFORMS
   /* Picture versions of pixmap and mask for compositing.  */
   Picture picture, mask_picture;
 # endif
diff --git a/src/image.c b/src/image.c
index 57b405f6db..8f951f0fbf 100644
--- a/src/image.c
+++ b/src/image.c
@@ -1669,7 +1669,7 @@ postprocess_image (struct frame *f, struct image *img)
     }
 }
 
-#if defined (HAVE_IMAGEMAGICK) || defined (HAVE_NATIVE_SCALING)
+#if defined (HAVE_IMAGEMAGICK) || defined (HAVE_NATIVE_TRANSFORMS)
 /* Scale an image size by returning SIZE / DIVISOR * MULTIPLIER,
    safely rounded and clipped to int range.  */
 
@@ -1768,49 +1768,241 @@ compute_image_size (size_t width, size_t height,
   *d_width = desired_width;
   *d_height = desired_height;
 }
-#endif /* HAVE_IMAGEMAGICK || HAVE_NATIVE_SCALING */
+#endif /* HAVE_IMAGEMAGICK || HAVE_NATIVE_TRANSFORMS */
 
 static void
-image_set_image_size (struct frame *f, struct image *img)
+image_set_rotation (struct image *img, double tm[3][3])
 {
-#ifdef HAVE_NATIVE_SCALING
+#ifdef HAVE_NATIVE_TRANSFORMS
 # ifdef HAVE_IMAGEMAGICK
-  /* ImageMagick images are already the correct size.  */
+  /* ImageMagick images are already rotated.  */
   if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
     return;
 # endif
 
-  int width, height;
-  compute_image_size (img->width, img->height, img->spec, &width, &height);
+# ifdef HAVE_XRENDER
+  if (!img->picture)
+    return;
+# endif
+
+  Lisp_Object value;
+  double t[3][3], rot[3][3], tmp[3][3], tmp2[3][3];
+  int rotation, cos_r, sin_r, width, height;
+
+  value = image_spec_value (img->spec, QCrotation, NULL);
+  if (! NUMBERP (value))
+    return;
+
+  rotation = XFLOATINT (value);
+  rotation = rotation % 360;
+
+  if (rotation < 0)
+    rotation += 360;
+
+  if (rotation == 0)
+    return;
+
+  if (rotation == 90)
+    {
+      width = img->height;
+      height = img->width;
+
+      cos_r = 0;
+      sin_r = 1;
+    }
+  else if (rotation == 180)
+    {
+      width = img->width;
+      height = img->height;
+
+      cos_r = -1;
+      sin_r = 0;
+    }
+  else if (rotation == 270)
+    {
+      width = img->height;
+      height = img->width;
+
+      cos_r = 0;
+      sin_r = -1;
+    }
+  else
+    {
+      image_error ("Native image rotation only supports multiples of 90 degrees");
+      return;
+    }
+
+  /* Translate so (0, 0) is in the centre of the image.  */
+  INIT_MATRIX (t);
+  t[2][0] = img->width/2;
+  t[2][1] = img->height/2;
+
+  MULT_MATRICES (tm, t, tmp);
+
+  /* Rotate.  */
+  INIT_MATRIX (rot);
+  rot[0][0] = cos_r;
+  rot[1][0] = sin_r;
+  rot[0][1] = - sin_r;
+  rot[1][1] = cos_r;
+
+  MULT_MATRICES (tmp, rot, tmp2);
+
+  /* Translate back.  */
+  INIT_MATRIX (t);
+  t[2][0] = - width/2;
+  t[2][1] = - height/2;
+
+  MULT_MATRICES (tmp2, t, tm);
 
-# ifdef HAVE_NS
-  ns_image_set_size (img->pixmap, width, height);
   img->width = width;
   img->height = height;
+#endif
+}
+
+static void
+image_set_crop (struct image *img, double tm[3][3])
+{
+#ifdef HAVE_NATIVE_TRANSFORMS
+# ifdef HAVE_IMAGEMAGICK
+  /* ImageMagick images are already cropped.  */
+  if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
+    return;
 # endif
 
 # ifdef USE_CAIRO
   img->width = width;
   img->height = height;
 # elif defined HAVE_XRENDER
-  if (img->picture)
+  if (!img->picture)
+    return;
+# endif
+
+  double m[3][3], tmp[3][3];
+  int left, top, width, height;
+  Lisp_Object x = Qnil;
+  Lisp_Object y = Qnil;
+  Lisp_Object w = Qnil;
+  Lisp_Object h = Qnil;
+  Lisp_Object crop = image_spec_value (img->spec, QCcrop, NULL);
+
+  if (!CONSP (crop))
+    return;
+  else
     {
-      double xscale = img->width / (double) width;
-      double yscale = img->height / (double) height;
+      w = XCAR (crop);
+      crop = XCDR (crop);
+      if (CONSP (crop))
+	{
+          h = XCAR (crop);
+	  crop = XCDR (crop);
+	  if (CONSP (crop))
+	    {
+              x = XCAR (crop);
+	      crop = XCDR (crop);
+	      if (CONSP (crop))
+                y = XCAR (crop);
+	    }
+	}
+    }
 
-      XTransform tmat
-	= {{{XDoubleToFixed (xscale), XDoubleToFixed (0), XDoubleToFixed (0)},
-	    {XDoubleToFixed (0), XDoubleToFixed (yscale), XDoubleToFixed (0)},
-	    {XDoubleToFixed (0), XDoubleToFixed (0), XDoubleToFixed (1)}}};
+  if (FIXNATP (w) && XFIXNAT (w) < img->width)
+    width = XFIXNAT (w);
+  else
+    width = img->width;
 
-      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
-			       0, 0);
-      XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+  if (TYPE_RANGED_FIXNUMP (int, x))
+    {
+      left = XFIXNUM (x);
+      if (left < 0)
+        left = img->width - width + left;
+    }
+  else
+    left = (img->width - width)/2;
+
+  if (FIXNATP (h) && XFIXNAT (h) < img->height)
+    height = XFIXNAT (h);
+  else
+    height = img->height;
+
+  if (TYPE_RANGED_FIXNUMP (int, y))
+    {
+      top = XFIXNUM (y);
+      if (top < 0)
+        top = img->height - height + top;
+    }
+  else
+    top = (img->height - height)/2;
+
+  /* Negative values operate from the right and bottom of the image
+     instead of the left and top.  */
+  if (left < 0)
+    {
+      width = img->width + left;
+      left = 0;
+    }
+
+  if (width + left > img->width)
+    width = img->width - left;
 
-      img->width = width;
-      img->height = height;
+  if (top < 0)
+    {
+      height = img->height + top;
+      top = 0;
     }
+
+  if (height + top > img->height)
+    height = img->height - top;
+
+  INIT_MATRIX (m);
+  m[2][0] = left;
+  m[2][1] = top;
+
+  MULT_MATRICES (tm, m, tmp);
+  COPY_MATRIX (tmp, tm);
+
+  img->width = width;
+  img->height = height;
+#endif
+}
+
+static void
+image_set_size (struct image *img, double tm[3][3])
+{
+#ifdef HAVE_NATIVE_TRANSFORMS
+# ifdef HAVE_IMAGEMAGICK
+  /* ImageMagick images are already the correct size.  */
+  if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
+    return;
+# endif
+
+# ifdef HAVE_XRENDER
+  if (!img->picture)
+    return;
+# endif
+
+  int width, height;
+
+  compute_image_size (img->width, img->height, img->spec, &width, &height);
+
+# if defined (HAVE_NS) || defined (HAVE_XRENDER)
+  double rm[3][3], tmp[3][3];
+  double xscale, yscale;
+
+  xscale = img->width / (double) width;
+  yscale = img->height / (double) height;
+
+  INIT_MATRIX (rm);
+  rm[0][0] = xscale;
+  rm[1][1] = yscale;
+
+  MULT_MATRICES (tm, rm, tmp);
+  COPY_MATRIX (tmp, tm);
+
+  img->width = width;
+  img->height = height;
 # endif
+
 # ifdef HAVE_NTGUI
   /* Under HAVE_NTGUI, we will scale the image on the fly, when we
      draw it.  See w32term.c:x_draw_image_foreground.  */
@@ -1820,6 +2012,36 @@ image_set_image_size (struct frame *f, struct image *img)
 #endif
 }
 
+static void
+image_set_transform (struct frame *f, struct image *img, double matrix[3][3])
+{
+  /* TODO: Add MS Windows support.  */
+#ifdef HAVE_NATIVE_TRANSFORMS
+# if defined (HAVE_NS)
+  /* Under NS the transform is applied to the drawing surface at
+     drawing time, so store it for later.  */
+  ns_image_set_transform (img->pixmap, matrix);
+# elif defined (HAVE_XRENDER)
+  if (img->picture)
+    {
+      XTransform tmat
+	= {{{XDoubleToFixed (matrix[0][0]),
+             XDoubleToFixed (matrix[1][0]),
+             XDoubleToFixed (matrix[2][0])},
+	    {XDoubleToFixed (matrix[0][1]),
+             XDoubleToFixed (matrix[1][1]),
+             XDoubleToFixed (matrix[2][1])},
+	    {XDoubleToFixed (matrix[0][2]),
+             XDoubleToFixed (matrix[1][2]),
+             XDoubleToFixed (matrix[2][2])}}};
+
+      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
+			       0, 0);
+      XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+    }
+# endif
+#endif
+}
 
 /* Return the id of image with Lisp specification SPEC on frame F.
    SPEC must be a valid Lisp image specification (see valid_image_p).  */
@@ -1875,7 +2097,16 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	     `:background COLOR'.  */
 	  Lisp_Object ascent, margin, relief, bg;
 	  int relief_bound;
-          image_set_image_size (f, img);
+
+#ifdef HAVE_NATIVE_TRANSFORMS
+          double transform_matrix[3][3];
+
+          INIT_MATRIX (transform_matrix);
+          image_set_size (img, transform_matrix);
+          image_set_crop (img, transform_matrix);
+          image_set_rotation (img, transform_matrix);
+          image_set_transform (f, img, transform_matrix);
+#endif
 
 	  ascent = image_spec_value (spec, QCascent, NULL);
 	  if (FIXNUMP (ascent))
@@ -9672,9 +9903,9 @@ DEFUN ("lookup-image", Flookup_image, Slookup_image, 1, 1, 0,
 			    Initialization
  ***********************************************************************/
 
-DEFUN ("image-scaling-p", Fimage_scaling_p, Simage_scaling_p, 0, 1, 0,
-       doc: /* Test whether FRAME supports resizing images.
-Return t if FRAME supports native scaling, nil otherwise.  */)
+DEFUN ("image-transforms-p", Fimage_transforms_p, Simage_transforms_p, 0, 1, 0,
+       doc: /* Test whether FRAME supports image transformation.
+Return t if FRAME supports native transforms, nil otherwise.  */)
      (Lisp_Object frame)
 {
 #if defined (USE_CAIRO) || defined (HAVE_NS) || defined (HAVE_NTGUI)
@@ -9934,7 +10165,7 @@ non-numeric, there is no explicit limit on the size of images.  */);
   defsubr (&Slookup_image);
 #endif
 
-  defsubr (&Simage_scaling_p);
+  defsubr (&Simage_transforms_p);
 
   DEFVAR_BOOL ("cross-disabled-images", cross_disabled_images,
     doc: /* Non-nil means always draw a cross over disabled images.
diff --git a/src/nsimage.m b/src/nsimage.m
index 0249d22aca..7268e66263 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -76,9 +76,8 @@ Updated by Christian Limpach (chris@nice.ch)
 {
   EmacsImage *eImg = nil;
   NSSize size;
-  Lisp_Object lisp_index, lisp_rotation;
+  Lisp_Object lisp_index;
   unsigned int index;
-  double rotation;
 
   NSTRACE ("ns_load_image");
 
@@ -87,9 +86,6 @@ Updated by Christian Limpach (chris@nice.ch)
   lisp_index = Fplist_get (XCDR (img->spec), QCindex);
   index = FIXNUMP (lisp_index) ? XFIXNAT (lisp_index) : 0;
 
-  lisp_rotation = Fplist_get (XCDR (img->spec), QCrotation);
-  rotation = NUMBERP (lisp_rotation) ? XFLOATINT (lisp_rotation) : 0;
-
   if (STRINGP (spec_file))
     {
       eImg = [EmacsImage allocInitFromFile: spec_file];
@@ -119,13 +115,6 @@ Updated by Christian Limpach (chris@nice.ch)
 
   img->lisp_data = [eImg getMetadata];
 
-  if (rotation != 0)
-    {
-      EmacsImage *temp = [eImg rotate:rotation];
-      [eImg release];
-      eImg = temp;
-    }
-
   size = [eImg size];
   img->width = size.width;
   img->height = size.height;
@@ -155,6 +144,12 @@ Updated by Christian Limpach (chris@nice.ch)
   [(EmacsImage *)img setSize:NSMakeSize (width, height)];
 }
 
+void
+ns_image_set_transform (void *img, double m[3][3])
+{
+  [(EmacsImage *)img setTransform:m];
+}
+
 unsigned long
 ns_get_pixel (void *img, int x, int y)
 {
@@ -225,6 +220,7 @@ - (void)dealloc
 {
   [stippleMask release];
   [bmRep release];
+  [transform release];
   [super dealloc];
 }
 
@@ -528,42 +524,16 @@ - (BOOL)setFrame: (unsigned int) index
   return YES;
 }
 
-- (instancetype)rotate: (double)rotation
+- (void)setTransform: (double[3][3]) m
 {
-  EmacsImage *new_image;
-  NSPoint new_origin;
-  NSSize new_size, size = [self size];
-  NSRect rect = { NSZeroPoint, [self size] };
-
-  /* Create a bezier path of the outline of the image and do the
-   * rotation on it.  */
-  NSBezierPath *bounds_path = [NSBezierPath bezierPathWithRect:rect];
-  NSAffineTransform *transform = [NSAffineTransform transform];
-  [transform rotateByDegrees: rotation * -1];
-  [bounds_path transformUsingAffineTransform:transform];
-
-  /* Now we can find out how large the rotated image needs to be.  */
-  new_size = [bounds_path bounds].size;
-  new_image = [[EmacsImage alloc] initWithSize:new_size];
-
-  new_origin = NSMakePoint((new_size.width - size.width)/2,
-                           (new_size.height - size.height)/2);
-
-  [new_image lockFocus];
-
-  /* Create the final transform.  */
-  transform = [NSAffineTransform transform];
-  [transform translateXBy:new_size.width/2 yBy:new_size.height/2];
-  [transform rotateByDegrees: rotation * -1];
-  [transform translateXBy:-new_size.width/2 yBy:-new_size.height/2];
-
-  [transform concat];
-  [self drawAtPoint:new_origin fromRect:NSZeroRect
-          operation:NSCompositingOperationCopy fraction:1];
-
-  [new_image unlockFocus];
-
-  return new_image;
+  transform = [[NSAffineTransform transform] retain];
+  NSAffineTransformStruct tm
+    = { m[0][0], m[0][1], m[1][0], m[1][1], m[2][0], m[2][1]};
+  [transform setTransformStruct:tm];
+
+  /* Because the transform is applied to the drawing surface, and not
+     the image itself, we need to invert it.  */
+  [transform invert];
 }
 
 @end
diff --git a/src/nsterm.h b/src/nsterm.h
index 1e56276ca3..567f462ec6 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -632,6 +632,8 @@ typedef id instancetype;
   unsigned char *pixmapData[5]; /* shortcut to access pixel data */
   NSColor *stippleMask;
   unsigned long xbm_fg;
+@public
+  NSAffineTransform *transform;
 }
 + (instancetype)allocInitFromFile: (Lisp_Object)file;
 - (void)dealloc;
@@ -648,7 +650,7 @@ typedef id instancetype;
 - (NSColor *)stippleMask;
 - (Lisp_Object)getMetadata;
 - (BOOL)setFrame: (unsigned int) index;
-- (instancetype)rotate: (double)rotation;
+- (void)setTransform: (double[3][3]) m;
 @end
 
 
@@ -1201,6 +1203,7 @@ extern bool ns_load_image (struct frame *f, struct image *img,
 extern int ns_image_width (void *img);
 extern int ns_image_height (void *img);
 extern void ns_image_set_size (void *img, int width, int height);
+extern void ns_image_set_transform (void *img, double m[3][3]);
 extern unsigned long ns_get_pixel (void *img, int x, int y);
 extern void ns_put_pixel (void *img, int x, int y, unsigned long argb);
 extern void ns_set_alpha (void *img, int x, int y, unsigned char a);
diff --git a/src/nsterm.m b/src/nsterm.m
index 0cae5e9d44..f12e98ebfd 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3813,21 +3813,34 @@ Function modeled after x_draw_glyph_string_box ().
   /* Draw the image... do we need to draw placeholder if img == nil?  */
   if (img != nil)
     {
-#ifdef NS_IMPL_COCOA
+      /* The idea here is that the clipped area is set in the normal
+         view coordinate system, then we transform the coordinate
+         system so that when we draw the image it is rotated, resized
+         or whatever as required.  This is kind of backwards, but
+         there's no way to apply the transform to the image without
+         creating a whole new bitmap.  */
       NSRect dr = NSMakeRect (x, y, s->slice.width, s->slice.height);
-      NSRect ir = NSMakeRect (s->slice.x,
-                              s->img->height - s->slice.y - s->slice.height,
-                              s->slice.width, s->slice.height);
-      [img drawInRect: dr
-             fromRect: ir
-             operation: NSCompositingOperationSourceOver
-              fraction: 1.0
-           respectFlipped: YES
-                hints: nil];
-#else
-      [img compositeToPoint: NSMakePoint (x, y + s->slice.height)
-                  operation: NSCompositingOperationSourceOver];
-#endif
+      NSRect ir = NSMakeRect (0, 0, [img size].width, [img size].height);
+
+      NSAffineTransform *setOrigin = [NSAffineTransform transform];
+
+      [[NSGraphicsContext currentContext] saveGraphicsState];
+
+      /* Because of the transforms it's far too difficult to work out
+         what portion of the original, untransformed, image will be
+         drawn, so the clipping area will ensure we draw only the
+         correct bit.  */
+      NSRectClip (dr);
+
+      [setOrigin translateXBy:x - s->slice.x yBy:y - s->slice.y];
+      [setOrigin concat];
+      [img->transform concat];
+
+      [img drawInRect:ir fromRect:ir
+            operation:NSCompositingOperationSourceOver
+             fraction:1.0 respectFlipped:YES hints:nil];
+
+      [[NSGraphicsContext currentContext] restoreGraphicsState];
     }
 
   if (s->hl == DRAW_CURSOR)
diff --git a/src/xterm.c b/src/xterm.c
index 559d1b4892..7b407d8971 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2856,10 +2856,7 @@ x_composite_image (struct glyph_string *s, Pixmap dest,
       destination = XRenderCreatePicture (display, dest,
                                           default_format, 0, &attr);
 
-      /* FIXME: It may make sense to use PictOpSrc instead of
-         PictOpOver, as I don't know if we care about alpha values too
-         much here.  */
-      XRenderComposite (display, PictOpOver,
+      XRenderComposite (display, PictOpSrc,
                         s->img->picture, s->img->mask_picture, destination,
                         srcX, srcY,
                         srcX, srcY,
-- 
2.21.0

Re: Native image rotation

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

> I’ve decided it is a waste of time and have removed it. I’ve also
> reverted the order of operations to match ImageMagick as the use cases
> I’d imagined requiring a different order aren’t so useful with 90
> degree rotations.

Sounds great -- this means that non-Imagemagick Emacs has almost feature
parity with Imagemagick Emacs (for the image formats that Emacs
supports, that is).  The only significant thing that's missing is using
exif data to get rotation correct, I think?

I previously looked at writing an exif parser in Emacs Lisp, and it
looks easy enough, and if we had that, we could also edit the exif data,
which is pretty useful.

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

Re: Native image rotation

[Alan Third]

On Sun, Jun 02, 2019 at 08:24:14PM +0200, Lars Ingebrigtsen wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > I’ve decided it is a waste of time and have removed it. I’ve also
> > reverted the order of operations to match ImageMagick as the use cases
> > I’d imagined requiring a different order aren’t so useful with 90
> > degree rotations.

I’ve just pushed this to master.

> Sounds great -- this means that non-Imagemagick Emacs has almost feature
> parity with Imagemagick Emacs (for the image formats that Emacs
> supports, that is).  The only significant thing that's missing is using
> exif data to get rotation correct, I think?

Yes, I believe so. ImageMagick seems to have a built‐in for
automatically orienting the image.

> I previously looked at writing an exif parser in Emacs Lisp, and it
> looks easy enough, and if we had that, we could also edit the exif data,
> which is pretty useful.

It would also give us the ability to display other tags, like location
data and keywords, which could be nice in an image browser type
application.
-- 
Alan Third

Re: Native image rotation

[Andy Moreton]

On Wed 05 Jun 2019, Alan Third wrote:

> On Sun, Jun 02, 2019 at 08:24:14PM +0200, Lars Ingebrigtsen wrote:
>> Alan Third <alan@idiocy.org> writes:
>> 
>> > I’ve decided it is a waste of time and have removed it. I’ve also
>> > reverted the order of operations to match ImageMagick as the use cases
>> > I’d imagined requiring a different order aren’t so useful with 90
>> > degree rotations.
>
> I’ve just pushed this to master.

Thanks for working on this.

On Windows, emacs set the image `:rotation' property in response to
`image-rotate', but does not draw a rotated image, so some further work
is needed on the Windows image display primitives.

    AndyM

Re: Native image rotation

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Thu, 06 Jun 2019 10:03:57 +0100
> 
> Thanks for working on this.
> 
> On Windows, emacs set the image `:rotation' property in response to
> `image-rotate', but does not draw a rotated image, so some further work
> is needed on the Windows image display primitives.

Native image rotation was not yet implemented on Windows.  Interested
individuals are encouraged to work on adding that, TIA.