Fix: Inconsistent size calculation of Grid when UseLayoutRounding is true (not paradox related)

[7imekeeper]

What does the pull request do?

Makes Grid Row/Column size calculation a bit more consistent when UseLayoutRounding is true.
Fixes #21052

What is the current behavior?

Grid children can sometimes incorrectly have more space allocated than necessary when UseLayoutRounding is true.

Minimal example:

<StackPanel VerticalAlignment="Top"
            HorizontalAlignment="Center">
  <Grid RowDefinitions="Auto,Auto" Width="100">
    <Border Grid.RowSpan="2" Background="Red">
      <Border Width="50" Height="3" Background="Lime" />
    </Border>
  </Grid>
</StackPanel>

What is the updated/expected behavior with this PR?

Grid children are now more accurately allocated their necessary space.

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

Row/Column size rounding now use MidpointRounding.ToZero to make equivalent decimal values behave consistently regardless of differing preceding integral values. (e.g., 3.5 and 4.5 now round to 3 and 4, respectively, instead of both rounding to 4) This means that several cases where space was over-allocated are now under-allocated instead.
LayoutHelper has had an additional helper method added to support this behavior without breaking anything.
Additionally, some LayoutHelper methods have had rounding behavior made more consistent between DPI-aware and unaware paths.

Checklist

• Added unit tests (if possible)?
• Added XML documentation to any related classes?
• Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Fixed issues

Fixes #21052

[7imekeeper]

Visual testing was done using the following xaml added to the MainWindow of the Sandbox project.

<StackPanel Spacing="12"
  VerticalAlignment="Top"
  HorizontalAlignment="Center">
  <NumericUpDown x:Name="nud" Value="10" />
  <StackPanel Spacing="12" Orientation="Horizontal">
    <Grid RowDefinitions="Auto,Auto" Width="100">
      <Border Grid.RowSpan="2" Background="Red">
        <Border Width="50" Height="{Binding #nud.Value}" Background="Lime" />
      </Border>
    </Grid>
    <Grid RowDefinitions="Auto,Auto,Auto" Width="100">
      <Border Grid.RowSpan="3" Background="Red">
        <Border Width="50" Height="{Binding #nud.Value}" Background="Lime" />
      </Border>
    </Grid>
    <Grid RowDefinitions="Auto,Auto,Auto,Auto" Width="100">
      <Border Grid.RowSpan="4" Background="Red">
        <Border Width="50" Height="{Binding #nud.Value}" Background="Lime" />
      </Border>
    </Grid>
    <Grid RowDefinitions="Auto,Auto,Auto,Auto,Auto" Width="100">
      <Border Grid.RowSpan="5" Background="Red">
        <Border Width="50" Height="{Binding #nud.Value}" Background="Lime" />
      </Border>
    </Grid>
    <Grid RowDefinitions="2*,3*,2*" Width="100" Height="{Binding #nud.Value}">
      <Border Grid.Row="0" Width="50" Background="Red" />
      <Border Grid.Row="1" Width="50" Background="Lime" />
      <Border Grid.Row="2" Width="50" Background="Blue" />
    </Grid>
  </StackPanel>
</StackPanel>

[cla-avalonia]

• All contributors have signed the CLA.

[7imekeeper]

@cla-avalonia agree

[avaloniaui-bot]

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

[jsuarezruiz]

Thanks for working on this. The current tests cover two specific scale-1 paths:

• 3 / 2 = 1.5, where changing ToEven to ToZero avoids rounding both rows up.
• 13 / 3, which exercises the floating-point normalization added to RoundLayoutSizeUp.

The manual sample includes grids with 2–5 rows, but it does not explicitly exercise different layout scales, larger spans, or the relevant non-midpoint physical values. Those cases can still reach the original inconsistency because MidpointRounding.ToZero only changes exact .5 ties.

There is also a simple remaining failure at scale 1: a 3-DIP child spanning four Auto rows gives each row an unrounded size of 0.75, which rounds to 1. When phase 5 attempts to reduce the first row, the result is clamped to MinSizeForArrange == 0.75. The actual reduction is therefore only 0.25, but adjustedSize is reduced by the full dpiIncrement of 1. The loop believes it has reached the target size of 3, while the actual definition total is 3.75.

I also reproduced the problem on the current PR head with TestRoot.LayoutScaling = 1.25. A 47-device-pixel child (Height = 47 / 1.25) spanning eight Auto rows produces:

• grid.Bounds.Height == 37.6
• sum(RowDefinitions.ActualHeight) == 38.3
• spanningBorder.Bounds.Height == 38.4 (48 device pixels)

Each initial row allocation is 4.7 DIPs, or 5.875 physical pixels, so it rounds up to 4.8 DIPs. The correction can reduce a row only to its 4.7-DIP minimum, but adjustedSize still assumes the full 0.8-DIP decrement.

Could we please:

1. Update phase 5 so that adjustedSize tracks the actual change applied after MinSizeForArrange clamping, and continue distributing the discrepancy until the actual definition total matches finalSize whenever the min/max constraints permit it.
2. Add test coverage for Height = 3 spanning four Auto rows at scale 1.
3. Add a test with TestRoot.LayoutScaling = 1.25, eight Auto rows, and Height = 47 / 1.25, asserting that the Grid, spanning border, and sum of ActualHeight values agree. Ideally, this could be parameterized across scales such as 1, 1.25, 1.5, and 2.

