pop-os / icon-theme

System76 Pop icon theme for Linux
Creative Commons Attribution Share Alike 4.0 International
203 stars 27 forks source link

SVG Format Icons #103

Closed isantop closed 2 years ago

isantop commented 2 years ago

Converts the icons from PNG to SVG format

SVG icons are smaller and scalable, so that we don't need to provide icons @2x (or @3x in the future) sizes. They are automatically pixel doubled (and scale sharply) to the required DPI. This allows us to support a theoretically infinite number of DPI scales, particularly integer scaling.

Additionally, if an Icon needs to be scaled up to fit a more arbitrary size, SVG should in theory provide a better experience because they can be scaled to any size. We do lose the crisp design of pixel-perfect integer scaling, but since they're scalable and not raster, it should be clearer than that.

QA: For testing, we need to make sure there are no missing icons and that all of the icons show up correctly in most apps. There aren't any expected issues in switching to SVG, but this should be independently verified.

isantop commented 2 years ago

When inspecting this change with gtk4-icon-browser, I found that in the View Controls category, the zoom-in, zoom-out, and zoom-original icons used to be themed:

Screenshot from 2022-02-28 14-02-36

After the change, they seem to be falling back to another theme:

Screenshot from 2022-02-28 14-03-17

This does not affect the symbolic version of those icons, just the normal version.

@jacobgkau Those are the Pop versions of the full-color icons. They were not rendering correctly prior to this PR.

jacobgkau commented 2 years ago

Spotted a regression on a HiDPI system; the regression is not happening on a LoDPI system. In the Archive Manager app, when creating a new archive, the Location: dropdown used to show these icons:

archive-old

Now, it shows folder icons instead:

archive-new

I'm almost inclined to believe the former are stock GNOME/Adwaita icons and the latter are Pop icons, but I'm not sure why the "correct" icons would only be showing up on HiDPI if that was the case.

isantop commented 2 years ago

@jacobgkau Both are Pop icons; the color versions are the 16x16 versions and the other versions are the larger 32x32+ versions.

It looks like the icon lookup was failing and falling back on the larger icons when it shouldn't have. I've setup a symlink system in the build that will allow the theme to fall back on the correct size icon files. While this means the files are technically half-size, because they're now SVG we can reliably scale them up 2x without them getting blurry, and the icon loader appears to do that successfully. The issue you saw seems to be resolved as of e59cf06

isantop commented 2 years ago

70a2ad1 should fix the broken symbolic icons. I would still like to re-render the whole theme, but the icons should at least be usable again

isantop commented 2 years ago

Re-rendering the theme resulted in no changes, so 70a2ad1 is ready.

13r0ck commented 2 years ago

Good call to use optimized svgs using the d value. :+1:

jacobgkau commented 2 years ago

On 70a2ad1, the Archive Manager folder icons are back to using folder icons on HiDPI:

Screenshot from 2022-03-16 09-23-07

LoDPI is still using the color versions:

Screenshot from 2022-03-16 09-24-51

isantop commented 2 years ago

@jacobgkau I think I figured out what was causing that. I've updated the packaging to ensure that that won't be an issue anymore. They should now use the full-color icons at both sizes for the small, 16x16 icons

jacobgkau commented 2 years ago

Installing this update on a Jammy system, I'm getting a lot of "directory not empty" warnings from dpkg:

Output: ``` Unpacking pop-icon-theme (3.0.0~1649180661~22.04~9fa7783) over (2.1.0~1637164842 ~22.04~9998b20) ... dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/8x8@2x/emble ms': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/64x64@2x/sta tus': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/64x64@2x/pla ces': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/64x64@2x/mim etypes': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/64x64@2x/emb lems': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/64x64@2x/dev ices': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/64x64@2x/cat egories': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/64x64@2x/app s': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/64x64@2x/act ions': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/512x512@2x/p laces': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/48x48@2x/sta tus': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/48x48@2x/pla ces': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/48x48@2x/mim etypes': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/48x48@2x/emb lems': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/48x48@2x/dev ices': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/48x48@2x/cat egories': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/48x48@2x/app s': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/48x48@2x/act ions': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/32x32@2x/sta tus': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/32x32@2x/pla ces': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/32x32@2x/mim etypes': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/32x32@2x/emb lems': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/32x32@2x/dev ices': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/32x32@2x/cat egories': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/32x32@2x/app s': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/32x32@2x/act ions': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/256x256@2x/s tatus': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/256x256@2x/p laces': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/256x256@2x/m imetypes': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/256x256@2x/d evices': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/256x256@2x/c ategories': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/256x256@2x/a pps': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/256x256@2x/a ctions': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/24x24@2x/sta tus': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/24x24@2x/pla ces': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/24x24@2x/mim etypes': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/24x24@2x/emb lems': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/24x24@2x/dev ices': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/24x24@2x/cat egories': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/24x24@2x/app s': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/24x24@2x/act ions': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/16x16@2x/sta tus': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/16x16@2x/pla ces': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/16x16@2x/mim etypes': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/16x16@2x/emb lems': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/16x16@2x/dev ices': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/16x16@2x/cat egories': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/16x16@2x/app s': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/16x16@2x/act ions': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/128x128@2x/s tatus': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/128x128@2x/p laces': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/128x128@2x/m imetypes': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/128x128@2x/d evices': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/128x128@2x/c ategories': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/128x128@2x/a pps': Directory not empty dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/128x128@2x/a ctions': Directory not empty Setting up pop-icon-theme (3.0.0~1649180661~22.04~9fa7783) ... ```

