bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third <alan@idiocy.org>]

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

On Sat, Nov 23, 2019 at 01:02:50AM +0100, Adam Sjøgren wrote:
> 
> Note, though, how the RGB image looks different than the two others, it
> has a border added.

This is caused by the smoothing of the image. After a LOT of searching
around I found out we need to set the repeat value for the Picture to
pad so that when it’s applying the filter and looking outside the
image bounds it will look for the nearest real pixel rather than
inventing a transparent black one.

Patch attached.

> I have run Gnus in Emacs for around an hour now without any crashes,
> with this patch.

Thanks!
-- 
Alan Third

[-- Attachment #2: v4-0001-Fix-image-scaling-with-masks-bug-38109.patch --]
[-- Type: text/plain, Size: 10173 bytes --]

From 4efbc28e264e2045d4836492bd7fe388e42da055 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 9 Nov 2019 17:04:25 +0000
Subject: [PATCH v4] Fix image scaling with masks (bug#38109)

* src/image.c (lookup_image): Move call to image_set_transform after
postprocess_image.
(image_create_x_image_and_pixmap_1): Use new function.
(image_set_transform): Apply the transform to the mask too.
(x_create_xrender_picture): New function.
(Create_Pixmap_From_Bitmap_Data):
(xpm_load): Use new function.
* src/xterm.c (x_composite_image): Use PictOpOver when there is a mask
so the transparency is honoured.
(x_draw_image_foreground_1): Use x_composite_image.
---
 src/image.c | 147 ++++++++++++++++++++++++++++++++++++----------------
 src/xterm.c |  11 ++--
 2 files changed, 106 insertions(+), 52 deletions(-)

diff --git a/src/image.c b/src/image.c
index 870f008b14..70d932f9ed 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2244,6 +2244,14 @@ image_set_transform (struct frame *f, struct image *img)
       XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
 			       0, 0);
       XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+
+      if (img->mask_picture)
+        {
+          XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->mask_picture,
+                                   FilterBest, 0, 0);
+          XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->mask_picture,
+                                      &tmat);
+        }
     }
 # elif defined HAVE_NTGUI
   /* Store the transform matrix for application at draw time.  */
@@ -2313,10 +2321,6 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	  Lisp_Object ascent, margin, relief, bg;
 	  int relief_bound;
 
-#ifdef HAVE_NATIVE_TRANSFORMS
-          image_set_transform (f, img);
-#endif
-
 	  ascent = image_spec_value (spec, QCascent, NULL);
 	  if (FIXNUMP (ascent))
 	    img->ascent = XFIXNUM (ascent);
@@ -2357,6 +2361,13 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	     don't have the image yet.  */
 	  if (!EQ (builtin_lisp_symbol (img->type->type), Qpostscript))
 	    postprocess_image (f, img);
+
+          /* postprocess_image above may modify the image or the mask,
+             relying on the image's real width and height, so
+             image_set_transform must be called after it.  */
+#ifdef HAVE_NATIVE_TRANSFORMS
+          image_set_transform (f, img);
+#endif
 	}
 
       unblock_input ();
@@ -2527,6 +2538,61 @@ x_destroy_x_image (XImage *ximg)
     }
 }
 
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+/* Create and return an XRender Picture for XRender transforms.  */
+static Picture
+x_create_xrender_picture (struct frame *f, Emacs_Pixmap pixmap, int depth)
+{
+  Picture p;
+  Display *display = FRAME_X_DISPLAY (f);
+  int event_basep, error_basep;
+
+  if (XRenderQueryExtension (display, &event_basep, &error_basep))
+    {
+      if (depth <= 0)
+	depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
+      if (depth == 32 || depth == 24 || depth == 8 || depth == 4 || depth == 1)
+        {
+          /* FIXME: Do we need to handle all possible bit depths?
+             XRenderFindStandardFormat supports PictStandardARGB32,
+             PictStandardRGB24, PictStandardA8, PictStandardA4,
+             PictStandardA1, and PictStandardNUM (what is this?!).
+
+             XRenderFindFormat may support more, but I don't
+             understand the documentation.  */
+          XRenderPictFormat *format;
+          format = XRenderFindStandardFormat (display,
+                                              depth == 32 ? PictStandardARGB32
+                                              : depth == 24 ? PictStandardRGB24
+                                              : depth == 8 ? PictStandardA8
+                                              : depth == 4 ? PictStandardA4
+                                              : PictStandardA1);
+
+          /* Set the Picture repeat to "pad".  This means when
+             operations look at pixels outside the image area they
+             will use the value of the nearest real pixel instead of
+             using a transparent black pixel.  */
+          XRenderPictureAttributes attr;
+          unsigned long attr_mask = CPRepeat;
+          attr.repeat = RepeatPad;
+
+          p = XRenderCreatePicture (display, pixmap, format, attr_mask, &attr);
+        }
+      else
+        {
+          image_error ("Specified image bit depth is not supported by XRender");
+          return 0;
+        }
+    }
+  else
+    {
+      /* XRender not supported on this display.  */
+      return 0;
+    }
+
+  return p;
+}
+# endif /* !defined USE_CAIRO && defined HAVE_XRENDER */
 #endif	/* HAVE_X_WINDOWS */
 
 /* Return true if XIMG's size WIDTH x HEIGHT doesn't break the
@@ -2579,36 +2645,8 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
   if (!x_create_x_image_and_pixmap (f, width, height, depth, pimg, pixmap))
     return 0;
 # ifdef HAVE_XRENDER
-  Display *display = FRAME_X_DISPLAY (f);
-  int event_basep, error_basep;
-  if (picture && XRenderQueryExtension (display, &event_basep, &error_basep))
-    {
-      if (depth <= 0)
-	depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
-      if (depth == 32 || depth == 24 || depth == 8)
-        {
-          XRenderPictFormat *format;
-          XRenderPictureAttributes attr;
-
-          /* FIXME: Do we need to handle all possible bit depths?
-             XRenderFindStandardFormat supports PictStandardARGB32,
-             PictStandardRGB24, PictStandardA8, PictStandardA4,
-             PictStandardA1, and PictStandardNUM (what is this?!).
-
-             XRenderFindFormat may support more, but I don't
-             understand the documentation.  */
-          format = XRenderFindStandardFormat (display,
-                                              depth == 32 ? PictStandardARGB32
-                                              : depth == 24 ? PictStandardRGB24
-                                              : PictStandardA8);
-          *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr);
-        }
-      else
-        {
-          image_error ("Specified image bit depth is not supported by XRender");
-          *picture = 0;
-        }
-    }
+  if (picture)
+    *picture = x_create_xrender_picture (f, *pixmap, depth);
 # endif
 
   return 1;
