Improve shadow rendering on Android, fix shadow clipping on iOS #26789

albyrock87 · 2024-12-23T16:46:03Z

Android Shadow Rendering

Description of Change

move the shadow computation to android level with an improved algorithm
- use Glide's LruBitmapPool to reduce the memory pressure on Java
- prepare Paint only when shadow changes
when we know we're going to render a solid shape we can use Canvas API to speedup the shadow rendering performance by a lot
remove unneeded saveLayer call from platform interop: it's a very expensive operation done for no reasons here
fix shadow rendering on Android which was still broken (despite Shadows not rendering as expected on Android and iOS #24414) under some conditions (like negative offsets)

Benchmark

Here I've executed the testing host app with return new ShadowBenchmark(); as main page and scrolling the CV once.

Before

After (with transparency)

You can see this is a bit faster, and for sure it is creating less GC/allocation work due to the bitmap pool.
Still timings are not good enough.

After (with solid background)

This time no need to create bitmaps and render descendants, we can simply draw the shadow.
And this is exactly the speed we want.

Notes

To collect those speedscopes I've temporary re-added:

protected override void DrawShadow(Canvas canvas, int viewWidth, int viewHeight)
{
	base.DrawShadow(canvas, viewWidth, viewHeight);
}

iOS Shadow Rendering

Removes unneeded shadow sublayer from wrapper view => we can simply apply the shadow to the WrapperView's main Layer considering it's the parent of the clipped content.
Make clear that the layer where we apply clipping is the content's main Layer
Ensure that when the clip is removed, we also clear the content's Layer's mask

Issues Fixed

Fixes #27156
Fixes #23630
Fixes #20518
Fixes #18202
Fixes #17886
Fixes #13981
Fixes #9539
Fixes #4106

Relates to #17881
Relates to #17257
Relates to #10401

PureWeen · 2024-12-23T16:53:29Z

/azp run

azure-pipelines · 2024-12-23T16:53:50Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2024-12-23T22:45:55Z

/rebase

PureWeen · 2024-12-23T22:47:37Z

/azp run

azure-pipelines · 2024-12-23T22:47:58Z

Azure Pipelines successfully started running 3 pipeline(s).

AmrAlSayed0 · 2024-12-24T13:45:43Z

I'm late to the party but an attempt to improve shadow's performance was made before here #10523. Moving the code to the Java side might help but I think what would benefit the most is a shadow cache. All views that have the same size and the same shadow properties (very common scenario in CollectionView items for example) should use the same bitmap. This will eliminate a huge number of calls including C# to Java ones. There is an implementation of the shadow cache in the PR.

albyrock87 · 2024-12-24T13:50:27Z

@AmrAlSayed0 thanks for sharing, I'll check it out.
This is still a very early WIP.
I also intend using Glide's bitmap pool for this, which should improve the performance by avoiding continuous allocations of the bitmaps.

src/Core/src/Platform/Android/WrapperView.cs

jonathanpeppers · 2025-01-10T15:32:06Z

src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/glide/ShadowBitmapPool.java

+                    // Use application context to avoid memory leaks
+                    Context applicationContext = context.getApplicationContext();


Do you have any details why to use getApplicationContext?

It looks like the ShadowBitmapPool is used once per PlatformWrapperView, which would have the same lifetime as an activity anyway?

If we used a single ShadowBitmapPool for the entire application, seems like getApplicationContext would be appropriate in that case.

See comment below: we use only one pool.

jonathanpeppers · 2025-01-10T15:33:12Z

src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/PlatformWrapperView.java

    public PlatformWrapperView(Context context) {
        super(context);
        this.viewBounds = new Rect();
+        this.bitmapPool = ShadowBitmapPool.get(context);


So, we use 1 pool per PlatformWrapperView.

Can we use 1 pool for the entire application? That would allow many views to share the pool.

Actually that static method returns one pool for the entire application, so exactly what you're asking :)

PureWeen · 2025-01-10T15:37:51Z

/azp run

azure-pipelines · 2025-01-10T15:38:12Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2025-01-12T17:09:59Z

/azp run

azure-pipelines · 2025-01-12T17:10:21Z

Azure Pipelines successfully started running 3 pipeline(s).

AmrAlSayed0 · 2025-01-13T08:37:18Z

Correct me if I'm wrong but what this is doing is using a pool not a cache. Which means that no 2 views will ever use the same bitmap, it's just that that bitmaps will be reused and recycled instead of being created each time it's needed to better conserve memory.

I was thinking of a cache for when, for example, 200 items in a CollectionView request the same exact bitmap cuz they have MeasureFirstItem set (hence same size) and have the same template (hence the same shadow properties) and the same bitmap can be set for both.

I know this PR is already doing too much so this maybe an optimization for a later time ..

albyrock87 · 2025-01-13T09:27:24Z

@AmrAlSayed0 there's a small "cache" at the view's level which reuses the bitmap content when not shadowInvalidated.

But from my analysis, this is probably ineffective as every time something changes on the descendants (requestLayout) the shadow is being invalidated.

The reason for this invalidation is that the shadow is based on content pixels.
For example, you may have a collection view containing simple Labels with a shadow.
Now every label may have the same height, but the shadow will be different considering the text is different.

This is really a pain as Android doesn't offer a better way to draw a shadow based on the content.

What I've done here though, is detecting if you have a shadow applied to a solid rectangle, or a solid Border.
If that's the case the shadow will be drawn based on the shape directly on the rendering Canvas: this is super fast and we don't have to worry about bitmaps at all.

~~Unfortunately there is an edge case not covered, so I'm discussing with the MAUI team what to do.~~

Thanks for your input on this!

PureWeen · 2025-01-14T20:52:30Z

/azp run

azure-pipelines · 2025-01-14T20:52:52Z

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz · 2025-01-15T10:49:13Z

src/Core/src/Platform/Android/WrapperView.cs

@@ -83,19 +54,55 @@ public override bool DispatchTouchEvent(MotionEvent e)

 		partial void ClipChanged()
 		{
-			_invalidateClip = _invalidateShadow = true;
+			_invalidateClip = true;


If you change the clipping of a View at runtime, and it has a shadow, should it be invalidated, would this still happen?

Yes, it will, and this is also covered in the new 24114 UI test.
The invalidation happens on Java side.

jsuarezruiz · 2025-01-15T10:50:21Z

src/Core/src/Platform/Android/WrapperView.cs

+				float[] positions;
+				float[] bounds;
+
+				switch (shadowPaint)


Nice one, but we don't have gradient shadows implemented on other platforms. After merging this, we should check if we can implement it.

This was already implemented android-only.
This can't be done on other platforms, for example iOS doesn't support this, it only takes a Color parameter.

jsuarezruiz · 2025-01-15T10:53:35Z

src/Core/src/Graphics/MauiDrawable.Android.cs

-				_backgroundColor = new AColor(color);
+				var backgroundColor = new AColor(color);
+				_backgroundColor = backgroundColor;
+				_isBackgroundSolid = backgroundColor.A is 255;


Could be an extension method in the ColorExtensions class.

jsuarezruiz · 2025-01-15T10:55:57Z

src/Core/src/Graphics/MauiDrawable.Android.cs

@@ -224,6 +234,7 @@ public void SetBorderBrush(RadialGradientPaint radialGradientPaint)

 			_borderColor = null;
 			_stroke = radialGradientPaint;
+			_isBorderSolid = radialGradientPaint.GradientStops.All(s => s.Color.Alpha == 1);


Like with Color, could be a nice addition to the PaintExtensions class.

jsuarezruiz · 2025-01-15T10:59:36Z

src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/PlatformWrapperView.java

+            shadowPaint = null;
+            shadowCanvas = null;
+            if (shadowBitmap != null) {
+                bitmapPool.put(shadowBitmap);


Good improvement!

jsuarezruiz · 2025-01-15T11:04:32Z

src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/PlatformWrapperView.java

+
+    private int getRadiusSafeSpace() {
+        if (radius <= 0) {
+            return 4;


Why use this specific value?

I forgot to remove this, thanks!

jsuarezruiz · 2025-01-15T11:05:28Z

src/Controls/tests/TestCases.HostApp/Issues/Issue24414.xaml.cs

+		UpdateLabel();
+	}
+
+	private void OnTapGestureRecognizerTapped(object sender, EventArgs e)


Could also update the sample adding another Button to remove/add the shadows at runtime?

jsuarezruiz · 2025-01-15T11:10:20Z

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24414.cs

@@ -16,8 +16,34 @@ public Issue24414(TestDevice device)
 		[Category(UITestCategories.Visual)]
 		public void Issue24414Test()


The test is failing in all the platforms by differences in the shadows:

Maybe commits 14eecfe after include the snapshots had an impact?

Could you review the tests? Let me know if needs help with it.

Actually I have to capture all the shapshots because I changed the UI test.
I will on next commit+azp.

jsuarezruiz · 2025-01-15T11:15:33Z

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24414.cs

@@ -16,8 +16,34 @@ public Issue24414(TestDevice device)
 		[Category(UITestCategories.Visual)]
 		public void Issue24414Test()


On Android, could use

maui/src/TestUtils/src/UITest.Appium/HelperExtensions.cs

Line 1771 in daf8866

public static IList<object> GetPerformanceData(this IApp app, string performanceDataType)

to get memory info and validate never exceed a value, and avoid detecting degradation in memory consumption due to any change

albyrock87 · 2025-01-15T15:11:57Z

@jsuarezruiz I should have addressed all your feedbacks (except for screenshots which will be updated after the next AZP)

# Conflicts: # src/TestUtils/src/UITest.Appium/HelperExtensions.cs

PureWeen · 2025-01-15T16:12:47Z

/azp run

azure-pipelines · 2025-01-15T16:13:11Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2025-01-15T16:40:10Z

/azp run

azure-pipelines · 2025-01-15T16:40:34Z

Azure Pipelines successfully started running 3 pipeline(s).

albyrock87 requested a review from a team as a code owner December 23, 2024 16:46

albyrock87 requested review from rmarinho and PureWeen December 23, 2024 16:46

albyrock87 marked this pull request as draft December 23, 2024 16:46

dotnet-policy-service bot added the community ✨ Community Contribution label Dec 23, 2024

PureWeen added this to the .NET 9 SR4 milestone Dec 23, 2024

github-actions bot force-pushed the android-shadow branch from 2a51aee to 2a218db Compare December 23, 2024 22:46

albyrock87 force-pushed the android-shadow branch 5 times, most recently from 2f0b86f to eccf37e Compare December 24, 2024 13:45

albyrock87 force-pushed the android-shadow branch 11 times, most recently from d40a883 to 5cb0404 Compare December 26, 2024 11:35

jonathanpeppers reviewed Jan 10, 2025

View reviewed changes

Update screenshots

a9b540b

albyrock87 force-pushed the android-shadow branch from d01b92c to a9b540b Compare January 12, 2025 16:57

Small speedup

14eecfe

albyrock87 force-pushed the android-shadow branch from e5e9143 to 14eecfe Compare January 14, 2025 18:38

jsuarezruiz requested changes Jan 15, 2025

View reviewed changes

jsuarezruiz reviewed Jan 15, 2025

View reviewed changes

Address feedbacks and fix iOS shadows/clipping

166b602

albyrock87 mentioned this pull request Jan 15, 2025

[iOS] Shadow for clipped visual element #23872

Open

Merge branch 'main' into android-shadow

7cee0bb

# Conflicts: # src/TestUtils/src/UITest.Appium/HelperExtensions.cs

albyrock87 changed the title ~~Android shadow performance~~ Improve shadow rendering on Android, fix shadow clipping on iOS Jan 15, 2025

Regenerate correct aar

cea78e7

		// Use application context to avoid memory leaks
		Context applicationContext = context.getApplicationContext();

		@@ -16,8 +16,34 @@ public Issue24414(TestDevice device)
		[Category(UITestCategories.Visual)]
		public void Issue24414Test()

Improve shadow rendering on Android, fix shadow clipping on iOS #26789

Are you sure you want to change the base?

Improve shadow rendering on Android, fix shadow clipping on iOS #26789

Conversation

albyrock87 commented Dec 23, 2024 • edited Loading

Android Shadow Rendering

Description of Change

Benchmark

iOS Shadow Rendering

Issues Fixed

PureWeen commented Dec 23, 2024

azure-pipelines bot commented Dec 23, 2024

PureWeen commented Dec 23, 2024

PureWeen commented Dec 23, 2024

azure-pipelines bot commented Dec 23, 2024

AmrAlSayed0 commented Dec 24, 2024

albyrock87 commented Dec 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albyrock87 Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

PureWeen commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

PureWeen commented Jan 12, 2025

azure-pipelines bot commented Jan 12, 2025

AmrAlSayed0 commented Jan 13, 2025

albyrock87 commented Jan 13, 2025 • edited Loading

PureWeen commented Jan 14, 2025

azure-pipelines bot commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albyrock87 commented Jan 15, 2025 • edited Loading

PureWeen commented Jan 15, 2025

azure-pipelines bot commented Jan 15, 2025

PureWeen commented Jan 15, 2025

azure-pipelines bot commented Jan 15, 2025

albyrock87 commented Dec 23, 2024 •

edited

Loading

albyrock87 Jan 10, 2025 •

edited

Loading

albyrock87 commented Jan 13, 2025 •

edited

Loading

albyrock87 commented Jan 15, 2025 •

edited

Loading