From 9e1c35bc5dfe5e76f38c7f6cc104409c9bbb3d79 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen Date: Sun, 26 Jul 2026 23:33:01 +1000 Subject: [PATCH] fix(grid): register assigned definition collections with their shared size group (#21848) * test(grid): reproduce shared size groups ignoring assigned definitions Definitions supplied through the ColumnDefinitions or RowDefinitions setter are already in the collection when the grid claims it, so they never pass through the collection-changed handler that joins them to the parent tree. They never register with their shared size group, and the definitions they replace never unregister from it. * fix(grid): join assigned definition collections to the parent tree DefinitionList.SetParent assigned each definition's Parent but never called OnEnterParentTree, which only ran from the collection-changed handler. Assigning Parent is not sufficient: OnEnterParentTree also sets InheritanceParent, and a definition cannot read the inherited PrivateSharedSizeScope that registers it with its group until that link exists. Definitions supplied through the ColumnDefinitions setter - an object initializer, a shared resource, or ColumnDefinitions="Auto,*" - were therefore silently absent from their shared size group. Enter and exit the parent tree from SetParent, and release the outgoing collection when Grid swaps one in. Without that release the replaced definitions stay registered with the group; nothing resets their measured minimum any more, so they pin it at whatever they last contributed. * test(grid): cover the definition ownership contract Removing a definition leaves it holding its old Parent and its property inheritance link, so it still reads the grid's shared size scope and can re-register itself into a scope it has left. Also covers moving a definition between grids, reassigning the same collection, and row definitions, which the assignment fix reached but nothing exercised. * refactor(grid): centralise definition parent-tree transitions Definition ownership was implemented twice, and the two paths disagreed: SetParent exited a definition and cleared its Parent, while removing one from the collection called OnExitParentTree but left Parent set. Detach was incomplete either way, since OnEnterParentTree establishes InheritanceParent but OnExitParentTree never cleared it - so a removed definition kept reading the grid's inherited PrivateSharedSizeScope, and the grid kept it alive as an inheritance child. Route every owner change through one transition that exits the old tree, assigns Parent, and enters the new one, and clear InheritanceParent on exit so detach mirrors attach. --------- Co-authored-by: Julien Lebosquain --- src/Avalonia.Controls/DefinitionBase.cs | 4 + src/Avalonia.Controls/DefinitionList.cs | 50 +++- src/Avalonia.Controls/Grid.cs | 12 + .../Avalonia.Controls.UnitTests/GridTests.cs | 238 ++++++++++++++++++ 4 files changed, 292 insertions(+), 12 deletions(-) diff --git a/src/Avalonia.Controls/DefinitionBase.cs b/src/Avalonia.Controls/DefinitionBase.cs index 020f6996aa..b7effa6232 100644 --- a/src/Avalonia.Controls/DefinitionBase.cs +++ b/src/Avalonia.Controls/DefinitionBase.cs @@ -60,6 +60,10 @@ namespace Avalonia.Controls } Parent?.InvalidateMeasure(); + + // while this link survives the definition still reads the grid's inherited + // PrivateSharedSizeScope, and the grid holds it as an inheritance child. + InheritanceParent = null; } /// diff --git a/src/Avalonia.Controls/DefinitionList.cs b/src/Avalonia.Controls/DefinitionList.cs index 63a54731e0..aa2f4f4080 100644 --- a/src/Avalonia.Controls/DefinitionList.cs +++ b/src/Avalonia.Controls/DefinitionList.cs @@ -25,17 +25,53 @@ namespace Avalonia.Controls private void SetParent(Grid? value) { + if (_parent == value) + { + return; + } + _parent = value; + // definitions already present when the grid claims the collection never pass through + // OnCollectionChanged, so they have to change trees here. var idx = 0; foreach (T definition in this) { - definition.Parent = value; + SetDefinitionParent(definition, value); definition.Index = idx++; } } + /// + /// Moves a definition from its current parent tree to . Every route + /// that changes a definition's owner goes through here. + /// + /// + /// Ownership is more than the Parent pointer: entering a tree also establishes the property + /// inheritance link a definition needs to see its shared size scope, and leaving one releases + /// that link and its shared size registration. + /// + private static void SetDefinitionParent(DefinitionBase definition, Grid? parent) + { + if (definition.Parent == parent) + { + return; + } + + if (definition.Parent is not null) + { + definition.OnExitParentTree(); + } + + definition.Parent = parent; + + if (parent is not null) + { + definition.OnEnterParentTree(); + } + } + internal void OnCollectionChanged(object? sender, NotifyCollectionChangedEventArgs e) { var idx = 0; @@ -62,17 +98,7 @@ namespace Avalonia.Controls for (var i = 0; i < count; i++) { - var definition = (DefinitionBase) items[i]!; - - if (wasRemoved) - { - definition.OnExitParentTree(); - } - else - { - definition.Parent = Parent; - definition.OnEnterParentTree(); - } + SetDefinitionParent((DefinitionBase)items[i]!, wasRemoved ? null : Parent); } } } diff --git a/src/Avalonia.Controls/Grid.cs b/src/Avalonia.Controls/Grid.cs index 992cc38ecc..2e15b07463 100644 --- a/src/Avalonia.Controls/Grid.cs +++ b/src/Avalonia.Controls/Grid.cs @@ -198,6 +198,12 @@ namespace Avalonia.Controls set { if (_extData == null) { _extData = new ExtendedData(); } + // otherwise the outgoing definitions stay registered with their shared size + // group and keep contributing to its minimum. + if (_extData.ColumnDefinitions is { } oldDefinitions && !ReferenceEquals(oldDefinitions, value)) + { + oldDefinitions.Parent = null; + } _extData.ColumnDefinitions = value; _extData.ColumnDefinitions.Parent = this; InvalidateMeasure(); @@ -220,6 +226,12 @@ namespace Avalonia.Controls set { if (_extData == null) { _extData = new ExtendedData(); } + // otherwise the outgoing definitions stay registered with their shared size + // group and keep contributing to its minimum. + if (_extData.RowDefinitions is { } oldDefinitions && !ReferenceEquals(oldDefinitions, value)) + { + oldDefinitions.Parent = null; + } _extData.RowDefinitions = value; _extData.RowDefinitions.Parent = this; InvalidateMeasure(); diff --git a/tests/Avalonia.Controls.UnitTests/GridTests.cs b/tests/Avalonia.Controls.UnitTests/GridTests.cs index 435bb79cf3..3a9a8cdaee 100644 --- a/tests/Avalonia.Controls.UnitTests/GridTests.cs +++ b/tests/Avalonia.Controls.UnitTests/GridTests.cs @@ -1309,6 +1309,244 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(10, plainGrid.ColumnDefinitions[0].ActualWidth); } + [Fact] + public void Shared_Size_Group_Is_Registered_For_Definitions_Assigned_As_A_Collection() + { + // Definitions supplied through the ColumnDefinitions setter - an object initializer, a + // shared resource, or ColumnDefinitions="Auto,*" - are already in the collection when the + // grid claims it, so they never pass through the collection-changed handler that joins + // them to the parent tree. + var grids = new[] + { + new Grid + { + ColumnDefinitions = new ColumnDefinitions + { + new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }, + new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }, + }, + }, + new Grid + { + ColumnDefinitions = new ColumnDefinitions + { + new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }, + new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }, + }, + }, + }; + grids[0].Children.Add(new Border { Width = 50, Height = 10 }); + + var scope = new StackPanel + { + [Grid.IsSharedSizeScopeProperty] = true, + Children = { grids[0], grids[1] }, + }; + var root = new TestRoot(scope); + + root.ExecuteInitialLayoutPass(); + // Shared groups validate after layout and apply any resulting invalidation on the next pass. + root.LayoutManager.ExecuteLayoutPass(); + + Assert.Equal(50, grids[0].ColumnDefinitions[0].ActualWidth); + Assert.Equal(50, grids[1].ColumnDefinitions[0].ActualWidth); + } + + [Fact] + public void Replacing_Definition_Collection_Releases_Its_Shared_Size_Group() + { + // The outgoing definitions are no longer reachable from the grid, so nothing resets their + // measured minimum. Left registered, they keep the group pinned at whatever size they + // last contributed. + var grid = new Grid(); + grid.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }); + grid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + grid.Children.Add(new Border { Width = 50, Height = 10 }); + + var other = new Grid(); + other.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }); + other.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + + var scope = new StackPanel + { + [Grid.IsSharedSizeScopeProperty] = true, + Children = { grid, other }, + }; + var root = new TestRoot(scope); + + root.ExecuteInitialLayoutPass(); + // Shared groups validate after layout and apply any resulting invalidation on the next pass. + root.LayoutManager.ExecuteLayoutPass(); + Assert.Equal(50, other.ColumnDefinitions[0].ActualWidth); + + grid.ColumnDefinitions = new ColumnDefinitions + { + new ColumnDefinition { Width = GridLength.Auto }, + new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }, + }; + root.LayoutManager.ExecuteLayoutPass(); + root.LayoutManager.ExecuteLayoutPass(); + + Assert.Equal(0, other.ColumnDefinitions[0].ActualWidth); + } + + [Fact] + public void Removing_Definition_Detaches_It_From_The_Grid() + { + var shared = new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }; + var grid = new Grid(); + grid.ColumnDefinitions.Add(shared); + grid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + grid.Children.Add(new Border { Width = 50, Height = 10 }); + + var other = new Grid(); + other.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }); + other.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + + var scope = new StackPanel + { + [Grid.IsSharedSizeScopeProperty] = true, + Children = { grid, other }, + }; + var root = new TestRoot(scope); + + root.ExecuteInitialLayoutPass(); + root.LayoutManager.ExecuteLayoutPass(); + Assert.Equal(50, other.ColumnDefinitions[0].ActualWidth); + + grid.ColumnDefinitions.Remove(shared); + root.LayoutManager.ExecuteLayoutPass(); + root.LayoutManager.ExecuteLayoutPass(); + Assert.Equal(0, other.ColumnDefinitions[0].ActualWidth); + Assert.Null(shared.Parent); + + // A definition that has left the grid must no longer see the grid's scope. + shared.SharedSizeGroup = null; + shared.SharedSizeGroup = "A"; + root.LayoutManager.ExecuteLayoutPass(); + root.LayoutManager.ExecuteLayoutPass(); + Assert.Equal(0, other.ColumnDefinitions[0].ActualWidth); + } + + [Fact] + public void Moving_Definition_Between_Grids_Moves_Its_Shared_Size_Registration() + { + var shared = new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }; + var source = new Grid(); + source.ColumnDefinitions.Add(shared); + source.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + source.Children.Add(new Border { Width = 50, Height = 10 }); + + var sourcePartner = new Grid(); + sourcePartner.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }); + sourcePartner.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + + var target = new Grid(); + target.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + + var targetPartner = new Grid(); + targetPartner.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }); + targetPartner.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + targetPartner.Children.Add(new Border { Width = 20, Height = 10 }); + + var sourceScope = new StackPanel + { + [Grid.IsSharedSizeScopeProperty] = true, + Children = { source, sourcePartner }, + }; + var targetScope = new StackPanel + { + [Grid.IsSharedSizeScopeProperty] = true, + Children = { target, targetPartner }, + }; + var root = new TestRoot(new StackPanel { Children = { sourceScope, targetScope } }); + + root.ExecuteInitialLayoutPass(); + root.LayoutManager.ExecuteLayoutPass(); + Assert.Equal(50, sourcePartner.ColumnDefinitions[0].ActualWidth); + Assert.Equal(20, targetPartner.ColumnDefinitions[0].ActualWidth); + + source.ColumnDefinitions.Remove(shared); + target.ColumnDefinitions.Insert(0, shared); + root.LayoutManager.ExecuteLayoutPass(); + root.LayoutManager.ExecuteLayoutPass(); + + Assert.Same(target, shared.Parent); + Assert.Equal(0, sourcePartner.ColumnDefinitions[0].ActualWidth); + Assert.Equal(20, target.ColumnDefinitions[0].ActualWidth); + } + + [Fact] + public void Reassigning_The_Same_Definition_Collection_Is_Inert() + { + var grid = new Grid(); + grid.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }); + grid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + grid.Children.Add(new Border { Width = 50, Height = 10 }); + + var other = new Grid(); + other.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" }); + other.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + + var scope = new StackPanel + { + [Grid.IsSharedSizeScopeProperty] = true, + Children = { grid, other }, + }; + var root = new TestRoot(scope); + + root.ExecuteInitialLayoutPass(); + root.LayoutManager.ExecuteLayoutPass(); + Assert.Equal(50, other.ColumnDefinitions[0].ActualWidth); + + var definitions = grid.ColumnDefinitions; + grid.ColumnDefinitions = definitions; + root.LayoutManager.ExecuteLayoutPass(); + root.LayoutManager.ExecuteLayoutPass(); + + Assert.Same(definitions, grid.ColumnDefinitions); + Assert.Equal(50, other.ColumnDefinitions[0].ActualWidth); + } + + [Fact] + public void Shared_Size_Group_Is_Registered_For_Row_Definitions_Assigned_As_A_Collection() + { + var grids = new[] + { + new Grid + { + RowDefinitions = new RowDefinitions + { + new RowDefinition { Height = GridLength.Auto, SharedSizeGroup = "A" }, + new RowDefinition { Height = new GridLength(1, GridUnitType.Star) }, + }, + }, + new Grid + { + RowDefinitions = new RowDefinitions + { + new RowDefinition { Height = GridLength.Auto, SharedSizeGroup = "A" }, + new RowDefinition { Height = new GridLength(1, GridUnitType.Star) }, + }, + }, + }; + grids[0].Children.Add(new Border { Width = 10, Height = 50 }); + + var scope = new StackPanel + { + Orientation = Layout.Orientation.Horizontal, + [Grid.IsSharedSizeScopeProperty] = true, + Children = { grids[0], grids[1] }, + }; + var root = new TestRoot(scope); + + root.ExecuteInitialLayoutPass(); + root.LayoutManager.ExecuteLayoutPass(); + + Assert.Equal(50, grids[0].RowDefinitions[0].ActualHeight); + Assert.Equal(50, grids[1].RowDefinitions[0].ActualHeight); + } + [Fact] public void Collection_Changes_Are_Tracked() {