Visitar URL original
fix(imaging): Don't cache IgnoreImageCache loads by mikernet · Pull Request #25100 · unoplatform/uno · GitHub
Skip to content

fix(imaging): Don't cache IgnoreImageCache loads - #25100

Merged
MartinZikmund merged 1 commit into
unoplatform:masterfrom
mikernet:dev/mikernet/ignore-image-cache-25098
Oct 10, 2026
Merged

fix(imaging): Don't cache IgnoreImageCache loads#25100
MartinZikmund merged 1 commit into
unoplatform:masterfrom
mikernet:dev/mikernet/ignore-image-cache-25098

Conversation

@mikernet

@mikernet mikernet commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

GitHub Issue: closes #25098

PR Type:

🐞 Bugfix

What changed? 🚀

On Skia, a BitmapImage with CreateOptions="IgnoreImageCache" skipped the lookup in the static _bitmapImageCache but still added its load task to it, so the decoded image was retained by the cache exactly as if the option were not set. The cache is bounded by entry count, not bytes, so a viewer that shows a few dozen full-size images pins hundreds of megabytes that no element owns; on WebAssembly that reaches the linear memory limit and terminates the runtime. IgnoreImageCache is the documented way to keep such images out of the cache and had no effect.

The add is now gated on the option as well as FeatureConfiguration.Image.EnableBitmapImageCache. That also skips the cancellation registration that removed the entry when the load was cancelled, which would otherwise evict an entry other consumers share for the same key when an ignoring load is cancelled. An ignoring load therefore neither reads from, writes to, nor replaces anything in the cache, and its decoded image lives only as long as the BitmapImage that produced it.

This cache is Uno-specific (WinUI caches in its native imaging pipeline, not on BitmapImage), so there is no WinUI source to port.

Tests

New Given_BitmapImage.When_IgnoreImageCache_Then_Not_Added_To_Cache runtime test, using an internal hook that resolves the URI the same way the loader does and returns the cached task for a key. It loads an ms-appx asset with distinct decode widths as keys and checks that an ignoring load is not cached, a normal load is, and a later ignoring load for the same key does not replace the existing entry. It fails on master (the ignoring load is found in the cache) and passes with this PR.

Validation on Skia desktop: Given_BitmapImage, Given_BitmapSource, Given_Image, Given_ImageSource and Given_ImageBrush all pass (60 tests, 5 pre-existing platform skips).

PR Checklist ✅

Copilot AI balanced review requested due to automatic review settings October 8, 2026 04:55
Copilot AI reviewed Oct 8, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The regression test can become inconclusive instead of validating the fix.

1 open finding
What changed in this PR

Prevents Skia BitmapImage loads using IgnoreImageCache from entering the static cache.

Changes:

  • Gates cache insertion on IgnoreImageCache.
  • Adds runtime coverage for cache insertion and replacement behavior.
File Description
BitmapImage.cs Skips cache insertion for ignored loads.
Given_BitmapImage.cs Adds regression coverage.

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread src/Uno.UI.RuntimeTests/Microsoft/UI/Xaml/Media/Imaging/Given_BitmapImage.cs Resolved
@mikernet mikernet mentioned this pull request Oct 8, 2026
Merged
2 of 5 tasks
@mikernet

mikernet commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

#25101 is stacked on this PR (same gate, expressed as a single useCache flag) and fixes the shared-download cancellation and error retention in the same cache. This one can merge on its own first; the other shrinks to its remaining commits once it does.

@mikernet
fix(imaging): Don't cache IgnoreImageCache loads
7c1c364 
`IgnoreImageCache` skipped the lookup in the static `_bitmapImageCache`
but still added its load task to it, so the decoded image was retained
by the cache exactly as if the option were not set. Gate the add (and
the cancellation-removal registration) on the option too, so the image
lives only as long as the `BitmapImage` that produced it and never
replaces an entry other consumers already share.

Fixes unoplatform#25098
Copilot AI balanced review requested due to automatic review settings October 8, 2026 05:43
@mikernet
mikernet force-pushed the dev/mikernet/ignore-image-cache-25098 branch from 78dc472 to 7c1c364 Compare October 8, 2026 05:43
Copilot AI reviewed Oct 8, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused fix matches the reported behavior and has adequate regression coverage.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

