Fix non-boundary layout nodes not being dirtied

Include/RmlUi/Core/ElementUtilities.h

@@ -172,6 +172,34 @@ public:
 				search_queue.push(element->GetChild(i));
 		}
 	}
+
+	template <typename Func>
+	static CallbackControlFlow VisitElementsDepthOrder(Element* element, Func&& func, int tree_depth = 0)
+	{
+		if constexpr (std::is_same_v<std::invoke_result_t<Func, Element*, int>, CallbackControlFlow>)
+		{
+			const CallbackControlFlow control_flow = func(element, tree_depth);
+			if (control_flow == CallbackControlFlow::Break)
+				return CallbackControlFlow::Break;
+			if (control_flow == CallbackControlFlow::SkipChildren)
+				return CallbackControlFlow::Continue;
+		}
+		else
+		{
+			func(element, tree_depth);
+		}
+
+		const int num_children = element->GetNumChildren();
+		for (int i = 0; i < num_children; i++)
+		{
+			const CallbackControlFlow control_flow = VisitElementsDepthOrder(element->GetChild(i), func, tree_depth + 1);
+			if (control_flow == CallbackControlFlow::Break)
+				return control_flow;
+			RMLUI_ASSERT(control_flow != CallbackControlFlow::SkipChildren);
+		}
+
+		return CallbackControlFlow::Continue;
+	}
 };
 
 } // namespace Rml

Source/Core/Element.cpp

@@ -177,11 +177,7 @@ void Element::Update(float dp_ratio, Vector2f vp_dimensions)
 	}
 
 	LayoutNode* layout_node = GetLayoutNode();
-	if (layout_node->IsSelfDirty())
-	{
-		if (LayoutNode* layout_boundary = layout_node->GetClosestLayoutBoundary())
-			layout_boundary->SetDirty(DirtyLayoutType::Child);
-	}
+	layout_node->DirtyUpToClosestLayoutBoundary();
 }
 
 void Element::UpdateProperties(const float dp_ratio, const Vector2f vp_dimensions)

Source/Core/ElementDocument.cpp

@@ -525,6 +525,15 @@ void ElementDocument::UpdateLayout()
 		RMLUI_ZoneScoped;
 		RMLUI_ZoneText(source_url.c_str(), source_url.size());
 
+		String tree_dirty_state;
+		ElementUtilities::VisitElementsDepthOrder(this, [&](Element* element, int tree_depth) {
+			tree_dirty_state += '\n' + String(size_t(4 * tree_depth), ' ');
+			tree_dirty_state += CreateString("%s: Dirty: %d  Dirty Self: %d", element->GetAddress().c_str(), element->GetLayoutNode()->IsDirty(),
+				element->GetLayoutNode()->IsSelfDirty());
+		});
+
+		Log::Message(Log::LT_INFO, "ElementDocument::UpdateLayout - Tree dirty state:\n%s\n\n", tree_dirty_state.c_str());
+
 		bool force_full_document_layout = false;
 		bool any_layout_updates = false;
 

Source/Core/Layout/ContainerBox.cpp

@@ -121,10 +121,10 @@ bool ContainerBox::IsScrollContainer() const
 	return LayoutDetails::IsScrollContainer(overflow_x, overflow_y);
 }
 
-bool ContainerBox::IsMaxContentConstraint() const
+bool ContainerBox::IsUnderMaxContentConstraint() const
 {
 	if (parent_container)
-		return parent_container->IsMaxContentConstraint();
+		return parent_container->IsUnderMaxContentConstraint();
 	// TODO: Very hacky
 	if (const Box* box = GetIfBox())
 		return box->GetSize().x >= 10'000.f;
@@ -382,4 +382,25 @@ String TableWrapper::DebugDumpTree(int depth) const
 	return String(depth * 2, ' ') + "TableWrapper" + " | " + LayoutDetails::GetDebugElementName(element);
 }
 
