Skip to content

Commit

Permalink
[android] avoid OnLayout() for Label (dotnet#21291)
Browse files Browse the repository at this point in the history
Context: dotnet#18505
Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip
Context: dotnet#21229 (review)

In profiling scrolling of an app with a `<CollectionView/>` and 12
`<Label/>`s, we see time spent in:

    1.9% Microsoft.Maui!Microsoft.Maui.Platform.MauiTextView.OnLayout(bool,int,int,int,int)

This is a callback from Java to C#, which has a performance cost.
Reviewing the code, we would only need to make this callback *at all*
if `Label.FormattedText` is not `null`. The bulk of all `Label`'s can
avoid this call?

To do this:

* Write a new `PlatformAppCompatTextView.java` that override `onLayout()`

* It only calls `onLayoutFormatted()` if a `isFormatted` `boolean`
  field is `true`

* We can set `isFormatted` if a formatted string is such as:
  `isFormatted = !(text instanceof String)`

With this change in place, the above `MauiTextView.OnLayout()` call is
completely gone from `dotnet-trace` output. Scrolling the sample also
"feels" a bit snappier.

This should improve the performance of all non-formatted `Label`s on
Android.

This is the mininum amount of API changes possible -- which seems like
what we should go for if we ship this change in .NET 8 servicing.
  • Loading branch information
jonathanpeppers authored Mar 27, 2024
1 parent 705b551 commit 83960b0
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.microsoft.maui;

import android.content.Context;

import androidx.annotation.NonNull;
import androidx.appcompat.widget.AppCompatTextView;

public class PlatformAppCompatTextView extends AppCompatTextView {
public PlatformAppCompatTextView(@NonNull Context context) {
super(context);
}

private boolean isFormatted;

/**
* Sets isFormatted=true, used for FormattedString content
* @param text
* @param type
*/
@Override
public void setText(CharSequence text, BufferType type) {
isFormatted = !(text instanceof String);
super.setText(text, type);
}

@Override
protected void onLayout(boolean changed, int left, int top, int right, int bottom) {
super.onLayout(changed, left, top, right, bottom);

if (isFormatted) {
onLayoutFormatted(changed, left, top, right, bottom);
}
}

/**
* To be overridden from C#
* @param changed
* @param left
* @param top
* @param right
* @param bottom
*/
protected void onLayoutFormatted(boolean changed, int left, int top, int right, int bottom) { }
}
6 changes: 2 additions & 4 deletions src/Core/src/Platform/Android/MauiTextView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,16 @@

namespace Microsoft.Maui.Platform
{
public class MauiTextView : AppCompatTextView
public class MauiTextView : PlatformAppCompatTextView
{
public MauiTextView(Context context) : base(context)
{
}

internal event EventHandler<LayoutChangedEventArgs>? LayoutChanged;

protected override void OnLayout(bool changed, int l, int t, int r, int b)
internal override void OnLayoutFormatted(bool changed, int l, int t, int r, int b)
{
base.OnLayout(changed, l, t, r, b);

LayoutChanged?.Invoke(this, new LayoutChangedEventArgs(l, t, r, b));
}
}
Expand Down
1 change: 0 additions & 1 deletion src/Core/src/PublicAPI/net-android/PublicAPI.Shipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2253,7 +2253,6 @@ override Microsoft.Maui.Platform.MauiSwipeView.DispatchTouchEvent(Android.Views.
override Microsoft.Maui.Platform.MauiSwipeView.OnAttachedToWindow() -> void
override Microsoft.Maui.Platform.MauiSwipeView.OnInterceptTouchEvent(Android.Views.MotionEvent? e) -> bool
override Microsoft.Maui.Platform.MauiSwipeView.OnTouchEvent(Android.Views.MotionEvent? e) -> bool
override Microsoft.Maui.Platform.MauiTextView.OnLayout(bool changed, int l, int t, int r, int b) -> void
override Microsoft.Maui.Platform.MauiWebChromeClient.Dispose(bool disposing) -> void
override Microsoft.Maui.Platform.MauiWebViewClient.Dispose(bool disposing) -> void
override Microsoft.Maui.Platform.MauiWebViewClient.OnPageFinished(Android.Webkit.WebView? view, string? url) -> void
Expand Down
6 changes: 6 additions & 0 deletions src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ Microsoft.Maui.KeyboardAcceleratorModifiers.None = 0 -> Microsoft.Maui.KeyboardA
Microsoft.Maui.KeyboardAcceleratorModifiers.Shift = 1 -> Microsoft.Maui.KeyboardAcceleratorModifiers
Microsoft.Maui.KeyboardAcceleratorModifiers.Windows = 16 -> Microsoft.Maui.KeyboardAcceleratorModifiers
Microsoft.Maui.Layouts.FlexBasis.Equals(Microsoft.Maui.Layouts.FlexBasis other) -> bool
Microsoft.Maui.PlatformAppCompatTextView
Microsoft.Maui.PlatformAppCompatTextView.PlatformAppCompatTextView(nint javaReference, Android.Runtime.JniHandleOwnership transfer) -> void
Microsoft.Maui.Platform.IImageSourcePartSetter
Microsoft.Maui.Platform.IImageSourcePartSetter.Handler.get -> Microsoft.Maui.IElementHandler?
Microsoft.Maui.Platform.IImageSourcePartSetter.ImageSourcePart.get -> Microsoft.Maui.IImageSourcePart?
Expand Down Expand Up @@ -64,10 +66,14 @@ override Microsoft.Maui.Handlers.ShapeViewHandler.GetDesiredSize(double widthCon
override Microsoft.Maui.Layouts.FlexBasis.Equals(object? obj) -> bool
override Microsoft.Maui.Layouts.FlexBasis.GetHashCode() -> int
override Microsoft.Maui.MauiAppCompatActivity.DispatchTouchEvent(Android.Views.MotionEvent? e) -> bool
override Microsoft.Maui.PlatformAppCompatTextView.JniPeerMembers.get -> Java.Interop.JniPeerMembers!
override Microsoft.Maui.PlatformAppCompatTextView.ThresholdClass.get -> nint
override Microsoft.Maui.PlatformAppCompatTextView.ThresholdType.get -> System.Type!
override Microsoft.Maui.Platform.ContentViewGroup.GetClipPath(int width, int height) -> Android.Graphics.Path?
override Microsoft.Maui.Platform.MauiMaterialButton.IconGravity.get -> int
override Microsoft.Maui.Platform.MauiMaterialButton.IconGravity.set -> void
override Microsoft.Maui.Platform.MauiScrollView.OnMeasure(int widthMeasureSpec, int heightMeasureSpec) -> void
*REMOVED*override Microsoft.Maui.Platform.MauiTextView.OnLayout(bool changed, int l, int t, int r, int b) -> void
override Microsoft.Maui.Platform.MauiMaterialButton.OnMeasure(int widthMeasureSpec, int heightMeasureSpec) -> void
override Microsoft.Maui.Platform.NavigationViewFragment.OnDestroy() -> void
override Microsoft.Maui.PlatformContentViewGroup.JniPeerMembers.get -> Java.Interop.JniPeerMembers!
Expand Down
8 changes: 5 additions & 3 deletions src/Core/src/Transforms/Metadata.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
<!-- Make all classes internal by default -->
<attr path="//class[@visibility='public']" name="visibility">internal</attr>
<!-- Public classes -->
<attr path="//class[@name='PlatformAppCompatTextView']" name="visibility">public</attr>
<attr path="//class[@name='PlatformContentViewGroup']" name="visibility">public</attr>
<attr path="//class[@name='PlatformWrapperView']" name="visibility">public</attr>
<attr path="//class[@name='MauiViewGroup']" name="visibility">public</attr>
<!-- Internal methods & constructors -->
<attr path="//class[@name='PlatformAppCompatTextView']/method" name="visibility">internal</attr>
<attr path="//class[@name='PlatformAppCompatTextView']/constructor" name="visibility">internal</attr>

<!-- We subclass this so it needs to be public -->
<attr path="//package[@name='com.microsoft.maui']/class[@name='MauiViewGroup']" name="visibility">public</attr>

<remove-node path="//package[@name='com.bumptech.glide']" />
<remove-node path="//package[@name='com.microsoft.maui.glide']" />
<remove-node path="//package[@name='com.microsoft.maui.glide.font']" />
Expand Down
Binary file modified src/Core/src/maui.aar
Binary file not shown.

0 comments on commit 83960b0

Please sign in to comment.