Browse Source

Skip rounding in flexbox layout, allow fractional sizes of flex items

Previously the flex item offset and size were snapped to the pixel grid. This made it so that flex items could have a slightly different size than their inner content. This could further cause a single pixel gap between the child of a flex item and that child's immediate cousin.
Michael Ragazzon 10 months ago
parent
commit
9363a12712

+ 9 - 18
Source/Core/Layout/FlexFormattingContext.cpp

@@ -34,7 +34,6 @@
 #include "../../../Include/RmlUi/Core/Types.h"
 #include "ContainerBox.h"
 #include "LayoutDetails.h"
-#include "LayoutEngine.h"
 #include <algorithm>
 #include <float.h>
 #include <numeric>
@@ -91,8 +90,6 @@ UniquePtr<LayoutBox> FlexFormattingContext::Format(ContainerBox* parent_containe
 			context.flex_content_containing_block.y = containing_block.y;
 		}
 
-		Math::SnapToPixelGrid(context.flex_content_offset, context.flex_available_content_size);
-
 		// Format the flexbox and all its children.
 		Vector2f flex_resulting_content_size, content_overflow_size;
 		float flex_baseline = 0.f;
@@ -120,8 +117,7 @@ UniquePtr<LayoutBox> FlexFormattingContext::Format(ContainerBox* parent_containe
 
 Vector2f FlexFormattingContext::GetMaxContentSize(Element* element)
 {
-	// A large but finite number is used here, because the flexbox formatting algorithm
-	// needs to round numbers, and it doesn't support infinities.
+	// A large but finite number is used here, since layouting doesn't always work well infinities.
 	const Vector2f infinity(10000.0f, 10000.0f);
 	RootBox root(infinity);
 	auto flex_container_box = MakeUnique<FlexContainer>(element, &root);
@@ -231,7 +227,7 @@ static void GetItemSizing(FlexItem::Size& destination, const ComputedAxisSize& c
 
 static float GetInnerUsedMainSize(const FlexItem& item)
 {
-	// Due to pixel snapping (rounding) of the outer size, `sum_edges` may be larger than it, so clamp the result to zero.
+	// Due to floating-point precision the outer size may be smaller than `sum_edges`, so clamp the result to zero.
 	return Math::Max(item.used_main_size - item.main.sum_edges, 0.f);
 }
 
@@ -570,9 +566,10 @@ void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector
 	}
 
 	// -- Align main axis (§9.5) --
-	// Main alignment is done before cross sizing. Due to rounding to the pixel grid, the main size can
-	// change slightly after main alignment/offseting. Also, the cross sizing depends on the main sizing
-	// so doing it in this order ensures no surprises (overflow/wrapping issues) due to pixel rounding.
+	// Main alignment is done before cross sizing. Previously, doing it in this order was important due to pixel
+	// rounding, since changing the main offset could change the main size after rounding, which in turn could influence
+	// the cross size. However, now we no longer do pixel rounding, so we may be free to do cross sizing first if we
+	// want to do it in that order for some particular reason.
 	for (FlexLine& line : container.lines)
 	{
 		const float remaining_free_space = used_main_size -
@@ -653,7 +650,7 @@ void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector
 			}
 		}
 
-		// Now find the offset and snap the outer edges to the pixel grid.
+		// Now find the offset for each item.
 		float cursor = 0.0f;
 		for (FlexItem& item : line.items)
 		{
@@ -663,7 +660,6 @@ void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector
 				item.main_offset = cursor + item.main.margin_a + item.main_auto_margin_size_a;
 
 			cursor += item.used_main_size + item.main_auto_margin_size_a + item.main_auto_margin_size_b;
-			Math::SnapToPixelGrid(item.main_offset, item.used_main_size);
 		}
 	}
 
@@ -748,7 +744,7 @@ void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector
 				})->hypothetical_cross_size;
 
 			// Currently, we don't handle the case where baseline alignment could extend the line's cross size, see CSS specs 9.4.8.
-			line.cross_size = Math::Max(0.0f, Math::Round(largest_hypothetical_cross_size));
+			line.cross_size = Math::Max(0.0f, largest_hypothetical_cross_size);
 
 			if (flex_single_line)
 				line.cross_size = Math::Clamp(line.cross_size, cross_min_size, cross_max_size);
