Fix: prevent StaggeredLayout floating point errors (#573)

* prevent floating point errors * fix spacing * Apply suggestions from code review Co-authored-by: Michael Hawker MSFT (XAML Llama) <[email protected]> --------- Co-authored-by: Michael Hawker MSFT (XAML Llama) <[email protected]>
CommunityToolkit · Dec 6, 2024 · 044f82f · 044f82f
1 parent 2e64c30
commit 044f82f
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 16 deletions.
diff --git a/components/Primitives/src/StaggeredLayout/StaggeredLayout.cs b/components/Primitives/src/StaggeredLayout/StaggeredLayout.cs
@@ -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);
         }
@@ -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
         {
@@ -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();
@@ -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;
@@ -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;
             }
@@ -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);
@@ -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/>

diff --git a/components/Primitives/src/StaggeredLayout/StaggeredLayoutState.cs b/components/Primitives/src/StaggeredLayout/StaggeredLayoutState.cs
@@ -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;