+bool CachedContainer::GetBaselineOfLastLine(float& out_baseline) const
+{
+	if (baseline_of_last_line.has_value())
+	{
+		out_baseline = *baseline_of_last_line;
+		return true;
+	}
+	return false;
+}
+
+float CachedContainer::GetShrinkToFitWidth() const
+{
+	RMLUI_ERRORMSG("Internal error: CachedContainer should not be used under a max-content constraint.");
+	return 0.f;
+}
+
+String CachedContainer::DebugDumpTree(int depth) const
+{
+	return String(depth * 2, ' ') + "CachedContainer" + " | " + LayoutDetails::GetDebugElementName(element);
+}
+
 } // namespace Rml

Source/Core/Layout/ContainerBox.h

@@ -63,7 +63,7 @@ public:
 	/// Returns true if this container can have scrollbars enabled, as determined by its overflow properties.
 	bool IsScrollContainer() const;
 	/// Returns true if this container is being layed-out under max-content constraint.
-	bool IsMaxContentConstraint() const;
+	bool IsUnderMaxContentConstraint() const;
 
 	void AssertMatchesParentContainer(ContainerBox* container_box) const
 	{
@@ -183,19 +183,26 @@ private:
 };
 
 /**
-    A box which is produced when we matched the existing layout.
-
+    A box which is produced after matching the existing layout.
 */
 class CachedContainer final : public ContainerBox {
 public:
-	CachedContainer(Vector2f containing_block) : ContainerBox(Type::Root, nullptr, nullptr), box(containing_block) {}
-	CachedContainer(const Box& box) : ContainerBox(Type::Root, nullptr, nullptr), box(box) {}
+	CachedContainer(Element* element, ContainerBox* parent_container, const Box& box, Vector2f visible_overflow_size,
+		Optional<float> baseline_of_last_line) :
+		ContainerBox(Type::CachedContainer, element, parent_container), box(box), baseline_of_last_line(baseline_of_last_line)
+	{
+		SetVisibleOverflowSize(visible_overflow_size);
+	}
 
 	const Box* GetIfBox() const override { return &box; }
+	bool GetBaselineOfLastLine(float& out_baseline) const override;
+	float GetShrinkToFitWidth() const override;
+
 	String DebugDumpTree(int depth) const override;
 
 private:
-	Box box;
+	const Box& box;
+	Optional<float> baseline_of_last_line;
 };
 
 } // namespace Rml

Source/Core/Layout/FormattingContext.cpp

@@ -98,37 +98,48 @@ UniquePtr<LayoutBox> FormattingContext::FormatIndependent(ContainerBox* parent_c
 	}
 #endif
 
-	const bool is_under_max_content_constraint = parent_container->IsMaxContentConstraint();
+	UniquePtr<LayoutBox> layout_box;
+
+	const bool is_under_max_content_constraint = parent_container->IsUnderMaxContentConstraint();
 	if (type != FormattingContextType::None && !is_under_max_content_constraint)
 	{
 		LayoutNode* layout_node = element->GetLayoutNode();
 		if (layout_node->CommittedLayoutMatches(parent_container->GetContainingBlockSize(Style::Position::Static),
 				parent_container->GetContainingBlockSize(Style::Position::Static), override_initial_box))
 		{
-			Log::Message(Log::LT_DEBUG, "Layout cache match on element: %s", element->GetAddress().c_str());
-			// TODO: Construct a new CachedContainer and return that.
+			Log::Message(Log::LT_INFO, "Layout cache match on element: %s", element->GetAddress().c_str());
 			// TODO: How to deal with ShrinkToFitWidth, in particular for the returned box? Store it in the LayoutNode?
-			// Maybe best not to use this committed layout at all during max-content layouting. Instead, skip this here,
-			// return zero in the CacheContainerBox, and make a separate LayoutNode cache for shrink-to-fit width that
-			// is fetched in LayoutDetails::ShrinkToFitWidth().
+			//   Maybe best not to use this committed layout at all during max-content layouting. Instead, skip this here,
+			//   return zero in the CacheContainerBox, and make a separate LayoutNode cache for shrink-to-fit width that
+			//   is fetched in LayoutDetails::ShrinkToFitWidth().
+			layout_box = MakeUnique<CachedContainer>(element, parent_container, element->GetBox(),
+				layout_node->GetCommittedLayout()->visible_overflow_size, layout_node->GetCommittedLayout()->baseline_of_last_line);
 		}
 	}
 
