Browse Source

Call Element::OnLayout only after all other layouting is done

To avoid multiple calls to OnLayout, e.g. after re-iterating over a set of elements that has had scrollbars added in a parent.
Michael Ragazzon 6 months ago
parent
commit
b6a6e7761e

+ 1 - 1
Include/RmlUi/Core/Element.h

@@ -706,7 +706,7 @@ private:
 	void DirtyFontFaceRecursive();
 
 	void ClampScrollOffset();
-	void ClampScrollOffsetRecursive();
+	void CommitLayoutRecursive();
 
 	/// Start an animation, replacing any existing animations of the same property name. If start_value is null, the element's current value is used.
 	ElementAnimationList::iterator StartAnimation(PropertyId property_id, const Property* start_value, int num_iterations, bool alternate_direction,

+ 13 - 2
Source/Core/Element.cpp

@@ -3019,12 +3019,23 @@ void Element::ClampScrollOffset()
 	meta->scroll.UpdateProperties();
 }
 
-void Element::ClampScrollOffsetRecursive()
+void Element::CommitLayoutRecursive()
 {
+	if (GetDisplay() == Style::Display::None)
+		return;
+
+	OnLayout();
+
+	// The size of the scrollable area might have changed, so clamp the scroll offset to avoid scrolling outside the
+	// scrollable area. During layouting, we might be changing the scrollable overflow area of the element several
+	// times, such as after enabling scrollbars. For this reason, we don't clamp the scroll offset during layouting, as
+	// that could inadvertently clamp it to a temporary size. Now that we know the final layout, including the size of
+	// each element's scrollable area, we can finally clamp the scroll offset.
 	ClampScrollOffset();
+
 	const int num_children = GetNumChildren();
 	for (int i = 0; i < num_children; ++i)
-		GetChild(i)->ClampScrollOffsetRecursive();
+		GetChild(i)->CommitLayoutRecursive();
 }
 
 } // namespace Rml

+ 0 - 2
Source/Core/Layout/BlockContainer.cpp

@@ -119,8 +119,6 @@ bool BlockContainer::Close(BlockContainer* parent_block_container)
 
 	EnsureEmptyInterruptedLineBox();
 
-	SubmitElementLayout();
-
 	return true;
 }
 

+ 0 - 7
Source/Core/Layout/ContainerBox.cpp

@@ -124,11 +124,6 @@ void ContainerBox::SetElementBaseline(float element_baseline)
 	element->SetBaseline(element_baseline);
 }
 
-void ContainerBox::SubmitElementLayout()
-{
-	element->OnLayout();
-}
-
 ContainerBox::ContainerBox(Type type, Element* element, ContainerBox* parent_container) :
 	LayoutBox(type), element(element), parent_container(parent_container)
 {
@@ -269,7 +264,6 @@ bool FlexContainer::Close(const Vector2f content_overflow_size, const Box& box,
 
 	ClosePositionedElements();
 
-	SubmitElementLayout();
 	SetElementBaseline(element_baseline);
 	return true;
 }
@@ -304,7 +298,6 @@ void TableWrapper::Close(const Vector2f content_overflow_size, const Box& box, f
 
 	ClosePositionedElements();
 
-	SubmitElementLayout();
 	SetElementBaseline(element_baseline);
 }
 

+ 0 - 2
Source/Core/Layout/ContainerBox.h

@@ -82,8 +82,6 @@ protected:
 
 	// Set the element's baseline (proxy for private access to Element).
 	void SetElementBaseline(float element_baseline);
-	// Calls Element::OnLayout (proxy for private access to Element).
-	void SubmitElementLayout();
 
 	// The element this box represents, if any.
 	Element* const element;

+ 0 - 1
Source/Core/Layout/InlineBox.cpp

@@ -147,7 +147,6 @@ void InlineBox::Submit(const PlacedFragment& placed_fragment)
 		element_offset = border_position;
 		element->SetOffset(border_position, placed_fragment.offset_parent);
 		element->SetBox(element_box);
-		SubmitElementOnLayout();
 	}
 	else
 	{

+ 0 - 6
Source/Core/Layout/InlineLevelBox.cpp

@@ -48,11 +48,6 @@ void InlineLevelBox::operator delete(void* chunk, size_t size)
 
 InlineLevelBox::~InlineLevelBox() {}
 
-void InlineLevelBox::SubmitElementOnLayout()
-{
-	element->OnLayout();
-}
-
 const FontMetrics& InlineLevelBox::GetFontMetrics() const
 {
 	if (FontFaceHandle handle = element->GetFontFaceHandle())
@@ -162,7 +157,6 @@ void InlineLevelBox_Atomic::Submit(const PlacedFragment& placed_fragment)
 
 	GetElement()->SetOffset(border_position, placed_fragment.offset_parent);
 	GetElement()->SetBox(box);
-	SubmitElementOnLayout();
 }
 
 InlineLevelBox_Text::InlineLevelBox_Text(ElementText* element) : InlineLevelBox(element) {}

+ 0 - 3
Source/Core/Layout/InlineLevelBox.h

@@ -82,9 +82,6 @@ protected:
 	// Set the inner-to-outer spacing (margin + border + padding) for inline boxes.
 	void SetInlineBoxSpacing(float spacing_left, float spacing_right);
 
-	// Calls Element::OnLayout (proxy for private access to Element).
-	void SubmitElementOnLayout();
-
 private:
 	float height_above_baseline = 0.f;
 	float depth_below_baseline = 0.f;

+ 2 - 7
Source/Core/Layout/LayoutEngine.cpp

@@ -48,13 +48,8 @@ void LayoutEngine::FormatElement(Element* element, Vector2f containing_block)
 	}
 
 	{
-		RMLUI_ZoneScopedN("ClampScrollOffsetRecursive");
-		// The size of the scrollable area might have changed, so clamp the scroll offset to avoid scrolling outside the
-		// scrollable area. During layouting, we might be changing the scrollable overflow area of the element several
-		// times, such as after enabling scrollbars. For this reason, we don't clamp the scroll offset during layouting,
-		// as that could inadvertently clamp it to a temporary size. Now that we know the final layout, including the
-		// size of each element's scrollable area, we can finally clamp the scroll offset.
-		element->ClampScrollOffsetRecursive();
+		RMLUI_ZoneScopedN("CommitLayoutRecursive");
+		element->CommitLayoutRecursive();
 	}
 }
 

+ 0 - 1
Source/Core/Layout/ReplacedFormattingContext.cpp

@@ -68,7 +68,6 @@ UniquePtr<LayoutBox> ReplacedFormattingContext::Format(ContainerBox* parent_cont
 void ReplacedBox::Close()
 {
 	element->SetBox(box);
-	element->OnLayout();
 }
 
 String ReplacedBox::DebugDumpTree(int depth) const