Skip to content

Commit

Permalink
Merge pull request #1138 from unoplatform/dev/cdb/autolayout/cycle-de…
Browse files Browse the repository at this point in the history 
…tected

Prevent AutoLayout from causing an invalidation loop on WinUI
  • Loading branch information
@carldebilly
carldebilly authored May 16, 2024
2 parents f906a5e + 8a877e1 commit d74db9d
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 67 deletions.
4 changes: 2 additions & 2 deletions src/Uno.Toolkit.RuntimeTests/Tests/AutoLayoutTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ public async Task When_PrimaryAxisAlignment_Centered_And_Filled_Elements_With_Ma

var border2 = new Border()
{
Background = new SolidColorBrush(Color.FromArgb(255, 0, 0, 255)),
Background = new SolidColorBrush(Color.FromArgb(255, 255, 0, 0)),
MaxHeight = 100,
MaxWidth = 100,
};
Expand Down Expand Up @@ -547,7 +547,7 @@ public async Task When_Stretched_PrimaryAlignment_In_ScrollViewer()

var border2 = new Border()
{
Background = new SolidColorBrush(Color.FromArgb(255, 0, 0, 255)),
Background = new SolidColorBrush(Color.FromArgb(255, 255, 0, 0)),
MinHeight = 25,
};

Expand Down
71 changes: 43 additions & 28 deletions src/Uno.Toolkit.UI/Controls/AutoLayout/AutoLayout.Layouting.Arrange.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,23 @@ namespace Uno.Toolkit.UI;

