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

From: Alan Third <[email-masked]>
Date: Sun, 17 Nov 2019 19:01:05 +0000

On Sun, Nov 17, 2019 at 07:49:56PM +0100, Adam Sjøgren wrote:
> Adam writes:
> 
> >> New patch attached.
> >
> > After a while with this patch running Gnus, Emacs crashed like this:
> 
> Here is a backtrace running Emacs with gdb (I don't really know how to
> use gdb, just "run" and "bt"):

Thanks for trying. I’m giving up on freeing the Pictures there,
although I strongly suspect it’s adding a memory leak.

New patch attached which should resize xpms and xbms. I noticed two problems:

  1. sometimes shrinking an image leaves some pixels behind in clear
  areas.
  
  2. In image mode if I shrink an image enough so that it disappears
  from view completely I can’t then scale it up with +. I suspect
  something’s going to zero and subsequent calculations all stay at
  zero.

-- 
Alan Third
>From 8aeed43bbbf1931c4b5ef1ca0e61103ca9fcaf6c Mon Sep 17 00:00:00 2001
From: Alan Third <[email-masked]>
Date: Sat, 9 Nov 2019 17:04:25 +0000
Subject: [PATCH v3] 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 | 139 +++++++++++++++++++++++++++++++++++-----------------
 src/xterm.c |   8 +--
 2 files changed, 97 insertions(+), 50 deletions(-)

diff --git a/src/image.c b/src/image.c
index 870f008b14..6c1898eae0 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,12 @@ 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,
+             so image_set_transform must be called after it.  */
+#ifdef HAVE_NATIVE_TRANSFORMS
+          image_set_transform (f, img);
+#endif
 	}
 
       unblock_input ();
@@ -2527,6 +2537,54 @@ 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)
+        {
+          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
+                                              : depth == 8 ? PictStandardA8
+                                              : depth == 4 ? PictStandardA4
+                                              : PictStandardA1);
+          p = XRenderCreatePicture (display, pixmap, format, 0, &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 +2637,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 +3417,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 +4394,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 44fbd27b11..6de1644cb0 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -3056,7 +3056,7 @@ x_composite_image (struct glyph_string *s, Pixmap dest,
       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,
@@ -3325,9 +3325,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