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

From: Alan Third <[email-masked]>
Date: Sun, 17 Nov 2019 17:22:08 +0000

On Sat, Nov 16, 2019 at 05:53:16PM +0100, Adam Sjøgren wrote:
> Alan writes:
> 
> > Hi Adam, can you try with the patch again, but undoing the changes to
> > image_clear_image_1 in image.c? There should be two, both wrapped in
> > ‘#ifdef HAVE_XRENDER’.
> >
> > Those changes weren’t required for the fix, but looked as though they
> > should be needed generally.
> >
> > If that doesn’t help can you try running under a debugger and sending
> > us the backtrace?
> 
> I'm trying the patch without those two sections now (finally). No
> crashes so far (around an hours worth of using Gnus).

I’m pretty sure we need to free the Pictures there, so perhaps we need
to free the Picture before freeing the associated pixmap...

New patch attached.
-- 
Alan Third
>From 00d2632c91bc8a930aaeb87018aa9f06dd5dbecd Mon Sep 17 00:00:00 2001
From: Alan Third <[email-masked]>
Date: Sat, 9 Nov 2019 17:04:25 +0000
Subject: [PATCH v2] Fix image scaling with masks (bug#38109)

* src/image.c (image_clear_image_1): Free the XRender Pictures.
(lookup_image): Move call to image_set_transform after
postprocess_image.
(image_create_x_image_and_pixmap_1): Add 1 bit image type for masks.
* 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 | 35 +++++++++++++++++++++++++++++------
 src/xterm.c |  8 ++++----
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/src/image.c b/src/image.c
index 870f008b14..f872cb31fb 100644
--- a/src/image.c
+++ b/src/image.c
@@ -1453,6 +1453,13 @@ image_clear_image_1 (struct frame *f, struct image *img, int flags)
     {
       if (img->pixmap)
 	{
+#if !defined USE_CAIRO && defined HAVE_XRENDER
+          if (img->picture)
+            {
+              XRenderFreePicture (FRAME_X_DISPLAY (f), img->picture);
+              img->picture = 0;
+            }
+# endif
 	  FRAME_TERMINAL (f)->free_pixmap (f, img->pixmap);
 	  img->pixmap = NO_PIXMAP;
 	  /* NOTE (HAVE_NS): background color is NOT an indexed color! */
@@ -1472,6 +1479,13 @@ image_clear_image_1 (struct frame *f, struct image *img, int flags)
     {
       if (img->mask)
 	{
+#if !defined USE_CAIRO && defined HAVE_XRENDER
+          if (img->mask_picture)
+            {
+              XRenderFreePicture (FRAME_X_DISPLAY (f), img->mask_picture);
+              img->mask_picture = 0;
+            }
+#endif
 	  FRAME_TERMINAL (f)->free_pixmap (f, img->mask);
 	  img->mask = NO_PIXMAP;
 	  img->background_transparent_valid = 0;
@@ -2244,6 +2258,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 +2335,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 +2375,10 @@ 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);
+
+#ifdef HAVE_NATIVE_TRANSFORMS
+          image_set_transform (f, img);
+#endif
 	}
 
       unblock_input ();
@@ -2585,7 +2607,7 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
     {
       if (depth <= 0)
 	depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
-      if (depth == 32 || depth == 24 || depth == 8)
+      if (depth == 32 || depth == 24 || depth == 8 || depth == 1)
         {
           XRenderPictFormat *format;
           XRenderPictureAttributes attr;
@@ -2600,7 +2622,8 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
           format = XRenderFindStandardFormat (display,
                                               depth == 32 ? PictStandardARGB32
                                               : depth == 24 ? PictStandardRGB24
-                                              : PictStandardA8);
+                                              : depth == 8 ? PictStandardA8
+                                              : PictStandardA1);
           *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr);
         }
       else
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