bug#44065: 28.0.50; SVG image not shown completely

[Alan Third <alan@idiocy.org>]

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

On Mon, Oct 19, 2020 at 11:10:02AM +0200, Stephen Berman wrote:
> On Mon, 19 Oct 2020 10:43:53 +0200 Lars Ingebrigtsen <larsi@gnus.org> wrote:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >>> > Maybe it depends on the version of librsvg? or some dependency of
> >>> > librsvg?
> >>>
> >>> My system uses librsvg-2.48.2.
> >>
> >> 2.40.1 here.
> >
> > 2.50.1 here.  So it looks like something changed between 2.40 and 2.48
> > somewhere.
> 
> One thing that changed with 2.41 is the implementation of librsvg:
> 
> https://download.gnome.org/sources/librsvg/2.41/librsvg-2.41.0.news
> 
>   Version 2.41.0
>   - The big news is that parts of librsvg are now implemented in the
>     Rust programming language, instead of C. [...]
>   - Code that has been converted to Rust:  marker orientations and
>     rendering, path data parser, path building, length normalization,
>     gradient inheritance, bounding boxes with affine transformations.
> 
> Maybe that led to the clipping?  (But I can't readily check how 2.41
> displays SVGs in Emacs.)

A lot of stuff is deprecated in 2.46, presumably because of this
change.

I've got something that works for me. I'm using 2.50, so I'd
appreciate it if someone using 2.45 or below could check that it
builds and isn't completely broken.

I don't expect this bug to be fixed on libsrvg 2.45 or below. I don't
see any obvious way around it while still being able to resize the
image and set background colours, etc., and since it works on recent
versions of librsvg I don't think it's worth putting too much effort
in.
-- 
Alan Third

[-- Attachment #2: 0001-Fix-SVG-image-dimension-calculations-bug-44065.patch --]
[-- Type: text/plain, Size: 6058 bytes --]

From 9da7933d902f69bbfe0ed185cf3d2889958ac63b Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Mon, 19 Oct 2020 21:19:57 +0100
Subject: [PATCH] Fix SVG image dimension calculations (bug#44065)

* src/image.c (svg_load_image): Calculate the image size by using the
viewBox size and applying it to the image.  Work out CSS so it remains
the same across all SVG calculations.
---
 src/image.c | 69 ++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 19 deletions(-)

diff --git a/src/image.c b/src/image.c
index 25d5af8a8d..aa305dfebf 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9736,7 +9736,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 		ptrdiff_t size, char *filename)
 {
   RsvgHandle *rsvg_handle;
-  RsvgDimensionData dimension_data;
+  double viewbox_width, viewbox_height;
   GError *err = NULL;
   GdkPixbuf *pixbuf;
   int width;
@@ -9745,6 +9745,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   int rowstride;
   char *wrapped_contents = NULL;
   ptrdiff_t wrapped_size;
+  char *css;
 
 #if ! GLIB_CHECK_VERSION (2, 36, 0)
   /* g_type_init is a glib function that must be called prior to
@@ -9788,22 +9789,60 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   if (err) goto rsvg_error;
 #endif
 
+  /* Generate the default CSS for the image.  */
+  int buffer_size;
+  Lisp_Object value;
+  unsigned long foreground = img->face_foreground;
+  value = image_spec_value (img->spec, QCforeground, NULL);
+  if (!NILP (value))
+    foreground = image_alloc_image_color (f, img, value, img->face_foreground);
+
+  /* TODO: Use the actual font size or at least make it configurable.  */
+  char *css_format = "svg {color: #%06X; fill: currentColor; font-size: 14;}";
+
+  /* Add 3 to cover the extra size of the color string and the null byte.  */
+  buffer_size = strlen (css_format) + 3;
+  css = xmalloc (buffer_size);
+  if (! css || buffer_size <= snprintf (css, buffer_size, css_format,
+                                        foreground & 0xFFFFFF))
+    goto rsvg_error;
+
+  rsvg_handle_set_stylesheet (rsvg_handle, css, strlen (css), NULL);
+
   /* Get the image dimensions.  */
+#if LIBRSVG_CHECK_VERSION (2, 46, 0)
+  RsvgRectangle zero_rect, viewbox;
+
+  rsvg_handle_get_geometry_for_layer (rsvg_handle, NULL,
+                                      &zero_rect, &viewbox,
+                                      NULL, NULL);
+  viewbox_width = viewbox.x + viewbox.width;
+  viewbox_height = viewbox.y + viewbox.height;
+#else
+  RsvgDimensionData dimension_data;
+
   rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
 
+  viewbox_width = dimension_data.width;
+  viewbox_height = dimension_data.height;
+#endif
+  compute_image_size (viewbox_width, viewbox_height, img->spec,
+                      &width, &height);
+
+  if (! check_image_size (f, width, height))
+    {
+      image_size_error ();
+      goto rsvg_error;
+    }
+
   /* We are now done with the unmodified data.  */
   g_object_unref (rsvg_handle);
 
-  /* Calculate the final image size.  */
-  compute_image_size (dimension_data.width, dimension_data.height,
-                      img->spec, &width, &height);
-
   /* Wrap the SVG data in another SVG.  This allows us to set the
      width and height, as well as modify the foreground and background
      colors.  */
   {
     Lisp_Object value;
-    unsigned long foreground = img->face_foreground;
     unsigned long background = img->face_background;
 
     Lisp_Object encoded_contents
@@ -9818,9 +9857,8 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
     const char *wrapper =
       "<svg xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
       "xmlns:xi=\"http://www.w3.org/2001/XInclude\" "
-      "style=\"color: #%06X; fill: currentColor;\" "
       "width=\"%d\" height=\"%d\" preserveAspectRatio=\"none\" "
-      "viewBox=\"0 0 %d %d\">"
+      "viewBox=\"0 0 %f %f\">"
       "<rect width=\"100%%\" height=\"100%%\" fill=\"#%06X\"/>"
       "<xi:include href=\"data:image/svg+xml;base64,%s\"></xi:include>"
       "</svg>";
@@ -9829,9 +9867,6 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
        width and height strings and things.  */
     int buffer_size = SBYTES (encoded_contents) + strlen (wrapper) + 64;
 
-    value = image_spec_value (img->spec, QCforeground, NULL);
-    if (!NILP (value))
-      foreground = image_alloc_image_color (f, img, value, img->face_foreground);
     value = image_spec_value (img->spec, QCbackground, NULL);
     if (!NILP (value))
       {
@@ -9844,8 +9879,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 
     if (!wrapped_contents
         || buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
-                                    foreground & 0xFFFFFF, width, height,
-                                    dimension_data.width, dimension_data.height,
+                                    width, height, viewbox_width, viewbox_height,
                                     background & 0xFFFFFF, SSDATA (encoded_contents)))
       goto rsvg_error;
 
@@ -9887,12 +9921,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   if (err) goto rsvg_error;
 #endif
 
-  rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
-  if (! check_image_size (f, dimension_data.width, dimension_data.height))
-    {
-      image_size_error ();
-      goto rsvg_error;
-    }
+  rsvg_handle_set_stylesheet (rsvg_handle, css, strlen (css), NULL);
 
   /* We can now get a valid pixel buffer from the svg file, if all
      went ok.  */
@@ -9971,6 +10000,8 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
     g_object_unref (rsvg_handle);
   if (wrapped_contents)
     xfree (wrapped_contents);
+  if (css)
+    xfree (css);
   /* FIXME: Use error->message so the user knows what is the actual
      problem with the image.  */
   image_error ("Error parsing SVG image `%s'", img->spec);
-- 
2.26.1