bug#64908: 29.1; svg parse failure

[Alan Third <alan@idiocy.org>]

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

On Sat, Aug 05, 2023 at 08:00:17PM +0200, David Ponce wrote:
> On 05/08/2023 19:39, Alan Third wrote:
> > On Sat, Aug 05, 2023 at 07:37:01PM +0200, David Ponce wrote:
> > > 
> > > I just tried patch V3, and the result is the same:
> > > insert-image produces the same black square of 22x22 pixels.
> > 
> > Can you try changing the initialisation of zero_rect to {0, 0, 10,
> > 10}?
> > 
> > I remember a lot of trial and error with this originally, and it looks
> > like they may have changed how it works.
> 
> It is better :-)
> 
> Before call to rsvg_handle_get_geometry_for_layer:
> 
> (gdb) print zero_rect
> $1 = {x = 0, y = 0, width = 10, height = 10}
> (gdb) print viewbox
> $2 = {x = 6.9533474147268633e-310, y = 2.3106462124703418e-319, width = 5.8283471538673388e-315, height = 9.8813129168249309e-323}
> (gdb) print out_logical_rect
> $3 = {x = 6.9533558057749482e-310, y = 6.9533473517748961e-310, width = 1.4821969375237396e-323, height = 3.0586981611317814e-317}
> 
> After call:
> 
> (gdb) print $eax
> $4 = 1
> (gdb) print zero_rect
> $5 = {x = 0, y = 0, width = 10, height = 10}
> (gdb) print viewbox
> $6 = {x = 7, y = 3, width = 8.70703125, height = 16}
> (gdb) print out_logical_rect
> $7 = {x = 7, y = 3, width = 8.70703125, height = 16}
> 
> Now I see the image, aligned at bottom right on the line (see the screenshot).

OK. I really don't understand how this works, and I'm sure some older
version of librsvg required a viewport of zero size, but I don't know
which, so this is all a bit of a stab in the dark. Anyway, patch
attached with a viewport that is hopefully big enough for all the SVGs
we'll ever see. :)

-- 
Alan Third