Compiling an older version to test, I don't get these warnings when installing 70a2ad1, only the latest commit (9fa7783). Additionally, the package actually failed to install when going from that older commit to the newer commit:

Output: ``` (Reading database ... 200964 files and directories currently installed.) Preparing to unpack .../pop-icon-theme_3.0.0~1649180661~22.04~9fa7783_all.deb ... rm: cannot remove '/usr/share/icons/Pop/8x8@2x': No such file or directory rm: cannot remove '/usr/share/icons/Pop/16x16@2x': No such file or directory rm: cannot remove '/usr/share/icons/Pop/24x24@2x': No such file or directory rm: cannot remove '/usr/share/icons/Pop/32x32@2x': No such file or directory rm: cannot remove '/usr/share/icons/Pop/48x48@2x': No such file or directory rm: cannot remove '/usr/share/icons/Pop/64x64@2x': No such file or directory rm: cannot remove '/usr/share/icons/Pop/128x128@2x': No such file or directory rm: cannot remove '/usr/share/icons/Pop/256x256@2x': No such file or directory rm: cannot remove '/usr/share/icons/Pop/512x512@2x': No such file or directory dpkg: error processing archive /var/cache/apt/archives/pop-icon-theme_3.0.0~1649180661~22.04~9fa7783_all.deb (--unpack): new pop-icon-theme package pre-installation script subprocess returned error exit status 1 Errors were encountered while processing: /var/cache/apt/archives/pop-icon-theme_3.0.0~1649180661~22.04~9fa7783_all.deb E: Sub-process /usr/bin/dpkg returned an error code (1) ```

Are either of these a problem?

isantop commented 2 years ago

@jacobgkau I believe the latest commit should have fixed the second issue. I think the first issue is also solved, however it shouldn't be a problem as long as the icons work correctly.

jacobgkau commented 2 years ago

7ac113c is still giving me the dpkg: warning: unable to delete old directory '/usr/share/icons/Pop/.../...': Directory not empty warnings when installing. As long as this doesn't invalidate my testing and doesn't mean files will look different on a fresh iso built with this update included, the warnings can be ignored.

Everything is looking good on a LoDPI system, but on a HiDPI system, launching gtk4-widget-factory fails with:

system76@pop-os:~$ gtk4-widget-factory 

(gtk4-widget-factory:7686): Gdk-ERROR **: 16:09:55.507: Resource path /org/pop-os/themes/Pop/4.0/assets/check@2-symbolic.symbolic.png is not a valid image: The resource at “/org/pop-os/themes/Pop/4.0/assets/check@2-symbolic.symbolic.png” does not exist
Trace/breakpoint trap (core dumped)
isantop commented 2 years ago

@jacobgkau

(gtk4-widget-factory:7686): Gdk-ERROR **: 16:09:55.507: Resource path /org/pop-os/themes/Pop/4.0/assets/check@2-symbolic.symbolic.png is not a valid image: The resource at “/org/pop-os/themes/Pop/4.0/assets/check@2-symbolic.symbolic.png” does not exist Trace/breakpoint trap (core dumped)

This is a GTK Theme Gresource error, not a theme problem

13r0ck commented 2 years ago

Screenshot from 2022-04-12 11-26-08

isantop commented 2 years ago

@13r0ck Files are exported from the source document using Inkscape actions, not by exporting manually from within the Inkscape GUI. I may be mistaken here, but I'm not sure there's a way to export SVGs with those options without manually exporting every possible icon file at every size (currently the count is over 2700 files).

13r0ck commented 2 years ago

@isantop inkscape I believe just uses https://github.com/scour-project/scour, can the export script just pass the icons thorough that after exporting from inkscape?

scour -i input.svg -o output.svg --enable-viewboxing --enable-id-stripping \
  --enable-comment-stripping --shorten-ids --indent=none