Skip to content

CmdPal: Icons (9/n) - Prevent stale recycled icons and reuse image presenters - #50189

Open
Jiří Polášek (jiripolasek) wants to merge 2 commits into
dev/jpolasek/f/49940-cmdpal-icon-protocol-app-iconsfrom
dev/jpolasek/f/49941-cmdpal-icon-presentation
Open

Jiří Polášek (jiripolasek) wants to merge 2 commits into
dev/jpolasek/f/49940-cmdpal-icon-protocol-app-iconsfrom
dev/jpolasek/f/49941-cmdpal-icon-presentation

Conversation

@jiripolasek

@jiripolasek Jiří Polášek (jiripolasek) commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Summary of the Pull Request

Part 9 of 12 in the CmdPal icon-loading series. Depends on #50188. Next: #50190.

This PR clears previous-item presentation immediately and reduces image-control churn.

  • Adds placement-owned fallbacks and resets resolved/request-specific fallback state when the source changes.
  • Alternates inactive Image slots in a stable Grid instead of rebuilding the image subtree.
  • Keeps app-image fallback policy separate from valid glyph/emoji hero rendering.

Motivation: a recycled row must not show another item's icon while loading, and repeated image updates should reuse presentation where WinUI permits it.

Evidence

Historical adjacent-stage comparison (2026-08-18); shared method and limitations: the diagnostics foundation PR #50181.

  • C cold-ish icon-element update average: 0.188 → 0.074 ms (−0.114 ms); all three pairs decreased.
  • C cold-ish elements created: 454 → 84; elements reused: 0 → 382.
  • C cold-ish cumulative measured icon UI time: 119.588 → 67.655 ms (51.933 ms less across an approximately 28-second search scenario).
  • C cold-ish applied average: 8.402 → 8.483 ms (+0.081 ms), inconsistent across pairs; no request-latency benefit is established.
  • Presentation-state tests cover reset and asynchronous fallback transitions. Final visual recycling still needs a UI smoke test.

The measurable benefit is less control construction and cumulative UI work, alongside the stale-icon correctness fix. A 0.114 ms update saving is not a claim that the app feels 61% faster.

Technical notes

  • Two lazily created image slots are an intentional workaround for observed blank/stale results when reusing one presenter. They are not a claim that all WinUI icon elements can safely be reused.
  • Request-specific fallback state must be reset together with the resolved source. Clearing only the displayed source can retain the previous item's fallback.
  • Recycled controls must not explicitly dispose shared cached image resources; those may still be used by other controls or deferred WinUI copies.
  • Ordinary glyphs and emojis remain valid hero sources. The shared bitmap placeholder is an app-icon policy, not a blanket ban on glyphs in details.

Implementation: src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/IconBox.cs.

PR Checklist

  • Communication: I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end-user-facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

🧭 PR intake

Visual evidence: Required — The pull request changes product UI files, so reviewers need to see the visible result. Visual evidence is currently missing.

Author action

Jiří Polášek (@jiripolasek), please update the following before review:

  • Add a screenshot, GIF, or video to the PR description so reviewers can validate the visible change.

See the contribution guide for the full checklist.

If there is no author response within 7 days, this PR will be automatically closed.

Automated PR intake; PowerToys maintainers make final decisions.

@jiripolasek Jiří Polášek (jiripolasek) added Ready for review and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Aug 28, 2026
@jiripolasek
Jiří Polášek (jiripolasek) force-pushed the dev/jpolasek/f/49941-cmdpal-icon-presentation branch from 3a11581 to a668536 Compare September 8, 2026 19:43
@github-actions github-actions Bot added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Ready for review labels Sep 8, 2026
@jiripolasek
Jiří Polášek (jiripolasek) force-pushed the dev/jpolasek/f/49941-cmdpal-icon-presentation branch from a668536 to ca84cbf Compare September 21, 2026 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Product-Command Palette Refers to the Command Palette utility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CmdPal: Icon loading (8/n) - Fix recycled icon presentation and reuse presenters

3 participants