Let the configured icon theme decide which icon the launcher draws - #9412
Closed
VykosMolt wants to merge 1 commit into
Closed
Let the configured icon theme decide which icon the launcher draws#9412VykosMolt wants to merge 1 commit into
VykosMolt wants to merge 1 commit into
Conversation
`indexIconLine()` keeps the first path it sees for a name, so the order `iconIndexScanCommand()` emits paths in is what decides which theme the launcher ends up drawing. The scan walked every icon root in one pass, which left that decision to readdir: `~/.local/share/icons` is searched before `/usr/share/icons`, so on a machine with several themes installed there an app's icon came from whichever theme was reached first, and could change between rescans. The theme in `org.gnome.desktop.interface icon-theme`, which `omarchy-theme-set-gnome` writes from the theme's `icons.theme`, was never consulted. The scan now emits the configured theme first, then hicolor, where an application installs its own icon, and only then everything else, which is what finds an icon no theme covers. Within a theme the paths are ranked so scalable comes first and larger fixed sizes come before smaller ones, because themes carry less detail in their small buckets and a row rendered large could otherwise be handed a 24x24 icon. The theme passes use `find -L`. Icon themes are built out of symlinked size directories -- Papirus's `128x128/apps` is a symlink to `../64x64/apps` -- and find does not descend those without it, so a large chunk of the configured theme was invisible to the scan even when it was reached. The catch-all pass stays without `-L`: following every symlink under every icon root is much slower, and that pass only exists as a fallback. Ordering is done in the scan rather than in the parser so the parser keeps its single rule, that the first path for a name wins. Sorting in the parser would mean holding every path for every name to compare them, for a list that is hundreds of thousands of lines on a machine with a few themes installed.
Author
|
Closing this as superseded by #9408, which fixes the same defect (#7552) in the same function. This PR orders the scan configured theme -> hicolor -> everything else. That is wrong: it lets hicolor outrank a theme the configured one actually inherits from, so an icon a parent theme carries is beaten by hicolor's copy. #9408 walks the inheritance chain properly instead: depth-first through Sorry for the duplicate - two open PRs against one issue was my mistake. #9408 is the one to review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
indexIconLine()keeps the first path it sees for a name, so the ordericonIndexScanCommand()emits paths in is what decides which theme the launcher draws. The scan walked every icon root in one pass, which left that to readdir:$HOME/.local/share/iconsis searched before/usr/share/icons, so on a machine with a few themes installed there an app's icon came from whichever theme was reached first, and could change between rescans. The theme inorg.gnome.desktop.interface icon-theme, whichomarchy-theme-set-gnomewrites from the theme'sicons.theme, was never read.The scan now emits the configured theme first, then hicolor, where an application installs its own icon, and only then everything else, which is what finds an icon no theme covers. Within a theme the paths are ranked so scalable comes first and larger fixed sizes come before smaller ones, because themes carry less detail in their small buckets and a row rendered large could otherwise be handed a 24x24 icon.
The theme passes use
find -L. Icon themes are built out of symlinked size directories — Papirus's128x128/appsis a symlink to../64x64/apps— and find does not descend those without it, so a large part of the configured theme was invisible to the scan even once it was reached. The catch-all pass stays without-L: following every symlink under every icon root is much slower, and that pass only exists as a fallback.The ordering is done in the scan rather than in the parser so the parser keeps its single rule, that the first path for a name wins. Sorting in the parser would mean holding every path for every name to compare them, on a list that runs to hundreds of thousands of lines with a few themes installed.
Testing
icon-index-theme-test.shpulls the command out ofAppLibrary.qmland runs it against a tree holding the same icon name in three themes, withgsettingsstubbed: the configured theme wins over a theme that is merely installed, an icon no chosen theme carries is still found, an application's own hicolor icon is still found, a symlinked size directory inside a theme is searched, the larger artwork wins inside a theme, and device icons are still indexed. The first assertion fails on current quattro, resolving to the unselected theme.Measured on this machine with
icon-themeset toPapirusand several other themes under~/.local/share/icons, resolving the 152 non-absoluteIcon=names from the installed desktop entries through the same first-hit-wins rule:The 26 that still come from elsewhere are correct: 14 are applications shipping their own hicolor icon, and the rest are names Papirus does not carry.
./test/shell— 218 of 222 files pass; the four failures (config-test,runtime-smoke-test,snapper-test,unowned-system-paths-test) fail identically on unmodified quattro on this machine.This comes out of https://github.com/VykosMolt/omarchy-desktop, where I have been running the Quattro shell as a plain Arch session and fixing what turned up. A couple of the other things I found there are worth sending separately, so more to follow.
Fixes #7552