This was referenced Oct 8, 2026
Closed
Merged
@mikernet

mikernet commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

For a servicing/6.8 backport: a port of this change onto the servicing/6.8 commit that 6.8.0-dev.212 shipped from is at Singulink@91ec14388a (branch Singulink:custom/6.8). The cache lives in BitmapImage.skia.cs there; same gate and test, runtime-tested on Skia desktop.

@azure-pipelines

azure-pipelines Bot commented Oct 9, 2026

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@MartinZikmund

MartinZikmund commented Oct 9, 2026

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

azure-pipelines Bot commented Oct 9, 2026

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@nventive-devops

nventive-devops commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

The build 238499 found UI Test snapshots differences: skia-linux-screenshots: 118, skia-windows-screenshots: 1153

Details

  • skia-linux-screenshots: 118 changed over 2458

    🚨🚨 Comparison Details (first 20) 🚨🚨
    • Buttons.png-dark
    • Buttons.png
    • ButtonClippingTestsControl.png-dark
    • ButtonClippingTestsControl.png
    • ClipboardTests.png-dark
    • CalendarView_Theming.png-dark
    • CalendarView_Theming.png
    • CompositionEffectBrush.png-dark
    • CompositionEffectBrush.png
    • CompositionNineGridBrush_Source_Changes.png-dark
    • CompositionNineGridBrush_Source_Changes.png
    • DisplayInformation.png-dark
    • DynamicFont.png-dark
    • DynamicFont.png
    • DoubleImageBrushInList.png-dark
    • DoubleImageBrushInList.png
    • DropDownButtonPage.png-dark
    • Focus_FocusVisual_Properties.png-dark
    • Focus_FocusVisual_Properties.png
    • ElementLevelTheme.png-dark

  • skia-windows-screenshots: 1153 changed over 2474

    🚨🚨 Comparison Details (first 20) 🚨🚨
    • Accessibility_ScreenReader.png
    • ApplicationViewSpanningRectsPage.png-dark
    • Basics_Automated.png-dark
    • Basics_Automated.png
    • BorderLeftwithmargins.png-dark
    • BorderLeftwithmargins.png
    • BorderVersusPanelClipping.png-dark
    • BorderVersusPanelClipping.png
    • Border_CornerRadius_Gradient.png-dark
    • Border_CornerRadius_Gradient.png
    • Button_Enabled_Control_Disabled.png-dark
    • Button_Enabled_Control_Disabled.png
    • Capture with overlap.png-dark
    • Capture with overlap.png
    • ClippingWithOpacity.png-dark
    • ClippingWithOpacity.png
    • CommandBar_With_Long_Sentences.png-dark
    • CommandBar_With_Long_Sentences.png
    • ContentControl_SelectorInheritance.png-dark
    • ContentControl_SelectorInheritance.png

@mergify

mergify Bot commented Oct 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@MartinZikmund
MartinZikmund merged commit c21e3a8 into unoplatform:master Oct 10, 2026
53 checks passed
@agneszitte agneszitte mentioned this pull request Nov 2, 2026
Merged
2 of 5 tasks
agneszitte added a commit that referenced this pull request Nov 2, 2026
@agneszitte
Merge pull request #25911 from unoplatform/dev/agzi/backport-25100-ig…
0375801 
…noreimagecache-6.8

fix(imaging): Don't cache IgnoreImageCache loads (backport #25100)
@mergify mergify Bot mentioned this pull request Nov 2, 2026
Merged
2 of 5 tasks
agneszitte added a commit that referenced this pull request Nov 2, 2026
@agneszitte
Merge pull request #25912 from unoplatform/mergify/bp/release/stable/…
c28ec75 
…6.7/pr-25911

fix(imaging): Don't cache IgnoreImageCache loads (backport #25100) (backport #25911)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ramezgerges ramezgerges ramezgerges approved these changes
@MartinZikmund MartinZikmund Awaiting requested review from MartinZikmund
@Xiaoy312 Xiaoy312 Awaiting requested review from Xiaoy312
@ajpinedam ajpinedam Awaiting requested review from ajpinedam

Assignees

@mikernet mikernet

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Skia] BitmapCreateOptions.IgnoreImageCache still adds the load to the static bitmap cache

5 participants

@mikernet @MartinZikmund @nventive-devops @ramezgerges