-	UniquePtr<LayoutBox> layout_box;
-	switch (type)
+	if (!layout_box)
 	{
-	case FormattingContextType::Block: layout_box = BlockFormattingContext::Format(parent_container, element, override_initial_box); break;
-	case FormattingContextType::Table: layout_box = TableFormattingContext::Format(parent_container, element, override_initial_box); break;
-	case FormattingContextType::Flex: layout_box = FlexFormattingContext::Format(parent_container, element, override_initial_box); break;
-	case FormattingContextType::Replaced: layout_box = ReplacedFormattingContext::Format(parent_container, element, override_initial_box); break;
-	case FormattingContextType::None: break;
+		switch (type)
+		{
+		case FormattingContextType::Block: layout_box = BlockFormattingContext::Format(parent_container, element, override_initial_box); break;
+		case FormattingContextType::Table: layout_box = TableFormattingContext::Format(parent_container, element, override_initial_box); break;
+		case FormattingContextType::Flex: layout_box = FlexFormattingContext::Format(parent_container, element, override_initial_box); break;
+		case FormattingContextType::Replaced: layout_box = ReplacedFormattingContext::Format(parent_container, element, override_initial_box); break;
+		case FormattingContextType::None: break;
+		}
 	}
 
 	if (layout_box && !is_under_max_content_constraint)
 	{
+		Optional<float> baseline_of_last_line;
+		float baseline_of_last_line_value = 0.f;
+		if (layout_box->GetBaselineOfLastLine(baseline_of_last_line_value))
+			baseline_of_last_line = baseline_of_last_line_value;
+
 		LayoutNode* layout_node = element->GetLayoutNode();
 		layout_node->CommitLayout(parent_container->GetContainingBlockSize(Style::Position::Static),
-			parent_container->GetContainingBlockSize(Style::Position::Absolute), override_initial_box);
+			parent_container->GetContainingBlockSize(Style::Position::Absolute), override_initial_box, layout_box->GetVisibleOverflowSize(),
+			baseline_of_last_line);
 	}
 
 #ifdef RMLUI_DEBUG
