ropensci / rsvg

SVG renderer for R based on librsvg2
Other
95 stars 1 forks source link

Writing SVG causes librsvg panic #41

Closed jimjam-slam closed 9 months ago

jimjam-slam commented 10 months ago

As part of some refactoring of {ggflags}, I'm trying to run flag SVGs from HatScripts/circle-flags through rsvg::rsvg_svg to ensure they'r Cairo-compatible.

However, running that function with any flag in the set causes a panic that kills my R session:

> rsvg_svg("svg/au.svg", "test1.svg")
thread '<unnamed>' panicked at 'assertion failed: width > 0 && height > 0', src/surface_utils/shared_surface.rs:217:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5
[1]    22622 abort      R

I also get this if I manually specify an output width and height:

> rsvg_svg("svg/au.svg", "test1.svg", width = 20, height = 20)
thread '<unnamed>' panicked at 'assertion failed: width > 0 && height > 0', src/surface_utils/shared_surface.rs:217:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5
[1]    22622 abort      R

I don't have any problems with other export functions:

> rsvg::rsvg_png("svg/au.svg", "test1.png")
>

I can also successfully run this example code from the package docs:

tmp <- tempfile()
svglite::svglite(tmp, width = 10, height = 7)
ggplot2::qplot(mpg, wt, data = mtcars, colour = factor(cyl))
dev.off()
rsvg_svg(tmp, "out.svg")

So I'm thinking there's something about these input SVGs that librsvg doesn't like exporting!

Here's what au.svg looks like (I've applied a formatter to it but otherwise left it untouched):

<svg xmlns="http://www.w3.org/2000/svg" width="512" height="512" viewBox="0 0 512 512">
  <mask id="a">
    <circle cx="256" cy="256" r="256" fill="#fff" />
  </mask>
  <g mask="url(#a)">
    <path fill="#0052b4" d="M0 0h512v512H0z" />
    <path fill="#eee"
      d="m154 300 14 30 32-8-14 30 25 20-32 7 1 33-26-21-26 21 1-33-33-7 26-20-14-30 32 8zm222-27h47l-38 27 15-44 14 44zm7-162 7 15 16-4-7 15 12 10-15 3v17l-13-11-13 11v-17l-15-3 12-10-7-15 16 4zm57 67 7 15 16-4-7 15 12 10-15 3v16l-13-10-13 11v-17l-15-3 12-10-7-15 16 4zm-122 22 7 15 16-4-7 15 12 10-15 3v16l-13-10-13 11v-17l-15-3 12-10-7-15 16 4zm65 156 7 15 16-4-7 15 12 10-15 3v17l-13-11-13 11v-17l-15-3 12-10-7-15 16 4zM0 0v32l32 32L0 96v160h32l32-32 32 32h32v-83l83 83h45l-8-16 8-15v-14l-83-83h83V96l-32-32 32-32V0H96L64 32 32 0Z" />
    <path fill="#d80027" d="M32 0v32H0v64h32v160h64V96h160V32H96V0Zm96 128 128 128v-31l-97-97z" />
  </g>
</svg>

I'm on macOS Sonoma 14.0 on an M1 Mac, with R 4.2.1 and rsvg 2.5.0 linking to librsvg 2.56.0.

jimjam-slam commented 10 months ago

When I modify au.svg above so that there's no longer a <mask> element and the <g> element no longer has a mask attribute, the SVG export works fine! (The masks are pretty integral though, as the flags are supposed to be circular, so unless there's a way I can pre-process them to replace the mask with something else, I might be a bit stuck!)

jimjam-slam commented 10 months ago

Okay, modifying au.svg so that it uses a clipPath for its circular frames instead of a mask seems to allow it to be processed without a problem:

