Added a support for GradientBrushes on Shape.Stroke#22208
Added a support for GradientBrushes on Shape.Stroke#22208kubaflo wants to merge 2 commits intodotnet:mainfrom
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
jsuarezruiz
left a comment
jsuarezruiz
left a comment
There was a problem hiding this comment.
There are no changes in public APIs, although the implementation in Windows is missing. I think if we decide to include this enhancement, I can apply changes in this PR by helping to implement on Windows.
Eilon
added
the
area-drawing
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 22208Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 22208" |
There was a problem hiding this comment.
Pull request overview
This PR adds support for applying GradientBrush (linear/radial) to Shape.Stroke, restoring expected behavior for Shapes (e.g., Path, Ellipse) on Android and iOS/macOS platforms, and adds UI test coverage plus baseline snapshots for issue #21983.
Changes:
- Use the stroke brush to set up gradient paint state during stroke rendering (
ShapeDrawable). - Implement gradient-aware stroke drawing for Paths on Android (shader) and MaciOS (stroked-path clipping + gradient fill).
- Add a HostApp repro page + Appium screenshot test and platform baseline images.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Graphics/src/Graphics/Platforms/MaciOS/PlatformCanvas.cs | When a gradient is active, stroke a path by converting it to a stroked outline and filling it with the gradient. |
| src/Graphics/src/Graphics/Platforms/Android/PlatformCanvas.cs | Applies the gradient shader to the stroke paint when drawing a path. |
| src/Core/src/Graphics/ShapeDrawable.cs | Uses the stroke brush to set up paint state before calling DrawPath. |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue21983.cs | Adds an Issues UI test that verifies gradient strokes via screenshot. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue21983.xaml | Adds XAML repro page demonstrating gradient stroke on Path and Ellipse. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue21983.xaml.cs | Code-behind for the new repro page. |
| src/Controls/tests/TestCases.Android.Tests/snapshots/android/GradientShouldBeAppliedToStrokes.png | Android screenshot baseline for the new UI test. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios/GradientShouldBeAppliedToStrokes.png | iOS screenshot baseline for the new UI test. |
| if (_shader != null) | ||
| { | ||
| CurrentState.StrokePaintWithAlpha.SetShader(_shader); | ||
| } | ||
| _canvas.DrawPath(platformPath, CurrentState.StrokePaintWithAlpha); |
There was a problem hiding this comment.
In PlatformDrawPath, the stroke Paint's Shader is only set when _shader != null, but it is never cleared when _shader is null. If a previous draw set a gradient shader, subsequent non-gradient strokes can accidentally keep using the old shader (especially in scenarios where callers draw multiple paths without a state restore). Consider explicitly clearing the shader (SetShader(null)) in the else path, or moving shader application into state so SaveState/RestoreState reliably restores it.
| if (_shader != null) | |
| { | |
| CurrentState.StrokePaintWithAlpha.SetShader(_shader); | |
| } | |
| _canvas.DrawPath(platformPath, CurrentState.StrokePaintWithAlpha); | |
| var strokePaint = CurrentState.StrokePaintWithAlpha; | |
| if (_shader != null) | |
| { | |
| strokePaint.SetShader(_shader); | |
| } | |
| else | |
| { | |
| // Ensure we don't reuse a shader from a previous draw when no shader is desired | |
| strokePaint.SetShader(null); | |
| } | |
| _canvas.DrawPath(platformPath, strokePaint); |
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.AppiumTests.Issues |
There was a problem hiding this comment.
This new Issues UITest uses the Microsoft.Maui.AppiumTests.Issues namespace, while other tests under TestCases.Shared.Tests/Tests/Issues commonly use the Microsoft.Maui.TestCases.Tests.Issues namespace. To keep test discovery/grouping consistent with the rest of the Issues suite, consider switching this file to the standard Issues test namespace used in this project.
| namespace Microsoft.Maui.AppiumTests.Issues | |
| namespace Microsoft.Maui.TestCases.Tests.Issues |
🤖 AI Summary📊 Expand Full Review🔍 Pre-Flight — Context & Validation📝 Review Session — Added a support for GradientBrushes on Shape.Stroke (#21983) ·
|
| Reviewer | Comment | Status |
|---|---|---|
| jsuarezruiz | Windows implementation is missing | |
| Copilot | Android: shader not cleared when _shader == null (gradient bleed risk) |
|
| Copilot | Test uses wrong namespace AppiumTests.Issues vs TestCases.Tests.Issues |
Concerns Identified
| File:Location | Issue | Severity |
|---|---|---|
PlatformCanvas.cs (Android):550-554 |
_shader applied to StrokePaintWithAlpha but never cleared when null - gradient could bleed to subsequent non-gradient strokes |
|
Issue21983.cs:6 |
Wrong namespace: Microsoft.Maui.AppiumTests.Issues instead of Microsoft.Maui.TestCases.Tests.Issues |
|
Issue21983.cs |
Missing newline at end of file | ℹ️ Minor |
Issue21983.cs:1 |
Wrapped in #if !WINDOWS - test skipped on Windows |
ℹ️ Intentional |
| Windows | No gradient stroke implementation for Windows |
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #22208 | SetFillPaint + platform-specific shader/gradient for stroke drawing | ⏳ PENDING (Gate) | ShapeDrawable.cs, Android/PlatformCanvas.cs, MaciOS/PlatformCanvas.cs |
Original PR |
🚦 Gate — Test Verification
📝 Review Session — Added a support for GradientBrushes on Shape.Stroke (#21983) · bb9a0fb
Result:
Platform: android
Mode: Full Verification (attempted)
| Check | Expected | Actual | Result |
|---|---|---|---|
| Tests WITHOUT fix | FAIL | FAIL | ✅ |
| Tests WITH fix | PASS | INCONCLUSIVE (ADB0010: Broken pipe) |
Tests FAIL without fix ✅ - Confirmed. The build succeeded and tests failed as expected (screenshot mismatch - gradient not applied to stroke).
Tests WITH fix - BLOCKED ❌ - The Android emulator became unresponsive after the first ~15-minute test run. The second deployment attempt failed with ADB0010: Mono.AndroidTools.InstallFailedException: Unexpected install output: cmd: Failure calling service package: Broken pipe (32). This is a transient infrastructure issue, not a code problem.
Decision: Proceeding to Report per autonomous execution rules (Gate fails due to environment → proceed to Report).
🔧 Fix — Analysis & Comparison
📝 Review Session — Added a support for GradientBrushes on Shape.Stroke (#21983) · bb9a0fb
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #22208 | SetFillPaint + platform shader/gradient for stroke drawing | 3 files | Original PR |
Exhausted: N/A (Skipped - Gate environment blocker prevented full verification)
Selected Fix: PR's fix (skipped try-fix exploration due to Gate not completing)
Reason for skipping: The Gate's "with fix" run failed due to an Android emulator infrastructure issue (ADB0010: Broken pipe). Per autonomous execution rules, try-fix exploration requires Gate to pass first. Skipping Phase 3 and proceeding to Report.
📋 Report — Final Recommendation
📝 Review Session — Added a support for GradientBrushes on Shape.Stroke (#21983) · bb9a0fb
⚠️ Final Recommendation: REQUEST CHANGES
Summary
PR #22208 implements GradientBrush support on Shape.Stroke for Android and iOS/macOS, fixing issue #21983. The approach is conceptually sound and the screenshot baseline images confirm the visual result is correct. However, there are two code quality issues that should be addressed before merging: an Android state management bug that could cause gradient bleed between consecutive non-gradient strokes, and a wrong test namespace that breaks test organization.
Root Cause
The DrawStrokePath method in ShapeDrawable previously only called canvas.StrokeColor = stroke.ToColor(), which resolves a GradientBrush to just its first color (via ToColor()). The fix adds canvas.SetFillPaint(stroke, dirtyRect) which passes the full gradient paint state to the platform canvas before DrawPath is called. Each platform canvas then uses that gradient state during stroke drawing:
- Android: Applies
_shadertoStrokePaintWithAlphainPlatformDrawPath - iOS/macOS: Converts the path to a stroked outline via
ReplacePathWithStrokedPath()then fills it withFillWithGradient
Issues Found
1. ⚠️ Android: Shader not cleared for non-gradient strokes (potential regression)
File: src/Graphics/src/Graphics/Platforms/Android/PlatformCanvas.cs:550-554
// Current code (PR):
if (_shader != null)
{
CurrentState.StrokePaintWithAlpha.SetShader(_shader);
}
_canvas.DrawPath(platformPath, CurrentState.StrokePaintWithAlpha);When _shader == null (solid color stroke), StrokePaintWithAlpha.SetShader() is never called. If a previous draw call set a gradient shader on StrokePaintWithAlpha, that shader persists. RestoreState() disposes _shader (sets it to null) but does not clear the shader reference on StrokePaintWithAlpha. This means a solid-color stroke rendered after a gradient stroke on the same canvas could accidentally inherit the old gradient.
Suggested fix:
var strokePaint = CurrentState.StrokePaintWithAlpha;
strokePaint.SetShader(_shader != null ? _shader : null); // Always set (clears if null)
_canvas.DrawPath(platformPath, strokePaint);Or use the Copilot-suggested else { strokePaint.SetShader(null); } pattern.
2. ⚠️ Wrong test namespace (breaks test organization)
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue21983.cs:6
// Current (wrong):
namespace Microsoft.Maui.AppiumTests.Issues
// Expected (correct):
namespace Microsoft.Maui.TestCases.Tests.IssuesAll other tests in Tests/Issues/ use Microsoft.Maui.TestCases.Tests.Issues. Using the wrong namespace may cause test runners to not discover or categorize the test correctly with the rest of the Issues test suite.
3. ℹ️ Minor: Missing newline at end of test file
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue21983.cs - No \n at end of file.
4. ℹ️ Windows not implemented
The PR adds a TODO comment and #if !WINDOWS guard on the test. This is acceptable for an initial implementation, but Windows support remains absent (noted by reviewer jsuarezruiz).
Fix Quality Assessment
The core fix approach is correct and the visual results are confirmed by the screenshot baselines in the PR. The platform implementations use appropriate native mechanisms (Android shader, iOS/macCatalyst stroked path fill). The main concern is the Android state management gap that could cause regressions in mixed gradient/solid-color rendering scenarios.
Gate Result
Requested Changes
- Fix Android shader clearing: Add
else { CurrentState.StrokePaintWithAlpha.SetShader(null); }inPlatformDrawPathto prevent gradient shader bleed. - Fix test namespace: Change
Microsoft.Maui.AppiumTests.Issues→Microsoft.Maui.TestCases.Tests.Issues. - Add newline: Add trailing newline to
Issue21983.cs.
📋 Expand PR Finalization Review
Title: ✅ Good
Current: Added a support for GradientBrushes on Shape.Stroke
Description: ⚠️ Needs Update
- Grammar: "Added a support" — git commit titles use imperative mood ("Add", not "Added a")
- Missing platform prefix — Windows is not fixed (still has a TODO for Windows in
ShapeDrawable.cs), so this PR is Android + iOS/macCatalyst only - Vague: doesn't call out which control category (
Shape)
✨ Suggested PR Description
[!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!
Root Cause
ShapeDrawable.DrawStrokePath had a TODO comment acknowledging that Paint (gradient) support was missing for Shape.Stroke. The method called canvas.StrokeColor = stroke.ToColor(), which extracts only the first color stop from any GradientBrush, effectively reducing it to a solid color. canvas.SetFillPaint() — the mechanism that configures gradient shaders on each platform — was never called for stroke paths, so gradient brushes on Stroke rendered as solid colors.
Description of Change
Three platform-level changes enable gradient stroke rendering on Android and iOS/macCatalyst:
src/Core/src/Graphics/ShapeDrawable.cs
- Added
canvas.SetFillPaint(stroke, dirtyRect)call beforecanvas.DrawPath(path)inDrawStrokePath - Updated the TODO comment to scope it to Windows only (since Android and iOS are now fixed)
- The existing
canvas.StrokeColor = stroke.ToColor()is preserved as the Windows fallback (solid color)
src/Graphics/src/Graphics/Platforms/Android/PlatformCanvas.cs
- In
PlatformDrawPath, applies_shader(set bySetFillPaint) toCurrentState.StrokePaintWithAlphabefore drawing, enabling gradient-stroked paths on Android
src/Graphics/src/Graphics/Platforms/MaciOS/PlatformCanvas.cs
- In
PlatformDrawPath, when_gradientis set (bySetFillPaint), uses the existingFillWithGradient()+CGContext.ReplacePathWithStrokedPath()pattern — the standard Core Graphics technique for clipping a gradient fill to the outline of a stroked path
Platforms Fixed
- ✅ Android
- ✅ iOS / macCatalyst
- ❌ Windows — still pending (
TODOinShapeDrawable.cs); falls back to solid color (first gradient stop)
Issues Fixed
Fixes #21983
Before / After
| Before | After |
|---|---|
 |
 |
Code Review: ⚠️ Issues Found
Code Review — PR #22208
🔴 Critical Issues
Android: Gradient shader not cleared after use — subsequent solid-color strokes will render with previous gradient
File: src/Graphics/src/Graphics/Platforms/Android/PlatformCanvas.cs
Problem:
StrokePaintWithAlpha returns the same _strokePaint object each time (it only updates the ARGB color via SetARGB). When PlatformDrawPath calls CurrentState.StrokePaintWithAlpha.SetShader(_shader), the shader persists on _strokePaint across subsequent draw calls. There is no path to clear it:
SetFillPaintShader(null)(called bySetFillPaintwhen switching to a non-gradient paint) only clearsFillPaint's shader — notStrokePaint's shader.- When the next shape draws a solid-color stroke,
_shaderisnulland theif (_shader != null)block is skipped — soStrokePaintWithAlpha's shader is never cleared.
In Android, a Paint with a Shader renders using the shader's colors (the solid color set via SetARGB is ignored). This means all subsequent solid-color stroke operations after any gradient stroke will still render using the previous gradient's colors.
Current code (buggy):
protected override void PlatformDrawPath(PathF aPath)
{
var platformPath = aPath.AsAndroidPath();
if (_shader != null)
{
CurrentState.StrokePaintWithAlpha.SetShader(_shader);
}
// ❌ If _shader is null, the old shader remains on StrokePaintWithAlpha
_canvas.DrawPath(platformPath, CurrentState.StrokePaintWithAlpha);
platformPath.Dispose();
}Recommended fix:
protected override void PlatformDrawPath(PathF aPath)
{
var platformPath = aPath.AsAndroidPath();
// Always set (or clear) the shader — passing null explicitly removes it from the Paint
CurrentState.StrokePaintWithAlpha.SetShader(_shader);
_canvas.DrawPath(platformPath, CurrentState.StrokePaintWithAlpha);
platformPath.Dispose();
}This same pattern is missing for all other PlatformDraw* methods that use StrokePaintWithAlpha (e.g. DrawLine, DrawArc, DrawRectangle, DrawRoundedRectangle, DrawOval) — they also don't apply the gradient shader. However, Shape.Stroke only routes through PlatformDrawPath, so those other methods are lower risk for this specific PR.
🟡 Suggestions
1. Missing newline at end of Issue21983.cs
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue21983.cs
The file ends without a trailing newline (\ No newline at end of file). This is a minor style issue that can cause noisy diffs in the future.
Fix: Add a newline after #endif.
2. PlatformAffected.All is misleading on the HostApp issue attribute
File: src/Controls/tests/TestCases.HostApp/Issues/Issue21983.xaml.cs
[Issue(IssueTracker.Github, 21983, "GradientBrushes are not supported on Shape.Stroke", PlatformAffected.All)]Windows still has the bug (Shape.Stroke gradient is still a TODO in ShapeDrawable.cs), and the UI test itself is wrapped in #if !WINDOWS. Using PlatformAffected.All implies the fix covers all platforms, which is inaccurate.
Recommended: Change to PlatformAffected.iOS | PlatformAffected.Android (or the equivalent enum values for iOS/macCatalyst + Android) to accurately reflect what this PR fixes.
✅ Looks Good
-
iOS/macCatalyst implementation is clean and correct. Using
CGContext.ReplacePathWithStrokedPath()to convert the stroke outline to a clippable fill path, then rendering the gradient viaFillWithGradient(), is the standard and well-established Core Graphics pattern. TheFillWithGradienthelper correctly handles the save/clip/draw/restore lifecycle. -
ShapeDrawable.csapproach is sound. CallingSetFillPaint(stroke, dirtyRect)beforeDrawPathcorrectly reuses the existing gradient infrastructure without duplicating gradient-setup logic. The TODO comment update accurately scopes the remaining Windows limitation. -
Test coverage is appropriate. A screenshot comparison test with two gradient brush types (Linear and Radial) on two shape types (Path and Ellipse) covers the key scenarios. The
#if !WINDOWSguard is correct given the Windows limitation. -
Before/after screenshots in the PR description clearly demonstrate the fix visually.
kubaflo
added
s/agent-changes-requested
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #22208 | Use stroke brush paint setup in ShapeDrawable, then add platform-specific gradient-aware stroke rendering for Android and MaciOS; add screenshot UITest coverage |
⏳ PENDING (Gate) | src/Core/src/Graphics/ShapeDrawable.cs, src/Graphics/src/Graphics/Platforms/Android/PlatformCanvas.cs, src/Graphics/src/Graphics/Platforms/MaciOS/PlatformCanvas.cs, tests |
Original PR; Windows not implemented |
🚦 Gate — Test Verification
Gate Result: ❌ FAILED
Platform: android
Mode: Full Verification
- Tests FAIL without fix: ✅
- Tests PASS with fix: ❌
Notes
- Verification artifacts reported the expected failure in the without-fix leg.
- The with-fix leg failed during Android package install, not during an executed UI assertion.
- Observed blocker:
ADB0010/Mono.AndroidTools.InstallFailedException: Unexpected install output: cmd: Failure calling service package: Broken pipe (32). - Because the install failed before the test could complete, this gate result is weaker than a normal behavioral failure and should be interpreted with caution.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Apply the stroke shader at setup time via Android canvas state instead of inside PlatformDrawPath |
✅ PASS | 4 files | Simplest passing alternative; also exposed the test namespace issue |
| 2 | try-fix | Convert the stroked path to a filled outline with Paint.GetFillPath() and render it with the fill paint/shader |
✅ PASS | 4 files + Android baseline | Avoids stroke shader state entirely, but requires snapshot update |
| 3 | try-fix | Apply the shader only for the current Android stroke draw, then restore the previous stroke shader immediately | ❌ FAIL | 2 files | Test command failed before Issue21983 executed because the UI test project hit compile errors |
| 4 | try-fix | Use explicit shader cleanup / temporary paint objects so cached stroke paint never retains a shader | ❌ FAIL | 3 files | Visual diff 3.68% from baseline |
| 5 | try-fix | Use saveLayer + PorterDuff.Mode.SrcIn compositing to mask the gradient into the stroke silhouette |
✅ PASS | 3 files | Passing alternative without mutating cached stroke paint state |
| 6 | try-fix | Use offscreen bitmap mask compositing instead of saveLayer |
❌ FAIL | 2 files | Visually correct but still diffed 3.68% from baseline |
| 7 | try-fix | Rebuild a dedicated stroke shader per draw from the original brush and path bounds | ❌ FAIL | 2 files | Visually correct but still diffed 3.68% from baseline |
| PR | PR #22208 | Use ShapeDrawable paint setup plus draw-time Android shader assignment and MaciOS gradient stroke rendering |
❌ FAILED (Gate) | 3 impl + tests | Gate failed on Android because the with-fix verification leg failed |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 1 | Yes | State-level stroke shader propagation instead of draw-time shader mutation |
| claude-sonnet-4.6 | 1 | Yes | Use GetFillPath() to convert the stroke to a fill outline and draw with gradient fill paint |
| gpt-5.3-codex | 1 | Yes | Use a scoped per-draw shader assignment/restore helper for Android stroke rendering |
| gemini-3-pro-preview | 1 | Yes | Use saveLayer + SRC_IN compositing to mask the gradient into the stroke |
| claude-opus-4.6 | 2 | Yes | Use an explicit bitmap alpha mask instead of saveLayer |
| claude-sonnet-4.6 | 2 | No | NO NEW IDEAS |
| gpt-5.3-codex | 2 | No | Rejected as duplicate of temporary paint approach |
| gemini-3-pro-preview | 2 | No | NO NEW IDEAS |
| claude-opus-4.6 | 3 | Yes | Rebuild a dedicated stroke shader from gradient stops and path bounds |
| claude-sonnet-4.6 | 3 | No | NO NEW IDEAS |
| gpt-5.3-codex | 3 | No | NO NEW IDEAS |
| gemini-3-pro-preview | 3 | No | Rejected as duplicate of temporary paint approach |
Exhausted: Yes
Selected Fix: Candidate #1 — it is the simplest passing alternative, keeps normal stroke rendering semantics, and avoids the PR's draw-time cached-paint shader mutation. Candidate #5 is also promising but more complex.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #21983 confirmed for Android/iOS; PR adds Android/iOS implementation plus UITest coverage. |
| Gate | ❌ FAILED | Android full verification reported FAIL without fix / FAIL with fix; the with-fix leg died during package install with ADB0010 broken-pipe, so this phase does not provide strong evidence in favor of the PR. |
| Try-Fix | ✅ COMPLETE | 7 attempts total, 3 passing candidates. Best alternative was Candidate #1. |
| Report | ✅ COMPLETE |
Summary
The PR is trying to fix the right problem, but the Android implementation is not the best version of the fix that we found. Independent try-fix exploration produced multiple Android alternatives that passed, including a simpler one that avoids mutating cached stroke paint state inside PlatformDrawPath.
In addition, the PR's added issue UITest was introduced under the wrong namespace convention (Microsoft.Maui.AppiumTests.Issues instead of the existing Microsoft.Maui.TestCases.Tests.Issues pattern used by this project), which multiple try-fix attempts had to correct before test validation could proceed reliably.
Root Cause
On Android, gradient brush information was being prepared for fills but not correctly applied to shape stroke rendering. The PR addresses this by pushing fill-paint setup from ShapeDrawable and then assigning the shader to the cached stroke paint during PlatformDrawPath.
The independent alternatives showed that the core Android issue can be fixed more cleanly by managing the stroke shader at setup time instead of mutating the cached stroke paint during draw, which reduces the risk of stale shader state carrying into later draws.
Fix Quality
- What is good: The PR identifies the correct functional gap and adds issue-specific UI coverage and baselines.
- What needs work: The Android fix relies on draw-time mutation of cached stroke paint state, while simpler passing alternatives exist.
- Additional review point: The new issue test should follow the existing
Microsoft.Maui.TestCases.Tests.Issuesnamespace convention. - Scope note: A prior human review also pointed out that Windows support is still absent even though the behavior has been reported there.
Selected Fix
Selected Fix: Candidate #1
Why: It passed on Android, is smaller and simpler than the compositing-based alternatives, preserves normal stroke rendering semantics, and avoids the PR's draw-time cached-paint shader mutation.
📋 Expand PR Finalization Review
Title: ✅ Good
Current: Added a support for GradientBrushes on Shape.Stroke
Description: ⚠️ Needs Update
- Grammar: "Added a support" — git commit titles use imperative mood ("Add", not "Added a")
- Missing platform prefix — Windows is not fixed (still has a TODO for Windows in
ShapeDrawable.cs), so this PR is Android + iOS/macCatalyst only - Vague: doesn't call out which control category (
Shape)
✨ Suggested PR Description
[!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!
Root Cause
ShapeDrawable.DrawStrokePath had a TODO comment acknowledging that Paint (gradient) support was missing for Shape.Stroke. The method called canvas.StrokeColor = stroke.ToColor(), which extracts only the first color stop from any GradientBrush, effectively reducing it to a solid color. canvas.SetFillPaint() — the mechanism that configures gradient shaders on each platform — was never called for stroke paths, so gradient brushes on Stroke rendered as solid colors.
Description of Change
Three platform-level changes enable gradient stroke rendering on Android and iOS/macCatalyst:
src/Core/src/Graphics/ShapeDrawable.cs
- Added
canvas.SetFillPaint(stroke, dirtyRect)call beforecanvas.DrawPath(path)inDrawStrokePath - Updated the TODO comment to scope it to Windows only (since Android and iOS are now fixed)
- The existing
canvas.StrokeColor = stroke.ToColor()is preserved as the Windows fallback (solid color)
src/Graphics/src/Graphics/Platforms/Android/PlatformCanvas.cs
- In
PlatformDrawPath, applies_shader(set bySetFillPaint) toCurrentState.StrokePaintWithAlphabefore drawing, enabling gradient-stroked paths on Android
src/Graphics/src/Graphics/Platforms/MaciOS/PlatformCanvas.cs
- In
PlatformDrawPath, when_gradientis set (bySetFillPaint), uses the existingFillWithGradient()+CGContext.ReplacePathWithStrokedPath()pattern — the standard Core Graphics technique for clipping a gradient fill to the outline of a stroked path
Platforms Fixed
- ✅ Android
- ✅ iOS / macCatalyst
- ❌ Windows — still pending (
TODOinShapeDrawable.cs); falls back to solid color (first gradient stop)
Issues Fixed
Fixes #21983
Before / After
| Before | After |
|---|---|
|
|
Code Review: ⚠️ Issues Found
Code Review — PR #22208
🔴 Critical Issues
Android: Gradient shader not cleared after use — subsequent solid-color strokes will render with previous gradient
File: src/Graphics/src/Graphics/Platforms/Android/PlatformCanvas.cs
Problem:
StrokePaintWithAlpha returns the same _strokePaint object each time (it only updates the ARGB color via SetARGB). When PlatformDrawPath calls CurrentState.StrokePaintWithAlpha.SetShader(_shader), the shader persists on _strokePaint across subsequent draw calls. There is no path to clear it:
SetFillPaintShader(null)(called bySetFillPaintwhen switching to a non-gradient paint) only clearsFillPaint's shader — notStrokePaint's shader.- When the next shape draws a solid-color stroke,
_shaderisnulland theif (_shader != null)block is skipped — soStrokePaintWithAlpha's shader is never cleared.
In Android, a Paint with a Shader renders using the shader's colors (the solid color set via SetARGB is ignored). This means all subsequent solid-color stroke operations after any gradient stroke will still render using the previous gradient's colors.
Current code (buggy):
protected override void PlatformDrawPath(PathF aPath)
{
var platformPath = aPath.AsAndroidPath();
if (_shader != null)
{
CurrentState.StrokePaintWithAlpha.SetShader(_shader);
}
// ❌ If _shader is null, the old shader remains on StrokePaintWithAlpha
_canvas.DrawPath(platformPath, CurrentState.StrokePaintWithAlpha);
platformPath.Dispose();
}Recommended fix:
protected override void PlatformDrawPath(PathF aPath)
{
var platformPath = aPath.AsAndroidPath();
// Always set (or clear) the shader — passing null explicitly removes it from the Paint
CurrentState.StrokePaintWithAlpha.SetShader(_shader);
_canvas.DrawPath(platformPath, CurrentState.StrokePaintWithAlpha);
platformPath.Dispose();
}This same pattern is missing for all other PlatformDraw* methods that use StrokePaintWithAlpha (e.g. DrawLine, DrawArc, DrawRectangle, DrawRoundedRectangle, DrawOval) — they also don't apply the gradient shader. However, Shape.Stroke only routes through PlatformDrawPath, so those other methods are lower risk for this specific PR.
🟡 Suggestions
1. Missing newline at end of Issue21983.cs
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue21983.cs
The file ends without a trailing newline (\ No newline at end of file). This is a minor style issue that can cause noisy diffs in the future.
Fix: Add a newline after #endif.
2. PlatformAffected.All is misleading on the HostApp issue attribute
File: src/Controls/tests/TestCases.HostApp/Issues/Issue21983.xaml.cs
[Issue(IssueTracker.Github, 21983, "GradientBrushes are not supported on Shape.Stroke", PlatformAffected.All)]Windows still has the bug (Shape.Stroke gradient is still a TODO in ShapeDrawable.cs), and the UI test itself is wrapped in #if !WINDOWS. Using PlatformAffected.All implies the fix covers all platforms, which is inaccurate.
Recommended: Change to PlatformAffected.iOS | PlatformAffected.Android (or the equivalent enum values for iOS/macCatalyst + Android) to accurately reflect what this PR fixes.
✅ Looks Good
-
iOS/macCatalyst implementation is clean and correct. Using
CGContext.ReplacePathWithStrokedPath()to convert the stroke outline to a clippable fill path, then rendering the gradient viaFillWithGradient(), is the standard and well-established Core Graphics pattern. TheFillWithGradienthelper correctly handles the save/clip/draw/restore lifecycle. -
ShapeDrawable.csapproach is sound. CallingSetFillPaint(stroke, dirtyRect)beforeDrawPathcorrectly reuses the existing gradient infrastructure without duplicating gradient-setup logic. The TODO comment update accurately scopes the remaining Windows limitation. -
Test coverage is appropriate. A screenshot comparison test with two gradient brush types (Linear and Radial) on two shape types (Path and Ellipse) covers the key scenarios. The
#if !WINDOWSguard is correct given the Windows limitation. -
Before/after screenshots in the PR description clearly demonstrate the fix visually.
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #22208 | Call canvas.SetFillPaint(stroke, dirtyRect) in stroke drawing path; Android PlatformDrawPath reads _shader |
❌ FAILED (Gate) | ShapeDrawable.cs, Android/PlatformCanvas.cs, MaciOS/PlatformCanvas.cs |
Reuses fill infrastructure for stroke; causes side effects |
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | IStrokePaintable internal interface — SetStrokePaint(Paint,RectF) sets _strokeShader directly on stroke paint, zero fill side effects, ScalingCanvas properly scales rect |
✅ PASS | 5 files (new interface + ScalingCanvas + Android PlatformCanvas + PlatformCanvasState + ShapeDrawable) | Cleanest separation of stroke/fill shader paths |
| 2 | try-fix (claude-sonnet-4.6) | Add SetStrokePaint to public ICanvas interface; AbstractCanvas virtual no-op; Android _strokeShader field |
❌ FAIL (3.68% diff) | 6 files + 8 PublicAPI.Unshipped.txt | Public API change; rect likely not scaled via ScalingCanvas |
| 3 | try-fix (gpt-5.3-codex) | Reflection-based routing through ScalingCanvas.Wrapped to PlatformCanvas |
❌ FAIL (IL2075) | 4 files | Reflection not allowed (trim-unsafe) |
| 4 | try-fix (gpt-5.4) | State-based: GradientStrokePaint on PlatformCanvasState, cast chain in ShapeDrawable |
❌ FAIL (3.68% diff) | 3 files | Same coordinate scaling issue as Attempt 2 |
| PR | PR #22208 | SetFillPaint reuse in stroke path; fill _shader read in PlatformDrawPath for stroke |
❌ FAILED (Gate) | ShapeDrawable.cs, Android/PlatformCanvas.cs, MaciOS/PlatformCanvas.cs |
Fill state side effects cause rendering failure |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | NO NEW IDEAS — Attempt 1 is the best available approach |
| gpt-5.3-codex | 2 | No | NO NEW IDEAS |
Exhausted: Yes — all 4 models queried, 2 cross-pollination responses, no new ideas.
Selected Fix: Attempt 1 (IStrokePaintable internal interface) — Reason: Only passing candidate. Architecturally cleanest: dedicated internal code path for stroke gradient, _strokeShader lifecycle separate from fill _shader, no public API changes, routes through ScalingCanvas for proper coordinate scaling. Zero fill-state side effects.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #21983 — gradient brush on Shape.Stroke, Android + iOS affected |
| Gate | ❌ FAILED | android — test did not behave as expected with PR's fix |
| Try-Fix | ✅ COMPLETE | 4 attempts, 1 passing (Attempt 1); 2 models cross-pollinated; exhausted |
| Report | ✅ COMPLETE | Selected Fix: Attempt 1 (IStrokePaintable internal interface) |
Summary
PR #22208 adds gradient brush support to Shape.Stroke on Android and iOS/macCatalyst. The motivation is correct and the iOS/macCatalyst fix (ReplacePathWithStrokedPath() + FillWithGradient()) is architecturally sound. However, the Android fix reuses the fill-paint infrastructure (SetFillPaint) to set up gradient state, which causes fill-state side effects that prevent the test from passing. The gate ❌ FAILED on Android.
Try-fix exploration found a cleaner alternative (Attempt 1, ✅ PASS) using a new internal IStrokePaintable interface that routes gradient setup directly to the stroke paint without touching fill state at all.
Root Cause
Why the PR's Android fix fails: ShapeDrawable.DrawStrokePath calls canvas.SetFillPaint(stroke, dirtyRect) before canvas.DrawPath(path). On Android, SetFillPaint is designed for fill operations — it:
- Disposes the old
_shader(fill shader) - Creates a new gradient
Shaderand sets it as_shader - Calls
CurrentState.SetFillPaintShader(_shader)— modifies the fill paint's shader - Sets
FillColor = Colors.White
The PR then reads _shader in PlatformDrawPath to apply it to StrokePaintWithAlpha. While this mechanically works, the fill-state pollution causes rendering failures: the fill shader and fill color are modified, which interferes with subsequent DrawFillPath calls on the same canvas. The combination causes incorrect rendering (gate screenshot fails).
Why Attempt 1 passes: It adds IStrokePaintable.SetStrokePaint(Paint, RectF) — an internal interface implemented by both PlatformCanvas and ScalingCanvas. The gradient shader is built and set directly on StrokePaint via CurrentState.SetStrokePaintShader(_strokeShader), with _strokeShader being a separate field from _shader (fill shader). Fill state is never touched. ScalingCanvas properly scales rectangle coordinates before delegating, matching what ScalingCanvas.SetFillPaint does.
Fix Quality
PR's fix issues:
- ❌ Android gate fails —
SetFillPaintside effects (fill-state pollution) cause incorrect rendering; gate ❌ FAILED - ❌
SetFillPaintsemantic mismatch — repurposing a fill API for stroke gradient setup is fragile and couples fill/stroke rendering state ⚠️ PlatformAffected.Allinaccurate — Windows explicitly NOT fixed (TODO comment); should bePlatformAffected.Android | PlatformAffected.iOS | PlatformAffected.MacCatalyst⚠️ Android snapshot may need regeneration — the committed snapshot was never verified against a passing run; should be regenerated from the working fix- ✅ iOS/macCatalyst fix is correct —
CGContext.ReplacePathWithStrokedPath()+FillWithGradient()is the standard Core Graphics pattern for gradient strokes - ✅ Stale shader clearing (
SetShader(null)inPlatformDrawPathelse-branch) — correct; author applied this from prior review feedback
Suggested fix (Attempt 1 pattern):
Replace canvas.SetFillPaint(stroke, dirtyRect) in ShapeDrawable.DrawStrokePath with a cast to IStrokePaintable:
if (stroke is not SolidPaint && canvas is IStrokePaintable strokePaintable)
strokePaintable.SetStrokePaint(stroke, dirtyRect);Add new src/Graphics/src/Graphics/IStrokePaintable.cs (internal interface):
internal interface IStrokePaintable
{
void SetStrokePaint(Paint paint, RectF rectangle);
}PlatformCanvas implements it with a dedicated _strokeShader field (lifecycle managed in ResetState/RestoreState). ScalingCanvas implements it by scaling the rectangle and delegating. PlatformCanvasState gets an internal SetStrokePaintShader(Shader) helper. No PlatformDrawPath modification needed — the shader is already on the stroke paint before DrawPath is called.
Fix diff is available in: CustomAgentLogsTmp/PRState/22208/PRAgent/try-fix/attempt-1/fix.diff
Code Review — PR #22208SummaryAdds gradient brush (
The platform implementations are architecturally sound. Backward compatibility for solid color strokes is maintained ( Findings❌ Wrong namespace in test file
📍
|
🟡 .NET MAUI Review - Changes SuggestedExpand Full Review -
|
- Clear shader on stroke paint when no gradient is active to prevent stale shader reuse from previous draws - Fix test namespace from AppiumTests.Issues to TestCases.Tests.Issues for consistency with the rest of the Issues test suite Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🚦 Gate - Test Before and After Fix📊 Expand Full Gate —
|
Issues Fixed
Code from this sample: https://github.com/crhalvorson/MauiPathGradientRepro
Fixes #21983