@@ -137,7 +148,9 @@ UniquePtr<LayoutBox> FormattingContext::FormatIndependent(ContainerBox* parent_c
 		debug_tracker->current_stack_level -= 1;
 		if (layout_box)
 		{
+			const bool from_cache = (layout_box->GetType() == LayoutBox::Type::CachedContainer);
 			tracker_entry->layout = Optional<FormatIndependentDebugTracker::Entry::LayoutResults>({
+				from_cache,
 				layout_box->GetVisibleOverflowSize(),
 				layout_box->GetIfBox() ? Optional<Box>{*layout_box->GetIfBox()} : std::nullopt,
 			});

Source/Core/Layout/FormattingContextDebug.cpp

@@ -107,7 +107,7 @@ String FormatIndependentDebugTracker::ToString() const
 
 		if (entry.layout)
 		{
-			result += newline + "|   Layout results:";
+			result += newline + "|   Layout results" + (entry.layout->from_cache ? " (cached)" : "") + ":";
 			result += newline + "|       Visible overflow: " + Rml::ToString(entry.layout->visible_overflow_size.x) + " x " +
 				Rml::ToString(entry.layout->visible_overflow_size.y);
 

Source/Core/Layout/FormattingContextDebug.h

@@ -52,6 +52,7 @@ public:
 		Optional<Box> override_box;
 		FormattingContextType type;
 		struct LayoutResults {
+			bool from_cache;
 			Vector2f visible_overflow_size;
 			Optional<Box> box;
 		};

Source/Core/Layout/LayoutBox.h

@@ -39,7 +39,7 @@ namespace Rml {
 */
 class LayoutBox {
 public:
-	enum class Type { Root, BlockContainer, InlineContainer, FlexContainer, TableWrapper, Replaced };
+	enum class Type { Root, BlockContainer, InlineContainer, FlexContainer, TableWrapper, Replaced, CachedContainer };
 
 	virtual ~LayoutBox() = default;
 

Source/Core/Layout/LayoutNode.cpp

@@ -34,15 +34,18 @@
 
 namespace Rml {
 
-LayoutNode* LayoutNode::GetClosestLayoutBoundary() const
+void LayoutNode::DirtyUpToClosestLayoutBoundary()
 {
+	if (!IsSelfDirty())
+		return;
+
 	for (Element* parent = element->GetParentNode(); parent; parent = parent->GetParentNode())
 	{
 		LayoutNode* parent_node = parent->GetLayoutNode();
+		parent_node->SetDirty(DirtyLayoutType::Child);
 		if (parent_node->IsLayoutBoundary())
-			return parent_node;
+			break;
 	}
-	return nullptr;
 }
 
 bool LayoutNode::IsLayoutBoundary() const

Source/Core/Layout/LayoutNode.h

@@ -60,8 +60,10 @@ inline DirtyLayoutType operator&(DirtyLayoutType lhs, DirtyLayoutType rhs)
 struct CommittedLayout {
 	Vector2f containing_block_size;
 	Vector2f absolutely_positioning_containing_block_size;
-
 	Optional<Box> override_box;
+
+	Vector2f visible_overflow_size;
+	Optional<float> baseline_of_last_line;
 };
 
 /*
@@ -73,7 +75,7 @@ public:
 
 	LayoutNode(Element* element) : element(element) {}
 
-	LayoutNode* GetClosestLayoutBoundary() const;
+	void DirtyUpToClosestLayoutBoundary();
 
 	void ClearDirty() { dirty_flag = DirtyLayoutType::None; }
 	void SetDirty(DirtyLayoutType dirty_type) { dirty_flag = dirty_flag | dirty_type; }
@@ -81,12 +83,15 @@ public:
 	bool IsDirty() const { return dirty_flag != DirtyLayoutType::None; }
 	bool IsSelfDirty() const { return !(dirty_flag == DirtyLayoutType::None || dirty_flag == DirtyLayoutType::Child); }
 
-	void CommitLayout(Vector2f containing_block_size, Vector2f absolutely_positioning_containing_block_size, const Box* override_box)
+	void CommitLayout(Vector2f containing_block_size, Vector2f absolutely_positioning_containing_block_size, const Box* override_box,
+		Vector2f visible_overflow_size, Optional<float> baseline_of_last_line)
 	{
 		committed_layout.emplace(CommittedLayout{
 			containing_block_size,
 			absolutely_positioning_containing_block_size,
 			override_box ? Optional<Box>(*override_box) : Optional<Box>(),
+			visible_overflow_size,
+			baseline_of_last_line,
 		});
 		ClearDirty();
 	}

Tests/Source/UnitTests/LayoutIsolation.cpp

@@ -209,7 +209,7 @@ TEST_CASE("LayoutIsolation.FullLayoutFormatIndependentCount")
 	document->Show();
 	TestsShell::RenderLoop();
 
-	Log::Message(Log::LT_DEBUG, "%s", format_independent_tracker.ToString().c_str());
+	Log::Message(Log::LT_INFO, "%s", format_independent_tracker.ToString().c_str());
 
 	const auto count_level_1 = std::count_if(format_independent_tracker.entries.begin(), format_independent_tracker.entries.end(),
 		[](const auto& entry) { return entry.level == 1; });
@@ -362,7 +362,7 @@ TEST_CASE("LayoutIsolation.Absolute")
 		CHECK(format_independent_tracker.entries.size() == 2);
 	}
 
-	Log::Message(Log::LT_DEBUG, "%s", format_independent_tracker.ToString().c_str());
+	Log::Message(Log::LT_INFO, "%s", format_independent_tracker.ToString().c_str());
 
 	document->Close();
 	TestsShell::ShutdownShell();