Skip to content

fix(grid): register assigned definition collections with their shared size group - #21848

Merged
MrJul merged 5 commits into
AvaloniaUI:mainfrom
NathanDrake2406:nathan/fix-definitionlist-setparent
Jul 26, 2026
Merged

fix(grid): register assigned definition collections with their shared size group#21848
MrJul merged 5 commits into
AvaloniaUI:mainfrom
NathanDrake2406:nathan/fix-definitionlist-setparent

Conversation

@NathanDrake2406

Copy link
Copy Markdown
Contributor

What does the pull request do?

Makes Grid join a definition collection to the parent tree when the collection is assigned, rather than only when definitions are added to an already-parented collection. Without this, definitions supplied through the ColumnDefinitions / RowDefinitions setter are silently absent from their SharedSizeGroup.

Found while addressing review feedback on #21837.

What is the current behavior?

DefinitionList.SetParent assigns each definition's Parent but never calls OnEnterParentTree, which only runs from the collection-changed handler. Definitions already present when the grid claims the collection therefore never join the parent tree.

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. The failure is silent: the grid lays out correctly in every other respect, and only the group membership is missing.

This affects every route through the setter:

new Grid
{
    ColumnDefinitions = new ColumnDefinitions
    {
        new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" },
    },
}

as well as ColumnDefinitions="Auto,*" and a ColumnDefinitions supplied as a shared resource.

The reverse case leaks. When a grid swaps in a new collection, the outgoing definitions keep their registration. They are no longer reachable from the grid, so nothing resets their measured minimum, and they pin the group at whatever size they last contributed — a shared column stays wide after the definitions that made it wide are gone.

What is the updated/expected behavior with this PR?

Definitions assigned as a collection register with their shared size group, and definitions replaced by a new collection unregister from it.

Validation on macOS ARM64 with .NET 10:

  • Added Shared_Size_Group_Is_Registered_For_Definitions_Assigned_As_A_Collection and Replacing_Definition_Collection_Releases_Its_Shared_Size_Group. Each fails against unmodified sources on its own defect (0 instead of 50; 50 instead of 0) and passes with the fix.
  • GridTests: 95 passed, 0 failed.
  • Avalonia.Controls.UnitTests: 3,651 total, 3,650 passed, 1 pre-existing skip, 0 failed.
  • Avalonia.Markup.Xaml.UnitTests: 591 total, 588 passed, 3 skipped, 0 failed.
  • Avalonia.Base.UnitTests: 2,994 total, 2,982 passed, 12 skipped, 0 failed.
  • Avalonia.Markup.UnitTests: 252 passed, 0 failed.

How was the solution implemented (if it's not obvious)?

SetParent now exits the old parent tree and enters the new one, so a definition's tree membership follows the collection however it was populated. It returns early when the parent is unchanged, which keeps the Grid setter's unconditional Parent = this idempotent.

Grid releases the previous collection before adopting a new one. That release is what unregisters the outgoing definitions, so it depends on the SetParent change.

The commits follow the repository's bug-fix convention: the first adds the failing behavioral tests, the second contains the fix.

Checklist

  • Added unit tests (if possible)?
  • Added XML documentation to any related classes? No public API was added or changed.
  • Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation. No documentation change is needed; this restores the behavior the existing documentation already describes.

Breaking changes

None. Both changes affect internal parent-tree bookkeeping only.

Obsoletions / Deprecations

None.

Fixed issues

None filed for this specific path. It may be the underlying cause of some reports of SharedSizeGroup not applying, but I have not verified any individual issue against it.

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.
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.
@MrJul MrJul added bug backport-candidate-12.1.x Consider this PR for backporting to 12.1 branch labels Jul 24, 2026
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.
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.
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.2.999-cibuild0067731-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

MrJul
MrJul previously approved these changes Jul 26, 2026

@MrJul MrJul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

…st-setparent

# Conflicts:
#	tests/Avalonia.Controls.UnitTests/GridTests.cs
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.2.999-cibuild0067791-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul
MrJul added this pull request to the merge queue Jul 26, 2026
Merged via the queue into AvaloniaUI:main with commit 9e1c35b Jul 26, 2026
10 checks passed
MrJul added a commit that referenced this pull request Jul 29, 2026
… 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 <julien@lebosquain.net>
@MrJul MrJul added backported-12.1.x and removed backport-candidate-12.1.x Consider this PR for backporting to 12.1 branch labels Jul 29, 2026
RedQuE3n added a commit to RedQuE3n/EmuSen.LunaP that referenced this pull request Aug 15, 2026
…the guard that could not have noticed

LunaTable put every column in a shared size group and set the shared size scope
on its root, which is Avalonia's own mechanism for making an Auto column in the
header line up with the Auto column in each row. The wiring was right and it
never did anything.

Avalonia 12.1.0 registers a definition with its shared size scope when the
definition is ADDED to the collection a Grid already owns, and does not when a
ready-made collection is ASSIGNED to the Grid. LunaTable assigned, in both
places - the header grid in Rebuild and each row grid in Row. An assigned
definition keeps a SharedSizeGroup that reads back correctly and shares nothing,
so every column sized alone while looking, from outside, exactly like a column
that was sharing.

Measured on a three-column table: the Auto column's heading started at x=416.0
and its cells at x=422.0. Six pixels, every row, for the life of the control.
Star and absolute columns resolve to the same number in both grids without
needing to share anything, so they lined up regardless - which is why this
shipped and why it stayed.

Fixed upstream by AvaloniaUI/Avalonia#21848, "register assigned definition
collections with their shared size group", merged 2026-07-26 - after 12.1.0 was
released on 2026-07-09. Populating works on 12.1.0 as it stands, so this costs no
version bump and stays correct when the upstream fix does arrive.

The guard is the part worth reading. It asserted that the SharedSizeGroup NAMES
matched between the header grid and a row grid and that none was empty. Both
were true the entire time nothing was sharing, so it passed every day of a defect
it was written to catch. It had a second hole and that one is more instructive:
the comment it was guarding says AUTO IS ACCEPTED AND MADE TO WORK, and no test
in the file had ever used an Auto column. The one feature the comment claimed was
the one feature the data could not exercise.

It is replaced by an assertion about where the text actually lands - for every
column, the heading's x equals its cell's x, through an Auto column whose heading
is deliberately wider than its cells. Made to fail on purpose per §22.5:
reverting Define to an assignment reports "Column 1 (classification) heading
starts at x=356.0 but its cell starts at x=422.0", a 66 pixel gap naming the site
to fix. The name assertion survives as a smaller claim, because it localizes a
failure the positional one cannot explain.

Two further guards. A 10,000 row table realizes 10 rows and 30 cells, which pins
the assumption that row virtualization comes free from the ListBox and that cell
virtualization follows from rows - a change of items panel would otherwise turn
that into 10,000 grids with nothing to announce it but a slow window. And an
upstream canary that fails the day a version carrying #21848 is taken, which is
the notice that Define's comment has become history rather than a live hazard.

Ui.Cols and Ui.Rows assign the same way and are deliberately left alone here.
Their definitions parse from a comma-separated string that has no syntax for a
SharedSizeGroup, so nothing there is trying to share and nothing is broken; it is
a trap rather than a defect, and it gets its own change.

Cites §27 rather than the correction subsection it wants, because that subsection
is not written yet and CitationTests fails the build for a § that does not
resolve.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants