Skip formatting when possible after FormatFitContentWidth

Source/Core/Layout/FlexFormattingContext.cpp

@@ -52,7 +52,7 @@ UniquePtr<LayoutBox> FlexFormattingContext::Format(ContainerBox* parent_containe
 	return FlexFormattingContext::FormatImpl(parent_container, element, initial_box, flex_min_size, flex_max_size);
 }
 
-float FlexFormattingContext::DetermineMaxContentWidth(Element* element, const Box& initial_box, const FormattingMode& formatting_mode)
+UniquePtr<LayoutBox> FlexFormattingContext::DetermineMaxContentWidth(Element* element, const Box& initial_box, const FormattingMode& formatting_mode)
 {
 	RMLUI_ASSERT(formatting_mode.constraint == FormattingMode::Constraint::MaxContent);
 	const Vector2f containing_block(-1.f);
@@ -60,9 +60,12 @@ float FlexFormattingContext::DetermineMaxContentWidth(Element* element, const Bo
 
 	const Vector2f min_flex_size(0.f);
 	const Vector2f max_flex_size(FLT_MAX);
-	UniquePtr<LayoutBox> layout_box = FlexFormattingContext::FormatImpl(&root, element, initial_box, min_flex_size, max_flex_size);
 
-	return layout_box->GetShrinkToFitWidth();
+	// Return the layout box if (and only if) we can consider the layout complete, despite formatting under a
+	// max-content constraint. This allows us to skip a formatting step in some cases. Later we may want to make some
+	// formatting simplifications during max-content sizing, optimized just for retrieving its width. In this case, we
+	// should make sure that the full flex formatting is done at least once during layouting in the end.
+	return FlexFormattingContext::FormatImpl(&root, element, initial_box, min_flex_size, max_flex_size);
 }
 
 UniquePtr<LayoutBox> FlexFormattingContext::FormatImpl(ContainerBox* parent_container, Element* element, const Box& initial_box,

Source/Core/Layout/FlexFormattingContext.h

@@ -48,7 +48,7 @@ public:
 	static UniquePtr<LayoutBox> Format(ContainerBox* parent_container, Element* element, Vector2f containing_block, const Box& initial_box);
 
 	/// Formats a flex container to determine its max-content width.
-	static float DetermineMaxContentWidth(Element* element, const Box& initial_box, const FormattingMode& formatting_mode);
+	static UniquePtr<LayoutBox> DetermineMaxContentWidth(Element* element, const Box& initial_box, const FormattingMode& formatting_mode);
 
 private:
 	static UniquePtr<LayoutBox> FormatImpl(ContainerBox* parent_container, Element* element, const Box& initial_box, Vector2f flex_min_size,

Source/Core/Layout/FormattingContext.cpp

@@ -119,7 +119,7 @@ UniquePtr<LayoutBox> FormattingContext::FormatIndependent(ContainerBox* parent_c
 	UniquePtr<LayoutBox> layout_box;
 
 	const FormattingMode& formatting_mode = parent_container->GetFormattingMode();
-	if (type != FormattingContextType::None && formatting_mode.constraint == FormattingMode::Constraint::None)
+	if (formatting_mode.constraint == FormattingMode::Constraint::None)
 	{
 		LayoutNode* layout_node = element->GetLayoutNode();
 		if (layout_node->CommittedLayoutMatches(parent_container->GetContainingBlockSize(Style::Position::Static),
@@ -154,21 +154,33 @@ UniquePtr<LayoutBox> FormattingContext::FormatIndependent(ContainerBox* parent_c
 
 			if (box.GetSize().x < 0.f)
 			{
-				FormatFitContentWidth(box, element, type, formatting_mode, containing_block);
-				LayoutDetails::ClampSizeAndBuildAutoMarginsForBlockWidth(box, containing_block, element);
+				layout_box = FormatFitContentWidth(box, element, type, formatting_mode, containing_block);
+				if (layout_box)
+				{
+					Box box_before = box;
+					LayoutDetails::ClampSizeAndBuildAutoMarginsForBlockWidth(box, containing_block, element);
+					// If the box is changed after clamping, we need to re-format the element with the new size, ignoring the returned layout box.
+					if (box != box_before)
+						layout_box.reset();
+				}
+				else
+				{
+					LayoutDetails::ClampSizeAndBuildAutoMarginsForBlockWidth(box, containing_block, element);
+				}
 			}
 		}
 
-		switch (type)
+		if (!layout_box)
 		{
-		case FormattingContextType::Block: layout_box = BlockFormattingContext::Format(parent_container, element, containing_block, box); break;
-		case FormattingContextType::Table: layout_box = TableFormattingContext::Format(parent_container, element, containing_block, box); break;
-		case FormattingContextType::Flex: layout_box = FlexFormattingContext::Format(parent_container, element, containing_block, box); break;
-		case FormattingContextType::Replaced: layout_box = ReplacedFormattingContext::Format(parent_container, element, box); break;
-		case FormattingContextType::None: RMLUI_ERROR; break;
+			switch (type)
+			{
+			case FormattingContextType::Block: layout_box = BlockFormattingContext::Format(parent_container, element, containing_block, box); break;
+			case FormattingContextType::Table: layout_box = TableFormattingContext::Format(parent_container, element, containing_block, box); break;
+			case FormattingContextType::Flex: layout_box = FlexFormattingContext::Format(parent_container, element, containing_block, box); break;
+			case FormattingContextType::Replaced: layout_box = ReplacedFormattingContext::Format(parent_container, element, box); break;
+			case FormattingContextType::None: RMLUI_ERROR; break;
+			}
 		}
-
-		// TODO: If MaxContent constraint, and containing block = -1, -1, store resulting size as max-content size.
 	}
 
 	if (layout_box && formatting_mode.constraint == FormattingMode::Constraint::None)
@@ -221,8 +233,8 @@ float FormattingContext::FormatFitContentWidth(ContainerBox* parent_container, E
 	return box.GetSize().x;
 }
 
-void FormattingContext::FormatFitContentWidth(Box& box, Element* element, FormattingContextType type, const FormattingMode& parent_formatting_mode,
-	const Vector2f containing_block)
+UniquePtr<LayoutBox> FormattingContext::FormatFitContentWidth(Box& box, Element* element, FormattingContextType type,
+	const FormattingMode& parent_formatting_mode, const Vector2f containing_block)
 {
 	RMLUI_ASSERT(element);
 #ifdef RMLUI_TRACY_PROFILING
@@ -232,15 +244,12 @@ void FormattingContext::FormatFitContentWidth(Box& box, Element* element, Format
 	RMLUI_ZoneText(zone_text.c_str(), zone_text.size());
 #endif
 
-	LayoutNode* layout_node = element->GetLayoutNode();
-
 	const float box_height = box.GetSize().y;
+	LayoutNode* layout_node = element->GetLayoutNode();
 
+	UniquePtr<LayoutBox> layout_box;
 	float max_content_width = -1.f;
-	// TODO: The shrink-to-fit width is only cached for every other nested flexbox during the initial
-	// GetShrinkToFitWidth. I.e. the first .outer flexbox below #nested is formatted outside of this function. Even
-	// though in principle I believe we should be able to store its formatted width. Maybe move this caching into
-	// FormatIndependent somehow?
+
 	if (Optional<float> cached_width = layout_node->GetMaxContentWidthIfCached())
 	{
 		max_content_width = *cached_width;
@@ -255,15 +264,28 @@ void FormattingContext::FormatFitContentWidth(Box& box, Element* element, Format
 
 		switch (type)
 		{
-		case FormattingContextType::Block: max_content_width = BlockFormattingContext::DetermineMaxContentWidth(element, box, formatting_mode); break;
+		case FormattingContextType::Block:
+		{
+			max_content_width = BlockFormattingContext::DetermineMaxContentWidth(element, box, formatting_mode);
+		}
+		break;
+		case FormattingContextType::Flex:
+		{
+			layout_box = FlexFormattingContext::DetermineMaxContentWidth(element, box, formatting_mode);
+			max_content_width = layout_box->GetShrinkToFitWidth();
+		}
+		break;
 		case FormattingContextType::Table:
+		{
 			// Currently we don't support shrink-to-fit width for tables, just use a zero-sized width.
 			max_content_width = 0.f;
-			break;
-		case FormattingContextType::Flex: max_content_width = FlexFormattingContext::DetermineMaxContentWidth(element, box, formatting_mode); break;
+		}
+		break;
 		case FormattingContextType::Replaced:
-			RMLUI_ERRORMSG("Replaced elements are expected to have a positive intrinsice size and do not perform shrink-to-fit layout.");
-			break;
+		{
+			RMLUI_ERRORMSG("Replaced elements are expected to have a positive intrinsice size and should not need max-content formatting.");
+		}
+		break;
 		case FormattingContextType::None: RMLUI_ERROR; break;
 		}
 
@@ -281,6 +303,8 @@ void FormattingContext::FormatFitContentWidth(Box& box, Element* element, Format
 	}
 
 	box.SetContent(Vector2f(fit_content_width, box_height));
+
+	return layout_box;
 }
 
 } // namespace Rml

Source/Core/Layout/FormattingContext.h

@@ -76,8 +76,11 @@ protected:
 	~FormattingContext() = default;
 
 private:
-	static void FormatFitContentWidth(Box& box, Element* element, FormattingContextType type, const FormattingMode& parent_formatting_mode,
-		Vector2f containing_block);
+	/// Formats the element under a max-content constraint and sets its (unclamped) fit-content width on the box.
+	/// @return Optionally, a layout box indicating that the formatted element can be used directly when under
+	/// max-content constraints.
+	static UniquePtr<LayoutBox> FormatFitContentWidth(Box& box, Element* element, FormattingContextType type,
+		const FormattingMode& parent_formatting_mode, Vector2f containing_block);
 };
 
 } // namespace Rml

Tests/Source/Benchmarks/Flexbox.cpp

@@ -667,3 +667,98 @@ TEST_CASE("flexbox.shrink-to-fit")
 
 	document->Close();
 }
+
+static const String rml_flexbox_shrink_to_fit_super_nested = R"(
+<rml>
+<head>
+    <title>Flex - Shrink-to-fit 01</title>
+    <link type="text/rcss" href="/../Tests/Data/style.rcss"/>
+	<style>
+		body { width: 1000px; }
+		.shrink-to-fit {
+			float: left;
+			clear: both;
+			margin: 10px 0;
+			border: 2px #e8e8e8;
+		}
+		.outer {
+			border: 1px red;
+			padding: 30px;
+		}
+		#basic .outer {
+			display: flex;
+		}
+		#nested .outer {
+			display: inline-flex;
+		}
+		.inner {
+			border: 1px blue;
+			padding: 30px;
+		}
+	</style>
+</head>
+<body>
+<div id="nested" class="shrink-to-fit">
+	Before
+	<div class="outer">
+		<div class="inner">
+			<div class="outer">
+				<div class="inner">
+					<div class="outer">
+						<div class="inner">Flex</div>
+					</div>
+				</div>
+			</div>
+		</div>
+	</div>
+	After
+</div>
+</body>
+</rml>
+)";
+
+TEST_CASE("flexbox.shrink-to-fit-super-nested")
+{
+	Context* context = TestsShell::GetContext();
+	REQUIRE(context);
+
+	nanobench::Bench bench;
+	bench.title("Flexbox shrink-to-fit");
+	bench.relative(true);
+
+	ElementDocument* document = context->LoadDocumentFromMemory(rml_flexbox_shrink_to_fit_super_nested);
+	Element* basic = document->GetElementById("basic");
+	Element* nested = document->GetElementById("nested");
+
+	document->Show();
+	TestsShell::RenderLoop();
+
+	{
+		document->Close();
+		return;
+	}
+
+	basic->SetProperty(PropertyId::Display, Style::Display::None);
+	nested->SetProperty(PropertyId::Display, Style::Display::None);
+
+	bench.run("Reference", [&] {
+		document->SetProperty(PropertyId::Display, Style::Display::None);
+		document->RemoveProperty(PropertyId::Display);
+		context->Update();
+		context->Render();
+	});
+	bench.run("Basic shrink-to-fit", [&] {
+		basic->RemoveProperty(PropertyId::Display);
+		nested->SetProperty(PropertyId::Display, Style::Display::None);
+		context->Update();
+		context->Render();
+	});
+	bench.run("Nested shrink-to-fit", [&] {
+		basic->SetProperty(PropertyId::Display, Style::Display::None);
+		nested->RemoveProperty(PropertyId::Display);
+		context->Update();
+		context->Render();
+	});
+
+	document->Close();
+}