Flex formatting: Avoid negative flex item size in some situations whe…

…n edge size is fractional, fixes #657
mikke89 · Sep 14, 2024 · 270efa1 · 270efa1
1 parent fa02110
commit 270efa1
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 6 deletions.
diff --git a/Source/Core/Layout/BlockContainer.cpp b/Source/Core/Layout/BlockContainer.cpp
@@ -40,10 +40,10 @@ namespace Rml {
 
 BlockContainer::BlockContainer(ContainerBox* _parent_container, FloatedBoxSpace* _space, Element* _element, const Box& _box, float _min_height,
 	float _max_height) :
-	ContainerBox(Type::BlockContainer, _element, _parent_container),
-	box(_box), min_height(_min_height), max_height(_max_height), space(_space)
+	ContainerBox(Type::BlockContainer, _element, _parent_container), box(_box), min_height(_min_height), max_height(_max_height), space(_space)
 {
 	RMLUI_ASSERT(element);
+	RMLUI_ASSERT(box.GetSize().x >= 0.f);
 
 	if (!space)
 	{

diff --git a/Source/Core/Layout/FlexFormattingContext.cpp b/Source/Core/Layout/FlexFormattingContext.cpp
@@ -229,6 +229,17 @@ 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.
+	return Math::Max(item.used_main_size - item.main.sum_edges, 0.f);
+}
+
+static float GetInnerUsedCrossSize(const FlexItem& item)
+{
+	return Math::Max(item.used_cross_size - item.cross.sum_edges, 0.f);
+}
+
 void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector2f& flex_content_overflow_size, float& flex_baseline) const
 {
 	// The following procedure is based on the CSS flexible box layout algorithm.
@@ -671,13 +682,12 @@ void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector
 		for (FlexItem& item : line.items)
 		{
 			const Vector2f content_size = item.box.GetSize();
-			const float used_main_size_inner = item.used_main_size - item.main.sum_edges;
 
 			if (main_axis_horizontal)
 			{
 				if (content_size.y < 0.0f)
 				{
-					item.box.SetContent(Vector2f(used_main_size_inner, content_size.y));
+					item.box.SetContent(Vector2f(GetInnerUsedMainSize(item), content_size.y));
 					FormattingContext::FormatIndependent(flex_container_box, item.element, &item.box, FormattingContextType::Block);
 					item.hypothetical_cross_size = item.element->GetBox().GetSize().y + item.cross.sum_edges;
 				}
@@ -690,7 +700,7 @@ void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector
 			{
 				if (content_size.x < 0.0f)
 				{
-					item.box.SetContent(Vector2f(content_size.x, used_main_size_inner));
+					item.box.SetContent(Vector2f(content_size.x, GetInnerUsedMainSize(item)));
 					item.hypothetical_cross_size =
 						LayoutDetails::GetShrinkToFitWidth(item.element, flex_content_containing_block) + item.cross.sum_edges;
 				}
@@ -946,7 +956,7 @@ void FlexFormattingContext::Format(Vector2f& flex_resulting_content_size, Vector
 	{
 		for (FlexItem& item : line.items)
 		{
-			const Vector2f item_size = MainCrossToVec2(item.used_main_size - item.main.sum_edges, item.used_cross_size - item.cross.sum_edges);
+			const Vector2f item_size = MainCrossToVec2(GetInnerUsedMainSize(item), GetInnerUsedCrossSize(item));
 			const Vector2f item_offset = MainCrossToVec2(item.main_offset, line.cross_offset + item.cross_offset);
 
 			item.box.SetContent(item_size);

diff --git a/Tests/Source/UnitTests/FlexFormatting.cpp b/Tests/Source/UnitTests/FlexFormatting.cpp
@@ -106,3 +106,99 @@ TEST_CASE("FlexFormatting")
 
 	TestsShell::ShutdownShell();
 }
+
+static const String document_flex_dp_ratio_rml = R"(
+<rml>
+<head>
+	<link type="text/rcss" href="/assets/rml.rcss"/>
+	<style>
+		body {
+			width: 100%;
+			height: 100%;
+		}
+		.window {
+			width: 1920dp;
+			height: 1080dp;
+			margin: 0 auto;
+			display: flex;
+			flex-direction: column;
+		}
+		.inner {
+			flex-grow: 1;
+			display: flex;
+			flex-direction: row;
+		}
+		.left {
+			width: 30%;
+			margin: 32dp;
+			overflow: auto;
+			background: #522;
+		}
+		.right {
+			flex: 1;
+			margin: 32dp;
+			overflow: auto;
+			background: #252;
+		}
+	</style>
+</head>
+
+<body>
+	<div class="window">
+		<div class="inner">
+			<div class="left"/>
+			<div class="right"/>
+		</div>
+	</div>
+</body>
+</rml>
+)";
+
+TEST_CASE("FlexFormatting.dp_ratio")
+{
+	Context* context = TestsShell::GetContext();
+	REQUIRE(context);
+
+	ElementDocument* document = context->LoadDocumentFromMemory(document_flex_dp_ratio_rml);
+	REQUIRE(document);
+	document->Show();
+
+	Element* window = document->GetChild(0);
+
+	constexpr float native_width = 1920.f;
+	constexpr float native_height = 1080.f;
+
+	const float test_window_widths[] = {
+		3440.f,
+		2960.f,
+		2880.f,
+		2560.f,
+		2400.f,
+		2048.f,
+		1921.f,
+		1920.f,
+		1919.f,
+		1600.f,
+		1366.f,
+		1280.f,
+		1024.f,
+	};
+
+	for (float window_width : test_window_widths)
+	{
+		const float dp_ratio = window_width / native_width;
+		CAPTURE(window_width);
+		CAPTURE(dp_ratio);
+
+		context->SetDensityIndependentPixelRatio(dp_ratio);
+
+		TestsShell::RenderLoop();
+
+		CHECK(window->GetBox().GetSize().x == window_width);
+		CHECK(window->GetBox().GetSize().y == doctest::Approx((window_width / native_width) * native_height));
+	}
+
+	document->Close();
+
+	TestsShell::ShutdownShell();
+}