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