-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[android] improve performance of ImageHandler.PlatformArrange()
#23665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[android] improve performance of ImageHandler.PlatformArrange()
#23665
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
8dcd951
to
d52ca17
Compare
Is this going into next SR? Seems like a great adition! |
Any news on this? |
Sorry to bump this, but feels like this is an important MR to improve performance overall in a basic element like Image in all applications |
Context: https://github.com/davidortinau/AllTheLists Profiling @davidortinau's app, I noticed the "Check-ins" sample felt the slowest on Android. One thing I noticed while scrolling: 98.75ms (0.90%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) 67.11ms (0.61%) Mono.Android!Android.Widget.ImageView.ScaleType.get_CenterCrop() In this case, `PlatformArrange()` is called a lot for every `<Image/>`: if (PlatformView.GetScaleType() == ImageView.ScaleType.CenterCrop) { var (left, top, right, bottom) = PlatformView.Context!.ToPixels(frame); var clipRect = new Android.Graphics.Rect(0, 0, right - left, bottom - top); PlatformView.ClipBounds = clipRect; } `ImageView.ScaleType` is a class, and so and some bookkeeping is done to lookup *the same* C# instance for a Java object. We can make this a bit better by writing a new Java method: public static boolean isImageViewCenterCrop(@nonnull ImageView imageView) { return imageView.getScaleType() == ImageView.ScaleType.CENTER_CROP; } Next, let's make a `PlatformView.ToPixels()` extension method that can avoid calling `View.Context` for the same reason. Lastly, we can make a `PlatformInterop.SetClipBounds()` method to avoid creating a `Android.Graphics.Rect` object in C#. With these changes, I can only see the topmost `PlatformArrange()` method now: 2.93ms (0.03%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) This should improve the layout performance of all .NET MAUI `<Image/>` on Android. I also "banned" `GetScaleType()` in `eng/BannedSymbols.txt`.
d52ca17
to
0ed8055
Compare
@bcaceiro I originally held off on this because it could be fixed here:
We'll try to merge it now, as those changes didn't land. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the performance of ImageHandler.PlatformArrange()
on Android by optimizing several expensive operations during image layout. The changes focus on reducing object allocations and method call overhead during frequent layout operations.
Key changes:
- Replaces expensive
GetScaleType()
calls with a native Java method that avoids C# object creation - Introduces extension methods to avoid unnecessary
View.Context
property access - Adds native Java methods to set clip bounds without creating
Android.Graphics.Rect
objects in C#
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/Core/src/Handlers/Image/ImageHandler.Android.cs |
Updates PlatformArrange() to use new optimized methods for scale type checking and clip bounds setting |
src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/PlatformInterop.java |
Adds native Java methods isImageViewCenterCrop() and setClipBounds() to avoid object allocation overhead |
src/Core/src/Platform/Android/ContextExtensions.cs |
Adds ToPixels() extension method for View to avoid accessing Context property |
src/Compatibility/Core/src/Android/FastRenderers/ImageElementManager.cs |
Updates compatibility layer to use new optimized scale type checking method |
eng/BannedSymbols.txt |
Bans GetScaleType() method to prevent future performance regressions |
src/Core/tests/DeviceTests/Core.DeviceTests.csproj |
Adds IsTestProject property for proper test project identification |
src/Controls/tests/DeviceTests/Controls.DeviceTests.csproj |
Adds IsTestProject property for proper test project identification |
@@ -122,6 +122,17 @@ static float ToPixelsUsingMetrics(double dp) | |||
return (float)Math.Ceiling((dp * s_displayDensity) - GeometryUtil.Epsilon); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new ToPixels extension method for View lacks XML documentation. Consider adding documentation explaining its purpose and how it differs from the Context.ToPixels method.
/// <summary> | |
/// Converts a <see cref="Graphics.Rect"/> from device-independent pixels (DP) to physical pixels using the display metrics associated with the specified <see cref="View"/>. | |
/// This differs from <see cref="Context.ToPixels(Graphics.Rect)"/> in that it uses the metrics of the <see cref="View"/>, which may be different from those of the <see cref="Context"/>. | |
/// </summary> | |
/// <param name="view">The <see cref="View"/> whose display metrics are used for conversion.</param> | |
/// <param name="rectangle">The rectangle in device-independent pixels (DP) to convert.</param> | |
/// <returns> | |
/// A tuple containing the left, top, right, and bottom coordinates of the rectangle in physical pixels. | |
/// </returns> |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's internal
, so I didn't document it yet. Its purpose is to avoid calling View.Context
in the first place.
…3665) Context: https://github.com/davidortinau/AllTheLists Profiling @davidortinau's app, I noticed the "Check-ins" sample felt the slowest on Android. One thing I noticed while scrolling: 98.75ms (0.90%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) 67.11ms (0.61%) Mono.Android!Android.Widget.ImageView.ScaleType.get_CenterCrop() In this case, `PlatformArrange()` is called a lot for every `<Image/>`: if (PlatformView.GetScaleType() == ImageView.ScaleType.CenterCrop) { var (left, top, right, bottom) = PlatformView.Context!.ToPixels(frame); var clipRect = new Android.Graphics.Rect(0, 0, right - left, bottom - top); PlatformView.ClipBounds = clipRect; } `ImageView.ScaleType` is a class, and so and some bookkeeping is done to lookup *the same* C# instance for a Java object. We can make this a bit better by writing a new Java method: public static boolean isImageViewCenterCrop(@nonnull ImageView imageView) { return imageView.getScaleType() == ImageView.ScaleType.CENTER_CROP; } Next, let's make a `PlatformView.ToPixels()` extension method that can avoid calling `View.Context` for the same reason. Lastly, we can make a `PlatformInterop.SetClipBounds()` method to avoid creating a `Android.Graphics.Rect` object in C#. With these changes, I can only see the topmost `PlatformArrange()` method now: 2.93ms (0.03%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) This should improve the layout performance of all .NET MAUI `<Image/>` on Android. I also "banned" `GetScaleType()` in `eng/BannedSymbols.txt`.
…3665) Context: https://github.com/davidortinau/AllTheLists Profiling @davidortinau's app, I noticed the "Check-ins" sample felt the slowest on Android. One thing I noticed while scrolling: 98.75ms (0.90%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) 67.11ms (0.61%) Mono.Android!Android.Widget.ImageView.ScaleType.get_CenterCrop() In this case, `PlatformArrange()` is called a lot for every `<Image/>`: if (PlatformView.GetScaleType() == ImageView.ScaleType.CenterCrop) { var (left, top, right, bottom) = PlatformView.Context!.ToPixels(frame); var clipRect = new Android.Graphics.Rect(0, 0, right - left, bottom - top); PlatformView.ClipBounds = clipRect; } `ImageView.ScaleType` is a class, and so and some bookkeeping is done to lookup *the same* C# instance for a Java object. We can make this a bit better by writing a new Java method: public static boolean isImageViewCenterCrop(@nonnull ImageView imageView) { return imageView.getScaleType() == ImageView.ScaleType.CENTER_CROP; } Next, let's make a `PlatformView.ToPixels()` extension method that can avoid calling `View.Context` for the same reason. Lastly, we can make a `PlatformInterop.SetClipBounds()` method to avoid creating a `Android.Graphics.Rect` object in C#. With these changes, I can only see the topmost `PlatformArrange()` method now: 2.93ms (0.03%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) This should improve the layout performance of all .NET MAUI `<Image/>` on Android. I also "banned" `GetScaleType()` in `eng/BannedSymbols.txt`.
…3665) Context: https://github.com/davidortinau/AllTheLists Profiling @davidortinau's app, I noticed the "Check-ins" sample felt the slowest on Android. One thing I noticed while scrolling: 98.75ms (0.90%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) 67.11ms (0.61%) Mono.Android!Android.Widget.ImageView.ScaleType.get_CenterCrop() In this case, `PlatformArrange()` is called a lot for every `<Image/>`: if (PlatformView.GetScaleType() == ImageView.ScaleType.CenterCrop) { var (left, top, right, bottom) = PlatformView.Context!.ToPixels(frame); var clipRect = new Android.Graphics.Rect(0, 0, right - left, bottom - top); PlatformView.ClipBounds = clipRect; } `ImageView.ScaleType` is a class, and so and some bookkeeping is done to lookup *the same* C# instance for a Java object. We can make this a bit better by writing a new Java method: public static boolean isImageViewCenterCrop(@nonnull ImageView imageView) { return imageView.getScaleType() == ImageView.ScaleType.CENTER_CROP; } Next, let's make a `PlatformView.ToPixels()` extension method that can avoid calling `View.Context` for the same reason. Lastly, we can make a `PlatformInterop.SetClipBounds()` method to avoid creating a `Android.Graphics.Rect` object in C#. With these changes, I can only see the topmost `PlatformArrange()` method now: 2.93ms (0.03%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) This should improve the layout performance of all .NET MAUI `<Image/>` on Android. I also "banned" `GetScaleType()` in `eng/BannedSymbols.txt`.
…3665) Context: https://github.com/davidortinau/AllTheLists Profiling @davidortinau's app, I noticed the "Check-ins" sample felt the slowest on Android. One thing I noticed while scrolling: 98.75ms (0.90%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) 67.11ms (0.61%) Mono.Android!Android.Widget.ImageView.ScaleType.get_CenterCrop() In this case, `PlatformArrange()` is called a lot for every `<Image/>`: if (PlatformView.GetScaleType() == ImageView.ScaleType.CenterCrop) { var (left, top, right, bottom) = PlatformView.Context!.ToPixels(frame); var clipRect = new Android.Graphics.Rect(0, 0, right - left, bottom - top); PlatformView.ClipBounds = clipRect; } `ImageView.ScaleType` is a class, and so and some bookkeeping is done to lookup *the same* C# instance for a Java object. We can make this a bit better by writing a new Java method: public static boolean isImageViewCenterCrop(@nonnull ImageView imageView) { return imageView.getScaleType() == ImageView.ScaleType.CENTER_CROP; } Next, let's make a `PlatformView.ToPixels()` extension method that can avoid calling `View.Context` for the same reason. Lastly, we can make a `PlatformInterop.SetClipBounds()` method to avoid creating a `Android.Graphics.Rect` object in C#. With these changes, I can only see the topmost `PlatformArrange()` method now: 2.93ms (0.03%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) This should improve the layout performance of all .NET MAUI `<Image/>` on Android. I also "banned" `GetScaleType()` in `eng/BannedSymbols.txt`.
…3665) Context: https://github.com/davidortinau/AllTheLists Profiling @davidortinau's app, I noticed the "Check-ins" sample felt the slowest on Android. One thing I noticed while scrolling: 98.75ms (0.90%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) 67.11ms (0.61%) Mono.Android!Android.Widget.ImageView.ScaleType.get_CenterCrop() In this case, `PlatformArrange()` is called a lot for every `<Image/>`: if (PlatformView.GetScaleType() == ImageView.ScaleType.CenterCrop) { var (left, top, right, bottom) = PlatformView.Context!.ToPixels(frame); var clipRect = new Android.Graphics.Rect(0, 0, right - left, bottom - top); PlatformView.ClipBounds = clipRect; } `ImageView.ScaleType` is a class, and so and some bookkeeping is done to lookup *the same* C# instance for a Java object. We can make this a bit better by writing a new Java method: public static boolean isImageViewCenterCrop(@nonnull ImageView imageView) { return imageView.getScaleType() == ImageView.ScaleType.CENTER_CROP; } Next, let's make a `PlatformView.ToPixels()` extension method that can avoid calling `View.Context` for the same reason. Lastly, we can make a `PlatformInterop.SetClipBounds()` method to avoid creating a `Android.Graphics.Rect` object in C#. With these changes, I can only see the topmost `PlatformArrange()` method now: 2.93ms (0.03%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) This should improve the layout performance of all .NET MAUI `<Image/>` on Android. I also "banned" `GetScaleType()` in `eng/BannedSymbols.txt`.
…3665) Context: https://github.com/davidortinau/AllTheLists Profiling @davidortinau's app, I noticed the "Check-ins" sample felt the slowest on Android. One thing I noticed while scrolling: 98.75ms (0.90%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) 67.11ms (0.61%) Mono.Android!Android.Widget.ImageView.ScaleType.get_CenterCrop() In this case, `PlatformArrange()` is called a lot for every `<Image/>`: if (PlatformView.GetScaleType() == ImageView.ScaleType.CenterCrop) { var (left, top, right, bottom) = PlatformView.Context!.ToPixels(frame); var clipRect = new Android.Graphics.Rect(0, 0, right - left, bottom - top); PlatformView.ClipBounds = clipRect; } `ImageView.ScaleType` is a class, and so and some bookkeeping is done to lookup *the same* C# instance for a Java object. We can make this a bit better by writing a new Java method: public static boolean isImageViewCenterCrop(@nonnull ImageView imageView) { return imageView.getScaleType() == ImageView.ScaleType.CENTER_CROP; } Next, let's make a `PlatformView.ToPixels()` extension method that can avoid calling `View.Context` for the same reason. Lastly, we can make a `PlatformInterop.SetClipBounds()` method to avoid creating a `Android.Graphics.Rect` object in C#. With these changes, I can only see the topmost `PlatformArrange()` method now: 2.93ms (0.03%) Microsoft.Maui!Microsoft.Maui.Handlers.ImageHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect) This should improve the layout performance of all .NET MAUI `<Image/>` on Android. I also "banned" `GetScaleType()` in `eng/BannedSymbols.txt`.
Context: https://github.com/davidortinau/AllTheLists
Profiling @davidortinau's app, I noticed the "Check-ins" sample felt the slowest on Android.
One thing I noticed while scrolling:
In this case,
PlatformArrange()
is called a lot for every<Image/>
:ImageView.ScaleType
is a class, and so and some bookkeeping is done to lookup the same C# instance for a Java object. We can make this a bit better by writing a new Java method:Next, let's make a
PlatformView.ToPixels()
extension method that can avoid callingView.Context
for the same reason.Lastly, we can make a
PlatformInterop.SetClipBounds()
method to avoid creating aAndroid.Graphics.Rect
object in C#.With these changes, I can only see the topmost
PlatformArrange()
method now:This should improve the layout performance of all .NET MAUI
<Image/>
on Android. I also "banned"GetScaleType()
ineng/BannedSymbols.txt
.