Follow the icon theme inheritance chain when building the app icon index - #9408
Follow the icon theme inheritance chain when building the app icon index#9408VykosMolt wants to merge 1 commit into
Conversation
AppLibrary's icon index walked every theme under the XDG icon directories and kept whichever basename `find` reached first. iconSource() consults that index before the themed lookup, so filesystem traversal order decided which theme an application icon came from, and `find` guarantees no order — the same name can resolve to a different theme on the next scan, and refreshIcons() runs whenever a consumer opens. With icon-theme set to Papirus and several other themes installed under ~/.local/share/icons, 2 of 150 desktop-entry Icon= names resolved into Papirus; of the 120 names Papirus ships, 118 came from a theme that was not selected. Scan the icon theme search order instead: the active theme, then each theme it inherits depth-first in declared order, then hicolor. That is the order the icon theme specification defines — a parent's own inheritance is searched before the next declared parent — so a derived theme resolves through its ancestors rather than dropping straight to hicolor. Tela-circle-dark, for instance, reaches AdwaitaLegacy through Adwaita before it falls back to breeze. A theme's icons may be spread across base directories, but only the first index.theme found defines it, so the walk reads that one and stops. hicolor is appended rather than followed through Inherits because it is the fallback of last resort and themes do list it early: Tela-circle-dark ships "Inherits=hicolor,Adwaita,breeze", which would otherwise search hicolor before either real parent. Themes that inherit each other cannot loop the walk. A theme is also exhausted before the next one is consulted, SVGs first and then PNGs within it. Emitting every SVG in the chain before any PNG would let a scalable icon further down the search order outrank the configured theme's raster one, which is the same defect one level along. The index keeps its apps/devices context limit, so it continues to guard generic names like "zoom" against resolving to an action icon, and it keeps its precedence over the themed lookup. Each pass is reverse-sorted so the result no longer depends on traversal order and "scalable" wins over the numeric size directories; symbolic icons are dropped because that order would otherwise prefer a monochrome glyph. The scan also gets cheaper: 52,556 lines instead of 104,959 here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
712ed50 to
2d4e722
Compare
|
Force-pushed one correctness fix, no change in scope. The scan had New assertion, which fails against the previous revision of this branch: Thirteen assertions now. Output on my machine is unchanged — 52,556 lines, same 119/14/13/3/1 split across Papirus, hicolor, the themed lookup, breeze and pixmaps — because nothing in that chain has a format collision for those names. The fixture is what pins the rule. I also prototyped resolving each theme's directories from its own |
AppLibrary's icon index walks every theme under the XDG icon directories and keeps whichever basenamefindreaches first.iconSource()consults that index before the themed lookup, so filesystem traversal order decides which theme an application icon comes from — andfindguarantees no order, so the same name can resolve differently on the next scan.With
icon-themeset toPapirusand several other themes installed under~/.local/share/icons, 2 of 150 desktop-entryIcon=names resolved into Papirus. Of the 120 names Papirus ships, 118 were served from a theme that was not selected.Why not just swap the two lookups. The index-first order is deliberate:
app-search-test.shasserts it, andQuickshell.iconPathreturns animage://URL, so there is no way to see what context it resolved into and reject anactions/hit after the fact. Swapping brings back thezoom-style collisions that ordering prevents. My first attempt at this instead scanned the active theme plushicolorand left inheritance to Qt — that is also wrong, because the index still wins, so a derived theme's icons lose tohicolorbefore Qt is ever asked. The index itself has to follow the chain.What changed. The scan walks the icon theme search order: the active theme, then each theme it inherits depth-first in declared order, then
hicolor. Depth-first is what the spec defines — a parent's own inheritance is searched before the next declared parent. A theme's icons may be spread across base directories, but only the firstindex.themefound defines it, so the walk reads that one and stops.hicoloris appended rather than followed throughInherits, because it is the fallback of last resort and themes do list it early:Tela-circle-darkshipsInherits=hicolor,Adwaita,breeze, which would otherwise searchhicolorahead of both real parents. Themes that inherit each other cannot loop the walk.The apps/devices context limit and the index's precedence over the themed lookup are both unchanged. Each pass is reverse-sorted so the result no longer depends on traversal order and
scalablewins over the numeric size directories; symbolic icons are excluded because that ordering would otherwise prefer a monochrome glyph.Resolved chains on my machine:
Resolution goes from 2/150 into the active theme to 119, with 14 from
hicolor, 3 frombreeze(Papirus's declared parent), 13 left toQuickshell.iconPath, 1 pixmap. The scan also gets cheaper: 52,556 lines instead of 104,959, 0.55s instead of 0.85s, and byte-identical across runs.Testing
app-search-test.shruns the command the QML actually ships against a fixture theme tree with a stubbedgsettings, and checks resolution precedence rather than the shape of the script: active theme over the themes it inherits, an inherited theme overhicolor, a parent's own inheritance before the next declared parent,hicolorlast even when a theme inherits it first, inheritance taken from the firstindex.themein base order and not from a shadowed copy,hicolorstill covering icons an app ships there, a name only an uninherited theme provides left to the themed lookup,scalableover fixed sizes, symbolic never chosen, an inheritance cycle terminating, and identical output across runs.Each of those catches a distinct defect. Removing any one of the three ordering rules fails exactly one assertion:
./test/shell— 217 of 221 files pass; the four failures (config-test,runtime-smoke-test,snapper-test,unowned-system-paths-test) fail identically on unmodified quattro on this machine.shellcheckreports nothing on the generated command and only the usualSC1091on the test file.The scan reads the theme name with
gsettings, the same settingomarchy-theme-set-gnomewrites. If that call fails the index falls back tohicoloronly, rather than to a full-tree walk.Fixes #7552