sile-typesetter / sile

The SILE Typesetter — Simon’s Improved Layout Engine
https://sile-typesetter.org
MIT License
1.67k stars 98 forks source link

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

Open ctrlcctrlv opened 3 years ago

ctrlcctrlv commented 3 years ago

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 commented 3 years ago

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

https://github.com/sile-typesetter/sile/blob/7e77f8941585af0f569d951c72ed7694745af53a/src/nanosvg.h#L1155

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

ctrlcctrlv commented 3 years ago

@erco77 has a patch here: https://github.com/memononen/nanosvg/issues/136#issuecomment-761201063

I think it will work to solve this.

alerque commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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.