Switch SVG loading from the Batik library to JSVG

[eirikbakke]

Per our discussion in #7463 (comment) and #7938 , here is a PR that switches SVG loading routine in ImageUtilities from the Batik library to the much more lightweight JSVG library.

It's a draft that needs more testing, so I have added the do-not-merge label for now. But I'm leaving the work here since it might benefit #7938.

[matthiasblaesing]

For testing the rendering you render render all svgs both with batik and jsvg and diff the two.

[eirikbakke]

There's a lot of bugs that can happen in the full application context, rather than in an isolated script, so I prefer to test the full app. (Dimensions, hints, HiDPI scaling, ImageProducer/ImageObserver/ColorModel stuff, caching etc.)

[matthiasblaesing]

There's a lot of bugs that can happen in the full application context, rather than in an isolated script, so I prefer to test the full app. (Dimensions, hints, HiDPI scaling, ImageProducer/ImageObserver/ColorModel stuff, caching etc.)

Sure, it was an idea and could catch the most obvious problem.

[eirikbakke]

Sorry for all the force-pushes... I was fidgeting to get the "paperwork" test to pass.

[mbien]

Sorry for all the force-pushes... I was fidgeting to get the "paperwork" test to pass.

no worries, thats what CI is for.

same comparison as in #7463 (comment)

batik is on the left, jsvg on the right (I think). I can't tell the difference - so its either not working or it works perfectly ;)

[matthiasblaesing]

First impression is good. I rebuild my "work" NetBeans with this applied and did not notice a difference.

[eirikbakke]

Yes, looks rather identical. Just to verify that the JSVG library is actually being used, I inserted a drawLine right after the call to com.github.weisj.jsvg.SVGDocument.renderWithPlatform. Confirmed:

Note that the PR does not actually remove the platform/libs.batik.read module, just the dependency on it from platform/openide.util.ui.svg . Removing the batik library entirely could be done separately if desired.

[eirikbakke]

I do see some cases where the icons for Undo, Redo, and Quick Search appear with the right shape, but a black fill, even though other icons look fine. Then in another case they looked OK. Will require some more investigation.

[eirikbakke]

OK, the "black shapes" problem was only with some older SVG files that we no longer use. They were generated using a now-deprecated setting in Adobe Illustrator.

(Full details: The problem occurred only on an my private build of NetBeans, which had some older versions of the SVG files that were generated with other Adobe Illustrator settings. All of those SVG files were regenerated in #7463, and replaced with versions that appear to work with JSVG. The README file merged in apache/netbeans-tools#67 lists the correct settings to use when generating SVGs from Adobe Illustrator. I filed an issue at weisJ/jsvg#96 to document the all-black icon issue, but it's not actually a problem for us as SVG files are now generated in a different way, and the setting that caused problems is now deprecated in Adobe Illustrator anyway.)

[eirikbakke]

I have now tested the patch on a build of master, in my working IDE (which is a private build based on NetBeans 22), and in my NetBeans Platform application (which has a bunch of additional icons beyond those in the IDE). The rendering seems to be entirely equivalent to Batik for our particular SVG files.

I think the PR is ready for review now, so I remove the do-not-merge label.

[matthiasblaesing]

Minimal nitpick, apart from these this looks good to me. Thank you.

I think it would be good to create an issue with target NB25 to remove batik library. It makes no sense to ship a library we don't use, on the other hand I think it would be good to give this some time in master before removing batik.

[eirikbakke]

I think it would be good to create an issue with target NB25 to remove batik library.

Here, I created it: #7969 (Also happy to create the PR for it once this current PR is merged.)

[matthiasblaesing]

Looks sane. Thanks again.

[eirikbakke]

Thanks for reviewing! Squashing the changes into a single commit before merging.