bug#36315: 27.0.50; SVG transparency handling is inaccurate

[YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>]

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

On Thu, 27 Jun 2019 00:57:48 +0900,
Eli Zaretskii wrote:
> 
> > > Maybe it's just me, but I'm uneasy to bypass librsvg and call Cairo
> > > directly for manipulating SVG images.  Why doesn't librsvg provide a
> > > way to do this via its own APIs?
> > >
> > > Does anyone else think it's unusual to make such direct calls to what
> > > is essentially a lower-level library?
> > 
> > What kind of operations do you think librsvg should provide us with,
> > instead of letting us use cairo?
> 
> Those for which you called the Cairo functions directly.

Which one, concretely?  Or you mean something in other parts?

+  cairo_surface_t *surface;
+#ifdef USE_CAIRO
+  surface = cairo_image_surface_create_for_data ((unsigned char *) ximg->data,
+						 CAIRO_FORMAT_RGB24,
+						 width, height,
+						 ximg->bytes_per_line);
+#else
+  surface = cairo_image_surface_create (CAIRO_FORMAT_RGB24, width, height);
+#endif
+  if (cairo_surface_status (surface) != CAIRO_STATUS_SUCCESS)
+    goto rsvg_error;
+  cairo_t *cr = cairo_create (surface);
+  cairo_set_source_rgb (cr, background.red / 65535.0,
+			background.green / 65535.0,
+			background.blue / 65535.0);
+  cairo_paint (cr);
+  cairo_set_source_rgb (cr, 0.0, 0.0, 0.0);
+  rsvg_handle_render_cairo (rsvg_handle, cr);
+  cairo_destroy (cr);
+  g_object_unref (rsvg_handle);

> I tried with the second patch.  I tried now again with the above one:
> still no rectangle.

Seems like a problem in DLL loading.  Please try the attached one.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

[-- Attachment #2: svg-cairo.diff --]
[-- Type: application/octet-stream, Size: 14916 bytes --]

diff --git a/configure.ac b/configure.ac
index 8ff0e21fbf6..bcd70d34cbb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2603,7 +2603,7 @@ AC_DEFUN
 HAVE_RSVG=no
 if test "${HAVE_X11}" = "yes" || test "${HAVE_NS}" = "yes" || test "${opsys}" = "mingw32"; then
   if test "${with_rsvg}" != "no"; then
-    RSVG_REQUIRED=2.11.0
+    RSVG_REQUIRED=2.14.0
     RSVG_MODULE="librsvg-2.0 >= $RSVG_REQUIRED"
 
     EMACS_CHECK_MODULES([RSVG], [$RSVG_MODULE])
diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index fef954eb7a3..285ca5d6967 100644
--- a/doc/lispref/os.texi
+++ b/doc/lispref/os.texi
@@ -3082,7 +3082,7 @@ Dynamic Libraries
         (tiff "libtiff3.dll" "libtiff.dll")
         (gif "giflib4.dll" "libungif4.dll" "libungif.dll")
         (svg "librsvg-2-2.dll")
-        (gdk-pixbuf "libgdk_pixbuf-2.0-0.dll")
+        (cairo "libcairo-2.dll")
         (glib "libglib-2.0-0.dll")
         (gobject "libgobject-2.0-0.dll")))
 @end example
diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index 044b82ed1e0..270b062e75a 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -277,7 +277,7 @@ libgnutls-version
 	     '(gif "libgif-6.dll" "giflib5.dll" "gif.dll")
 	 '(gif "libgif-5.dll" "giflib4.dll" "libungif4.dll" "libungif.dll")))
        '(svg "librsvg-2-2.dll")
