Browse Source

Skip layout cache if box is different, and on the first layout without layout constraints

This fixes one CSS test in particular (blocks-025) where the cached layout would produce a slightly different result.
Michael Ragazzon 4 months ago
parent
commit
e40b04def7

+ 4 - 3
Source/Core/Layout/FormattingContext.cpp

@@ -125,7 +125,8 @@ UniquePtr<LayoutBox> FormattingContext::FormatIndependent(ContainerBox* parent_c
 		const Vector2f absolute_containing_block = parent_container->GetContainingBlockSize(Style::Position::Absolute);
 
 		LayoutNode* layout_node = element->GetLayoutNode();
-		if (layout_node->CommittedLayoutMatches(containing_block, absolute_containing_block, override_initial_box))
+		if (layout_node->CommittedLayoutMatches(containing_block, absolute_containing_block, override_initial_box,
+				formatting_mode.constraint != FormattingMode::Constraint::None))
 		{
 #ifdef RMLUI_DEBUG
 			Log::Message(Log::LT_INFO, "Layout cache match on element%s: %s",
@@ -179,8 +180,8 @@ UniquePtr<LayoutBox> FormattingContext::FormatIndependent(ContainerBox* parent_c
 
 			LayoutNode* layout_node = element->GetLayoutNode();
 			layout_node->CommitLayout(parent_container->GetContainingBlockSize(Style::Position::Static),
-				parent_container->GetContainingBlockSize(Style::Position::Absolute), override_initial_box, layout_box->GetVisibleOverflowSize(),
-				baseline_of_last_line);
+				parent_container->GetContainingBlockSize(Style::Position::Absolute), override_initial_box,
+				formatting_mode.constraint != FormattingMode::Constraint::None, layout_box->GetVisibleOverflowSize(), baseline_of_last_line);
 		}
 	}
 

+ 2 - 1
Source/Core/Layout/LayoutNode.cpp

@@ -84,7 +84,7 @@ void LayoutNode::SetDirty(DirtyLayoutType dirty_type)
 }
 
 void LayoutNode::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)
+	bool layout_constraint, Vector2f visible_overflow_size, Optional<float> baseline_of_last_line)
 {
 	// TODO: This is mixing slightly different concepts. Rather, it might be advantageous to separate what is the input
 	// to the layout of the current element, and what is the output. That way we can e.g. set the containing block size
@@ -99,6 +99,7 @@ void LayoutNode::CommitLayout(Vector2f containing_block_size, Vector2f absolutel
 		containing_block_size,
 		absolutely_positioning_containing_block_size,
 		override_box ? Optional<Box>(*override_box) : Optional<Box>(),
+		layout_constraint,
 		visible_overflow_size,
 		baseline_of_last_line,
 	});

+ 17 - 14
Source/Core/Layout/LayoutNode.h

@@ -61,6 +61,7 @@ struct CommittedLayout {
 	Vector2f containing_block_size;
 	Vector2f absolutely_positioning_containing_block_size;
 	Optional<Box> override_box;
+	bool layout_constraint;
 
 	Vector2f visible_overflow_size;
 	Optional<float> baseline_of_last_line;
@@ -86,7 +87,7 @@ public:
 	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,
-		Vector2f visible_overflow_size, Optional<float> baseline_of_last_line);
+		bool layout_constraint, Vector2f visible_overflow_size, Optional<float> baseline_of_last_line);
 
 	// TODO: Currently, should only be used by `Context::SetDimensions`. Ideally, we would remove this and "commit" the
 	// containing block (without the layout result, see comment in `CommitLayout`).
@@ -96,7 +97,8 @@ public:
 		committed_max_content_width.reset();
 	}
 
-	bool CommittedLayoutMatches(Vector2f containing_block_size, Vector2f absolutely_positioning_containing_block_size, const Box* override_box) const
+	bool CommittedLayoutMatches(Vector2f containing_block_size, Vector2f absolutely_positioning_containing_block_size, const Box* override_box,
+		bool layout_constraint) const
 	{
 		if (IsDirty())
 			return false;
@@ -106,24 +108,25 @@ public:
 			committed_layout->absolutely_positioning_containing_block_size != absolutely_positioning_containing_block_size)
 			return false;
 
+		// Layout under a constraint may make some simplifications that requires re-evaluation under a normal formatting mode.
+		if (committed_layout->layout_constraint && !layout_constraint)
+			return false;
+
 		if (!override_box)
 			return !committed_layout->override_box.has_value();
 
 		const Box& compare_box = committed_layout->override_box.has_value() ? *committed_layout->override_box : element->GetBox();
-		if (!override_box->EqualAreaEdges(*committed_layout->override_box))
-			return false;
-
-		if (override_box->GetSize() == compare_box.GetSize())
-			return true;
 
-		// Lastly, if we have an indefinite size on the committed box, see if the layed-out size matches the input override box.
-		Vector2f compare_size = compare_box.GetSize();
-		if (compare_size.x < 0.f)
-			compare_size.x = element->GetBox().GetSize().x;
-		if (compare_size.y < 0.f)
-			compare_size.y = element->GetBox().GetSize().y;
+		// In some situations, if we have an indefinite size on the committed box, we could see if the layed-out size
+		// matches the input override box and use the cache here. However, because of cyclic-percentage rules with
+		// containing block sizes, this is only correct in certain situations, particularly when the vertical size of
+		// the containing block is indefinite. In this case the used containing block size should resolve to indefinite,
+		// even after it is sized. Although, we don't actually implement this behavior for now, but once we do, we could
+		// implement caching here in this case. For the horizontal axis, we are always required to re-evaluate any
+		// children for which this box acts as a containing block for, thus we cannot use a cache mechanism here. See:
+		// https://drafts.csswg.org/css-sizing/#cyclic-percentage-contribution
 
-		return override_box->GetSize() == compare_size;
+		return *override_box == compare_box;
 	}
 
 	Optional<float> GetMaxContentWidthIfCached() const { return committed_max_content_width; }