Skip to content

Commit

Permalink
Improve measure invalidation and legacy Layout(s) performance
Browse files Browse the repository at this point in the history
- Do not bubble up `MeasureInvalidated` on pages
- Invoke `MeasureInvalidated` just once when `BindingContext` is changing
- Do not synchronously measure/arrange children on legacy layout upon child measure invalidated: simply wait for the next native layout pass
  • Loading branch information
albyrock87 committed Sep 7, 2024
1 parent bcba26f commit 3153543
Show file tree
Hide file tree
Showing 12 changed files with 133 additions and 69 deletions.
2 changes: 1 addition & 1 deletion src/Controls/src/Core/BindableObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ void SetValueActual(BindableProperty property, BindablePropertyContext context,
}
}

void ApplyBindings(bool fromBindingContextChanged)
internal virtual void ApplyBindings(bool fromBindingContextChanged)
{
var prop = _properties.Values.ToArray();

Expand Down
31 changes: 17 additions & 14 deletions src/Controls/src/Core/LegacyLayouts/Layout.cs
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,12 @@ private protected override IList<Element> LogicalChildrenInternalBackingStore
public override SizeRequest Measure(double widthConstraint, double heightConstraint, MeasureFlags flags = MeasureFlags.None)
{
SizeRequest size = base.Measure(widthConstraint - Padding.HorizontalThickness, heightConstraint - Padding.VerticalThickness, flags);
return new SizeRequest(new Size(size.Request.Width + Padding.HorizontalThickness, size.Request.Height + Padding.VerticalThickness),
new Size(size.Minimum.Width + Padding.HorizontalThickness, size.Minimum.Height + Padding.VerticalThickness));
var request = new Size(size.Request.Width + Padding.HorizontalThickness, size.Request.Height + Padding.VerticalThickness);
var minimum = new Size(size.Minimum.Width + Padding.HorizontalThickness, size.Minimum.Height + Padding.VerticalThickness);

DesiredSize = request;

return new SizeRequest(request, minimum);
}

/// <summary>
Expand Down Expand Up @@ -294,13 +298,19 @@ public void RaiseChild(View view)
OnChildrenReordered();
}

internal virtual void InvalidateLayoutInternal()
{
_hasDoneLayout = false;
InvalidateMeasureCacheInternal();
}

