search

jnsh / arc-theme

A flat theme with transparent elements (actively maintained fork)
GNU General Public License v3.0
900 stars 77 forks source link

Use SVG instead of PNG assets for GTK3 #132

Closed tomKPZ closed 3 years ago

tomKPZ commented 3 years ago

This achieves several results:

jnsh commented 3 years ago

Thank you! This certainly seems like a good idea, and in addition to the benefits you listed should improve build times.

The question is why the upstream default GTK theme isn't using SVG assets as well, so I'm a bit concerned there could be some non-obvious caveat from this. One possible reason I found was from this commit message, suggesting that using SVG assets would make librsvg a hard dependency. However, I don't think that should be an issue with FOSS distributions, and therefore for Arc. Have you investigated this yourself by any chance?

Also, have you tested that this works with the older GTK3 versions? There's a note on the GTK3 NEWS file suggesting that loading SVG images from CSS might only be possible after 3.20, but the wording is a bit vague and I couldn't find the actual commit, so I'm not sure if that matters for this PR. It would be nice to apply this for every GTK3 theme version to avoid extra version checks on the build file, if there are no issues with older versions.

Either way, since this is not something the upstream does, it must be very thoroughly tested.

The changes look mostly good at quick glance, although I didn't review everything thoroughly. Only thing I noticed so far is the --export-svg= option for inkscape <1.0. I don't see that option in inkscape 0.92 docs. There is --export-plain-svg though, which I think should work here.

Also please prepend the first line of the commit message with gtk3: for cleaner commit logs, and add full URL to this PR at the end of the commit message for future reference.

Let me know if you've looked at any of my concerns mentioned above. Generally I'm in favor of merging this, but I need to be sure it won't cause any problems.

tomKPZ commented 3 years ago

Sorry for just noticing this. The assets appear to be slightly blurry when rendered as SVG, so this is likely why upstream does not do this. This is probably a deal breaker, so I'll just close this out.

jnsh commented 3 years ago

That might be the case. PNG assets are definitely more reliable in terms of rendering, as they are always "pixel perfect". Indeed, decrease in visual quality is not a worthy trade off for the non-visible benefits from this.

Thank you for your effort nevertheless :)