sammycage / lunasvg

LunaSVG is a lightweight library for efficient rendering and manipulation of SVG files.
MIT License
857 stars 124 forks source link

Some gradients don't render correctly #108

Closed leonstyhre closed 1 year ago

leonstyhre commented 1 year ago

Hi!

Some gradients don't render correctly in LunaSVG, please see these images for an example:

https://es-de.org/temp/LunaSVG_gradients.png https://es-de.org/temp/NanoSVG_gradients.png

Here's an example of a problematic file: https://gitlab.com/es-de/emulationstation-de/-/blob/master/themes/slate-DE/n3ds/images/consolegame.svg

Interestingly it doesn't render correctly in the GitLab SVG renderer either but it does render correctly in both NanoSVG and Inkscape.

sammycage commented 1 year ago

Nanosvg and Inkscape do not fully support gradient reference. Learn more here https://svgwg.org/svg2-draft/pservers.html#PaintServerTemplates

leonstyhre commented 1 year ago

Problem is Inkscape is an incredibly popular application used to create vector graphics so not being able to render its files correctly in my application is a showstopper. There are also hundreds of EmulationStation themes created over the past ten years or so and not supporting gradients in the same manner as in NanoSVG breaks backward compatibility with an unknown number of these.

As well it's not very helpful that you close my issues without any discussion or giving me a chance to comment, the same is true also for https://github.com/sammycage/lunasvg/issues/104 which is very much a real problem that needs to be resolved before I'll consider switching to LunaSVG. When running at low resolution the edge artifacts are very pronounced and distracting for a lot of the graphic files.

sammycage commented 1 year ago

As well it's not very helpful that you close my issues without any discussion or giving me a chance to comment.

My bad

sammycage commented 1 year ago

I'm not sure about Inkscape. The main problem is that gradientTransform is ambiguous. Is the file created by hand? You can fix the problem by simply removing gradientTransform on the affected elements.

leonstyhre commented 1 year ago

Thanks for opening this issue again!

Yes I made this file myself using Inkscape, it's done from scratch so no conversion from raster image or similar has been done. My concern is that I did nothing strange with this file and it triggers no warning in Inkscape and as mentioned it also rendered correctly in NanoSVG. Even if Inkscape would be slightly non-conformant (although I'm not sure if that's what you say?) it's still a fact that it's a very popular application and that a huge amount of images have been created with it during a number of years. As such there are likely many, many files with similar gradient issues if rendered with LunaSVG and that would introduce quite severe regressions in my application. As mentioned there are hundreds of themes available for EmulationStation, many of which are not really maintained any longer so there is nobody that would even take responsibility for updating all the SVG files to fix any compatibility issues with LunaSVG.

As a side comment, replacing the SVG rendering library is a very important change for my application so I have done thorough comparison with NanoSVG. After your changes for the rlottie conflict it now works correctly in Linux, BSD Unix, macOS and Windows and it compiles without any warnings with Clang, GCC and MSVC. It has more accurate rasterization than NanoSVG, it supports more SVG files and it has better performance and a much better API. Overall it's a great library as far as I can tell and I would really like to use it, but the regressions have to be fixed so that it will always render at least as well as NanoSVG in all instances. I think it's very close, there are only a few issues left and I really hope that you'll be able to fix them. Thanks in advance!

sammycage commented 1 year ago

I have tested this file in Firefox and Chrome. They produce the same results as lunasvg. Once NanoSVG implements this, the results will all be the same.

leonstyhre commented 1 year ago

Thanks for testing this, I may then have to accept this regression. But it's very annoying that a popular application like Inkscape generate non-standard SVG files. Let me do some investigations on how such files could be modified in Inkscape to render correctly in LunaSVG and I'll also check a lot of old themes to see how many files are affected.

leonstyhre commented 1 year ago

I've done some more investigations about this now and I have managed to find a fix for the issue for the problematic image files. By moving the shapes that didn't render correctly (i.e. missing gradients) ever so slightly to the left or right they now render correctly with LunaSVG as well as in Firefox and Chromium. Although I'm for sure not an expert in SVG rasterization this really does look like a bug to me, which would then be present also in Firefox and Chromium as they also don't render the files correctly unless they're modified. I mean, just moving a shape slightly should not disable or enable gradients?

I tried different things like changing grouping, saving in plain SVG instead of Inkscape SVG format etc. but nothing helps except moving the shapes slightly. I've now merged the updated images to my master branch, but here are three files from an older branch that have this problem and that I've now worked around using the method just described.

Old versions (these don't render correctly): https://gitlab.com/es-de/emulationstation-de/-/blob/stable-1.2/themes/rbsimple-DE/n3ds/images/consolegame.svg https://gitlab.com/es-de/emulationstation-de/-/blob/stable-1.2/themes/rbsimple-DE/saturnjp/images/consolegame.svg https://gitlab.com/es-de/emulationstation-de/-/blob/stable-1.2/themes/rbsimple-DE/xbox360/images/consolegame.svg

New versions (fixed by moving the shapes slightly): https://gitlab.com/es-de/emulationstation-de/-/blob/master/themes/slate-DE/n3ds/images/consolegame.svg https://gitlab.com/es-de/emulationstation-de/-/blob/master/themes/slate-DE/saturnjp/images/consolegame.svg https://gitlab.com/es-de/emulationstation-de/-/blob/master/themes/slate-DE/xbox360/images/consolegame.svg

sammycage commented 1 year ago

Although I'm for sure not an expert in SVG rasterization this really does look like a bug to me, which would then be present also in Firefox and Chromium as they also don't render the files correctly unless they're modified.

Inkspace is buggy

Nanosvg and Inkscape do not fully support gradient reference. Learn more here https://svgwg.org/svg2-draft/pservers.html#PaintServerTemplates

Links to support my claims

https://gitlab.com/inkscape/inkscape/-/blob/master/src/object/sp-gradient.cpp#L457 https://gitlab.com/inkscape/inkscape/-/issues/1061 https://github.com/memononen/nanosvg/blob/64d59e4d53308104181c6a7ad71fbf423f71910f/src/nanosvg.h#L831

leonstyhre commented 1 year ago

Maybe, but regardless it doesn't matter much as I've now found a way to work around such issues.

Today I also merged LunaSVG into the master branch of ES-DE so it's officially part of the project now! This is really large improvement over NanoSVG as many files that didn't work previously are now rendered correctly and performance and rendering accuracy is much improved. Thanks a lot for fixing the bugs I've reported, I'm looking forward to seeing what improvements you'll make to LunaSVG in the future. I'll make sure to incorporate your updates every now and then. I also added a link to your project in my CREDITS.md file and if you would be interested in including my application in your list of projects using LunaSVG then please do so :)

It will be a while before a stable release of ES-DE 2.0.0 is out that uses LunaSVG but I release somehow regular alpha releases and then later on there will be beta releases etc.

Anyway, you can go ahead and close this issue as I guess there is not much more that can/will be done about it.

sammycage commented 1 year ago

Thanks for helping to improve this library <3 <3 <3