Skip to content

Commit

Permalink
[android] avoid OnLayout() for Label
Browse files Browse the repository at this point in the history
Context: #18505
Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip
Context: #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.TextType` is not `TextType.Text`. 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` when `Label.TextType` changes to something
  other than `TextType.Text`.

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 `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 committed Mar 19, 2024
1 parent 93940d6 commit e6216a5
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 9 deletions.
8 changes: 8 additions & 0 deletions src/Controls/src/Core/Label/Label.Android.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ public static void MapMaxLines(ILabelHandler handler, Label label)
handler.PlatformView?.UpdateMaxLines(label);
}

void UpdateTextType(TextType value)
{
if (Handler?.PlatformView is MauiTextView textView)
{
textView.SetIsFormatted(value != TextType.Text);
}
}

void OnLayoutChanged(object sender, LayoutChangedEventArgs args)
{
if (Handler is not ILabelHandler labelHandler)
Expand Down
11 changes: 10 additions & 1 deletion src/Controls/src/Core/Label/Label.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,16 @@ public virtual string UpdateFormsText(string source, TextTransform textTransform

/// <summary>Bindable property for <see cref="TextType"/>.</summary>
public static readonly BindableProperty TextTypeProperty = BindableProperty.Create(nameof(TextType), typeof(TextType), typeof(Label), TextType.Text,
propertyChanged: (bindable, oldvalue, newvalue) => ((Label)bindable).InvalidateMeasureInternal(InvalidationTrigger.MeasureChanged));
propertyChanged: (bindable, oldvalue, newvalue) =>
{
if (bindable is Label label)
{
#if ANDROID
label.UpdateTextType((TextType)newvalue);
#endif
label.InvalidateMeasureInternal(InvalidationTrigger.MeasureChanged);
}
});

readonly Lazy<PlatformConfigurationRegistry<Label>> _platformConfigurationRegistry;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
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;

public void setIsFormatted(boolean value) {
if (isFormatted != value) {
isFormatted = value;
requestLayout();
}
}

@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);
}
}

protected void onLayoutFormatted(boolean changed, int left, int top, int right, int bottom) {
// Overridden in C#
}
}
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 e6216a5

Please sign in to comment.