From 5ed78276aac8dbb27b3fb900438f4aeeb02bb993 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Mon, 2 Sep 2024 12:19:17 +0200 Subject: [PATCH 01/17] Improve measure invalidation and legacy Layout(s) performance - 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 --- src/Controls/src/Core/BindableObject.cs | 2 +- src/Controls/src/Core/LegacyLayouts/Layout.cs | 31 +++++---- .../src/Core/LegacyLayouts/StackLayout.cs | 8 ++- src/Controls/src/Core/Page/Page.cs | 53 ++++++++------- .../src/Core/VisualElement/VisualElement.cs | 68 +++++++++++++++++-- .../tests/Core.UnitTests/ButtonUnitTest.cs | 4 +- .../tests/Core.UnitTests/GridTests.cs | 8 +-- .../Core.UnitTests/ImageButtonUnitTest.cs | 8 +-- .../tests/Core.UnitTests/ImageTests.cs | 6 +- .../tests/Core.UnitTests/LabelTests.cs | 2 +- .../Core.UnitTests/StackLayoutUnitTests.cs | 4 +- .../tests/Core.UnitTests/ViewUnitTests.cs | 8 +-- 12 files changed, 133 insertions(+), 69 deletions(-) diff --git a/src/Controls/src/Core/BindableObject.cs b/src/Controls/src/Core/BindableObject.cs index 7bdb15610463..eb0685b3da95 100644 --- a/src/Controls/src/Core/BindableObject.cs +++ b/src/Controls/src/Core/BindableObject.cs @@ -663,7 +663,7 @@ void SetValueActual(BindableProperty property, BindablePropertyContext context, } } - void ApplyBindings(bool fromBindingContextChanged) + internal virtual void ApplyBindings(bool fromBindingContextChanged) { var prop = _properties.Values.ToArray(); diff --git a/src/Controls/src/Core/LegacyLayouts/Layout.cs b/src/Controls/src/Core/LegacyLayouts/Layout.cs index 0f3b6d0b0d7f..254943e7cbc5 100644 --- a/src/Controls/src/Core/LegacyLayouts/Layout.cs +++ b/src/Controls/src/Core/LegacyLayouts/Layout.cs @@ -192,8 +192,12 @@ private protected override IList 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); } /// @@ -294,13 +298,19 @@ public void RaiseChild(View view) OnChildrenReordered(); } + internal virtual void InvalidateLayoutInternal() + { + _hasDoneLayout = false; + InvalidateMeasureCacheInternal(); + } + /// /// Invalidates the current layout. /// /// Calling this method will invalidate the measure and triggers a new layout cycle. protected virtual void InvalidateLayout() { - _hasDoneLayout = false; + InvalidateLayoutInternal(); InvalidateMeasureInternal(InvalidationTrigger.MeasureChanged); if (!_hasDoneLayout) { @@ -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); } /// @@ -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; } @@ -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) diff --git a/src/Controls/src/Core/LegacyLayouts/StackLayout.cs b/src/Controls/src/Core/LegacyLayouts/StackLayout.cs index db341b2df4d4..8932bf274730 100644 --- a/src/Controls/src/Core/LegacyLayouts/StackLayout.cs +++ b/src/Controls/src/Core/LegacyLayouts/StackLayout.cs @@ -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); } diff --git a/src/Controls/src/Core/Page/Page.cs b/src/Controls/src/Core/Page/Page.cs index 53b083d64504..3d4ab1bd62a6 100644 --- a/src/Controls/src/Core/Page/Page.cs +++ b/src/Controls/src/Core/Page/Page.cs @@ -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)); } @@ -510,8 +535,7 @@ internal override void OnChildMeasureInvalidatedInternal(VisualElement child, In /// The event arguments. 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 } /// @@ -583,29 +607,6 @@ protected void UpdateChildrenLayout() } } - internal virtual void OnChildMeasureInvalidated(VisualElement child, InvalidationTrigger trigger) - { - var container = this as IPageContainer; - 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) diff --git a/src/Controls/src/Core/VisualElement/VisualElement.cs b/src/Controls/src/Core/VisualElement/VisualElement.cs index 24946eb7c3d5..aa074b5fa1fa 100644 --- a/src/Controls/src/Core/VisualElement/VisualElement.cs +++ b/src/Controls/src/Core/VisualElement/VisualElement.cs @@ -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; @@ -991,7 +992,7 @@ public bool IsPlatformStateConsistent internal event EventHandler PlatformEnabledChanged; /// - /// 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. /// /// For internal use only. This API can be changed or removed without notice at any time. [EditorBrowsable(EditorBrowsableState.Never)] @@ -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); + } + } + } + /// /// Invalidates the measure of an element. /// @@ -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 @@ -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); } - + + /// + /// Clears the measure cache of a visual element. + /// + internal void InvalidateMeasureCacheInternal() + { + _measureCache.Clear(); + } + internal virtual void OnChildMeasureInvalidatedInternal(VisualElement child, InvalidationTrigger trigger) { switch (trigger) @@ -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: @@ -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; diff --git a/src/Controls/tests/Core.UnitTests/ButtonUnitTest.cs b/src/Controls/tests/Core.UnitTests/ButtonUnitTest.cs index f5ffd00a3d8d..154bd8508078 100644 --- a/src/Controls/tests/Core.UnitTests/ButtonUnitTest.cs +++ b/src/Controls/tests/Core.UnitTests/ButtonUnitTest.cs @@ -10,7 +10,7 @@ public class ButtonUnitTest : VisualElementCommandSourceTests