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: 1152 bytes --]

On Fri, 21 Jun 2019 05:26:53 +0900,
Pip Cet wrote:
> 
> [1  <text/plain; UTF-8 (7bit)>]
> Evaluate the following in emacs -Q:
> 
> (require 'svg)
> 
> (defun make-image (color)
>   (let ((svg (svg-create 100 100)))
>     (svg-rectangle svg 0 0 100 100 :fill color)
>     (svg-image svg)))
> 
> (set-frame-parameter (window-frame) 'background-color "black")
> 
> (insert (propertize " " 'display (make-image "#f00000")))
> 
> The expected result is a rectangle (on black background) of color
> #f00000. The actual result is a rectangle of color #ef0000. For black
> backgrounds, white is no longer representable.
> 
> This is related to bug #36304, but much easier to fix.
> 
> Patch attached.

An alternative way would be to use rsvg_handle_render_cairo, which is
recommended by librsvg, and let it blend with the background color.

Patch attached.  Note that this does not require --with-cairo.
Raising the required version of librsvg to 2.14 is not a problem, as
we are already using rsvg_handle_get_dimensions that requires that
version.  Is Windows librsvg DLL compiled with libcairo?

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

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

diff --git a/configure.ac b/configure.ac
index 0507f58054a..4fe9700b03e 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/src/image.c b/src/image.c
index 7b648c46ae9..f93898aedd0 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9379,17 +9379,17 @@ 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 (gboolean, rsvg_handle_render_cairo, (RsvgHandle *, cairo_t *));
 
-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 (cairo_surface_t *, cairo_image_surface_create,
+	    (cairo_format_t, int, int));
+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 (unsigned char *, cairo_image_surface_get_data, (cairo_surface_t *));
+DEF_DLL_FN (int, cairo_image_surface_get_stride, (cairo_surface_t *));
 
 #  if ! GLIB_CHECK_VERSION (2, 36, 0)
 DEF_DLL_FN (void, g_type_init, (void));
@@ -9400,14 +9400,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))
+  if (!(cairo = w32_delayed_load (Qcairo))
+      || !(glib = w32_delayed_load (Qglib))
       || !(gobject = w32_delayed_load (Qgobject))
-      || !(gdklib = w32_delayed_load (Qgdk_pixbuf))
       || !(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 +9417,16 @@ 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 (library, rsvg_handle_render_cairo);
 
-  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 (cairo, cairo_image_surface_create);
+  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_image_surface_get_data);
+  LOAD_DLL_FN (cairo, cairo_image_surface_get_stride);
 
 #  if ! GLIB_CHECK_VERSION (2, 36, 0)
   LOAD_DLL_FN (gobject, g_type_init);
@@ -9441,32 +9440,30 @@ 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 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 g_clear_error fn_g_clear_error
 #  define g_object_unref fn_g_object_unref
 #  if ! GLIB_CHECK_VERSION (2, 36, 0)
@@ -9474,8 +9471,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 +9547,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 +9588,78 @@ 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 (pixbuf);
-	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 (rsvg_handle);
+      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);
-
-    /* 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));
-	  }
+  /* 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);
+
+  cairo_surface_t *surface = cairo_image_surface_create (CAIRO_FORMAT_RGB24,
+							 width, height);
+  cairo_t *cr = cairo_create (surface);
+  cairo_set_source_rgb (cr, background.red >> 8,
+			background.green >> 8, background.blue >> 8);
+  cairo_paint (cr);
+  rsvg_handle_render_cairo (rsvg_handle, cr);
+  cairo_destroy (cr);
+  g_object_unref (rsvg_handle);
 
-	pixels += rowstride - 4 * width;
-      }
+  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) << 8;
+	  int green = ((rgb >> 8) & 0xff) << 8;
+	  int blue  = (rgb & 0xff) << 8;
+	  PUT_PIXEL (ximg, x, y, lookup_rgb_color (f, red, green, blue));
+	}
+      data += stride;
+    }
 
 #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 +10215,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  */