Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: prevent StaggeredLayout floating point errors #573

Merged
merged 5 commits into from
Dec 6, 2024
Merged

Fix: prevent StaggeredLayout floating point errors #573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
88e1946
prevent floating point errors
Poker-sang Nov 28, 2024
802caaf
fix spacing
Poker-sang Dec 3, 2024
0c96f8d
Apply suggestions from code review
Poker-sang Dec 4, 2024
d4b19b5
Merge branch 'main' into prevent-floating-point-errors
Poker-sang Dec 4, 2024
cca5a88
Merge branch 'main' into prevent-floating-point-errors
michael-hawker Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions components/Primitives/src/StaggeredLayout/StaggeredLayout.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ protected override Size MeasureOverride(VirtualizingLayoutContext context, Size
return new Size(availableSize.Width, 0);
}

if ((context.RealizationRect.Width == 0) && (context.RealizationRect.Height == 0))
if (context.RealizationRect is { Width: 0, Height: 0 })
{
return new Size(availableSize.Width, 0.0f);
}
Expand All @@ -163,7 +163,7 @@ protected override Size MeasureOverride(VirtualizingLayoutContext context, Size
if (ItemsStretch is StaggeredLayoutItemsStretch.None)
{
columnWidth = double.IsNaN(DesiredColumnWidth) ? availableWidth : Math.Min(DesiredColumnWidth, availableWidth);
numColumns = Math.Max(1, (int)Math.Floor(availableWidth / columnWidth));
numColumns = Math.Max(1, (int)Math.Floor(availableWidth / (columnWidth + ColumnSpacing)));
}
else
{
Expand All @@ -174,13 +174,14 @@ protected override Size MeasureOverride(VirtualizingLayoutContext context, Size
}
else
{
var tempAvailableWidth = availableWidth + ColumnSpacing;
numColumns = (int)Math.Floor(tempAvailableWidth / DesiredColumnWidth);
// 0.0001 is to prevent floating point errors
var tempAvailableWidth = availableWidth + ColumnSpacing - 0.0001;
numColumns = (int)Math.Floor(tempAvailableWidth / (DesiredColumnWidth + ColumnSpacing));
columnWidth = tempAvailableWidth / numColumns - ColumnSpacing;
}
}

if (columnWidth != state.ColumnWidth)
if (Math.Abs(columnWidth - state.ColumnWidth) > double.Epsilon)
{
// The items will need to be remeasured
state.Clear();
Expand All @@ -205,10 +206,10 @@ protected override Size MeasureOverride(VirtualizingLayoutContext context, Size
state.ClearColumns();
}

if (RowSpacing != state.RowSpacing)
if (Math.Abs(this.RowSpacing - state.RowSpacing) > double.Epsilon)
{
// If the RowSpacing changes the height of the rows will be different.
// The columns stores the height so we'll want to clear them out to
// The columns store the height so we'll want to clear them out to
// get the proper height
state.ClearColumns();
state.RowSpacing = RowSpacing;
Expand All @@ -228,7 +229,7 @@ protected override Size MeasureOverride(VirtualizingLayoutContext context, Size
{
// Item has not been measured yet. Get the element and store the values
item.Element = context.GetOrCreateElementAt(i);
item.Element.Measure(new Size((float)state.ColumnWidth, (float)availableHeight));
item.Element.Measure(new Size(state.ColumnWidth, availableHeight));
item.Height = item.Element.DesiredSize.Height;
measured = true;
}
Expand Down Expand Up @@ -260,12 +261,12 @@ protected override Size MeasureOverride(VirtualizingLayoutContext context, Size

deadColumns.Add(columnIndex);
}
else if (measured == false)
else if (!measured)
{
// We ALWAYS want to measure an item that will be in the bounds
item.Element = context.GetOrCreateElementAt(i);
item.Element.Measure(new Size((float)state.ColumnWidth, (float)availableHeight));
if (item.Height != item.Element.DesiredSize.Height)
item.Element.Measure(new Size(state.ColumnWidth, availableHeight));
if (Math.Abs(item.Height - item.Element.DesiredSize.Height) > double.Epsilon)
{
// this item changed size; we need to recalculate layout for everything after this
state.RemoveFromIndex(i + 1);
Expand All @@ -282,7 +283,7 @@ protected override Size MeasureOverride(VirtualizingLayoutContext context, Size

double desiredHeight = state.GetHeight();

return new Size((float)availableWidth, (float)desiredHeight);
return new Size(availableWidth, desiredHeight);
}

/// <inheritdoc/>
Expand Down
8 changes: 4 additions & 4 deletions components/Primitives/src/StaggeredLayout/StaggeredLayoutState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,14 @@ internal void ClearColumns()
/// </summary>
/// <returns>The estimated height of the layout.</returns>
/// <remarks>
/// If all of the items have been calculated then the actual height will be returned.
/// If all of the items have not been calculated then an estimated height will be calculated based on the average height of the items.
/// If all the items have been calculated then the actual height will be returned.
/// If all the items have not been calculated then an estimated height will be calculated based on the average height of the items.
/// </remarks>
internal double GetHeight()
{
double desiredHeight = Enumerable.Max(_columnLayout.Values, c => c.Height);
double desiredHeight = _columnLayout.Values.Max(c => c.Height);

var itemCount = Enumerable.Sum(_columnLayout.Values, c => c.Count);
var itemCount = _columnLayout.Values.Sum(c => c.Count);
if (itemCount == _context.ItemCount)
{
return desiredHeight;
Expand Down
Loading