Add configurable ResizeQuality for Resizetizer - #34559
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34559Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34559" |
There was a problem hiding this comment.
Pull request overview
This PR adds a configurable ResizeQuality option to Resizetizer so image resampling quality can be controlled via MSBuild metadata, while keeping existing behavior unchanged by default (Auto).
Changes:
- Introduces an internal
ResizeQualityenum and plumbs it throughResizeImageInfo→ tool creation → resize operations. - Replaces use of obsolete
SKPaint.FilterQualitywith explicitSKSamplingOptionsselection based onResizeQuality. - Adds unit + end-to-end tests validating parsing, sampling-option mapping, and output differences/regressions.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/SingleProject/Resizetizer/src/ResizeQuality.cs | Adds the ResizeQuality enum used to control resampling behavior. |
| src/SingleProject/Resizetizer/src/ResizeImageInfo.cs | Adds Quality property + parses ResizeQuality MSBuild metadata. |
| src/SingleProject/Resizetizer/src/SkiaSharpTools.cs | Adds quality-aware factory/ctor and maps ResizeQuality to SKSamplingOptions. |
| src/SingleProject/Resizetizer/src/SkiaSharpRasterTools.cs | Passes quality through constructor chain; raster draw uses SamplingOptions. |
| src/SingleProject/Resizetizer/src/SkiaSharpSvgTools.cs | Passes quality through; uses SamplingOptions for raster downscaling path. |
| src/SingleProject/Resizetizer/src/SkiaSharpImaginaryTools.cs | Updates base ctor call to include default quality. |
| src/SingleProject/Resizetizer/src/SkiaSharpAppIconTools.cs | Ensures app icon background/foreground tool creation respects info.Quality. |
| src/SingleProject/Resizetizer/src/Resizer.cs | Threads Info.Quality into tool creation. |
| src/SingleProject/Resizetizer/src/AndroidAdaptiveIconGenerator.cs | Threads Info.Quality into adaptive icon resizing. |
| src/SingleProject/Resizetizer/test/UnitTests/ResizeImageInfoTests.cs | Adds tests for defaulting + case-insensitive parsing + invalid value handling. |
| src/SingleProject/Resizetizer/test/UnitTests/SkiaSharpRasterToolsTests.cs | Adds sampling-option mapping tests and functional output comparisons for raster images. |
| src/SingleProject/Resizetizer/test/UnitTests/SkiaSharpSvgToolsTests.cs | Adds sampling-option mapping tests and functional output comparisons for SVG downscaling. |
| src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs | Adds end-to-end MSBuild task pipeline tests for ResizeQuality. |
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
This PR adds a configurable ResizeQuality option to the Resizetizer pipeline so callers can control image resampling behavior (via MSBuild item metadata), while preserving the existing default output.
Changes:
- Introduces an internal
ResizeQualityenum and plumbs it throughResizeImageInfoparsing and all resizing/tooling entry points. - Switches resampling control to
SKSamplingOptions(removing the obsoleteSKPaint.FilterQualityusage) and mapsResizeQualityvalues to the desired sampling options. - Adds unit and end-to-end tests covering parsing, sampling option mapping, and output equivalence/differences.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/SingleProject/Resizetizer/src/ResizeQuality.cs | Adds the new ResizeQuality enum used to select resampling behavior. |
| src/SingleProject/Resizetizer/src/ResizeImageInfo.cs | Adds Quality with default, and parses ResizeQuality from MSBuild metadata. |
| src/SingleProject/Resizetizer/src/SkiaSharpTools.cs | Plumbs ResizeQuality into constructors/factories and maps it to SKSamplingOptions; removes obsolete FilterQuality. |
| src/SingleProject/Resizetizer/src/SkiaSharpRasterTools.cs | Threads quality through constructors so raster drawing uses the configured sampling. |
| src/SingleProject/Resizetizer/src/SkiaSharpSvgTools.cs | Threads quality through constructors so SVG downscaling uses the configured sampling. |
| src/SingleProject/Resizetizer/src/SkiaSharpImaginaryTools.cs | Updates base constructor call to provide a default ResizeQuality for manufactured images. |
| src/SingleProject/Resizetizer/src/Resizer.cs | Ensures ResizeQuality flows into SkiaSharpTools.Create(...) when resizing. |
| src/SingleProject/Resizetizer/src/SkiaSharpAppIconTools.cs | Ensures app icon background/foreground tool creation passes through ResizeQuality. |
| src/SingleProject/Resizetizer/src/AndroidAdaptiveIconGenerator.cs | Ensures adaptive icon background/foreground resizing passes through ResizeQuality. |
| src/SingleProject/Resizetizer/test/UnitTests/ResizeImageInfoTests.cs | Adds tests for defaulting and case-insensitive parsing of ResizeQuality metadata. |
| src/SingleProject/Resizetizer/test/UnitTests/SkiaSharpRasterToolsTests.cs | Adds tests validating sampling option mapping and output invariants/differences for raster resizing. |
| src/SingleProject/Resizetizer/test/UnitTests/SkiaSharpSvgToolsTests.cs | Adds tests validating sampling option mapping and output invariants/differences for SVG resizing. |
| src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs | Adds end-to-end MSBuild pipeline tests verifying metadata is respected and defaults preserve baseline behavior. |
You can also share your feedback on Copilot code review. Take the survey.
Code Review SummaryResizeQuality for Resizetizer — Adds Findings
Overall Assessment
Otherwise the design, test coverage, and backward compatibility story are solid. Review performed by Copilot CLI |
a438af9 to
28be2d7
Compare
Code Review — PR #34559Independent AssessmentWhat this changes: Adds configurable Target branch: Reconciliation with PR NarrativeAgreement: ✅ Code matches description. Addresses @mattleibow's feedback on #25686: uses custom enum (not deprecated Findings✅ Good — Correct SkiaSharp API usageThe old code relied on // SkiaSharpRasterTools.DrawUnscaled
canvas.DrawImage(img, 0, 0, SamplingOptions, Paint);
// SkiaSharpSvgTools.DrawUnscaled (downscale path)
canvas.DrawImage(img, 0, 0, SamplingOptions, Paint);SVG upscaling ( ✅ Good — Quality mappings are well-chosen
✅ Good — Backwards compatible
✅ Good — Clean constructor threadingQuality parameter added to all constructor chains consistently:
✅ Good — Incremental build support
✅ Good — Comprehensive tests (22 new)
💡 Observation —
|
28be2d7 to
6989901
Compare
🧪 PR Test EvaluationOverall Verdict: ✅ Tests are adequate This PR adds 37 new unit tests with excellent coverage of the core
📊 Expand Full EvaluationPR Test Evaluation ReportPR: #34559 — Add configurable ResizeQuality for Resizetizer Overall Verdict✅ Tests are adequate Strong unit test coverage of the core quality mapping, MSBuild metadata parsing, and pixel-level regression guarantees. Two minor gaps in coverage for the 1. Fix Coverage — ✅The tests trace directly to the changed code paths:
If the quality mapping 2. Edge Cases & Gaps —
|
| Test | Assertion | Quality |
|---|---|---|
| Sampling options mapping | Assert.Equal(new SKSamplingOptions(...), tools.SamplingOptions) |
✅ Exact match |
| Pixel identity (default == Auto) | Pixel-by-pixel loop Assert.Equal(pixel1, pixel2) |
✅ Very strong |
| Pixel difference | Assert.True(differentPixels > 0, ...) |
✅ Adequate with message |
| Output dimensions | Assert.Equal(256, resultImage.Width) |
✅ Specific |
| Enum parsing | Assert.Equal(expected, info.Quality) |
✅ Exact match |
Assertions are specific and meaningful throughout. The pixel-by-pixel equality for the regression test (DefaultQualityProducesIdenticalOutputToExplicitAuto) is particularly strong.
9. Fix-Test Alignment — ✅
Fix → Test mapping:
| Changed File | Test Coverage |
|---|---|
ResizeQuality.cs (new enum) |
Used in all test files ✅ |
ResizeImageInfo.cs (Quality property + parsing) |
ResizeImageInfoTests.ResizeQualityTests ✅ |
SkiaSharpTools.cs (sampling options switch) |
SkiaSharpRasterToolsTests.ResizeQualityTests, SkiaSharpSvgToolsTests.ResizeQualityTests ✅ |
SkiaSharpRasterTools.cs / SkiaSharpSvgTools.cs / SkiaSharpImaginaryTools.cs (constructors) |
Covered transitively ✅ |
Resizer.cs (passes quality through) |
Covered by E2E ResizetizeImagesTests ✅ |
SkiaSharpAppIconTools.cs (passes quality) |
|
AndroidAdaptiveIconGenerator.cs (passes quality) |
|
Microsoft.Maui.Resizetizer.After.targets (build invalidation) |
Recommendations
- (Nice-to-have) Add a quality test to
SkiaSharpAppIconToolsTests— Add a test that creates an app icon withResizeQuality=Fastestand verifies the output differs fromAuto. Since the existingSkiaSharpAppIconToolsTestsalready test pixel output, this would be a straightforward addition. - (Nice-to-have) Add SVG
BestvsAutopixel comparison — MirrorBestQualityProducesDifferentPixelOutputThanAutofromSkiaSharpRasterToolsTestsinSkiaSharpSvgToolsTeststo ensure SVG quality behaves as expected forBestmode. - (Low priority) Consider a
ResizeQualityE2E test for app icons — A test inResizetizeImagesTeststhat uses an app icon item withResizeQualityset would close the gap for theAndroidAdaptiveIconGeneratorcode path.
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
dc.services.visualstudio.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "dc.services.visualstudio.com"See Network Configuration for more information.
Note
🔒 Integrity filtering filtered 2 items
Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.
- pr:Add configurable ResizeQuality for Resizetizer #34559 (
pull_request_read: Resource 'pr:Add configurable ResizeQuality for Resizetizer #34559' has lower integrity than agent requires. Agent would need to drop integrity tags [unapproved:all approved:all] to trust this resource.) - #25686 (
search_pull_requests: Resource 'pr:made FilterQuality configurable for Resizetizer #25686' has lower integrity than agent requires. Agent would need to drop integrity tags [approved:all] to trust this resource.)
🧪 Test evaluation by Evaluate PR Tests
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/azp run maui-pr |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
|
/azp run maui-pr |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Warning 🔍 Automated review could not completeA stage of the reviewer pipeline was stopped by its hang-safety timeout — most often the test-verification gate stalling on an emulator/simulator boot or an Appium hang on the CI agent. This is almost always a transient infrastructure issue, not a problem with your PR. Because the pipeline could not finish, no review was produced for this run. Please re-comment 🔍 Automated message from the .NET MAUI Copilot reviewer pipeline · build log |
… runtime The ReviewPR 'Boot iOS Simulator' step hard-exit-1'd when an AcesShared macOS agent had no usable iOS simulator runtime (empty 'simctl list runtimes' even unfiltered). That failure cascaded to SKIP Task 2 (Gate), Task 3 (expert Review), Task 4 (Post), the Deep UI Tests stage, and Post AI Summary — so the PR received NO review at all from an agent-provisioning gap unrelated to the PR (build 14669117, #34559 ios: runtime list empty -> boot exit 1 -> everything skipped). Make both boot-failure branches (no runtime / no device) graceful: warn and continue WITHOUT a booted simulator, leaving DEVICE_UDID empty. The expert AI review (Task 3, succeededOrFailed) still reads the diff and posts findings with no device, and the Gate self-degrades to INCONCLUSIVE — its verify-tests-fail retry loop already reports exit 3 (INCONCLUSIVE) when it cannot boot a device, so this produces an honest inconclusive rather than a false FAILED or a lost review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15d2af20-e4ab-4e88-9011-cfbd83513bc0
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
Adds a configurable
ResizeQualityoption to Resizetizer, allowing developers to control image resampling quality when resizing images.This is a clean re-implementation of community PR #25686 (by @thisisthekap), rebased onto
net11.0and updated based on review feedback.Key changes
Adds a new
ResizeQualityenum withAuto(default),Best, andFastestvalues.Adds
ResizeQualityMSBuild metadata support, for example:Maps
ResizeQualityto SkiaSharpSKSamplingOptionsinstead of the obsoletePaint.FilterQualitypath.Preserves existing behavior for
Auto, including default output compatibility.Propagates quality metadata through app icons, adaptive icons, monochrome icons, splash images, and generated image inputs so metadata-only changes correctly invalidate outputs.
Applies quality choices consistently for raster and SVG resizing, including SVG upscale/downscale paths, tint handling, and fractional SVG dimensions.
Adds a defensive in-tree BuildTasks import guard in
Maui.InTree.props/Maui.InTree.targetsso partial.buildtasksoutputs do not fail with missing Core props/targets imports before the friendly BuildTasks diagnostic can run.Quality mappings
AutoBestFastestTests
Adds and updates Resizetizer unit coverage for:
Autocompatibility.Validation
Latest
maui-prrun is green: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1511407Fixes #25686