Added a support for GradientBrushes on Shape.Stroke#22208
Added a support for GradientBrushes on Shape.Stroke#22208kubaflo wants to merge 7 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
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 -
|
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
kubaflo
added
the
s/agent-fix-implemented
- 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>
Replace shader-on-StrokePaint approach with GetFillPath to convert stroke geometry into a fill path, then draw with FillPaintWithAlpha. This mirrors the iOS ReplacePathWithStrokedPath pattern and correctly handles stroke caps, joins, and dash patterns. Changes: - Android PlatformDrawPath: use Paint.GetFillPath + FillPaintWithAlpha - ShapeDrawable: add explanatory comment for SetFillPaint usage - Test file: add trailing newline - Remove stale Android snapshot (needs recapture with new rendering) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use path.Bounds instead of dirtyRect for SetFillPaint in stroke rendering. dirtyRect is the full canvas dirty rectangle, so gradients would be positioned relative to the entire view rather than the shape geometry. path.Bounds ensures gradients map to the actual shape bounds, matching the expected visual output. Also add retryTimeout to VerifyScreenshot for animation stability, and remove stale snapshots that will be regenerated from CI with the corrected gradient coordinates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MauiBot
added
s/agent-fix-pr-picked
- Android PlatformCanvas: use 'using var' for strokedOutline Path to ensure proper disposal even if an exception occurs - HostApp Issue21983: remove redundant usings, XamlCompilation attribute (default since MAUI), and stray blank line Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Issues Fixed
Code from this sample: https://github.com/crhalvorson/MauiPathGradientRepro
Fixes #21983