@@ -878,10 +874,6 @@ void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector
 				}
 			}
 		}
-
-		// Snap the outer item cross edges to the pixel grid.
-		for (FlexItem& item : line.items)
-			Math::SnapToPixelGrid(item.cross_offset, item.used_cross_size);
 	}
 
 	const float accumulated_lines_cross_size = std::accumulate(container.lines.begin(), container.lines.end(), 0.f,
@@ -963,7 +955,6 @@ void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector
 				line.cross_offset = cursor + line.cross_spacing_a;
 
 			cursor += line.cross_spacing_a + line.cross_size + line.cross_spacing_b;
-			Math::SnapToPixelGrid(line.cross_offset, line.cross_size);
 		}
 	}
 
@@ -986,7 +977,7 @@ void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector
 			UniquePtr<LayoutBox> item_layout_box =
 				FormattingContext::FormatIndependent(flex_container_box, item.element, &item.box, FormattingContextType::Block);
 
-			// Set the position of the element within the the flex container
+			// Set the position of the element within the flex container
 			item.element->SetOffset(flex_content_offset + item_offset, element_flex);
 
 			// The flex container baseline is simply set to the first flex item that has a baseline.

+ 1 - 1
Source/Core/Layout/FlexFormattingContext.h

@@ -55,7 +55,7 @@ private:
 	/// Format the flexbox and its children.
 	/// @param[out] flex_resulting_content_size The final content size of the flex container.
 	/// @param[out] flex_content_overflow_size Overflow size in case flex items or their contents overflow the container.
-	/// @param[out] flex_baseline The baseline of the flex contaienr, in terms of the vertical distance from its top-left border corner.
+	/// @param[out] flex_baseline The baseline of the flex container, in terms of the vertical distance from its top-left border corner.
 	void Format(Vector2f& flex_resulting_content_size, Vector2f& flex_content_overflow_size, float& flex_baseline) const;
 
 	Vector2f flex_available_content_size;

+ 69 - 0
Tests/Data/VisualTests/fractional-dimensions-03.rml

@@ -0,0 +1,69 @@
+<rml>
+<head>
+	<link type="text/rcss" href="/../Tests/Data/style.rcss"/>
+	<title>Fractional dimensions 03 - Flexbox</title>
+	<meta name="Description" content="Layout of nested elements with fractional height within a flex container."/>
+	<style>
+		handle {
+			background-color: red;
+			display: flex;
+			flex-direction: column;
+			justify-content: flex-end;
+			position: relative;
+			width: auto;
+			height: auto;
+			cursor: move;
+		}
+		div.outer {
+			background-color: red;
+		}
+		div.inner {
+			border: 1.23dp #333;
+			background-color: #666;
+			height: 15.23333dp;
+		}
+		div.outer:nth-child(odd) > div.inner {
+			background-color: #999;
+		}
+	</style>
+</head>
+<body>
+	<p>Borders should be placed edge-to-edge, and there should be no red.</p>
+	<handle move_target="#self" edge_margin="none">
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+		<div class="outer"><div class="inner"></div></div>
+	</handle>
+</body>
+</rml>

+ 2 - 0
changelog.md

@@ -25,6 +25,7 @@ Fixes several situations with single pixel gaps and overlaps:
 - Gap of 1px between border or backgrounds of neighboring elements.
 - Overlap of 1px between border or backgrounds of neighboring elements.
 - Table cell backgrounds overlaps the table border by 1px.
+- Gap between nested elements in a flex container.
 - Clipping area offset by 1px compared to the border area.
 
 ![Single pixel gap fix examples - before and after comparisons](https://github.com/user-attachments/assets/f1b29382-4686-4fea-a4dc-ea9628669b80)
@@ -154,6 +155,7 @@ The font face will be inherited from the element it is being applied to. However
 
 ### Breaking changes
 
+- Layouts may see 1px shifts in various places due to the improvements to prevent single pixel gaps.
 - The target of the `<handle>` element will no longer move outside its containing block by default, see above for how to override this behavior.
 - Changed the signature of `MeshUtilities::GenerateBackground` and  `MeshUtilities::GenerateBackgroundBorder`.
   - They now take the new `RenderBox` class as input. The `Element::GetRenderBox` method can be used to construct it.