bug#44065: 28.0.50; SVG image not shown completely

bug#44065: 28.0.50; SVG image not shown completely

[styang]

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

Emacs stopped showing SVG files completely (i.e. it is shown cropped), with 8f42b94fe43 the offending commit. This commit intends to resolve bug#40845.

Please find the SVG file (1.svg) I use and the render image (1.png) in attachment. Notice the cropped part at the bottom and on the right side. The SVG file can be correctly viewed by Emacs 27, and other photo viewers like eog or gThumb. The file attached embeds an SVG element inside another, which seems to be discouraged by https://debbugs.gnu.org/cgi/bugreport.cgi?bug=40845#68. However, according to Mozilla MDN (https://developer.mozilla.org/en-US/docs/Web/SVG/Element/svg),

> The svg element is a container that defines a new coordinate system and viewport. It is used as the outermost element of SVG documents, but it can also be used to embed an SVG fragment inside an SVG or HTML document.

> Note: The xmlns attribute is only required on the outermost svg element of SVG documents. It is unnecessary for inner svg elements or inside HTML documents.

So the file IS a valid SVG file. 



[-- Attachment #2: offending svg --]
[-- Type: image/svg+xml, Size: 21845 bytes --]

[-- Attachment #3: rendered --]
[-- Type: image/png, Size: 14900 bytes --]

[-- Attachment #4: Type: text/plain, Size: 178 bytes --]



-- 
Sheng Yang(杨圣), PhD candidate
Computer Science Department
University of Maryland, College Park
E-mail: styang@fastmail.com
E-mail(old): yangsheng6810@gmail.com

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> From: styang@fastmail.com
> Date: Sat, 17 Oct 2020 18:27:30 -0500
> 
> Emacs stopped showing SVG files completely (i.e. it is shown cropped), with 8f42b94fe43 the offending commit. This commit intends to resolve bug#40845.

On what OS and with which version of librsvg?

> Please find the SVG file (1.svg) I use and the render image (1.png) in attachment. Notice the cropped part at the bottom and on the right side. The SVG file can be correctly viewed by Emacs 27, and other photo viewers like eog or gThumb. The file attached embeds an SVG element inside another, which seems to be discouraged by https://debbugs.gnu.org/cgi/bugreport.cgi?bug=40845#68. However, according to Mozilla MDN (https://developer.mozilla.org/en-US/docs/Web/SVG/Element/svg),

I have no problem displaying 1.svg with today's master.  How did you
try to display it?  Please show a complete recipe starting from
"emacs -Q".  I just visited the file, and it displayed correctly.

bug#44065: 28.0.50; SVG image not shown completely

[Sheng Yang]

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

> On what OS and with which version of librsvg?

Arch Linux, librsvg 2:2.50.1-1.

> I have no problem displaying 1.svg with today's master.  How did you
> try to display it?  Please show a complete recipe starting from
> "emacs -Q".  I just visited the file, and it displayed correctly.

I checked on today's master (commit 2c0cd90083), and the problem persists. Recipe of showing:

./src/emacs -Q /tmp/1.svg



Sheng Yang(杨圣), PhD candidate
Computer Science Department
University of Maryland, College Park
E-mail: styang@fastmail.com
E-mail (old but still used): yangsheng6810@gmail.com


[-- Attachment #2: Type: text/html, Size: 1236 bytes --]

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> Date: Sun, 18 Oct 2020 11:02:54 -0500
> From: "Sheng Yang" <styang@fastmail.com>
> Cc: 44065@debbugs.gnu.org
> 
> I checked on today's master (commit 2c0cd90083), and the problem persists. Recipe of showing:
> 
> ./src/emacs -Q /tmp/1.svg

No problem here with this recipe.

Does anyone else succeed in reproducing the problem?

bug#44065: 28.0.50; SVG image not shown completely

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Does anyone else succeed in reproducing the problem?

The svg image is cropped for me (Debian bullseye) to the right in Emacs
28.  Firefox displays it correctly, but Imagemagick display refuses to
show the image:

larsi@xo:~/src/emacs/trunk$ display /tmp/1.svg
display-im6.q16: must specify image size `/tmp/magick-GeZIabcfFHYCvUwmkbtM-oODO_uzDIU-' @ error/mvg.c/ReadMVGImage/186.


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: "Sheng Yang" <styang@fastmail.com>,  44065@debbugs.gnu.org
> Date: Sun, 18 Oct 2020 18:24:58 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Does anyone else succeed in reproducing the problem?
> 
> The svg image is cropped for me (Debian bullseye) to the right in Emacs
> 28.

By "cropped" do you mean that the rightmost closing parenthesis is
only partially visible?  If so, I see the same, but Emacs 27 and
Emacs 26.3 show the same "cropping", whereas the OP says the problem
started happening at some recent commit to master.

> Firefox displays it correctly, but Imagemagick display refuses to
> show the image:
> 
> larsi@xo:~/src/emacs/trunk$ display /tmp/1.svg
> display-im6.q16: must specify image size `/tmp/magick-GeZIabcfFHYCvUwmkbtM-oODO_uzDIU-' @ error/mvg.c/ReadMVGImage/186.

So is the conclusion that the image is faulty in some way, and we are
looking at "undefined behavior" of some kind?

bug#44065: 28.0.50; SVG image not shown completely

[Stephen Berman]

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

On Sun, 18 Oct 2020 20:03:25 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Cc: "Sheng Yang" <styang@fastmail.com>,  44065@debbugs.gnu.org
>> Date: Sun, 18 Oct 2020 18:24:58 +0200
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> > Does anyone else succeed in reproducing the problem?
>>
>> The svg image is cropped for me (Debian bullseye) to the right in Emacs
>> 28.
>
> By "cropped" do you mean that the rightmost closing parenthesis is
> only partially visible?  If so, I see the same, but Emacs 27 and
> Emacs 26.3 show the same "cropping", whereas the OP says the problem
> started happening at some recent commit to master.
>
>> Firefox displays it correctly, but Imagemagick display refuses to
>> show the image:
>>
>> larsi@xo:~/src/emacs/trunk$ display /tmp/1.svg
>> display-im6.q16: must specify image size
>> `/tmp/magick-GeZIabcfFHYCvUwmkbtM-oODO_uzDIU-' @
>> error/mvg.c/ReadMVGImage/186.
>
> So is the conclusion that the image is faulty in some way, and we are
> looking at "undefined behavior" of some kind?

I see what the OP reports: the image is cropped on the right -- the
closing `)' --  and the bottom -- `i-1' in master but not in emacs-27,
see the attached picture.

Steve Berman


[-- Attachment #2: 1.png --]
[-- Type: image/png, Size: 44570 bytes --]

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  44065@debbugs.gnu.org,
>   styang@fastmail.com
> Date: Sun, 18 Oct 2020 19:42:46 +0200
> 
> I see what the OP reports: the image is cropped on the right -- the
> closing `)' --  and the bottom -- `i-1' in master but not in emacs-27,
> see the attached picture.

That's not what I see here: I see identical ("cropped") displays on
master, in Emacs 27, and in Emacs 26.3.

Maybe it depends on the version of librsvg? or some dependency of
librsvg?

bug#44065: 28.0.50; SVG image not shown completely

[Stephen Berman]

On Sun, 18 Oct 2020 20:48:32 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  44065@debbugs.gnu.org,
>>   styang@fastmail.com
>> Date: Sun, 18 Oct 2020 19:42:46 +0200
>>
>> I see what the OP reports: the image is cropped on the right -- the
>> closing `)' --  and the bottom -- `i-1' in master but not in emacs-27,
>> see the attached picture.
>
> That's not what I see here: I see identical ("cropped") displays on
> master, in Emacs 27, and in Emacs 26.3.

I also see it uncropped in Emacs 26.3 as well as in Firefox and two
image viewers I have, but with ImageMagick I get the same error Lars
reported.

> Maybe it depends on the version of librsvg? or some dependency of
> librsvg?

My system uses librsvg-2.48.2.

Steve Berman

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: larsi@gnus.org,  44065@debbugs.gnu.org,  styang@fastmail.com
> Date: Sun, 18 Oct 2020 20:00:58 +0200
> 
> >> I see what the OP reports: the image is cropped on the right -- the
> >> closing `)' --  and the bottom -- `i-1' in master but not in emacs-27,
> >> see the attached picture.
> >
> > That's not what I see here: I see identical ("cropped") displays on
> > master, in Emacs 27, and in Emacs 26.3.
> 
> I also see it uncropped in Emacs 26.3 as well as in Firefox and two
> image viewers I have, but with ImageMagick I get the same error Lars
> reported.

If the image is corrupted or invalid, we could see "chaotic" results
due to unrelated factors.

> > Maybe it depends on the version of librsvg? or some dependency of
> > librsvg?
> 
> My system uses librsvg-2.48.2.

2.40.1 here.

bug#44065: 28.0.50; SVG image not shown completely

[Sheng Yang]

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

> I also see it uncropped in Emacs 26.3 as well as in Firefox and two
> image viewers I have, but with ImageMagick I get the same error Lars
> reported.
> 

I tried ImageMagick (or display), I do not get any errors, but it shows a blank canvas. (Arch Linux, imagemagick 7.0.10.34-1, librsvg 2:2.50.1-1.)

Sheng Yang(杨圣), PhD candidate
Computer Science Department
University of Maryland, College Park
E-mail: styang@fastmail.com
E-mail (old but still used): yangsheng6810@gmail.com


[-- Attachment #2: Type: text/html, Size: 962 bytes --]

bug#44065: 28.0.50; SVG image not shown completely

[Alan Third]

On Sat, Oct 17, 2020 at 06:27:30PM -0500, styang@fastmail.com wrote:
> Emacs stopped showing SVG files completely (i.e. it is shown
> cropped), with 8f42b94fe43 the offending commit. This commit intends
> to resolve bug#40845.
> 
> Please find the SVG file (1.svg) I use and the render image (1.png)
> in attachment. Notice the cropped part at the bottom and on the
> right side. The SVG file can be correctly viewed by Emacs 27, and
> other photo viewers like eog or gThumb. The file attached embeds an
> SVG element inside another, which seems to be discouraged by
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=40845#68. However,
> according to Mozilla MDN
> (https://developer.mozilla.org/en-US/docs/Web/SVG/Element/svg),
> 
> > The svg element is a container that defines a new coordinate
> > system and viewport. It is used as the outermost element of SVG
> > documents, but it can also be used to embed an SVG fragment inside
> > an SVG or HTML document.

I think you've misunderstood my message in that bug thread. Emacs
can't blindly embed SVG files within other SVGs because we can't
guarantee the files will only contain the <svg>...</svg> section.
There may be other bits, specifically XML doctype declarations, that
cannot be inserted inside an SVG.

It appears that by adding the wrapper SVG we change the scale of
the image even though we're not asking to. I can't see what we're
doing wrong.

I tried switching from using rsvg_handle_get_dimensions since it uses
a deprecated struct, although the function itself isn't deprecated
(shades of Apple's APIs). However rsvg_handle_get_geometry_for_layer
just plain doesn't return anything for at least one of my test SVG
files and still seems to produce the wrong values for the viewport
dimensions.

It's getting late. I suppose I'll come back to this tomorrow.

-- 
Alan Third

bug#44065: 28.0.50; SVG image not shown completely

[Lars Ingebrigtsen]

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.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44065: 28.0.50; SVG image not shown completely

[Stephen Berman]

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.)

Steve Berman

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Stephen Berman <stephen.berman@gmx.net>,  44065@debbugs.gnu.org,
>   styang@fastmail.com
> Date: Mon, 19 Oct 2020 10:43:53 +0200
> 
> 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.

But is it interesting, given that Imagemagick doesn't like the image?
Invalid images can cause all kinds of weird problems which depend on
versions of the image libraries in use.

bug#44065: 28.0.50; SVG image not shown completely

[Alan Third]

[-- 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

bug#44065: 28.0.50; SVG image not shown completely

[Sheng Yang]

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


> 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 checked the patch on Debian 10.6, with librsvg2 2.44.10-2.1. Compilation failed with the following error (path sanitized)

/usr/bin/ld: image.o: in function `svg_load_image':
emacs/src/image.c:9810: undefined reference to `rsvg_handle_set_stylesheet'
/usr/bin/ld: emacs/src/image.c:9924: undefined reference to `rsvg_handle_set_stylesheet'
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:651：temacs] error 1
make[1]: leaving directory “emacs/src”
make: *** [Makefile:424：src] error 2

BTW, the patch fixes the problem on Arch Linux, with librsvg 2:2.50.1-1.

Sheng Yang(杨圣), PhD candidate
Computer Science Department
University of Maryland, College Park
E-mail: styang@fastmail.com
E-mail (old but still used): yangsheng6810@gmail.com


[-- Attachment #2: Type: text/html, Size: 1573 bytes --]

bug#44065: 28.0.50; SVG image not shown completely

[Alan Third]

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

On Mon, Oct 19, 2020 at 05:34:45PM -0500, Sheng Yang wrote:
> 
> > 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 checked the patch on Debian 10.6, with librsvg2 2.44.10-2.1. Compilation failed with the following error (path sanitized)
> 
> /usr/bin/ld: image.o: in function `svg_load_image':
> emacs/src/image.c:9810: undefined reference to `rsvg_handle_set_stylesheet'
> /usr/bin/ld: emacs/src/image.c:9924: undefined reference to `rsvg_handle_set_stylesheet'
> collect2: error: ld returned 1 exit status
> make[1]: *** [Makefile:651：temacs] error 1
> make[1]: leaving directory “emacs/src”
> make: *** [Makefile:424：src] error 2

Thanks. I've attached a new one that doesn't try playing with
stylesheets since that's not necessary for this fix.

> BTW, the patch fixes the problem on Arch Linux, with librsvg 2:2.50.1-1.

Excellent, good to know, thanks.
-- 
Alan Third

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

From 760fe87e1dcb7451a5fa9ff19b80f940b8e03304 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Mon, 19 Oct 2020 21:19:57 +0100
Subject: [PATCH v2] 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.
---
 src/image.c | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/src/image.c b/src/image.c
index 25d5af8a8d..c734623739 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;
@@ -9789,15 +9789,34 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 #endif
 
   /* 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.  */
@@ -9820,7 +9839,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
       "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>";
@@ -9845,8 +9864,9 @@ 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,
-                                    background & 0xFFFFFF, SSDATA (encoded_contents)))
+                                    viewbox_width, viewbox_height,
+                                    background & 0xFFFFFF,
+                                    SSDATA (encoded_contents)))
       goto rsvg_error;
 
     wrapped_size = strlen (wrapped_contents);
@@ -9887,12 +9907,6 @@ 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;
-    }
 
   /* We can now get a valid pixel buffer from the svg file, if all
      went ok.  */
-- 
2.26.1

bug#44065: 28.0.50; SVG image not shown completely

[Sheng Yang]

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


> Thanks. I've attached a new one that doesn't try playing with
> stylesheets since that's not necessary for this fix.

This time compiles with librsvg 2.44.10-2.1 (on Debian 10.6), but the bug persists. For librsvg 2.50 (on Arch Linux), the patch continues to work smoothly.


Sheng Yang(杨圣), PhD candidate
Computer Science Department
University of Maryland, College Park
E-mail: styang@fastmail.com
E-mail (old but still used): yangsheng6810@gmail.com


[-- Attachment #2: Type: text/html, Size: 957 bytes --]

bug#44065: 28.0.50; SVG image not shown completely

[Alan Third]

On Tue, Oct 20, 2020 at 12:15:25PM -0500, Sheng Yang wrote:
> 
> > Thanks. I've attached a new one that doesn't try playing with
> > stylesheets since that's not necessary for this fix.
> 
> This time compiles with librsvg 2.44.10-2.1 (on Debian 10.6), but
> the bug persists. For librsvg 2.50 (on Arch Linux), the patch
> continues to work smoothly.

I suppose if we're very concerned about it, we can remove the ability
to resize and set colours when using librsvg < 2.45 and therefore get
rid of this bug everywhere.

Does anyone have an opinion?
-- 
Alan Third

bug#44065: 28.0.50; SVG image not shown completely

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

> I suppose if we're very concerned about it, we can remove the ability
> to resize and set colours when using librsvg < 2.45 and therefore get
> rid of this bug everywhere.
>
> Does anyone have an opinion?

I guess that's a cleaner solution -- it'd just be a missing feature
instead of displaying the SVG wrong.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44065: 28.0.50; SVG image not shown completely

[Corwin Brust]

Hi Lars & Alan,

On Wed, Oct 21, 2020 at 5:56 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
> Alan Third <alan@idiocy.org> writes:
>
> > I suppose if we're very concerned about it, we can remove the ability
> > to resize and set colours when using librsvg < 2.45 and therefore get
> > rid of this bug everywhere.
> >
> > Does anyone have an opinion?
>
> I guess that's a cleaner solution -- it'd just be a missing feature
> instead of displaying the SVG wrong.

I think I would prefer a potentially slightly manged image in most
cases, vs adding special case feature detection logic to avoid
run-time errors from code that works with SVGs.

Would it make sense/be possible to warn with "consider upgrading
librsvg" or similar when we detect Emacs is using a "too old" version
of librsvg?


Thank you.
Corwin

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> Date: Tue, 20 Oct 2020 20:54:44 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: 44065@debbugs.gnu.org, Lars Ingebrigtsen <larsi@gnus.org>,
>  Stephen Berman <stephen.berman@gmx.net>
> 
> On Tue, Oct 20, 2020 at 12:15:25PM -0500, Sheng Yang wrote:
> > 
> > > Thanks. I've attached a new one that doesn't try playing with
> > > stylesheets since that's not necessary for this fix.
> > 
> > This time compiles with librsvg 2.44.10-2.1 (on Debian 10.6), but
> > the bug persists. For librsvg 2.50 (on Arch Linux), the patch
> > continues to work smoothly.
> 
> I suppose if we're very concerned about it, we can remove the ability
> to resize and set colours when using librsvg < 2.45 and therefore get
> rid of this bug everywhere.

Sounds too drastic to me.  The bug seems to affect rare images, and
then only crops a few pixels (I initially didn't even see the
cropping).  By contrast, losing the background feature is quite a
loss.  And with my relatively ancient version of librsvg I see the bug
even before the face background feature was installed, so this is not
a regression for me.

Can't we just say this bug will persist unless new enough librsvg is
used?

bug#44065: 28.0.50; SVG image not shown completely

[Sheng Yang]

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

On Wed, Oct 21, 2020, at 11:31, Eli Zaretskii wrote:
> Sounds too drastic to me.  The bug seems to affect rare images, and
> then only crops a few pixels (I initially didn't even see the
> cropping).  By contrast, losing the background feature is quite a
> loss.  And with my relatively ancient version of librsvg I see the bug
> even before the face background feature was installed, so this is not
> a regression for me.
> 
> Can't we just say this bug will persist unless new enough librsvg is
> used?
> 

+1 for the suggestion of saying the bug will persist unless new enough librsvg is used.


Sheng Yang(杨圣), PhD candidate
Computer Science Department
University of Maryland, College Park
E-mail: styang@fastmail.com
E-mail (old but still used): yangsheng6810@gmail.com


[-- Attachment #2: Type: text/html, Size: 1388 bytes --]

bug#44065: 28.0.50; SVG image not shown completely

[Alan Third]

On Wed, Oct 21, 2020 at 07:31:02PM +0300, Eli Zaretskii wrote:
> > Date: Tue, 20 Oct 2020 20:54:44 +0100
> > From: Alan Third <alan@idiocy.org>
> > 
> > I suppose if we're very concerned about it, we can remove the ability
> > to resize and set colours when using librsvg < 2.45 and therefore get
> > rid of this bug everywhere.
> 
> Sounds too drastic to me.  The bug seems to affect rare images, and
> then only crops a few pixels (I initially didn't even see the
> cropping).  By contrast, losing the background feature is quite a
> loss.  And with my relatively ancient version of librsvg I see the bug
> even before the face background feature was installed, so this is not
> a regression for me.

That seems fair. I'd forgotten that the cropping is visible even with
the old code.

> Can't we just say this bug will persist unless new enough librsvg is
> used?

Should I put that note in documentation somewhere or just leave it as
a comment in the code?
-- 
Alan Third

bug#44065: 28.0.50; SVG image not shown completely

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

> Should I put that note in documentation somewhere or just leave it as
> a comment in the code?

A comment in the code would be fine, I think.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  styang@fastmail.com,
>   stephen.berman@gmx.net,  44065@debbugs.gnu.org
> Date: Thu, 22 Oct 2020 13:42:48 +0200
> 
> Alan Third <alan@idiocy.org> writes:
> 
> > Should I put that note in documentation somewhere or just leave it as
> > a comment in the code?
> 
> A comment in the code would be fine, I think.

Right.  Maybe we should also have a PROBLEMS entry about this?

bug#44065: 28.0.50; SVG image not shown completely

[Alan Third]

On Thu, Oct 22, 2020 at 03:51:21PM +0300, Eli Zaretskii wrote:
> > From: Lars Ingebrigtsen <larsi@gnus.org>
> > Cc: Eli Zaretskii <eliz@gnu.org>,  styang@fastmail.com,
> >   stephen.berman@gmx.net,  44065@debbugs.gnu.org
> > Date: Thu, 22 Oct 2020 13:42:48 +0200
> > 
> > Alan Third <alan@idiocy.org> writes:
> > 
> > > Should I put that note in documentation somewhere or just leave it as
> > > a comment in the code?
> > 
> > A comment in the code would be fine, I think.
> 
> Right.  Maybe we should also have a PROBLEMS entry about this?

I've done both and pushed to master. I don't think there's anything
else to be done here so I'm closing the bug report. Thanks all.
-- 
Alan Third

bug#44065: 28.0.50; SVG image not shown completely

[Andy Moreton]

On Thu 22 Oct 2020, Alan Third wrote:

> On Thu, Oct 22, 2020 at 03:51:21PM +0300, Eli Zaretskii wrote:
>> > From: Lars Ingebrigtsen <larsi@gnus.org>
>> > Cc: Eli Zaretskii <eliz@gnu.org>,  styang@fastmail.com,
>> >   stephen.berman@gmx.net,  44065@debbugs.gnu.org
>> > Date: Thu, 22 Oct 2020 13:42:48 +0200
>> > 
>> > Alan Third <alan@idiocy.org> writes:
>> > 
>> > > Should I put that note in documentation somewhere or just leave it as
>> > > a comment in the code?
>> > 
>> > A comment in the code would be fine, I think.
>> 
>> Right.  Maybe we should also have a PROBLEMS entry about this?
>
> I've done both and pushed to master. I don't think there's anything
> else to be done here so I'm closing the bug report. Thanks all.

These changes do not build on Windows (64bit mingw64 on MSYS2):

C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: image.o: in function `svg_load_image':
C:/emacs/git/emacs/master/src/image.c:9795: undefined reference to `rsvg_handle_get_geometry_for_layer'

The installed rsvg headers have:

    #define LIBRSVG_VERSION "2.48.8"

Can you please take a look ?

    AndyM

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Fri, 23 Oct 2020 21:17:19 +0100
> 
> These changes do not build on Windows (64bit mingw64 on MSYS2):
> 
> C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: image.o: in function `svg_load_image':
> C:/emacs/git/emacs/master/src/image.c:9795: undefined reference to `rsvg_handle_get_geometry_for_layer'
> 
> The installed rsvg headers have:
> 
>     #define LIBRSVG_VERSION "2.48.8"
> 
> Can you please take a look ?

I tried to fix that now, please check.

(We cannot just call a new function from an image library without
loading it from its DLL at run time on MS-Windows.)

bug#44065: 28.0.50; SVG image not shown completely

[Andy Moreton]

On Sat 24 Oct 2020, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Fri, 23 Oct 2020 21:17:19 +0100
>> 
>> These changes do not build on Windows (64bit mingw64 on MSYS2):
>> 
>> C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
>> image.o: in function `svg_load_image':
>> C:/emacs/git/emacs/master/src/image.c:9795: undefined reference to `rsvg_handle_get_geometry_for_layer'
>> 
>> The installed rsvg headers have:
>> 
>>     #define LIBRSVG_VERSION "2.48.8"
>> 
>> Can you please take a look ?
>
> I tried to fix that now, please check.
>
> (We cannot just call a new function from an image library without
> loading it from its DLL at run time on MS-Windows.)

Ah, I should have remebered that. All working now - thanks Eli.

    AndyM

bug#44065: 28.0.50; SVG image not shown completely

[Alan Third]

On Sat, Oct 24, 2020 at 10:09:59AM +0300, Eli Zaretskii wrote:
> 
> (We cannot just call a new function from an image library without
> loading it from its DLL at run time on MS-Windows.)

Ah, that's not something I was aware of. Thanks for fixing it.
-- 
Alan Third

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> Date: Sat, 24 Oct 2020 18:01:24 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: Andy Moreton <andrewjmoreton@gmail.com>, 44065@debbugs.gnu.org
> 
> On Sat, Oct 24, 2020 at 10:09:59AM +0300, Eli Zaretskii wrote:
> > 
> > (We cannot just call a new function from an image library without
> > loading it from its DLL at run time on MS-Windows.)
> 
> Ah, that's not something I was aware of. Thanks for fixing it.

No sweat.  And thanks for fixing the original problem to begin with.

bug#44065: 28.0.50; SVG image not shown completely

[Andy Moreton]

On Sat 24 Oct 2020, Eli Zaretskii wrote:

>> Date: Sat, 24 Oct 2020 18:01:24 +0100
>> From: Alan Third <alan@idiocy.org>
>> Cc: Andy Moreton <andrewjmoreton@gmail.com>, 44065@debbugs.gnu.org
>> 
>> On Sat, Oct 24, 2020 at 10:09:59AM +0300, Eli Zaretskii wrote:
>> > 
>> > (We cannot just call a new function from an image library without
>> > loading it from its DLL at run time on MS-Windows.)
>> 
>> Ah, that's not something I was aware of. Thanks for fixing it.
>
> No sweat.  And thanks for fixing the original problem to begin with.

A new report in bug#44206 shows that this patch caused other problems.

The docs for rsvg_handle_get_geometry_for_layer show it does not report
minimum sizes, as it ignores clipping regions. Thus for an SVG file
which contains a small clipping region applied to a larger image, the
reported sizes are incorrect.

Also, rsvg_handle_get_geometry_for_layer returns a gboolean, which the
docs do not describe, but other API functions return TRUE for success
and FALSE for failure. This should be checked.

Running under gdb with the image from bug#44206 shows that the bounds
reported by rsvg_handle_get_geometry_for_layer are zero (so the
functions may have failed and returned FALSE).

The original report here showed that rsvg_handle_get_dimensions did not
alwyas return the correct results. It is documented to require a prior
call to rsvg_handle_set_dpi to give correct results, but that is not
called in image.c.

Perhaps this can be fixed by reverting to the original code with
addition of a call to rsvg_handle_set_dpi or rsvg_handle_set_dpi_x_y
before calling rsvg_handle_get_dimensions.

    AndyM

bug#44065: 28.0.50; SVG image not shown completely

[Alan Third]

On Sun, Oct 25, 2020 at 12:26:59PM +0000, Andy Moreton wrote:
> A new report in bug#44206 shows that this patch caused other problems.
> 
> The docs for rsvg_handle_get_geometry_for_layer show it does not report
> minimum sizes, as it ignores clipping regions. Thus for an SVG file
> which contains a small clipping region applied to a larger image, the
> reported sizes are incorrect.
> 
> Also, rsvg_handle_get_geometry_for_layer returns a gboolean, which the
> docs do not describe, but other API functions return TRUE for success
> and FALSE for failure. This should be checked.
> 
> Running under gdb with the image from bug#44206 shows that the bounds
> reported by rsvg_handle_get_geometry_for_layer are zero (so the
> functions may have failed and returned FALSE).
> 
> The original report here showed that rsvg_handle_get_dimensions did not
> alwyas return the correct results. It is documented to require a prior
> call to rsvg_handle_set_dpi to give correct results, but that is not
> called in image.c.
> 
> Perhaps this can be fixed by reverting to the original code with
> addition of a call to rsvg_handle_set_dpi or rsvg_handle_set_dpi_x_y
> before calling rsvg_handle_get_dimensions.

No, it makes no difference to set the dpi. As far as I can tell
setting the dpi doesn't even change the image dimensions which seems a
little odd.

The problem isn't that rsvg_handle_get_dimensions is wrong, it's that
we're wrapping the original SVG in another SVG and in order to get it
to display correctly we need to know the exact details of the original
SVG. rsvg_handle_get_dimensions does not return enough of the
information we need.

The librsvg documentation specifically tries to warn us off from
querying for dimensions and suggest if we REALLY want to do that we
should be doing it through Cairo.

As I see it the main problem here is that librsvg is designed to work
either with Cairo or in a very specific way and we're not doing
either.

I'm basically at the stage of saying we cannot have lossless,
arbitrarily resized SVGs using librsvg, but we can probably use the
stylesheet function added in 2.46 to set the background and foreground
colours.

As far as I can see there are no other real alternatives to librsvg
either, but I haven't investigated in detail.

-- 
Alan Third

bug#44065: 28.0.50; SVG image not shown completely

[Andy Moreton]

On Sun 25 Oct 2020, Alan Third wrote:

> On Sun, Oct 25, 2020 at 12:26:59PM +0000, Andy Moreton wrote:
>> Perhaps this can be fixed by reverting to the original code with
>> addition of a call to rsvg_handle_set_dpi or rsvg_handle_set_dpi_x_y
>> before calling rsvg_handle_get_dimensions.
>
> No, it makes no difference to set the dpi. As far as I can tell
> setting the dpi doesn't even change the image dimensions which seems a
> little odd.

I tried this on Windows using MSYS2 64bit (mingw64) and librsvg 2.48.8.
If I revert the original patch in this bug and add a call to:
    rsvg_handle_set_dpi(rsvg_handle, 96.0);

immediately before the call to rsvg_handle_get_dimensions, then:
 - bug44206 image 222.svg is rendered correctly
 - bug44065 image 1.svg is still clipped on the bottom and right edges.

Both of these images render correctly in emacs 27.1.50 built from the
emacs-27 branch, using the same librsvg headers and runtime library.

> The problem isn't that rsvg_handle_get_dimensions is wrong, it's that
> we're wrapping the original SVG in another SVG and in order to get it
> to display correctly we need to know the exact details of the original
> SVG. rsvg_handle_get_dimensions does not return enough of the
> information we need.

What information is missing specifically ?

Both rsvg_handle_get_geometry_for_layer and
rsvg_handle_get_intrinsic_dimensions are documented as not taking
clipping regions into account.

That means if these functions report a non-zero size of an unclipped
image, that may still fail to load as it could exceed the limit set by
`max-image-size' and cause check_image_size to report a failure, even if
the clipped image would fit within the limit set by the user.

> The librsvg documentation specifically tries to warn us off from
> querying for dimensions and suggest if we REALLY want to do that we
> should be doing it through Cairo.

Please show where it says that: I have not found such an admonition in
the docs.

> As I see it the main problem here is that librsvg is designed to work
> either with Cairo or in a very specific way and we're not doing
> either.

I'm not convinced, as the experiments above show there is something else
going on that we are missing.

    AndyM

bug#44065: 28.0.50; SVG image not shown completely

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Sun, 25 Oct 2020 17:12:24 +0000
> 
> immediately before the call to rsvg_handle_get_dimensions, then:
>  - bug44206 image 222.svg is rendered correctly
>  - bug44065 image 1.svg is still clipped on the bottom and right edges.
> 
> Both of these images render correctly in emacs 27.1.50 built from the
> emacs-27 branch, using the same librsvg headers and runtime library.

Emacs 27 doesn't wrap SVGs with another SVG, so the fact that 27 works
doesn't tell us much, right?  This code is entirely new in Emacs 28,
and AFAIR we introduced it because we wanted to be able to use our own
colors with images that don't specify theirs.

bug#44065: 28.0.50; SVG image not shown completely

[Alan Third]

On Sun, Oct 25, 2020 at 05:12:24PM +0000, Andy Moreton wrote:
> On Sun 25 Oct 2020, Alan Third wrote:
> 
> > On Sun, Oct 25, 2020 at 12:26:59PM +0000, Andy Moreton wrote:
> >> Perhaps this can be fixed by reverting to the original code with
> >> addition of a call to rsvg_handle_set_dpi or rsvg_handle_set_dpi_x_y
> >> before calling rsvg_handle_get_dimensions.
> >
> > No, it makes no difference to set the dpi. As far as I can tell
> > setting the dpi doesn't even change the image dimensions which seems a
> > little odd.
> 
> I tried this on Windows using MSYS2 64bit (mingw64) and librsvg 2.48.8.
> If I revert the original patch in this bug and add a call to:
>     rsvg_handle_set_dpi(rsvg_handle, 96.0);
> 
> immediately before the call to rsvg_handle_get_dimensions, then:
>  - bug44206 image 222.svg is rendered correctly
>  - bug44065 image 1.svg is still clipped on the bottom and right edges.
> 
> Both of these images render correctly in emacs 27.1.50 built from the
> emacs-27 branch, using the same librsvg headers and runtime library.

As Eli has already pointed out, the master branch is wrapping the SVG
inside another SVG so we can set various parameters, such as the width
and height of the final image. Emacs 27 doesn't do that.

> > The problem isn't that rsvg_handle_get_dimensions is wrong, it's that
> > we're wrapping the original SVG in another SVG and in order to get it
> > to display correctly we need to know the exact details of the original
> > SVG. rsvg_handle_get_dimensions does not return enough of the
> > information we need.
> 
> What information is missing specifically ?

I believe that rsvg_handle_get_dimensions returns a width and height
value that may not count the entirety of the whitespace on the left
and top of the image.

So we set up a viewBox with an x coord of 0 and the width returned by
rsvg_handle_get_dimensions, and that is not then wide enough to cover
the entire image, as we had no way to know to add the 3 pixels or so
of whitespace on the left.

> Both rsvg_handle_get_geometry_for_layer and
> rsvg_handle_get_intrinsic_dimensions are documented as not taking
> clipping regions into account.

rsvg_handle_get_intrinsic_dimensions is documented as returning the
dimensions defined in the top level of the SVG if they exist, so if
they don't cover all the clipping regions (or, in fact, anything at
all) then that's not really our problem as the person who created the
image set those dimensions specifically.

> That means if these functions report a non-zero size of an unclipped
> image, that may still fail to load as it could exceed the limit set by
> `max-image-size' and cause check_image_size to report a failure, even if
> the clipped image would fit within the limit set by the user.

Anything could exceed the maximum size, the problem we ran into with
the latest bug report was it returning a zero sized rectangle.

> > The librsvg documentation specifically tries to warn us off from
> > querying for dimensions and suggest if we REALLY want to do that we
> > should be doing it through Cairo.
> 
> Please show where it says that: I have not found such an admonition in
> the docs.

https://developer.gnome.org/rsvg/stable/RsvgHandle.html:

The preferred way to render an already-loaded RsvgHandle is to use
rsvg_handle_render_cairo(). Please see its documentation for details.

Alternatively, you can use rsvg_handle_get_pixbuf() to directly obtain
a GdkPixbuf with the rendered image. This is simple, but it does not
let you control the size at which the SVG will be rendered. It will
just be rendered at the size which rsvg_handle_get_dimensions() would
return, which depends on the dimensions that librsvg is able to
compute from the SVG data.

Also plenty of notices like this:

rsvg_pixbuf_from_file_at_zoom is deprecated and should not be used in
newly-written code.

Set up a cairo matrix and use rsvg_handle_new_from_file() +
rsvg_handle_render_cairo() instead.

and while this page doesn't explicitly deny us the ability to request
dimensions, it does make it clear they don't think we should be doing
that:

https://developer.gnome.org/rsvg/stable/recommendations-assets.html

Maybe I'm reading too much into what these pages say, but I haven't
yet found any simple way to do what we want to do other than using
Cairo or using deprecated functions (if they still work).

-- 
Alan Third