Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Writing SVG causes librsvg panic #41

Closed
jimjam-slam opened this issue Sep 30, 2023 · 13 comments
Closed

Writing SVG causes librsvg panic #41

jimjam-slam opened this issue Sep 30, 2023 · 13 comments

Comments

@jimjam-slam
Copy link

jimjam-slam commented Sep 30, 2023

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
Copy link
Author

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
Copy link
Author

jimjam-slam commented Sep 30, 2023

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
Copy link
Member

jeroen commented Oct 2, 2023

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

@jeroen
Copy link
Member

jeroen commented Oct 2, 2023

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
Copy link
Author

jimjam-slam commented Oct 2, 2023

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
Copy link
Author

jimjam-slam commented Oct 2, 2023

Seems like this could be related to Kozea/CairoSVG#214

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

@jeroen
Copy link
Member

jeroen commented Oct 3, 2023

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
Copy link
Member

jeroen commented Oct 3, 2023

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
Copy link
Member

jeroen commented Oct 3, 2023

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
Copy link
Member

jeroen commented Oct 3, 2023

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 #36 and we need to try what is suggested here: #29 (comment)

jeroen added a commit that referenced this issue Oct 4, 2023
I think this may possibly address #41
@jeroen
Copy link
Member

jeroen commented Oct 4, 2023

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
Copy link
Member

jeroen commented Oct 8, 2023

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
Copy link
Author

jimjam-slam commented Oct 9, 2023

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants