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

packages/svg can't understand RGB colors expressed in percentage #1197

Open
ctrlcctrlv opened this issue Aug 31, 2021 · 5 comments · Fixed by memononen/nanosvg#198
Open

packages/svg can't understand RGB colors expressed in percentage #1197

ctrlcctrlv opened this issue Aug 31, 2021 · 5 comments · Fixed by memononen/nanosvg#198
Labels
bug Software bug issue Not My Bug 3rd party issue outside of SILE's scope
Milestone

Comments

@ctrlcctrlv
Copy link
Member

I commonly use cairosvg on the command line to clean up SVG files I intend to put into SILE documents. One thing that it does that I need is it takes <use/> elements and puts whatever is in the referent <symbol/> there, while keeping track of transforms. (SILE cannot understand <use/> at all.)

Unfortunately, cairosvg also transforms color elements as part of its cleanup. It transforms fill:#4d4d4d to fill:rgb(30.196078%,30.196078%,30.196078%). SILE interprets this as cyan, not the expected grey.

Minimal example

These two SVG should be rendered equivalently:

box.svg

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg width="14pt" height="14pt" viewBox="0 0 14 14" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:svg="http://www.w3.org/2000/svg">
  <rect style="fill:#4d4d4d" width="10" height="10" x="2" y="2" />
</svg>

box2.svg

<?xml version="1.0" encoding="UTF-8"?>
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="14pt" height="14pt" viewBox="0 0 14 14" version="1.1">
  <path style="fill:rgb(30.196078%,30.196078%,30.196078%);" d="M 2 2 L 12 2 L 12 12 L 2 12 Z M 2 2 "/>
</svg>

min.sil

\begin[papersize=28pt x 28pt]{document}
\neverindent\nofolios
\script[src=packages/svg]
\svg[width=10pt, src=box.svg]

\svg[width=10pt, src=box2.svg]
\end{document}

Output

image

@ctrlcctrlv
Copy link
Member Author

I have found the issue. It is in the nanosvg.h library, in the function nsvg__parseColorRGB.

sscanf(str + 4, "%d%[%%, \t]%d%[%%, \t]%d", &r, s1, &g, s2, &b);

It cannot understand decimal values in the percentage. If I use fill:rgb(30%,30%,30%), it's all fine.

@ctrlcctrlv
Copy link
Member Author

@erco77 has a patch here: memononen/nanosvg#136 (comment)

I think it will work to solve this.

@alerque
Copy link
Member

alerque commented Sep 3, 2021

Given that the upstream nanosvg maintainer is paying attention, I suggest the best thing to do is agree on a solution and put together a good PR upstream first. Once it is fixed there we can update our vendored version. If it was unmaintained or there was another roadblock I might consider patching locally, but in this case I don't see any reason this can't get fixed as far upstream as it possible.

(Also keep a weather eye out on Rust crates that might make viable alternatives for SVG‌ handling in the future.)

@ctrlcctrlv
Copy link
Member Author

@alerque memononen/nanosvg#136 is now closed by memononen/nanosvg#198 and just needs a release. However, NanoSVG has needed a release since April (memononen/nanosvg#197). If one isn't made soon hopefully using the development version is not such a big deal for SILE.

@alerque alerque added this to the v0.12.x milestone Sep 21, 2021
@alerque alerque added bug Software bug issue Not My Bug 3rd party issue outside of SILE's scope labels Sep 21, 2021
@alerque
Copy link
Member

alerque commented Sep 21, 2021

No, I don't think using the HEAD version would be a problem for us. I'd rather they cut a release like they clearly need to, but we could deal with using anything on their default branch I suppose.

cf. #1205

I'm pushing this to the next release cycle because "just one more thing" is already significantly delaying the math release and I want to get that cleaned up and out the door.

@alerque alerque modified the milestones: v0.12.1, v0.12.x Jan 12, 2022
@alerque alerque modified the milestones: v0.12.3, v0.12.x Mar 2, 2022
@alerque alerque modified the milestones: v0.12.x, v0.13.x Apr 18, 2022
@alerque alerque modified the milestones: v0.13.x, v0.14.x Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Software bug issue Not My Bug 3rd party issue outside of SILE's scope
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants