From eea91d302537b9b9287f95032679168c071308be Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 22 Jun 2022 17:11:20 -0500 Subject: [PATCH] [android] avoid View.Context during ContextView scrolling (#8243) Context: https://github.com/dotnet/maui/issues/8012 Context: https://github.com/Kalyxt/Test_CollectionView We had some reports of poor `CollectionView` performance while scrolling on an older Android device. Reviewing `dotnet trace` output, I did find some issues similar to #8001: 317.42ms (1.1%) mono.android!Android.Views.View.get_Context() 1% of the time is spent in repeated calls to `View.Context` inside the `ItemContentView` class. Making a new overload for `ContextExtensions.FromPixel()`, I was able to remove all of these calls. This results in only a couple `View.Context` calls on startup now, much better: 1.30ms (0.01%) mono.android!Android.Views.View.get_Context() Using the "janky frames" metric from the latest profiler in Android Studio Dolphin: https://developer.android.com/studio/profile/jank-detection With my slowest Android 12+ device, a Pixel 4a, I could actually see a few "janky frames" while scrolling the sample. With these changes in place, I only see 1 "janky frame" now. I also compared the before and after with the visual GPU profiler: https://developer.android.com/topic/performance/rendering/inspect-gpu-rendering It appears at a glance that these changes are better. I am unsure at what point the performance will be good enough to close #8012, but this helps! --- .../Handlers/Items/Android/ItemContentView.cs | 18 +++++++++--------- .../Android/RecyclerViewScrollListener.cs | 10 ++++------ .../src/Platform/Android/ContextExtensions.cs | 12 ++++++++++++ 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/Controls/src/Core/Handlers/Items/Android/ItemContentView.cs b/src/Controls/src/Core/Handlers/Items/Android/ItemContentView.cs index 5067a8e1b025..b194b3d79df4 100644 --- a/src/Controls/src/Core/Handlers/Items/Android/ItemContentView.cs +++ b/src/Controls/src/Core/Handlers/Items/Android/ItemContentView.cs @@ -65,7 +65,7 @@ protected override void OnLayout(bool changed, int l, int t, int r, int b) return; } - var size = Context.FromPixels(r - l, b - t); + var size = this.FromPixels(r - l, b - t); //TODO: RUI Is this the best way? //View.Arrange(new Rectangle(Point.Zero, size)); @@ -99,23 +99,23 @@ protected override void OnMeasure(int widthMeasureSpec, int heightMeasureSpec) var width = MeasureSpec.GetMode(widthMeasureSpec) == MeasureSpecMode.Unspecified ? double.PositiveInfinity - : Context.FromPixels(pixelWidth); + : this.FromPixels(pixelWidth); var height = MeasureSpec.GetMode(heightMeasureSpec) == MeasureSpecMode.Unspecified ? double.PositiveInfinity - : Context.FromPixels(pixelHeight); + : this.FromPixels(pixelHeight); var measure = View.Measure(width, height); if (pixelWidth == 0) { - pixelWidth = (int)Context.ToPixels(measure.Width); + pixelWidth = (int)this.ToPixels(measure.Width); } if (pixelHeight == 0) { - pixelHeight = (int)Context.ToPixels(measure.Height); + pixelHeight = (int)this.ToPixels(measure.Height); } _reportMeasure?.Invoke(new Size(pixelWidth, pixelHeight)); @@ -144,10 +144,10 @@ void UpdateContentLayout() if (mauiControlsView == null || aview == null) return; - var x = (int)Context.ToPixels(mauiControlsView.X); - var y = (int)Context.ToPixels(mauiControlsView.Y); - var width = Math.Max(0, (int)Context.ToPixels(mauiControlsView.Width)); - var height = Math.Max(0, (int)Context.ToPixels(mauiControlsView.Height)); + var x = (int)this.ToPixels(mauiControlsView.X); + var y = (int)this.ToPixels(mauiControlsView.Y); + var width = Math.Max(0, (int)this.ToPixels(mauiControlsView.Width)); + var height = Math.Max(0, (int)this.ToPixels(mauiControlsView.Height)); aview.Layout(x, y, width, height); diff --git a/src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs b/src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs index bc3be780ee54..4bdeb5c08231 100644 --- a/src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs +++ b/src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs @@ -41,14 +41,12 @@ public override void OnScrolled(RecyclerView recyclerView, int dx, int dy) _verticalOffset += dy; var (First, Center, Last) = GetVisibleItemsIndex(recyclerView); - - var context = recyclerView.Context; var itemsViewScrolledEventArgs = new ItemsViewScrolledEventArgs { - HorizontalDelta = context.FromPixels(dx), - VerticalDelta = context.FromPixels(dy), - HorizontalOffset = context.FromPixels(_horizontalOffset), - VerticalOffset = context.FromPixels(_verticalOffset), + HorizontalDelta = recyclerView.FromPixels(dx), + VerticalDelta = recyclerView.FromPixels(dy), + HorizontalOffset = recyclerView.FromPixels(_horizontalOffset), + VerticalOffset = recyclerView.FromPixels(_verticalOffset), FirstVisibleItemIndex = First, CenterItemIndex = Center, LastVisibleItemIndex = Last diff --git a/src/Core/src/Platform/Android/ContextExtensions.cs b/src/Core/src/Platform/Android/ContextExtensions.cs index 38b0418b4a17..5b084a10ed2c 100644 --- a/src/Core/src/Platform/Android/ContextExtensions.cs +++ b/src/Core/src/Platform/Android/ContextExtensions.cs @@ -32,6 +32,13 @@ public static class ContextExtensions // TODO FromPixels/ToPixels is both not terribly descriptive and also possibly sort of inaccurate? // These need better names. It's really To/From Device-Independent, but that doesn't exactly roll off the tongue. + internal static double FromPixels(this View view, double pixels) + { + if (s_displayDensity != float.MinValue) + return pixels / s_displayDensity; + return view.Context.FromPixels(pixels); + } + public static double FromPixels(this Context? self, double pixels) { EnsureMetrics(self); @@ -39,6 +46,11 @@ public static double FromPixels(this Context? self, double pixels) return pixels / s_displayDensity; } + internal static Size FromPixels(this View view, double width, double height) + { + return new Size(view.FromPixels(width), view.FromPixels(height)); + } + public static Size FromPixels(this Context context, double width, double height) { return new Size(context.FromPixels(width), context.FromPixels(height));