[-- Attachment #2: v4-0001-Fix-percentage-sizes-in-SVG-display-bug-64908.patch --]
[-- Type: text/x-diff, Size: 5541 bytes --]

From b37126c0c72b6949783a3044b44896e0ebbd9606 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 5 Aug 2023 10:39:31 +0100
Subject: [PATCH v4] Fix percentage sizes in SVG display (bug#64908)

* src/image.c (svg_css_length_to_pixels): Make percent units always
return zero and handle font size based units better.
(svg_load_image): Don't rely on the width and height values from the
SVG actually having any useful data, even if they're explicitly set.
---
 src/image.c | 82 +++++++++++++++++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 30 deletions(-)

diff --git a/src/image.c b/src/image.c
index c9420b48f4a..bc42f3c55f9 100644
--- a/src/image.c
+++ b/src/image.c
@@ -11137,6 +11137,12 @@ svg_css_length_to_pixels (RsvgLength length, double dpi, int font_size)
 {
   double value = length.length;
 
+#if ! LIBRSVG_CHECK_VERSION (2, 48, 0)
+  /* librsvg 2.48 lets us define the font size, but earlier versions
+     default to 12 pixels.  */
+  font_size = 12;
+#endif
+
   switch (length.unit)
     {
     case RSVG_UNIT_PX:
@@ -11161,16 +11167,31 @@ svg_css_length_to_pixels (RsvgLength length, double dpi, int font_size)
     case RSVG_UNIT_IN:
       value *= dpi;
       break;
-#if LIBRSVG_CHECK_VERSION (2, 48, 0)
-      /* We don't know exactly what font size is used on older librsvg
-	 versions.  */
     case RSVG_UNIT_EM:
       value *= font_size;
       break;
-#endif
+    case RSVG_UNIT_EX:
+      /* librsvg uses an ex height of half the em height, so we match
+	 that here.  */
+      value = value * font_size / 2.0;
+      break;
+    case RSVG_UNIT_PERCENT:
+      /* Percent is a ratio of the containing "viewport".  We don't
+	 have a viewport, as such, as we try to draw the image to it's
+	 'natural' size rather than dictate the size as if we were
+	 drawing icons on a toolbar or similar.  This means that
+	 percent values are useless to us and we are best off just
+	 drawing the image according to whatever other sizes we can
+	 derive.
+
+	 If we do set explicit width and height values in the image
+	 spec, this will work out correctly as librsvg will still
+	 honour the percentage sizes in its final rendering no matter
+	 what size we make the image.  */
+      value = 0;
+      break;
     default:
-      /* Probably ex or %.  We can't know what the pixel value is
-         without more information.  */
+      /* We should never reach this.  */
       value = 0;
     }
 
@@ -11301,7 +11322,8 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
     }
   else
     {
-      RsvgRectangle zero_rect, viewbox, out_logical_rect;
+      RsvgRectangle  viewbox;
+      double explicit_width = 0, explicit_height = 0;
 
       /* Try the intrinsic dimensions first.  */
       gboolean has_width, has_height;
@@ -11313,34 +11335,27 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 					    &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, dpi,
+      if (has_width)
+	explicit_width = svg_css_length_to_pixels (iwidth, dpi,
+						   img->face_font_size);
+      if (has_height)
+	explicit_height = svg_css_length_to_pixels (iheight, dpi,
 						    img->face_font_size);
-	  viewbox_height = svg_css_length_to_pixels (iheight, dpi,
-						     img->face_font_size);
 
-	  /* Here one dimension could be zero because in percent unit.
-	     So calculate this dimension with the other.  */
-	  if (! (0 < viewbox_width) && (iwidth.unit == RSVG_UNIT_PERCENT))
-	    viewbox_width = (viewbox_height * viewbox.width / viewbox.height)
-	      * iwidth.length;
-	  else if (! (0 < viewbox_height) && (iheight.unit == RSVG_UNIT_PERCENT))
-	    viewbox_height = (viewbox_width * viewbox.height / viewbox.width)
-	      * iheight.length;
+      if (explicit_width > 0 && explicit_height > 0)
+	{
+	  viewbox_width = explicit_width;
+	  viewbox_height = explicit_height;
 	}
-      else if (has_width && has_viewbox)
+      else if (explicit_width > 0 && has_viewbox)
 	{
-	  viewbox_width = svg_css_length_to_pixels (iwidth, dpi,
-						    img->face_font_size);
-	  viewbox_height = viewbox_width * viewbox.height / viewbox.width;
+	  viewbox_width = explicit_width;
+	  viewbox_height = explicit_width * viewbox.height / viewbox.width;
 	}
-      else if (has_height && has_viewbox)
+      else if (explicit_height > 0 && has_viewbox)
 	{
-	  viewbox_height = svg_css_length_to_pixels (iheight, dpi,
-						     img->face_font_size);
-	  viewbox_width = viewbox_height * viewbox.width / viewbox.height;
+	  viewbox_height = explicit_height;
+	  viewbox_width = explicit_height * viewbox.width / viewbox.height;
 	}
       else if (has_viewbox)
 	{
@@ -11354,8 +11369,15 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 	{
 	  /* We haven't found a usable set of sizes, so try working out
 	     the visible area.  */
+
+	  /* FIXME: I'm not sure exactly how librsvg uses this
+	     viewport input here, so I'm not sure what values I should
+	     set. */
+	  RsvgRectangle max_viewport_rect = {0, 0, UINT_MAX, UINT_MAX};
+	  RsvgRectangle out_logical_rect;
+
 	  rsvg_handle_get_geometry_for_layer (rsvg_handle, NULL,
-					      &zero_rect, &viewbox,
+					      &max_viewport_rect, &viewbox,
 					      &out_logical_rect, NULL);
 	  viewbox_width = viewbox.x + viewbox.width;
 	  viewbox_height = viewbox.y + viewbox.height;
-- 
2.40.1