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 scrollable views and pages
- 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 6, 2024
1 parent bcba26f commit a8780b1
Show file tree
Hide file tree
Showing 14 changed files with 109 additions and 66 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
5 changes: 5 additions & 0 deletions src/Controls/src/Core/Items/ItemsView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -224,5 +224,10 @@ protected override void OnBindingContextChanged()
if (InternalItemsLayout is BindableObject bo)
SetInheritedBindingContext(bo, BindingContext);
}

internal override void OnChildMeasureInvalidatedInternal(VisualElement child, InvalidationTrigger trigger)
{
// No need to trigger and propagate MeasureInvalidated given this is a scrollable area
}
}
}
5 changes: 5 additions & 0 deletions src/Controls/src/Core/ItemsView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,10 @@ static void OnItemsSourceChanged(BindableObject bindable, object oldValue, objec
}

protected virtual bool ValidateItemTemplate(DataTemplate template) => true;

internal override void OnChildMeasureInvalidatedInternal(VisualElement child, InvalidationTrigger trigger)
{
// No need to trigger and propagate MeasureInvalidated given this is a scrollable area
}
}
}
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
28 changes: 2 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,7 @@ 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
// No need to trigger and propagate MeasureInvalidated considering Page is the root node
OnChildMeasureInvalidated(child, new InvalidationEventArgs(trigger));
}

Expand All @@ -510,8 +510,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 +582,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
56 changes: 53 additions & 3 deletions src/Controls/src/Core/VisualElement/VisualElement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,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 +1345,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 @@ -1355,9 +1374,32 @@ public void InvalidateMeasureNonVirtual(InvalidationTrigger trigger)
InvalidateMeasureInternal(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 @@ -1377,7 +1419,15 @@ internal virtual void InvalidateMeasureInternal(InvalidationTrigger trigger)
MeasureInvalidated?.Invoke(this, new InvalidationEventArgs(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 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
6 changes: 3 additions & 3 deletions src/Controls/tests/Core.UnitTests/ImageTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void TestFillSizingWithConstrainedWidth()
[Fact]
public void TestSizeChanged()
{
var image = new Image { Source = "File0.png" };
var image = new Image { Source = "File0.png", IsPlatformEnabled = true };
Assert.Equal("File0.png", ((FileImageSource)image.Source).File);

var preferredSizeChanged = false;
Expand Down Expand Up @@ -165,7 +165,7 @@ public void TestFileImageSourceChanged()
public void TestFileImageSourcePropertiesChangedTriggerResize()
{
var source = new FileImageSource();
var image = new Image { Source = source };
var image = new Image { Source = source, IsPlatformEnabled = true };
bool fired = false;
image.MeasureInvalidated += (sender, e) => fired = true;
Assert.Null(source.File);
Expand All @@ -178,7 +178,7 @@ public void TestFileImageSourcePropertiesChangedTriggerResize()
public void TestStreamImageSourcePropertiesChangedTriggerResize()
{
var source = new StreamImageSource();
var image = new Image { Source = source };
var image = new Image { Source = source, IsPlatformEnabled = true };
bool fired = false;
image.MeasureInvalidated += (sender, e) => fired = true;
Assert.Null(source.Stream);
Expand Down
2 changes: 1 addition & 1 deletion src/Controls/tests/Core.UnitTests/LabelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void TextAndAttributedTextMutuallyExclusive()
[Fact]
public void InvalidateMeasureWhenTextChanges()
{
var label = new Label();
var label = new Label { IsPlatformEnabled = true };

bool fired;
label.MeasureInvalidated += (sender, args) =>
Expand Down
Loading

0 comments on commit a8780b1

Please sign in to comment.