Skip to content

Commit

Permalink
[android] avoid View.Context during ContextView scrolling (#8243)
Browse files Browse the repository at this point in the history
Context: #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!
  • Loading branch information
jonathanpeppers authored Jun 22, 2022
1 parent 0b6976e commit eea91d3
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 15 deletions.
18 changes: 9 additions & 9 deletions src/Controls/src/Core/Handlers/Items/Android/ItemContentView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions src/Core/src/Platform/Android/ContextExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,25 @@ 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);

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));
Expand Down

0 comments on commit eea91d3

Please sign in to comment.