bug#44206: 28.0.50; SVG image fail to show

[Alan Third <alan@idiocy.org>]

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

On Sat, Oct 24, 2020 at 10:25:03PM -0500, styang@fastmail.com wrote:
> Some SVG images fail to show due to regression caused by:
> b42481e22e * | Fix SVG image dimension calculations (bug#44065)
> 
> Please find the offending SVG file in attachment (in fact, all
> custom avatars in telega.el are not showing).

Thanks. It appears there are more ways to define an SVG size than I've
had hot dinners.

librsvg is really not helping here. The documentation makes it pretty
clear that they don't want us to be querying the SVG dimensions and
would prefer us to just give them the dimensions we want and/or to use
Cairo.

That doesn't really work for us because we don't know up-front what
dimensions are usable for the image. It would be a different matter if
we were using the SVGs as GUI components like buttons or something.

Anyway, I've thrown in another attempt at calculating the image size,
and it works for this and also for the previous images. This one is
more complex because it's trying to convert CSS sizes to pixel sizes
and I don't think we can be entirely sure of some of them (like ex
height? Maybe we can query that, but then we have to know font and
font size).

I have no doubt that there are many more SVG files out there that
won't display properly even with this patch.

> Affected librsvg version: 2:2.50.1-1 in Archlinux, and 2.44.10-2.1 in Debian.

I'm concerned that librsvg 2.44 is affected as it should be using the
same code path as before the commit in question.
-- 
Alan Third

[-- Attachment #2: 0001-Calculate-SVG-image-sizes-more-accurately-bug-44206.patch --]
[-- Type: text/plain, Size: 4703 bytes --]

From a178c8b58e37550a897062f1edfe1f674bf7d210 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sun, 25 Oct 2020 15:45:07 +0000
Subject: [PATCH] Calculate SVG image sizes more accurately (bug#44206)

* src/image.c (svg_css_length_to_pixels): New function.
(svg_load_image): Try more methods to work out the image size.
---
 src/image.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 93 insertions(+), 5 deletions(-)

diff --git a/src/image.c b/src/image.c
index 5f6d9f4c44..2f9c21cbcc 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9724,6 +9724,44 @@ svg_load (struct frame *f, struct image *img)
   return success_p;
 }
 
+static double
+svg_css_length_to_pixels (RsvgLength length)
+{
+  /* FIXME: 96 appears to be a pretty standard DPI but we should
+     probably use the real DPI if we can get it.  */
+  double dpi = 96;
+  double value = length.length;
+
+  switch (length.unit)
+    {
+    case RSVG_UNIT_PX:
+      /* Already a pixel value.  */
+      break;
+    case RSVG_UNIT_CM:
+      /* 2.54 cm in an inch.  */
+      value = dpi * value / 2.54;
+    case RSVG_UNIT_MM:
+      /* 25.4 mm in an inch.  */
+      value = dpi * value / 25.4;
+      break;
+    case RSVG_UNIT_PT:
+      /* 72 points in an inch.  */
+      value = dpi * value / 72;
+    case RSVG_UNIT_PC:
+      /* 6 picas in an inch.  */
+      value = dpi * value / 6;
+    case RSVG_UNIT_IN:
+      value *= dpi;
+      break;
+    default:
+      /* Probably one of em, ex, or %.  We can't know what the pixel
+         value is without more information.  */
+      value = 0;
+    }
+
+  return value;
+}
+
 /* Load frame F and image IMG.  CONTENTS contains the SVG XML data to
    be parsed, SIZE is its size, and FILENAME is the name of the SVG
    file being loaded.
@@ -9792,11 +9830,48 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 #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;
+  /* Try the instrinsic dimensions first.  */
+  gboolean has_width, has_height, has_viewbox;
+  RsvgLength iwidth, iheight;
+
+  rsvg_handle_get_intrinsic_dimensions (rsvg_handle,
+                                        &has_width, &iwidth,
+                                        &has_height, &iheight,
+                                        &has_viewbox, &viewbox);
+
+  if (has_width && has_height)
+    {
+      /* Success!  We can use these values directly.  */
+      viewbox_width = svg_css_length_to_pixels (iwidth);
+      viewbox_height = svg_css_length_to_pixels (iheight);
+    }
+  else if (has_width && has_viewbox)
+    {
+      viewbox_width = svg_css_length_to_pixels (iwidth);
+      viewbox_height = svg_css_length_to_pixels (iwidth)
+        * viewbox.width / viewbox.height;
+    }
+  else if (has_height && has_viewbox)
+    {
+      viewbox_height = svg_css_length_to_pixels (iheight);
+      viewbox_width = svg_css_length_to_pixels (iheight)
+        * viewbox.height / viewbox.width;
+    }
+  else if (has_viewbox)
+    {
+      viewbox_width = viewbox.width;
+      viewbox_height = viewbox.height;
+    }
+  else
+    {
+      /* We haven't found a useable set of sizes, so try working out
+         the visible area.  */
+      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
   /* The function used above to get the geometry of the visible area
      of the SVG are only available in librsvg 2.46 and above, so in
@@ -9809,6 +9884,19 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   viewbox_width = dimension_data.width;
   viewbox_height = dimension_data.height;
 #endif
+
+  if (viewbox_width == 0 || viewbox_height == 0)
+    {
+      /* We do not have any usable dimensions, so make some up.  The
+         values below are supposedly the default values most web
+         browsers use for SVGs with no set size.  */
+      /* FIXME: At this stage we should perhaps consider rendering the
+         image out to a bitmap and getting the dimensions from
+         that.  */
+      viewbox_width = 300;
+      viewbox_height = 150;
+    }
+
   compute_image_size (viewbox_width, viewbox_height, img->spec,
                       &width, &height);
 
-- 
2.26.1