<svg xmlns="http://www.w3.org/2000/svg" width="512" height="512" viewBox="0 0 512 512">
  <clipPath id="a">
    <circle cx="256" cy="256" r="256" />
  </clipPath>
  <g clip-path="url(#a)">
    <path fill="#0052b4" d="M0 0h512v512H0z" />
    <path fill="#eee"
      d="m154 300 14 30 32-8-14 30 25 20-32 7 1 33-26-21-26 21 1-33-33-7 26-20-14-30 32 8zm222-27h47l-38 27 15-44 14 44zm7-162 7 15 16-4-7 15 12 10-15 3v17l-13-11-13 11v-17l-15-3 12-10-7-15 16 4zm57 67 7 15 16-4-7 15 12 10-15 3v16l-13-10-13 11v-17l-15-3 12-10-7-15 16 4zm-122 22 7 15 16-4-7 15 12 10-15 3v16l-13-10-13 11v-17l-15-3 12-10-7-15 16 4zm65 156 7 15 16-4-7 15 12 10-15 3v17l-13-11-13 11v-17l-15-3 12-10-7-15 16 4zM0 0v32l32 32L0 96v160h32l32-32 32 32h32v-83l83 83h45l-8-16 8-15v-14l-83-83h83V96l-32-32 32-32V0H96L64 32 32 0Z" />
    <path fill="#d80027" d="M32 0v32H0v64h32v160h64V96h160V32H96V0Zm96 128 128 128v-31l-97-97z" />
  </g>
</svg>

All of the flags in this set are structured the same way, so I think I can pre-process them to work for my purposes! If mask support is not expected, a friendlier error might be useful here—or, if it is, there might be a bug that needs to be pursued. I hope this helps, though!

jeroen commented 10 months ago

Thanks! I can reproduce this on MacOS 13. Maybe there is an issue with our build of librsvg.

jeroen commented 10 months ago

Can you test if the problem is gone with the latest build:

install.packages('rsvg', repos = 'https://ropensci.r-universe.dev')

Update: hmm for me rsvg_svg no longer crashes, but rsvg_svg() does not give useful output either. Not quite sure why this svg won't roundtrip properly.

jimjam-slam commented 10 months ago

Mmmm, I can reproduce the result you're getting with an essentially empty SVG on the development version! Actually, I'm getting that empty SVG even if I include my previous workaround (substituting the <mask> for a <clipPath>).

jimjam-slam commented 10 months ago

Seems like this could be related to https://github.com/Kozea/CairoSVG/issues/214

EDIT: alas, adding attributes to the <mask> still resulted in a runtime error.

jeroen commented 10 months ago

Yeah there must be an issue with our static builds of the rsvg stack. Using the rsvg command line seems to work:

brew install librsvg
curl -OL https://raw.githubusercontent.com/HatScripts/circle-flags/gh-pages/flags/au.svg
rsvg-convert au.svg -f svg > out.svg

I noticed the output svg contains an embedded png image, so that is perhaps where things go wrong in our static librsvg.

jeroen commented 10 months ago

Hmm I see the same problem now on Windows after updating librsvg to 2.57.0. Not sure how to fix this, it seems to be a problem with recent versions of librsvg that only appears when when compiled statically :/

jeroen commented 10 months ago

I tried with librsvg 2.52 and 2.54 but they both crash as well. We could downgrade to 2.50 which seems to work for this example but who knows what side effects that has.

jeroen commented 10 months ago

OK some progress: this problem seems triggered by the "new" scaling methods that in our C code is conditional on LIBRSVG_CHECK_VERSION(2,52,0). If I remove the new code, things are working (but not scaling correctly).

So I think this is related to https://github.com/ropensci/rsvg/issues/36 and we need to try what is suggested here: https://github.com/ropensci/rsvg/issues/29#issuecomment-1059459290

jeroen commented 10 months ago

This code needs some rewriting, but I think the immediate issue is fixed. Can you try the latest version from https://ropensci.r-universe.dev/rsvg ?

jeroen commented 9 months ago

This should be released on rsvg 2.6.0, which is just released on CRAN. I hope this resolves your problems, but if you do find any issues, please open a new issue.

Thanks for reporting this.

jimjam-slam commented 9 months ago

Can confirm these are successfully processing ion rsvg 2.6.0! (Apologies for the delay getting back to you too!). The masks aren't rendering correctly yet, but that could be a separate problem with {grImport2}, which we're using to display them (I see @pmur002 is talking about some updates to that package in #40).

I should also note that the flags are quite a bit larger after processing—about 15-18 KB each, compared to less than 1 KB before processing. That's mostly because the mask (a black circle) is being base-64 encoded. I'm not sure if that's required on Cairo's side for rendering in R, and it's not really a problem as far as our package is concerned (except for folks rendering back out to SVG, perhaps), but it does blow the size out quite a lot!

EDIT: I'll file a separate issue for this second par!