The PR substantially reduces the existing failures, but we need to cover more cases. Let me know if can help with anything.

[7imekeeper]

Thanks for the feedback. I've expanded the test method to allow scaling and added the two specified tests, but both of them actually pass.

There is also a simple remaining failure at scale 1: a 3-DIP child spanning four Auto rows gives each row an unrounded size of 0.75, which rounds to 1. When phase 5 attempts to reduce the first row, the result is clamped to MinSizeForArrange == 0.75. The actual reduction is therefore only 0.25, but adjustedSize is reduced by the full dpiIncrement of 1. The loop believes it has reached the target size of 3, while the actual definition total is 3.75.

I tried to reproduce the described behavior but was unable to do so. It seems that MidpointRounding.ToZero actually truncates all decimal values instead of just affecting exact .5 ties which I confirmed via source:

// Directed rounding: Round to the nearest value, toward to zero
case MidpointRounding.ToZero:
    return Truncate(value);

Meaning in this scenario all definitions get rounded to zero and are then sequentially re-allocated a single pixel until the target height matches (resulting in heights of 1, 1, 1, 0 which I think is reasonable)

Similarly in the other scenario, all of the definitions get rounded to 5.6 pixels with some being re-allocated an additional 0.8 pixels.

It seems a (somewhat intentional) side-effect of this change is all (or nearly all) adjustment of roundedTakenSize seems to now occur in the second branch (roundedTakenSize < finalSize). The core discrepancy isn't directly fixed, but I haven't observed it since the changes.

I'm leery of making more changes to phase 5 without further discussion on exactly what behavior is desired. The current clamping is prone to undoing the rounding.

[avaloniaui-bot]

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

[jsuarezruiz]

Thanks for the feedback. I've expanded the test method to allow scaling and added the two specified tests, but both of them actually pass.

There is also a simple remaining failure at scale 1: a 3-DIP child spanning four Auto rows gives each row an unrounded size of 0.75, which rounds to 1. When phase 5 attempts to reduce the first row, the result is clamped to MinSizeForArrange == 0.75. The actual reduction is therefore only 0.25, but adjustedSize is reduced by the full dpiIncrement of 1. The loop believes it has reached the target size of 3, while the actual definition total is 3.75.

I tried to reproduce the described behavior but was unable to do so. It seems that MidpointRounding.ToZero actually truncates all decimal values instead of just affecting exact .5 ties which I confirmed via source:

// Directed rounding: Round to the nearest value, toward to zero
case MidpointRounding.ToZero:
    return Truncate(value);

Meaning in this scenario all definitions get rounded to zero and are then sequentially re-allocated a single pixel until the target height matches (resulting in heights of 1, 1, 1, 0 which I think is reasonable)

Similarly in the other scenario, all of the definitions get rounded to 5.6 pixels with some being re-allocated an additional 0.8 pixels.

It seems a (somewhat intentional) side-effect of this change is all (or nearly all) adjustment of roundedTakenSize seems to now occur in the second branch (roundedTakenSize < finalSize). The core discrepancy isn't directly fixed, but I haven't observed it since the changes.

I'm leery of making more changes to phase 5 without further discussion on exactly what behavior is desired. The current clamping is prone to undoing the rounding.

Thanks for the detailed explanation!
You’re right about MidpointRounding.ToZero, it truncates every fractional value but I reproduced one remaining issue. The new (47, 8, 1.25) case currently uses 47 DIPs, while I tried to report a case that uses 47 physical pixels, meaning Height = 47 / 1.25. With that input, the grid is 37.6, but the spanning border and row total are 38.4. I think that the extra pixel comes from floating-point accumulation. Could we keep this input as a regression test and investigate it?

[7imekeeper]

Thanks for the detailed explanation! You’re right about MidpointRounding.ToZero, it truncates every fractional value but I reproduced one remaining issue. The new (47, 8, 1.25) case currently uses 47 DIPs, while I tried to report a case that uses 47 physical pixels, meaning Height = 47 / 1.25. With that input, the grid is 37.6, but the spanning border and row total are 38.4. I think that the extra pixel comes from floating-point accumulation. Could we keep this input as a regression test and investigate it?

Ah, I see! Sorry for the misunderstanding. I've updated the test method to use double for targetHeight (really should have done this originally) and updated the specified test to target 47 / 1.25.

You are correct, this extra pixel comes from floating-point errors. Ultimately a comparison is done between 37.59999999999998 and 37.6 using MathUtilities.AreClose which returns false after calculating -eps = -1.8918200339612665E-14 which is not less than the delta of -2.1316282072803006E-14.

I updated adjustedSize += dpiIncrement; to adjustedSize = LayoutHelper.RoundLayoutValueUp(adjustedSize + dpiIncrement, dpi); which solves this comparison, but the assertion in the test also failed due to floating point errors after the fix (comparing 37.6 to 37.599999999999994). The test is easy to fix by adding a precision to the assert (I've chosen 8 to align with the LayoutHelper.RoundTo8Digits method) which I think is reasonable.

Something to note is these tiny floating-point inconsistencies are likely also affecting the order the definitions are sorted into, which could also potentially lead to some unexpected behavior.

[avaloniaui-bot]

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

[avaloniaui-bot]

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