numixproject / numix-core

Builder for App Icon Themes
GNU General Public License v3.0
765 stars 146 forks source link

Aviator icon breaks apart in nomacs viewer #5816

Open mrmeszaros opened 1 year ago

mrmeszaros commented 1 year ago

The aviator icon looks fine in most SVG viewers, but breaks apart in Nomacs.

Looked into it, and the issue is related to white-spacing of some arc commands like: a4 4 0 00-4 4 4 4 0 004 4 Most viewers recognize the 004 as two flags and the x offset, but nomacs does not. It is a reported rendering bug, not yet fixed.

Many icons I submitted were viewed with nomacs, so they have these superfluous white-spaces, but I see many do not have.

What is the numix standpoint towards this?

Screenshots

nomacs-rendering-bug

palob commented 1 year ago

I can't reproduce this with the Nomacs (3.17.2206-8) and qt5-svg (5.15.7+kde+r9-1) versions currently present in Arch. Neither with Gwenview as an alternative Qt viewer.

mrmeszaros commented 1 year ago

I tested it in Nomacs (3.12.0+dfsg-3) libqt5svg5 (5.12.8-0ubuntu1) currently present in Ubuntu 20.04 (GNOME desktop). I think Nomacs uses QtSVG for rendering the SVG. So between 5.12 and 5.15 this parsing/rendering issue must have been solved. Though I hadn't found it yet it the Qt issue tracker.

I'll check whether 22.04 has the newer version of Qt5.

palob commented 1 year ago

librsvg had a similar issue it appears https://github.com/numixproject/numix-icon-theme-circle/issues/151

mrmeszaros commented 1 year ago

I just tested this in a newly created Ubuntu 22.04 virtual instance. It it still an issue with the available Nomacs (3.12.0+dfsg-3build3) and libqt5svg5 (5.15.3-1).

After more investigation, I found a Qt bug report (QTBUG-92184). It is marked to be fixed in 5.15.6.

palob commented 1 year ago

Is this issue caused by a behaviour change of Inkscape/Scour or is it also visible in older icons? Can't remember I've changed my optimise options

mrmeszaros commented 1 year ago

There is a checkbox in Inkscape (Save as... Optimized SVG) that says "Work around renderer bugs", and that leaves in the spaces needed to render correctly.

I combed through all icons from A to C (cytus-2). There were 6 where the symbol completely broke:

Moreover, there are a solid few, where the baseplate dropshadow wonks out:

Did not check yet when these have been added, but it seems that this is not a singleton issue.

palob commented 1 year ago

So since the Qt bug is in Ubuntu LTS this might be worth fixing. Scour (which is what Inkscape uses for optimisation (and which looks unmaintained)) has the CLI option --renderer-workaround so this should be fixable batch-wise.

palob commented 1 year ago

Looks like Scour has no batch functionality so for instance this batch snippet is needed

find . -name "*.svg" -exec scour -i {} -o {}.svg --renderer-workaround  \;
palob commented 1 year ago

Well, this does Scours standard cleaning routine + --renderer-workaround ,need to find how to run only the latter.

palob commented 1 year ago

Even if I specify the options the options according to our guidelines this will still touch older icons which were saved in a different way (which do not need a renderer fix). So this may cause unneeded side effects and many icon may need to be checked visually. Is there a way to tell the "broken" icons script-wise?

mrmeszaros commented 1 year ago

I think there is. Since QtSvg breaks when rendering arcs with "missing" flag separators, I made a script that finds all such occurrances: https://gist.github.com/mrmeszaros/6384ae6769ce17e07bc9412cb8464244

The start of it's output matches with what I found manually, so I would extrapolate that it is correct for the rest.

I think it should be easy to change the script to fix the files.

palob commented 1 year ago

Thank you, will try this. Another option would be to set a certain date of last edit for the files to be rewritten.

mrmeszaros commented 1 year ago

@palob I thought about this, and I am mainly concerned with consistency:

Since (apart from me) no one is yet affected (and even I am only partially: when using Nomacs), so we might be fixing something that ain't broken. And for that I am mainly concerned with size-optimization, consistency between icons, and what baseplate to use for future (one of the baseplate shadows has arcs in them that are affected).

palob commented 1 year ago

If the buggy Qt version is in Ubuntu 22.04 LTS this I suppose might affect users of Kubuntu (KDE) and Lubuntu (LXQt) who want to use the Numix themes in a way the icons look broken not only when viewed in an image viewer but in any place.

palob commented 1 year ago

I find 172 affected files for Circle (which means I must have added 3 new ones 😬) and 61 for Square. So the standard is still working around renderer bugs and I think that's a manageable number of icons to check visually (maybe icons which have not yet our current optimisation options applied can break) .

mrmeszaros commented 1 year ago

Okay! So You want to take this, or may I do it?

Regarding the new icons, inkscape's optimized svg has the option to work around renderer bugs (such as this). Additionally, we may extend the CI to check for this renderer bug. That might be a good safeguard.

palob commented 1 year ago

I might not be able to do this in the following days, for some weeks now my laptop appears to be about to give up the ghost. During boot or shortly after I'm presented with spectacular arrangements of colours most of the time. After several briefly successful attempts to fix this I'm now sure it's a hardware issue. The lucid moments are getting rare.

palob commented 1 year ago

Got it running again with the GPU deactivated (everything fairly sluggish now). Now as a last step before doing this maybe we should check with a Lubuntu/Kubuntu 22.04 iso whether this arc issue actually has any negative effects?

mrmeszaros commented 1 year ago

Yeah, good idea, I'll do a vanilla Kubuntu + Lubuntu install (from ISO I guess? Virtualbox or maybe with some Libvirt/KVM/Qemu?). And check whether the icons show correctly. Hopefully this week.

palob commented 1 year ago

Yeah, thanks for doing that so I don't have to fiddle kernel parameters into the isos to have the GPU deactivated pre-boot.