fix: Icons not refreshing after changing icon preferences - #6738
Conversation
📝 WalkthroughWalkthroughThis PR changes LawnchairIconProvider to compute ChangesIcon state tracking
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hello, is this PR finish? Looking forward to merge this when it's ready. |
wellorbetter
left a comment
wellorbetter
left a comment
There was a problem hiding this comment.
Root cause: IconProvider.getStateForApp() — used as the DB freshnessId — only encodes system state and APK path.Lawnchair's own icon-affecting preferences (icon pack, themed icons, force monochrome, etc.) are not part of the key. Once a bitmap is written to app_icons.db, any subsequent preference change produces the same freshnessId, so IconCacheUpdateHandler always considers entries fresh and never re-invokes getIcon().
Fix: Override getStateForApp() in LawnchairIconProvider to append a fingerprint of all Lawnchair prefs that influence icon rendering. A preference change now produces a different freshnessId → AOSP's existing per-entry freshness check invalidates stale rows → icons re-render on demand.
Hey, yes it's ready — no further changes planned. Feel free to merge when convenient. Sorry for the slow response, been busy with work lately. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lawnchair/src/app/lawnchair/icons/LawnchairIconProvider.kt`:
- Around line 185-194: The cached boolean themedIconsEnabled is stale when prefs
change; replace the frozen field with a dynamic property so reads always call
prefs.themedIcons.get() (e.g., change the private val themedIconsEnabled =
prefs.themedIcons.get() to a getter-style property), ensuring getIcon and the
themeMap getter observe preference changes the same way getStateForApp does;
update only the themedIconsEnabled declaration so all uses (getIcon, themeMap,
etc.) read the current prefs.themedIcons.get().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0333212e-b9e3-4901-8adb-8f3cf7692be1
📒 Files selected for processing (1)
lawnchair/src/app/lawnchair/icons/LawnchairIconProvider.kt
No worries. I'll approve and merge your PR now. |
validcube
changed the title
2 participants
Description
Changing any icon-related preference (Icon Pack, Themed icon source, Themed icons scope, Force monochrome, etc.) fails to refresh drawer/workspace icons. Once
app_icons.dbentries are generated, no preference change invalidates them — icons stay stale untilpm clearorversionCodebumps.Root cause:
IconProvider.getStateForApp()(used as the DBfreshnessId) only encodes system state andappInfo.sourceDir. Lawnchair's own preferences that drive icon rendering are not included, soIconCacheUpdateHandleralways considers cached entries up-to-date.Fix: override
getStateForApp()inLawnchairIconProviderto append a fingerprint of all Lawnchair prefs that affect rendering.Fixes #6725
Fixes #6731
Reproduction (100% reliable after
pm clear)BitmapInfoto DB)Why it's deterministic
PreferenceManager.reloadIconsonly callsiconCache.clearMemoryCache(). The DB is untouched. The momentmodel.reloadIfActive()triggers LoaderTask,IconCacheUpdateHandlercompares:dbRow.freshnessId == iconProvider.getStateForApp(app)Since
getStateForApp()never sees Lawnchair prefs, both sides stay equal across steps 5→7→8, entries are considered fresh, andgetIcon()is never re-invoked. The poisoned bitmap is reloaded from DB into memory — "refresh" is a no-op.Why extending
getStateForAppover clearing DBgetStateForApp.One-time cost: existing users' DB entries all lack the
|lc:suffix, so the first post-upgrade LoaderTask re-generates every icon once. Steady-state afterward is per-app on-demand.Testing
adb shell pm clear app.lawnchairthen repro steps 1–7 aboveType of change
✅ Bug fix (A non-breaking change that fixes an issue)
❌ New feature (A non-breaking change that adds functionality)
❌ Breaking change (A fix or feature that would cause existing functionality to not work as expected)
❌ Refactor (A code change that neither fixes a bug nor adds a feature)
❌ Performance (A code change that improves performance)
❌ Style (Code style changes)
❌ Docs (Changes to documentation)
❌ Chore (Changes to the build process or other tooling)
Summary by CodeRabbit