From 0ed805509c82479d1d514d069beebaf60d1ff14d Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 17 Jul 2024 15:11:20 -0500 Subject: [PATCH] [android] improve performance of `ImageHandler.PlatformArrange()` 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 ``: 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 `` on Android. I also "banned" `GetScaleType()` in `eng/BannedSymbols.txt`. --- eng/BannedSymbols.txt | 1 + .../Android/FastRenderers/ImageElementManager.cs | 2 +- .../tests/DeviceTests/Controls.DeviceTests.csproj | 1 + .../java/com/microsoft/maui/PlatformInterop.java | 15 +++++++++++++++ .../src/Handlers/Image/ImageHandler.Android.cs | 7 +++---- .../src/Platform/Android/ContextExtensions.cs | 11 +++++++++++ .../tests/DeviceTests/Core.DeviceTests.csproj | 1 + 7 files changed, 33 insertions(+), 5 deletions(-) diff --git a/eng/BannedSymbols.txt b/eng/BannedSymbols.txt index 9adcf5a06bf3..12123bcfc629 100644 --- a/eng/BannedSymbols.txt +++ b/eng/BannedSymbols.txt @@ -1,5 +1,6 @@ M:Microsoft.Extensions.DependencyInjection.Extensions.ServiceCollectionDescriptorExtensions.TryAddSingleton`2(Microsoft.Extensions.DependencyInjection.IServiceCollection);Use a Factory method to create the service instead M:Android.Content.Res.ColorStateList.#ctor(System.Int32[][],System.Int32[]);Use Microsoft.Maui.PlatformInterop.Get*ColorStateList() Java methods instead +M:Android.Widget.ImageView.GetScaleType();Use PlatformInterop.IsImageViewCenterCrop instead (or add a new method) P:Microsoft.Maui.MauiWinUIApplication.Services;Use the IPlatformApplication.Current.Services instead P:Microsoft.Maui.MauiWinUIApplication.Application;Use the IPlatformApplication.Current.Application instead P:Microsoft.UI.Xaml.Window.AppWindow;This API doesn't have null safety. Use GetAppWindow() and make sure to account for the possibility that GetAppWindow() might be null. diff --git a/src/Compatibility/Core/src/Android/FastRenderers/ImageElementManager.cs b/src/Compatibility/Core/src/Android/FastRenderers/ImageElementManager.cs index 15ae12b64739..9d4e7e5fc9c0 100644 --- a/src/Compatibility/Core/src/Android/FastRenderers/ImageElementManager.cs +++ b/src/Compatibility/Core/src/Android/FastRenderers/ImageElementManager.cs @@ -26,7 +26,7 @@ static void OnLayoutChange(object sender, global::Android.Views.View.LayoutChang { if (sender is IVisualElementRenderer renderer && renderer.View is ImageView imageView) #pragma warning disable CS0618 // Obsolete - AViewCompat.SetClipBounds(imageView, imageView.GetScaleType() == AScaleType.CenterCrop ? new ARect(0, 0, e.Right - e.Left, e.Bottom - e.Top) : null); + AViewCompat.SetClipBounds(imageView, PlatformInterop.IsImageViewCenterCrop(imageView) ? new ARect(0, 0, e.Right - e.Left, e.Bottom - e.Top) : null); #pragma warning restore CS0618 // Obsolete } diff --git a/src/Controls/tests/DeviceTests/Controls.DeviceTests.csproj b/src/Controls/tests/DeviceTests/Controls.DeviceTests.csproj index 729086c4deaf..5456480ffe41 100644 --- a/src/Controls/tests/DeviceTests/Controls.DeviceTests.csproj +++ b/src/Controls/tests/DeviceTests/Controls.DeviceTests.csproj @@ -7,6 +7,7 @@ Microsoft.Maui.DeviceTests Microsoft.Maui.Controls.DeviceTests $(NoWarn),CA1416 + true maccatalyst-x64 maccatalyst-arm64 diff --git a/src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/PlatformInterop.java b/src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/PlatformInterop.java index dc6934f574ff..10f724424e45 100644 --- a/src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/PlatformInterop.java +++ b/src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/PlatformInterop.java @@ -689,4 +689,19 @@ public static Animatable getAnimatable(Drawable drawable) { } return null; } + + /* + * Checks if the ScaleType of the ImageView is CENTER_CROP. + * Avoids returning an object to C#. + */ + public static boolean isImageViewCenterCrop(@NonNull ImageView imageView) { + return imageView.getScaleType() == ImageView.ScaleType.CENTER_CROP; + } + + /* + * Sets View.ClipBounds without creating a Rect object in C# + */ + public static void setClipBounds(@NonNull View view, int left, int top, int right, int bottom) { + view.setClipBounds(new Rect(left, top, right, bottom)); + } } diff --git a/src/Core/src/Handlers/Image/ImageHandler.Android.cs b/src/Core/src/Handlers/Image/ImageHandler.Android.cs index bb741ae1a1f8..81e764423ad2 100644 --- a/src/Core/src/Handlers/Image/ImageHandler.Android.cs +++ b/src/Core/src/Handlers/Image/ImageHandler.Android.cs @@ -76,14 +76,13 @@ await handler public override void PlatformArrange(Graphics.Rect frame) { - if (PlatformView.GetScaleType() == ImageView.ScaleType.CenterCrop) + if (PlatformInterop.IsImageViewCenterCrop(PlatformView)) { // If the image is center cropped (AspectFill), then the size of the image likely exceeds // the view size in some dimension. So we need to clip to the view's bounds. - var (left, top, right, bottom) = PlatformView.Context!.ToPixels(frame); - var clipRect = new Android.Graphics.Rect(0, 0, right - left, bottom - top); - PlatformView.ClipBounds = clipRect; + var (left, top, right, bottom) = PlatformView.ToPixels(frame); + PlatformInterop.SetClipBounds(PlatformView, 0, 0, right - left, bottom - top); } else { diff --git a/src/Core/src/Platform/Android/ContextExtensions.cs b/src/Core/src/Platform/Android/ContextExtensions.cs index fbb877a6e8c1..a175c270619e 100644 --- a/src/Core/src/Platform/Android/ContextExtensions.cs +++ b/src/Core/src/Platform/Android/ContextExtensions.cs @@ -122,6 +122,17 @@ static float ToPixelsUsingMetrics(double dp) return (float)Math.Ceiling((dp * s_displayDensity) - GeometryUtil.Epsilon); } + internal static (int left, int top, int right, int bottom) ToPixels(this View view, Graphics.Rect rectangle) + { + return + ( + (int)view.ToPixels(rectangle.Left), + (int)view.ToPixels(rectangle.Top), + (int)view.ToPixels(rectangle.Right), + (int)view.ToPixels(rectangle.Bottom) + ); + } + public static (int left, int top, int right, int bottom) ToPixels(this Context context, Graphics.Rect rectangle) { return diff --git a/src/Core/tests/DeviceTests/Core.DeviceTests.csproj b/src/Core/tests/DeviceTests/Core.DeviceTests.csproj index 7a64c076fdf7..dc7f9a9915f3 100644 --- a/src/Core/tests/DeviceTests/Core.DeviceTests.csproj +++ b/src/Core/tests/DeviceTests/Core.DeviceTests.csproj @@ -7,6 +7,7 @@ Microsoft.Maui.DeviceTests Microsoft.Maui.Core.DeviceTests $(NoWarn),CA1416 + true maccatalyst-x64 maccatalyst-arm64