/// <summary>
/// Invalidates the current layout.
/// </summary>
/// <remarks>Calling this method will invalidate the measure and triggers a new layout cycle.</remarks>
protected virtual void InvalidateLayout()
{
_hasDoneLayout = false;
InvalidateLayoutInternal();
InvalidateMeasureInternal(InvalidationTrigger.MeasureChanged);
if (!_hasDoneLayout)
{
Expand All @@ -321,8 +331,8 @@ protected virtual void InvalidateLayout()

internal override void OnChildMeasureInvalidatedInternal(VisualElement child, InvalidationTrigger trigger)
{
// TODO: once we remove old Xamarin public signatures we can invoke `OnChildMeasureInvalidated(VisualElement, InvalidationTrigger)` directly
OnChildMeasureInvalidated(child, new InvalidationEventArgs(trigger));
base.OnChildMeasureInvalidatedInternal(child, trigger);
}

/// <summary>
Expand Down Expand Up @@ -503,7 +513,7 @@ internal virtual void OnChildMeasureInvalidated(VisualElement child, Invalidatio
int count = children.Count;
for (var index = 0; index < count; index++)
{
if (LogicalChildrenInternal[index] is VisualElement v && v.IsVisible && (!v.IsPlatformEnabled || !v.IsPlatformStateConsistent))
if (LogicalChildrenInternal[index] is VisualElement { IsVisible: true } v && (!v.IsPlatformEnabled || !v.IsPlatformStateConsistent))
{
return;
}
Expand All @@ -517,20 +527,13 @@ internal virtual void OnChildMeasureInvalidated(VisualElement child, Invalidatio
{
return;
}
if (trigger == InvalidationTrigger.HorizontalOptionsChanged || trigger == InvalidationTrigger.VerticalOptionsChanged)
if (trigger is InvalidationTrigger.HorizontalOptionsChanged or InvalidationTrigger.VerticalOptionsChanged)
{
ComputeConstraintForView(view);
}
}

if (trigger == InvalidationTrigger.RendererReady)
{
InvalidateMeasureInternal(InvalidationTrigger.RendererReady);
}
else
{
InvalidateMeasureInternal(InvalidationTrigger.MeasureChanged);
}
InvalidateLayoutInternal();
}

internal override void OnIsVisibleChanged(bool oldValue, bool newValue)
Expand Down
8 changes: 7 additions & 1 deletion src/Controls/src/Core/LegacyLayouts/StackLayout.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,15 @@ internal override void ComputeConstraintForView(View view)
ComputeConstraintForView(view, false);
}

internal override void InvalidateMeasureInternal(InvalidationTrigger trigger)
internal override void InvalidateLayoutInternal()
{
base.InvalidateLayoutInternal();
_layoutInformation = new LayoutInformation();
}

internal override void InvalidateMeasureInternal(InvalidationTrigger trigger)
{
InvalidateLayoutInternal();
base.InvalidateMeasureInternal(trigger);
}

Expand Down
53 changes: 27 additions & 26 deletions src/Controls/src/Core/Page/Page.cs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,32 @@ protected override void OnBindingContextChanged()

internal override void OnChildMeasureInvalidatedInternal(VisualElement child, InvalidationTrigger trigger)
{
// TODO: once we remove old Xamarin public signatures we can invoke `OnChildMeasureInvalidated(VisualElement, InvalidationTrigger)` directly
// This time we won't propagate MeasureInvalidated up to the parent
// considering the page is the top level element for this feature
switch (trigger)
{
case InvalidationTrigger.VerticalOptionsChanged:
case InvalidationTrigger.HorizontalOptionsChanged:
// When a child changes its HorizontalOptions or VerticalOptions
// the size of the parent won't change, so we don't have to invalidate the measure
break;
case InvalidationTrigger.RendererReady:
// Undefined happens in many cases, including when `IsVisible` changes
case InvalidationTrigger.Undefined:
InvokeMeasureInvalidated(trigger);
break;
default:
// When visibility changes `InvalidationTrigger.Undefined` is used,
// so here we're sure that visibility didn't change
if (child.IsVisible)
{
// We need to invalidate measures only if child is actually visible
InvokeMeasureInvalidated(trigger);
}
break;
}

// We still need to call the legacy OnChildMeasureInvalidated to keep the compatibility.
OnChildMeasureInvalidated(child, new InvalidationEventArgs(trigger));
}

Expand All @@ -510,8 +535,7 @@ internal override void OnChildMeasureInvalidatedInternal(VisualElement child, In
/// <param name="e">The event arguments.</param>
protected virtual void OnChildMeasureInvalidated(object sender, EventArgs e)
{
InvalidationTrigger trigger = (e as InvalidationEventArgs)?.Trigger ?? InvalidationTrigger.Undefined;
OnChildMeasureInvalidated((VisualElement)sender, trigger);
// Nothing to do here: platform will take care of arranging the children if needed on the next layout pass
}

/// <summary>
Expand Down Expand Up @@ -583,29 +607,6 @@ protected void UpdateChildrenLayout()
}
}

internal virtual void OnChildMeasureInvalidated(VisualElement child, InvalidationTrigger trigger)
{
var container = this as IPageContainer<Page>;
if (container != null)
{
Page page = container.CurrentPage;
if (page != null && page.IsVisible && (!page.IsPlatformEnabled || !page.IsPlatformStateConsistent))
return;
}
else
{
var logicalChildren = this.InternalChildren;
for (var i = 0; i < logicalChildren.Count; i++)
{
var v = logicalChildren[i] as VisualElement;
if (v != null && v.IsVisible && (!v.IsPlatformEnabled || !v.IsPlatformStateConsistent))
return;
}
}

InvalidateMeasureInternal(InvalidationTrigger.MeasureChanged);
}

internal void OnAppearing(Action action)
{
if (_hasAppeared)
Expand Down
68 changes: 62 additions & 6 deletions src/Controls/src/Core/VisualElement/VisualElement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Collections.Generic;
using System.ComponentModel;
using System.Globalization;
using System.Runtime.CompilerServices;
using Microsoft.Maui.Controls.Internals;
using Microsoft.Maui.Controls.Shapes;
using Microsoft.Maui.Graphics;
Expand Down Expand Up @@ -991,7 +992,7 @@ public bool IsPlatformStateConsistent
internal event EventHandler PlatformEnabledChanged;

/// <summary>
/// Gets or sets a value that indicates whether this elements's platform equivalent element is enabled.
/// Gets or sets a value that indicates whether this element's platform equivalent element is enabled.
/// </summary>
/// <remarks>For internal use only. This API can be changed or removed without notice at any time.</remarks>
[EditorBrowsable(EditorBrowsableState.Never)]
Expand Down Expand Up @@ -1345,6 +1346,25 @@ internal void InvokeFocusChangeRequested(FocusRequestArgs args) =>
FocusChangeRequested?.Invoke(this, args);
internal bool HasFocusChangeRequestedEvent => FocusChangeRequested is not null;

internal override void ApplyBindings(bool fromBindingContextChanged)
{
try
{
_isApplyingBindings = true;
base.ApplyBindings(fromBindingContextChanged);
}
finally
{
_isApplyingBindings = false;

if (_applyingBindingsInvalidationTrigger is {} invalidationTrigger)
{
_applyingBindingsInvalidationTrigger = null;
InvalidateMeasureInternal(invalidationTrigger);
}
}
}

/// <summary>
/// Invalidates the measure of an element.
/// </summary>
Expand All @@ -1354,10 +1374,38 @@ public void InvalidateMeasureNonVirtual(InvalidationTrigger trigger)
{
InvalidateMeasureInternal(trigger);
}

internal void InvokeMeasureInvalidated(InvalidationTrigger trigger)
{
MeasureInvalidated?.Invoke(this, new InvalidationEventArgs(trigger));
}

bool _isApplyingBindings;
InvalidationTrigger? _applyingBindingsInvalidationTrigger;

internal virtual void InvalidateMeasureInternal(InvalidationTrigger trigger)
{
_measureCache.Clear();
if (!IsPlatformEnabled)
{
// No need to invalidate measure if there's no platform view
return;
}

if (_isApplyingBindings)
{
if (_applyingBindingsInvalidationTrigger == null &&
trigger is InvalidationTrigger.HorizontalOptionsChanged or InvalidationTrigger.VerticalOptionsChanged)
{
_applyingBindingsInvalidationTrigger = trigger;
}
else
{
_applyingBindingsInvalidationTrigger = InvalidationTrigger.Undefined;
}
return;
}

InvalidateMeasureCacheInternal();

// TODO ezhart Once we get InvalidateArrange sorted, HorizontalOptionsChanged and
// VerticalOptionsChanged will need to call ParentView.InvalidateArrange() instead
Expand All @@ -1374,10 +1422,18 @@ internal virtual void InvalidateMeasureInternal(InvalidationTrigger trigger)
break;
}

MeasureInvalidated?.Invoke(this, new InvalidationEventArgs(trigger));
InvokeMeasureInvalidated(trigger);
(Parent as VisualElement)?.OnChildMeasureInvalidatedInternal(this, trigger);
}


/// <summary>
/// Clears the measure cache of a visual element.
/// </summary>
internal void InvalidateMeasureCacheInternal()
{
_measureCache.Clear();
}

internal virtual void OnChildMeasureInvalidatedInternal(VisualElement child, InvalidationTrigger trigger)
{
switch (trigger)
Expand All @@ -1390,7 +1446,7 @@ internal virtual void OnChildMeasureInvalidatedInternal(VisualElement child, Inv
case InvalidationTrigger.RendererReady:
// Undefined happens in many cases, including when `IsVisible` changes
case InvalidationTrigger.Undefined:
MeasureInvalidated?.Invoke(this, new InvalidationEventArgs(trigger));
InvokeMeasureInvalidated(trigger);
(Parent as VisualElement)?.OnChildMeasureInvalidatedInternal(this, trigger);
return;
default:
Expand All @@ -1399,7 +1455,7 @@ internal virtual void OnChildMeasureInvalidatedInternal(VisualElement child, Inv
if (child.IsVisible)
{
// We need to invalidate measures only if child is actually visible
MeasureInvalidated?.Invoke(this, new InvalidationEventArgs(InvalidationTrigger.MeasureChanged));
InvokeMeasureInvalidated(trigger);
(Parent as VisualElement)?.OnChildMeasureInvalidatedInternal(this, InvalidationTrigger.MeasureChanged);
}
return;
Expand Down
4 changes: 2 additions & 2 deletions src/Controls/tests/Core.UnitTests/ButtonUnitTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public class ButtonUnitTest : VisualElementCommandSourceTests<Button>
[Fact]
public void MeasureInvalidatedOnTextChange()
{
var button = new Button();
var button = new Button { IsPlatformEnabled = true };

bool fired = false;
button.MeasureInvalidated += (sender, args) => fired = true;
Expand Down Expand Up @@ -123,7 +123,7 @@ public void TestBindingContextPropagation()
public void TestImageSourcePropertiesChangedTriggerResize()
{
var source = new FileImageSource();
var button = new Button { ImageSource = source };
var button = new Button { ImageSource = source, IsPlatformEnabled = true };
bool fired = false;
button.MeasureInvalidated += (sender, e) => fired = true;
Assert.Null(source.File);
Expand Down
8 changes: 4 additions & 4 deletions src/Controls/tests/Core.UnitTests/GridTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1329,7 +1329,7 @@ public void TestEnd()
[Fact]
public void TestDefaultRowSpacing()
{
var layout = new Grid();
var layout = new Grid { IsPlatformEnabled = true };

bool preferredSizeChanged = false;
layout.MeasureInvalidated += (sender, args) =>
Expand All @@ -1349,7 +1349,7 @@ public void TestDefaultRowSpacing()
[Fact]
public void TestDefaultColumnSpacing()
{
var layout = new Grid();
var layout = new Grid { IsPlatformEnabled = true };

bool preferredSizeChanged = false;
layout.MeasureInvalidated += (sender, args) =>
Expand All @@ -1369,7 +1369,7 @@ public void TestDefaultColumnSpacing()
[Fact]
public void TestAddCell()
{
var layout = new Grid();
var layout = new Grid { IsPlatformEnabled = true };
bool preferredSizeChanged = false;
layout.MeasureInvalidated += (sender, args) => preferredSizeChanged = true;

Expand All @@ -1383,7 +1383,7 @@ public void TestAddCell()
[Fact]
public void TestMoveCell()
{
var layout = new Grid();
var layout = new Grid { IsPlatformEnabled = true };
var label = new Label();
layout.Children.Add(label, 0, 0);

Expand Down
8 changes: 4 additions & 4 deletions src/Controls/tests/Core.UnitTests/ImageButtonUnitTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public void TestFillSizingWithConstrainedWidth()
[Fact]
public void TestSizeChanged()
{
var image = new ImageButton { Source = "File0.png" };
var image = new ImageButton { Source = "File0.png", IsPlatformEnabled = true };
Assert.Equal("File0.png", ((FileImageSource)image.Source).File);

var preferredSizeChanged = false;
Expand Down Expand Up @@ -163,7 +163,7 @@ public void TestFileImageSourceChanged()
public void TestFileImageSourcePropertiesChangedTriggerResize()
{
var source = new FileImageSource();
var image = new ImageButton { Source = source };
var image = new ImageButton { Source = source, IsPlatformEnabled = true };
bool fired = false;
image.MeasureInvalidated += (sender, e) => fired = true;
Assert.Null(source.File);
Expand All @@ -176,7 +176,7 @@ public void TestFileImageSourcePropertiesChangedTriggerResize()
public void TestStreamImageSourcePropertiesChangedTriggerResize()
{
var source = new StreamImageSource();
var image = new ImageButton { Source = source };
var image = new ImageButton { Source = source, IsPlatformEnabled = true };
bool fired = false;
EventHandler eventHandler = (sender, e) => fired = true;
image.MeasureInvalidated += eventHandler;
Expand Down Expand Up @@ -370,7 +370,7 @@ public void TestBindingContextPropagation()
public void TestImageSourcePropertiesChangedTriggerResize()
{
var source = new FileImageSource();
var button = new ImageButton { Source = source };
var button = new ImageButton { Source = source, IsPlatformEnabled = true };
bool fired = false;
button.MeasureInvalidated += (sender, e) => fired = true;
Assert.Null(source.File);
Expand Down
Loading

0 comments on commit 3153543

Please sign in to comment.