partial class AutoLayout
{
private bool _isArranging;

protected override Size ArrangeOverride(Size finalSize)
{
_isArranging = true;

try
{
return InnerArrange(finalSize);
}
finally
{
_isArranging = false;
}
}

private Size InnerArrange(in Size finalSize)
{
// Local cache of DependencyProperties
var children = Children;
Expand All @@ -40,20 +56,17 @@ protected override Size ArrangeOverride(Size finalSize)
var startPadding = isHorizontal ? padding.Left : padding.Top;
var endPadding = isHorizontal ? padding.Right : padding.Bottom;

if (_calculatedChildren is null)
if (_calculatedChildren is null || children.Count != _calculatedChildren.Length)
{
// If the panel has not been measured yet, we need to measure it now.
InvalidateMeasure();
return finalSize;
}

if (children.Count != _calculatedChildren.Length
|| finalSize != DesiredSize)
if (finalSize != DesiredSize)
{
// If the number of children has changed, or the final size if different than
// what was measured, we need to re-measure the children using the new final size.
// This usually happens when the panel is used in a ScrollViewer, and the ScrollViewer
// is measuring the panel with an infinite size, and then arranging it with a finite size.
// The _calculatedChildren is calculated for the available size, so we need to re-measure it
// using the new final size.
MeasureOverride(finalSize);
}

Expand Down Expand Up @@ -144,7 +157,7 @@ protected override Size ArrangeOverride(Size finalSize)
? filledChildrenSize + (filledMaxSurplus / (numberOfFilledChildren - numberOfFilledChildrenWithMax))
: filledChildrenSize + GetChildrenLowerThanAllocateSurplus(_calculatedChildren, filledChildrenSize);

EnsureZeroFloor(ref filledChildrenSizeAfterMaxSize);
EnsureZeroFloor(ref filledChildrenSizeAfterMaxSize);

// Establish actual inter-element spacing.
var spacingOffset = justify == AutoLayoutJustify.SpaceBetween && !atLeastOneFilledChild
Expand Down Expand Up @@ -260,7 +273,7 @@ protected override Size ArrangeOverride(Size finalSize)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void ApplyMinMaxValues(UIElement element, Orientation orientation, ref Size desiredSize)
private static void ApplyMinMaxValues(in UIElement element, in Orientation orientation, ref Size desiredSize)
{
if (element is not FrameworkElement frameworkElement)
{
Expand Down Expand Up @@ -305,11 +318,11 @@ private static void ApplyMinMaxValues(UIElement element, Orientation orientation

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static double ComputeCounterAlignmentOffset(
AutoLayoutAlignment? counterAlignment,
double childCounterLength,
double availableCounterLength,
Thickness padding,
bool isHorizontal)
in AutoLayoutAlignment? counterAlignment,
in double childCounterLength,
in double availableCounterLength,
in Thickness padding,
in bool isHorizontal)
{
switch (counterAlignment)
{
Expand All @@ -332,9 +345,9 @@ private static double ComputeCounterAlignmentOffset(

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static double ComputeSpaceBetween(
double availableSizeForStackedChildren,
double totalNonFilledStackedSize,
int numberOfStackedChildren)
in double availableSizeForStackedChildren,
in double totalNonFilledStackedSize,
in int numberOfStackedChildren)
{
var availableSizeForStackedSpaceBetween = availableSizeForStackedChildren - totalNonFilledStackedSize;

Expand All @@ -345,16 +358,16 @@ private static double ComputeSpaceBetween(

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static double PrimaryAxisAlignmentOffsetSize(
double totalOfFillMaxSize,
AutoLayoutAlignment autoLayoutAlignment,
double availableSizeForStackedChildren,
double totalNonFilledStackedSize,
bool haveMoreFilled,
double spacing,
int numberOfStackedChildren,
double filledChildrenSizeAfterMaxSize,
double startPadding,
double endPadding)
in double totalOfFillMaxSize,
in AutoLayoutAlignment autoLayoutAlignment,
in double availableSizeForStackedChildren,
in double totalNonFilledStackedSize,
in bool haveMoreFilled,
in double spacing,
in int numberOfStackedChildren,
in double filledChildrenSizeAfterMaxSize,
in double startPadding,
in double endPadding)
{
if (haveMoreFilled || autoLayoutAlignment is AutoLayoutAlignment.Start)
{
Expand Down Expand Up @@ -384,7 +397,9 @@ private static double PrimaryAxisAlignmentOffsetSize(
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static double GetChildrenLowerThanAllocateSurplus(CalculatedChildren[] calculatedChildren, double filledChildrenSizeBeforeMaxSize)
private static double GetChildrenLowerThanAllocateSurplus(
in Span<CalculatedChildren> calculatedChildren,
in double filledChildrenSizeBeforeMaxSize)
{
int countFilledChildrenGreaterThanAllocated = 0;
double sumSurplusFromLower = 0d;
Expand Down
82 changes: 47 additions & 35 deletions src/Uno.Toolkit.UI/Controls/AutoLayout/AutoLayout.Layouting.Measure.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,15 @@ protected override Size MeasureOverride(Size availableSize)
{
Orientation.Horizontal => new Size(availableSize.Width, desiredCounterSize + counterPaddingSize),
Orientation.Vertical => new Size(desiredCounterSize + counterPaddingSize, availableSize.Height),
_ => throw new ArgumentOutOfRangeException(),
_ => throw new InvalidOperationException(),
};
}
else
{
// 8b. Calculate the desired size of the panel, when there's at least one filled child
var stackedChildrenDesiredSize =
_calculatedChildren!
.Where(child => child.IsVisible)
.Where(child => child.IsVisible && child.Role != AutoLayoutRole.Filled)
.Select(c => c.MeasuredLength)
.Sum()
+ totalSpacingSize;
Expand All @@ -111,7 +111,7 @@ protected override Size MeasureOverride(Size availableSize)
Orientation.Vertical => new Size(
width: desiredCounterSize + counterPaddingSize,
height: desiredSizeInPrimaryOrientation + paddingSize),
_ => throw new ArgumentOutOfRangeException(),
_ => throw new InvalidOperationException(),
};
}

Expand Down Expand Up @@ -146,7 +146,7 @@ private void ApplyMinMaxValues(ref Size desiredSize)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private (int, bool) EstablishChildrenRoles(IList<UIElement> children, bool filledAsHug, Orientation orientation)
private (int, bool) EstablishChildrenRoles(in IList<UIElement> children, in bool filledAsHug, in Orientation orientation)
{
if(_calculatedChildren == null || _calculatedChildren.Length != children.Count)
{
Expand Down Expand Up @@ -208,11 +208,11 @@ private void ApplyMinMaxValues(ref Size desiredSize)

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void MeasureFixedChildren(
Orientation orientation,
double availableCounterSize,
in Orientation orientation,
in double availableCounterSize,
ref double remainingSize,
ref double desiredCounterSize,
double counterPaddingSize)
in double counterPaddingSize)
{
for (var i = 0; i < _calculatedChildren!.Length; i++)
{
Expand Down Expand Up @@ -240,11 +240,11 @@ private void MeasureFixedChildren(

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void MeasureHugChildren(
Orientation orientation,
double availableCounterSize,
in Orientation orientation,
in double availableCounterSize,
ref double remainingSize,
ref double desiredCounterSize,
double counterPaddingSize)
in double counterPaddingSize)
{
for (var i = 0; i < _calculatedChildren!.Length; i++)
{
Expand Down Expand Up @@ -273,11 +273,11 @@ private void MeasureHugChildren(

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool MeasureFilledChildren(
Orientation orientation,
double availableCounterSize,
in Orientation orientation,
in double availableCounterSize,
ref double remainingSize,
ref double desiredCounterSize,
double counterPaddingSize)
in double counterPaddingSize)
{
if (double.IsInfinity(remainingSize))
{
Expand Down Expand Up @@ -314,37 +314,42 @@ private bool MeasureFilledChildren(

MeasureChild(child.Element, orientation, filledSize, availableCounterSize, ref desiredCounterSize, counterPaddingSize, CounterAxisAlignment);

// MeasuredLength meaning for Filled children is the maximum length of the element,
// which could be the fixed size or the max size defined on the element.
if (child.Element is FrameworkElement fe)
{
var maxLength = fe.GetMaxLength(orientation);
var length = fe.GetPrimaryLength(orientation);

if (maxLength.IsFinite())
if (length.IsFinite())
{
child.MeasuredLength = length.IsFinite() ? Math.Min(maxLength, length) : maxLength;
}
else if (length.IsFinite())
{
child.MeasuredLength = length;
if (maxLength.IsFinite())
{
child.MeasuredLength = length.IsFinite() ? Math.Min(maxLength, length) : maxLength;
}
else
{
child.MeasuredLength = length;
}
}
else
{
child.MeasuredLength = double.MaxValue;
child.MeasuredLength = maxLength.IsFinite() ? maxLength : double.MaxValue;
}
}
}

return true; // at least one filled child
}

private static double MeasureChild(
UIElement child,
Orientation orientation,
double availableSize,
double availableCounterSize,
private double MeasureChild(
in UIElement child,
in Orientation orientation,
in double availableSize,
in double availableCounterSize,
ref double desiredCounterSize,
double counterPaddingSize,
AutoLayoutAlignment counterAxisAlignment)
in double counterPaddingSize,
in AutoLayoutAlignment counterAxisAlignment)
{
var isOrientationHorizontal = orientation is Orientation.Horizontal;
var isPrimaryAlignmentStretch = GetPrimaryAlignment(child) is AutoLayoutPrimaryAlignment.Stretch;
Expand All @@ -371,7 +376,10 @@ private static double MeasureChild(
? new Size(availableSize, availableCounterSize - (isCounterAlignmentStretch ? counterPaddingSize : 0))
: new Size(availableCounterSize - (isCounterAlignmentStretch ? counterPaddingSize : 0), availableSize);

child.Measure(availableSizeForChild);
if (!_isArranging)
{
child.Measure(availableSizeForChild);
}

double desiredSize;
if (isOrientationHorizontal)
Expand All @@ -390,7 +398,7 @@ private static double MeasureChild(
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void Decrement(ref double value, double decrement)
private static void Decrement(ref double value, in double decrement)
{
if (double.IsInfinity(value))
{
Expand All @@ -411,16 +419,16 @@ private static void EnsureZeroFloor(ref double sizeValue)
}

private double MeasureIndependentChildren(
Size availableSize,
Thickness borderThickness,
Orientation orientation,
in Size availableSize,
in Thickness borderThickness,
in Orientation orientation,
ref double desiredCounterSize)
{
var resultSize = new Size();
var anyIndependent = false;

// Actual available size for independent children is the available size minus the border thickness
availableSize = new Size(
var availableSizeIncludingBorder = new Size(
availableSize.Width - (borderThickness.Left + borderThickness.Right),
availableSize.Height - (borderThickness.Top + borderThickness.Bottom));

Expand All @@ -435,7 +443,11 @@ private double MeasureIndependentChildren(

anyIndependent = true;

child.Element.Measure(availableSize);
if (!_isArranging)
{
child.Element.Measure(availableSizeIncludingBorder);
}

var desiredSize = child.Element.DesiredSize;

// Adjust resulting desired size to the largest of the children
Expand Down Expand Up @@ -509,7 +521,7 @@ private enum AutoLayoutRole : byte
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void UpdateCounterAlignment(ref FrameworkElement frameworkElement, bool isHorizontal, bool isPrimaryAlignmentStretch, AutoLayoutAlignment counterAlignment)
private static void UpdateCounterAlignment(ref FrameworkElement frameworkElement, in bool isHorizontal, in bool isPrimaryAlignmentStretch, in AutoLayoutAlignment counterAlignment)
{
if (isHorizontal)
{
Expand Down
11 changes: 9 additions & 2 deletions src/Uno.Toolkit.UI/Controls/AutoLayout/AutoLayout.Properties.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public AutoLayoutAlignment CounterAxisAlignment
"PrimaryAlignment",
typeof(AutoLayoutPrimaryAlignment),
typeof(AutoLayout),
new PropertyMetadata(default(AutoLayoutPrimaryAlignment), propertyChangedCallback: InvalidateArrangeCallback));
new PropertyMetadata(default(AutoLayoutPrimaryAlignment), propertyChangedCallback: InvalidateParentAutoLayoutArrangeCallback));

[DynamicDependency(nameof(GetPrimaryAlignment))]
public static void SetPrimaryAlignment(DependencyObject element, AutoLayoutPrimaryAlignment value)
Expand All @@ -116,7 +116,7 @@ public static AutoLayoutPrimaryAlignment GetPrimaryAlignment(DependencyObject el
"CounterAlignment",
typeof(AutoLayoutAlignment),
typeof(AutoLayout),
new PropertyMetadata(default(AutoLayoutAlignment), propertyChangedCallback: InvalidateArrangeCallback));
new PropertyMetadata(default(AutoLayoutAlignment), propertyChangedCallback: InvalidateParentAutoLayoutArrangeCallback));

[DynamicDependency(nameof(GetCounterAlignment))]
public static void SetCounterAlignment(DependencyObject element, AutoLayoutAlignment value)
Expand Down Expand Up @@ -205,4 +205,11 @@ private static void InvalidateArrangeCallback(DependencyObject d, DependencyProp
element.InvalidateArrange();
}
}
private static void InvalidateParentAutoLayoutArrangeCallback(DependencyObject d, DependencyPropertyChangedEventArgs e)
{
if (d is FrameworkElement { Parent: AutoLayout al })
{
al.InvalidateArrange();
}
}
}

0 comments on commit d74db9d

Please sign in to comment.