-       '(gdk-pixbuf "libgdk_pixbuf-2.0-0.dll")
+       '(cairo "libcairo-2.dll")
        '(glib "libglib-2.0-0.dll")
        '(gobject "libgobject-2.0-0.dll")
        (if (>= libgnutls-version 30400)
diff --git a/src/image.c b/src/image.c
index 7b648c46ae9..d56dd998908 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9379,17 +9379,20 @@ DEF_DLL_FN (void, rsvg_handle_get_dimensions,
 DEF_DLL_FN (gboolean, rsvg_handle_write,
 	    (RsvgHandle *, const guchar *, gsize, GError **));
 DEF_DLL_FN (gboolean, rsvg_handle_close, (RsvgHandle *, GError **));
-DEF_DLL_FN (GdkPixbuf *, rsvg_handle_get_pixbuf, (RsvgHandle *));
 DEF_DLL_FN (void, rsvg_handle_set_base_uri, (RsvgHandle *, const char *));
-
-DEF_DLL_FN (int, gdk_pixbuf_get_width, (const GdkPixbuf *));
-DEF_DLL_FN (int, gdk_pixbuf_get_height, (const GdkPixbuf *));
-DEF_DLL_FN (guchar *, gdk_pixbuf_get_pixels, (const GdkPixbuf *));
-DEF_DLL_FN (int, gdk_pixbuf_get_rowstride, (const GdkPixbuf *));
-DEF_DLL_FN (GdkColorspace, gdk_pixbuf_get_colorspace, (const GdkPixbuf *));
-DEF_DLL_FN (int, gdk_pixbuf_get_n_channels, (const GdkPixbuf *));
-DEF_DLL_FN (gboolean, gdk_pixbuf_get_has_alpha, (const GdkPixbuf *));
-DEF_DLL_FN (int, gdk_pixbuf_get_bits_per_sample, (const GdkPixbuf *));
+DEF_DLL_FN (gboolean, rsvg_handle_render_cairo, (RsvgHandle *, cairo_t *));
+
+DEF_DLL_FN (cairo_surface_t *, cairo_image_surface_create,
+	    (cairo_format_t, int, int));
+DEF_DLL_FN (cairo_status_t, cairo_surface_status, (cairo_surface_t *));
+DEF_DLL_FN (cairo_t *, cairo_create, (cairo_surface_t *));
+DEF_DLL_FN (void, cairo_set_source_rgb, (cairo_t *, double, double, double));
+DEF_DLL_FN (void, cairo_paint, (cairo_t *));
+DEF_DLL_FN (void, cairo_destroy, (cairo_t *));
+DEF_DLL_FN (void, cairo_surface_flush, (cairo_surface_t *));
+DEF_DLL_FN (unsigned char *, cairo_image_surface_get_data, (cairo_surface_t *));
+DEF_DLL_FN (int, cairo_image_surface_get_stride, (cairo_surface_t *));
+DEF_DLL_FN (void, cairo_surface_destroy, (cairo_surface_t *));
 
 #  if ! GLIB_CHECK_VERSION (2, 36, 0)
 DEF_DLL_FN (void, g_type_init, (void));
@@ -9400,14 +9403,14 @@ DEF_DLL_FN (void, g_clear_error, (GError **));
 static bool
 init_svg_functions (void)
 {
-  HMODULE library, gdklib = NULL, glib = NULL, gobject = NULL;
+  HMODULE library, cairo = NULL, glib = NULL, gobject = NULL;
 
   if (!(glib = w32_delayed_load (Qglib))
       || !(gobject = w32_delayed_load (Qgobject))
-      || !(gdklib = w32_delayed_load (Qgdk_pixbuf))
+      || !(cairo = w32_delayed_load (Qcairo))
       || !(library = w32_delayed_load (Qsvg)))
     {
-      if (gdklib)  FreeLibrary (gdklib);
+      if (cairo)   FreeLibrary (cairo);
       if (gobject) FreeLibrary (gobject);
       if (glib)    FreeLibrary (glib);
       return 0;
@@ -9417,17 +9420,19 @@ init_svg_functions (void)
   LOAD_DLL_FN (library, rsvg_handle_get_dimensions);
   LOAD_DLL_FN (library, rsvg_handle_write);
   LOAD_DLL_FN (library, rsvg_handle_close);
-  LOAD_DLL_FN (library, rsvg_handle_get_pixbuf);
   LOAD_DLL_FN (library, rsvg_handle_set_base_uri);
-
-  LOAD_DLL_FN (gdklib, gdk_pixbuf_get_width);
-  LOAD_DLL_FN (gdklib, gdk_pixbuf_get_height);
-  LOAD_DLL_FN (gdklib, gdk_pixbuf_get_pixels);
-  LOAD_DLL_FN (gdklib, gdk_pixbuf_get_rowstride);
-  LOAD_DLL_FN (gdklib, gdk_pixbuf_get_colorspace);
-  LOAD_DLL_FN (gdklib, gdk_pixbuf_get_n_channels);
-  LOAD_DLL_FN (gdklib, gdk_pixbuf_get_has_alpha);
-  LOAD_DLL_FN (gdklib, gdk_pixbuf_get_bits_per_sample);
+  LOAD_DLL_FN (library, rsvg_handle_render_cairo);
+
+  LOAD_DLL_FN (cairo, cairo_image_surface_create);
+  LOAD_DLL_FN (cairo, cairo_surface_status);
+  LOAD_DLL_FN (cairo, cairo_create);
+  LOAD_DLL_FN (cairo, cairo_set_source_rgb);
+  LOAD_DLL_FN (cairo, cairo_paint);
+  LOAD_DLL_FN (cairo, cairo_destroy);
+  LOAD_DLL_FN (cairo, cairo_surface_flush);
+  LOAD_DLL_FN (cairo, cairo_image_surface_get_data);
+  LOAD_DLL_FN (cairo, cairo_image_surface_get_stride);
+  LOAD_DLL_FN (cairo, cairo_surface_destroy);
 
 #  if ! GLIB_CHECK_VERSION (2, 36, 0)
   LOAD_DLL_FN (gobject, g_type_init);
@@ -9441,32 +9446,36 @@ init_svg_functions (void)
 /* The following aliases for library functions allow dynamic loading
    to be used on some platforms.  */
 
-#  undef gdk_pixbuf_get_bits_per_sample
-#  undef gdk_pixbuf_get_colorspace
-#  undef gdk_pixbuf_get_has_alpha
-#  undef gdk_pixbuf_get_height
-#  undef gdk_pixbuf_get_n_channels
-#  undef gdk_pixbuf_get_pixels
-#  undef gdk_pixbuf_get_rowstride
-#  undef gdk_pixbuf_get_width
+#  undef cairo_create
+#  undef cairo_destroy
+#  undef cairo_image_surface_create
+#  undef cairo_image_surface_get_data
+#  undef cairo_image_surface_get_stride
+#  undef cairo_paint
+#  undef cairo_set_source_rgb
+#  undef cairo_surface_destroy
+#  undef cairo_surface_flush
+#  undef cairo_surface_status
 #  undef g_clear_error
 #  undef g_object_unref
 #  undef g_type_init
 #  undef rsvg_handle_close
 #  undef rsvg_handle_get_dimensions
-#  undef rsvg_handle_get_pixbuf
 #  undef rsvg_handle_new
+#  undef rsvg_handle_render_cairo
 #  undef rsvg_handle_set_base_uri
 #  undef rsvg_handle_write
 
-#  define gdk_pixbuf_get_bits_per_sample fn_gdk_pixbuf_get_bits_per_sample
-#  define gdk_pixbuf_get_colorspace fn_gdk_pixbuf_get_colorspace
-#  define gdk_pixbuf_get_has_alpha fn_gdk_pixbuf_get_has_alpha
-#  define gdk_pixbuf_get_height fn_gdk_pixbuf_get_height
-#  define gdk_pixbuf_get_n_channels fn_gdk_pixbuf_get_n_channels
-#  define gdk_pixbuf_get_pixels fn_gdk_pixbuf_get_pixels
-#  define gdk_pixbuf_get_rowstride fn_gdk_pixbuf_get_rowstride
-#  define gdk_pixbuf_get_width fn_gdk_pixbuf_get_width
+#  define cairo_create fn_cairo_create
+#  define cairo_destroy fn_cairo_destroy
+#  define cairo_image_surface_create fn_cairo_image_surface_create
+#  define cairo_image_surface_get_data fn_cairo_image_surface_get_data
+#  define cairo_image_surface_get_stride fn_cairo_image_surface_get_stride
+#  define cairo_paint fn_cairo_paint
+#  define cairo_set_source_rgb fn_cairo_set_source_rgb
+#  define cairo_surface_destroy fn_cairo_surface_destroy
+#  define cairo_surface_flush fn_cairo_surface_flush
+#  define cairo_surface_status fn_cairo_surface_status
 #  define g_clear_error fn_g_clear_error
 #  define g_object_unref fn_g_object_unref
 #  if ! GLIB_CHECK_VERSION (2, 36, 0)
@@ -9474,8 +9483,8 @@ init_svg_functions (void)
 #  endif
 #  define rsvg_handle_close fn_rsvg_handle_close
 #  define rsvg_handle_get_dimensions fn_rsvg_handle_get_dimensions
-#  define rsvg_handle_get_pixbuf fn_rsvg_handle_get_pixbuf
 #  define rsvg_handle_new fn_rsvg_handle_new
+#  define rsvg_handle_render_cairo fn_rsvg_handle_render_cairo
 #  define rsvg_handle_set_base_uri fn_rsvg_handle_set_base_uri
 #  define rsvg_handle_write fn_rsvg_handle_write
 
@@ -9550,11 +9559,6 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   RsvgHandle *rsvg_handle;
   RsvgDimensionData dimension_data;
   GError *err = NULL;
-  GdkPixbuf *pixbuf;
-  int width;
-  int height;
-  const guint8 *pixels;
-  int rowstride;
 
 #if ! GLIB_CHECK_VERSION (2, 36, 0)
   /* g_type_init is a glib function that must be called prior to
@@ -9596,104 +9600,92 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   #endif
 
   rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
-  if (! check_image_size (f, dimension_data.width, dimension_data.height))
+  int width = dimension_data.width, height = dimension_data.height;
+  if (! check_image_size (f, width, height))
     {
       image_size_error ();
       goto rsvg_error;
     }
 
-  /* We can now get a valid pixel buffer from the svg file, if all
-     went ok.  */
-  pixbuf = rsvg_handle_get_pixbuf (rsvg_handle);
-  if (!pixbuf) goto rsvg_error;
-  g_object_unref (rsvg_handle);
-
-  /* Extract some meta data from the svg handle.  */
-  width     = gdk_pixbuf_get_width (pixbuf);
-  height    = gdk_pixbuf_get_height (pixbuf);
-  pixels    = gdk_pixbuf_get_pixels (pixbuf);
-  rowstride = gdk_pixbuf_get_rowstride (pixbuf);
-
-  /* Validate the svg meta data.  */
-  eassert (gdk_pixbuf_get_colorspace (pixbuf) == GDK_COLORSPACE_RGB);
-  eassert (gdk_pixbuf_get_n_channels (pixbuf) == 4);
-  eassert (gdk_pixbuf_get_has_alpha (pixbuf));
-  eassert (gdk_pixbuf_get_bits_per_sample (pixbuf) == 8);
+  /* Try to create a x pixmap to hold the svg pixmap.  */
+  Emacs_Pix_Container ximg;
+  if (!image_create_x_image_and_pixmap (f, img, width, height, 0, &ximg, 0))
+    {
+      g_object_unref (rsvg_handle);
+      return 0;
+    }
 
-  {
-    /* Try to create a x pixmap to hold the svg pixmap.  */
-    Emacs_Pix_Container ximg;
-    if (!image_create_x_image_and_pixmap (f, img, width, height, 0, &ximg, 0))
-      {
-	g_object_unref (pixbuf);
-	return 0;
-      }
+  init_color_table ();
 
-    init_color_table ();
+  /* Handle alpha channel by combining the image with a background
+     color.  */
+  Emacs_Color background;
+  Lisp_Object specified_bg = image_spec_value (img->spec, QCbackground, NULL);
+  if (!STRINGP (specified_bg)
+      || !FRAME_TERMINAL (f)->defined_color_hook (f,
+						  SSDATA (specified_bg),
+						  &background,
+						  false,
+						  false))
+    FRAME_TERMINAL (f)->query_frame_background_color (f, &background);
 
-    /* Handle alpha channel by combining the image with a background
-       color.  */
-    Emacs_Color background;
-    Lisp_Object specified_bg = image_spec_value (img->spec, QCbackground, NULL);
-    if (!STRINGP (specified_bg)
-	|| !FRAME_TERMINAL (f)->defined_color_hook (f,
-                                                    SSDATA (specified_bg),
-                                                    &background,
-                                                    false,
-                                                    false))
-      FRAME_TERMINAL (f)->query_frame_background_color (f, &background);
-
-    /* SVG pixmaps specify transparency in the last byte, so right
-       shift 8 bits to get rid of it, since emacs doesn't support
-       transparency.  */
-    background.red   >>= 8;
-    background.green >>= 8;
-    background.blue  >>= 8;
-
-    /* This loop handles opacity values, since Emacs assumes
-       non-transparent images.  Each pixel must be "flattened" by
-       calculating the resulting color, given the transparency of the
-       pixel, and the image background color.  */
-    for (int y = 0; y < height; ++y)
-      {
-	for (int x = 0; x < width; ++x)
-	  {
-	    int red     = *pixels++;
-	    int green   = *pixels++;
-	    int blue    = *pixels++;
-	    int opacity = *pixels++;
-
-	    red   = ((red * opacity)
-		     + (background.red * ((1 << 8) - opacity)));
-	    green = ((green * opacity)
-		     + (background.green * ((1 << 8) - opacity)));
-	    blue  = ((blue * opacity)
-		     + (background.blue * ((1 << 8) - opacity)));
-
-	    PUT_PIXEL (ximg, x, y, lookup_rgb_color (f, red, green, blue));
-	  }
+  cairo_surface_t *surface;
+#ifdef USE_CAIRO
+  surface = cairo_image_surface_create_for_data ((unsigned char *) ximg->data,
+						 CAIRO_FORMAT_RGB24,
+						 width, height,
+						 ximg->bytes_per_line);
+#else
+  surface = cairo_image_surface_create (CAIRO_FORMAT_RGB24, width, height);
+#endif
+  if (cairo_surface_status (surface) != CAIRO_STATUS_SUCCESS)
+    goto rsvg_error;
+  cairo_t *cr = cairo_create (surface);
+  cairo_set_source_rgb (cr, background.red / 65535.0,
+			background.green / 65535.0,
+			background.blue / 65535.0);
+  cairo_paint (cr);
+  cairo_set_source_rgb (cr, 0.0, 0.0, 0.0);
+  rsvg_handle_render_cairo (rsvg_handle, cr);
+  cairo_destroy (cr);
+  g_object_unref (rsvg_handle);
 
-	pixels += rowstride - 4 * width;
-      }
+  cairo_surface_flush (surface);
+#ifndef USE_CAIRO
+  unsigned char *data = cairo_image_surface_get_data (surface);
+  int stride = cairo_image_surface_get_stride (surface);
+  for (int y = 0; y < height; ++y)
+    {
+      guint32 *pixels = (guint32 *) data;
+      for (int x = 0; x < width; ++x)
+	{
+	  guint32 rgb = *pixels++;
+	  int red   = ((rgb >> 16) & 0xff) * 0x101;
+	  int green = ((rgb >> 8) & 0xff) * 0x101;
+	  int blue  = (rgb & 0xff) * 0x101;
+	  PUT_PIXEL (ximg, x, y, lookup_rgb_color (f, red, green, blue));
+	}
+      data += stride;
+    }
+#endif	/* !USE_CAIRO */
 
 #ifdef COLOR_TABLE_SUPPORT
-    /* Remember colors allocated for this image.  */
-    img->colors = colors_in_color_table (&img->ncolors);
-    free_color_table ();
+  /* Remember colors allocated for this image.  */
+  img->colors = colors_in_color_table (&img->ncolors);
+  free_color_table ();
 #endif /* COLOR_TABLE_SUPPORT */
 
-    g_object_unref (pixbuf);
+  cairo_surface_destroy (surface);
 
-    img->width  = width;
-    img->height = height;
+  img->width  = width;
+  img->height = height;
 
-    /* Maybe fill in the background field while we have ximg handy.
-       Casting avoids a GCC warning.  */
-    IMAGE_BACKGROUND (img, f, (Emacs_Pix_Context)ximg);
+  /* Maybe fill in the background field while we have ximg handy.
+     Casting avoids a GCC warning.  */
+  IMAGE_BACKGROUND (img, f, (Emacs_Pix_Context)ximg);
 
-    /* Put ximg into the image.  */
-    image_put_x_image (f, img, ximg, 0);
-  }
+  /* Put ximg into the image.  */
+  image_put_x_image (f, img, ximg, 0);
 
   return 1;
 
@@ -10249,7 +10241,7 @@ non-numeric, there is no explicit limit on the size of images.  */);
   add_image_type (Qsvg);
 #ifdef HAVE_NTGUI
   /* Other libraries used directly by svg code.  */
-  DEFSYM (Qgdk_pixbuf, "gdk-pixbuf");
+  DEFSYM (Qcairo, "cairo");
   DEFSYM (Qglib, "glib");
   DEFSYM (Qgobject, "gobject");
 #endif /* HAVE_NTGUI  */