@@ -3387,6 +3425,11 @@ Create_Pixmap_From_Bitmap_Data (struct frame *f, struct image *img, char *data,
 				   img->width, img->height,
 				   fg, bg,
 				   DefaultDepthOfScreen (FRAME_X_SCREEN (f)));
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+  if (img->pixmap)
+    img->picture = x_create_xrender_picture (f, img->pixmap, 0);
+# endif
+
 #elif defined HAVE_NTGUI
   img->pixmap
     = w32_create_pixmap_from_bitmap_data (img->width, img->height, data);
@@ -4359,18 +4402,30 @@ xpm_load (struct frame *f, struct image *img)
 	  image_clear_image (f, img);
 	  rc = XpmNoMemory;
 	}
-      else if (img->mask_img)
-	{
-          img->mask = XCreatePixmap (FRAME_X_DISPLAY (f), FRAME_X_DRAWABLE (f),
-				     img->mask_img->width,
-				     img->mask_img->height,
-				     img->mask_img->depth);
-	  if (img->mask == NO_PIXMAP)
-	    {
-	      image_clear_image (f, img);
-	      rc = XpmNoMemory;
-	    }
-	}
+      else
+        {
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+          img->picture = x_create_xrender_picture (f, img->pixmap,
+                                                   img->ximg->depth);
+# endif
+          if (img->mask_img)
+            {
+              img->mask = XCreatePixmap (FRAME_X_DISPLAY (f), FRAME_X_DRAWABLE (f),
+                                         img->mask_img->width,
+                                         img->mask_img->height,
+                                         img->mask_img->depth);
+              if (img->mask == NO_PIXMAP)
+                {
+                  image_clear_image (f, img);
+                  rc = XpmNoMemory;
+                }
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+              else
+                img->mask_picture = x_create_xrender_picture
+                  (f, img->mask, img->mask_img->depth);
+# endif
+            }
+        }
     }
 #endif
 
diff --git a/src/xterm.c b/src/xterm.c
index d55bc3890d..9a6eda4488 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -3049,14 +3049,12 @@ x_composite_image (struct glyph_string *s, Pixmap dest,
       XRenderPictFormat *default_format;
       XRenderPictureAttributes attr;
 
-      /* FIXME: Should we do this each time or would it make sense to
-         store destination in the frame struct?  */
       default_format = XRenderFindVisualFormat (display,
                                                 DefaultVisual (display, 0));
       destination = XRenderCreatePicture (display, dest,
                                           default_format, 0, &attr);
 
-      XRenderComposite (display, PictOpSrc,
+      XRenderComposite (display, s->img->mask_picture ? PictOpOver : PictOpSrc,
                         s->img->picture, s->img->mask_picture, destination,
                         srcX, srcY,
                         srcX, srcY,
@@ -3315,6 +3313,7 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap)
 	     trust on the shape extension to be available
 	     (XShapeCombineRegion).  So, compute the rectangle to draw
 	     manually.  */
+          /* FIXME: Do we need to do this when using XRender compositing?  */
 	  unsigned long mask = (GCClipMask | GCClipXOrigin | GCClipYOrigin
 				| GCFunction);
 	  XGCValues xgcv;
@@ -3325,9 +3324,9 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap)
 	  xgcv.function = GXcopy;
 	  XChangeGC (display, s->gc, mask, &xgcv);
 
-	  XCopyArea (display, s->img->pixmap, pixmap, s->gc,
-		     s->slice.x, s->slice.y,
-		     s->slice.width, s->slice.height, x, y);
+	  x_composite_image (s, pixmap,
+                             s->slice.x, s->slice.y,
+                             x, y, s->slice.width, s->slice.height);
 	  XSetClipMask (display, s->gc, None);
 	}
       else
-- 
2.21.0