Commit 5ea405fd authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[LayoutNG] Consider inline-level OOF-positioned nodes as "adjoining".

"adjoining" objects are used to indicate that a particular node might
need relayout once its BFC block-offset is resolved. Previously we
thought that we just needed to know about "adjoining-floats".

However inline-level OOF-positioned nodes also needs to know its BFC
block-offset as the static-position of these nodes depend on where
floats are.

Previously as we didn't know that we needed to relayout these nodes
we'd get the incorrect static-position. Now these nodes get the
correct static-position.

Bug: 982403, 980908
Change-Id: I89f18298fd7379358a681b98514891d8d35bc38e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1692627Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676263}
parent 3939afbc
...@@ -503,6 +503,8 @@ void NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects( ...@@ -503,6 +503,8 @@ void NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects(
DCHECK(line_info.IsEmptyLine() || !line_box_metrics.IsEmpty()) DCHECK(line_info.IsEmptyLine() || !line_box_metrics.IsEmpty())
<< "Non-empty lines must have a valid set of linebox metrics."; << "Non-empty lines must have a valid set of linebox metrics.";
bool is_empty_inline = Node().IsEmptyInline();
// All children within the linebox are positioned relative to the baseline, // All children within the linebox are positioned relative to the baseline,
// then shifted later using NGLineBoxFragmentBuilder::MoveInBlockDirection. // then shifted later using NGLineBoxFragmentBuilder::MoveInBlockDirection.
LayoutUnit baseline_adjustment = LayoutUnit baseline_adjustment =
...@@ -533,10 +535,10 @@ void NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects( ...@@ -533,10 +535,10 @@ void NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects(
// To correctly determine which "line" block-level out-of-flow positioned // To correctly determine which "line" block-level out-of-flow positioned
// object is placed on, we need to keep track of if there is any inline-level // object is placed on, we need to keep track of if there is any inline-level
// content preceeding it. // content preceeding it.
bool has_preceeding_inline_level_content = false; bool has_preceding_inline_level_content = false;
for (NGLineBoxFragmentBuilder::Child& child : line_box_) { for (NGLineBoxFragmentBuilder::Child& child : line_box_) {
has_preceeding_inline_level_content |= child.HasInFlowFragment(); has_preceding_inline_level_content |= child.HasInFlowFragment();
LayoutObject* box = child.out_of_flow_positioned_box; LayoutObject* box = child.out_of_flow_positioned_box;
if (!box) if (!box)
...@@ -547,11 +549,20 @@ void NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects( ...@@ -547,11 +549,20 @@ void NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects(
// An inline-level OOF element positions itself within the line, at the // An inline-level OOF element positions itself within the line, at the
// position it would have been if it was in-flow. // position it would have been if it was in-flow.
static_offset.inline_offset = child.offset.inline_offset; static_offset.inline_offset = child.offset.inline_offset;
// The static-position of inline-level OOF-positioned nodes depends on
// previous floats (if any).
//
// If we are an empty-inline we may not have the correct BFC block-offset
// yet. Due to this we need to mark this node as having adjoining
// objects, and perform a re-layout if our position shifts.
if (is_empty_inline)
container_builder_.AddAdjoiningFloatTypes(kAdjoiningInlineOutOfFlow);
} else { } else {
// A block-level OOF element positions itself on the "next" line. However // A block-level OOF element positions itself on the "next" line. However
// only shifts down if there is inline-level content. // only shifts down if there is inline-level content.
static_offset.inline_offset = block_level_inline_offset; static_offset.inline_offset = block_level_inline_offset;
if (has_preceeding_inline_level_content) if (has_preceding_inline_level_content)
static_offset.block_offset += line_height; static_offset.block_offset += line_height;
} }
...@@ -798,11 +809,16 @@ scoped_refptr<const NGLayoutResult> NGInlineLayoutAlgorithm::Layout() { ...@@ -798,11 +809,16 @@ scoped_refptr<const NGLayoutResult> NGInlineLayoutAlgorithm::Layout() {
unsigned handled_leading_floats_index = unsigned handled_leading_floats_index =
PositionLeadingFloats(&initial_exclusion_space, &leading_floats); PositionLeadingFloats(&initial_exclusion_space, &leading_floats);
// Only empty-inlines should have the "forced" BFC block-offset set.
DCHECK(is_empty_inline || !ConstraintSpace().ForcedBfcBlockOffset());
// We query all the layout opportunities on the initial exclusion space up // We query all the layout opportunities on the initial exclusion space up
// front, as if the line breaker may add floats and change the opportunities. // front, as if the line breaker may add floats and change the opportunities.
const LayoutOpportunityVector opportunities = const LayoutOpportunityVector opportunities =
initial_exclusion_space.AllLayoutOpportunities( initial_exclusion_space.AllLayoutOpportunities(
ConstraintSpace().BfcOffset(), {ConstraintSpace().BfcOffset().line_offset,
ConstraintSpace().ForcedBfcBlockOffset().value_or(
ConstraintSpace().BfcOffset().block_offset)},
ConstraintSpace().AvailableSize().inline_size); ConstraintSpace().AvailableSize().inline_size);
NGExclusionSpace exclusion_space; NGExclusionSpace exclusion_space;
...@@ -919,10 +935,6 @@ scoped_refptr<const NGLayoutResult> NGInlineLayoutAlgorithm::Layout() { ...@@ -919,10 +935,6 @@ scoped_refptr<const NGLayoutResult> NGInlineLayoutAlgorithm::Layout() {
continue; continue;
} }
if (opportunity.rect.BlockStartOffset() >
ConstraintSpace().BfcOffset().block_offset)
container_builder_.SetIsPushedByFloats();
// Success! // Success!
container_builder_.SetBreakToken(line_breaker.CreateBreakToken(line_info)); container_builder_.SetBreakToken(line_breaker.CreateBreakToken(line_info));
...@@ -939,6 +951,10 @@ scoped_refptr<const NGLayoutResult> NGInlineLayoutAlgorithm::Layout() { ...@@ -939,6 +951,10 @@ scoped_refptr<const NGLayoutResult> NGInlineLayoutAlgorithm::Layout() {
// our adjoining floats, and shouldn't propagate this information // our adjoining floats, and shouldn't propagate this information
// to siblings. // to siblings.
container_builder_.ResetAdjoiningFloatTypes(); container_builder_.ResetAdjoiningFloatTypes();
if (opportunity.rect.BlockStartOffset() >
ConstraintSpace().BfcOffset().block_offset)
container_builder_.SetIsPushedByFloats();
} }
break; break;
} }
......
...@@ -803,10 +803,26 @@ void NGBlockLayoutAlgorithm::HandleOutOfFlowPositioned( ...@@ -803,10 +803,26 @@ void NGBlockLayoutAlgorithm::HandleOutOfFlowPositioned(
static_offset.block_offset += previous_inflow_position.margin_strut.Sum(); static_offset.block_offset += previous_inflow_position.margin_strut.Sum();
if (child.Style().IsOriginalDisplayInlineType()) { if (child.Style().IsOriginalDisplayInlineType()) {
// The static-position of inline-level OOF-positioned nodes depends on
// previous floats (if any).
//
// Due to this we need to mark this node as having adjoining objects, and
// perform a re-layout if our position shifts.
if (!container_builder_.BfcBlockOffset()) {
container_builder_.AddAdjoiningFloatTypes(kAdjoiningInlineOutOfFlow);
abort_when_bfc_block_offset_updated_ = true;
}
LayoutUnit origin_bfc_block_offset =
container_builder_.BfcBlockOffset().value_or(
ConstraintSpace().ForcedBfcBlockOffset().value_or(
ConstraintSpace().BfcOffset().block_offset)) +
static_offset.block_offset;
NGBfcOffset origin_bfc_offset = { NGBfcOffset origin_bfc_offset = {
ConstraintSpace().BfcOffset().line_offset + ConstraintSpace().BfcOffset().line_offset +
border_scrollbar_padding_.LineLeft(Style().Direction()), border_scrollbar_padding_.LineLeft(Style().Direction()),
BfcBlockOffset() + static_offset.block_offset}; origin_bfc_block_offset};
static_offset.inline_offset += CalculateOutOfFlowStaticInlineLevelOffset( static_offset.inline_offset += CalculateOutOfFlowStaticInlineLevelOffset(
Style(), origin_bfc_offset, exclusion_space_, Style(), origin_bfc_offset, exclusion_space_,
......
...@@ -1365,15 +1365,17 @@ TEST_F(NGBlockLayoutAlgorithmTest, PositionFloatInsideEmptyBlocks) { ...@@ -1365,15 +1365,17 @@ TEST_F(NGBlockLayoutAlgorithmTest, PositionFloatInsideEmptyBlocks) {
offset = offset =
To<NGPhysicalLineBoxFragment>(linebox_fragment)->Children()[0].offset; To<NGPhysicalLineBoxFragment>(linebox_fragment)->Children()[0].offset;
// inline 10 = left float's margin(10) // The floats are positioned outside the line-box as the line-box is
// "avoiding" these floats.
// inline -35 = inline-size of left-float (including margins).
// block 10 = left float's margin // block 10 = left float's margin
EXPECT_THAT(offset, PhysicalOffset(10, 10)); EXPECT_THAT(offset, PhysicalOffset(-35, 10));
offset = offset =
To<NGPhysicalLineBoxFragment>(linebox_fragment)->Children()[1].offset; To<NGPhysicalLineBoxFragment>(linebox_fragment)->Children()[1].offset;
// inline offset 135 = right float's margin(10) + right float offset(125) // inline offset 90 = right float's margin(10) + right float offset(80)
// block offset 15 = right float's margin // block offset 15 = right float's margin
LayoutUnit right_float_offset = LayoutUnit(125); LayoutUnit right_float_offset = LayoutUnit(80);
EXPECT_THAT(offset, PhysicalOffset(LayoutUnit(10) + right_float_offset, EXPECT_THAT(offset, PhysicalOffset(LayoutUnit(10) + right_float_offset,
LayoutUnit(15))); LayoutUnit(15)));
......
...@@ -580,7 +580,7 @@ class CORE_EXPORT NGConstraintSpace final { ...@@ -580,7 +580,7 @@ class CORE_EXPORT NGConstraintSpace final {
} }
unsigned has_rare_data : 1; unsigned has_rare_data : 1;
unsigned adjoining_floats : 2; // NGFloatTypes unsigned adjoining_floats : 3; // NGFloatTypes
unsigned writing_mode : 3; unsigned writing_mode : 3;
unsigned direction : 1; unsigned direction : 1;
......
...@@ -23,11 +23,13 @@ struct NGUnpositionedFloat; ...@@ -23,11 +23,13 @@ struct NGUnpositionedFloat;
typedef Vector<NGPositionedFloat, 8> NGPositionedFloatVector; typedef Vector<NGPositionedFloat, 8> NGPositionedFloatVector;
// TODO(ikilpatrick): Rename this to NGAdjoiningObjectTypes.
enum NGFloatTypeValue { enum NGFloatTypeValue {
kFloatTypeNone = 0b00, kFloatTypeNone = 0b000,
kFloatTypeLeft = 0b01, kFloatTypeLeft = 0b001,
kFloatTypeRight = 0b10, kFloatTypeRight = 0b010,
kFloatTypeBoth = 0b11 kFloatTypeBoth = 0b011,
kAdjoiningInlineOutOfFlow = 0b100
}; };
typedef int NGFloatTypes; typedef int NGFloatTypes;
......
...@@ -329,7 +329,7 @@ class CORE_EXPORT NGLayoutResult : public RefCounted<NGLayoutResult> { ...@@ -329,7 +329,7 @@ class CORE_EXPORT NGLayoutResult : public RefCounted<NGLayoutResult> {
unsigned is_self_collapsing : 1; unsigned is_self_collapsing : 1;
unsigned is_pushed_by_floats : 1; unsigned is_pushed_by_floats : 1;
unsigned adjoining_floats : 2; // NGFloatTypes unsigned adjoining_floats : 3; // NGFloatTypes
unsigned is_initial_block_size_indefinite : 1; unsigned is_initial_block_size_indefinite : 1;
unsigned has_descendant_that_depends_on_percentage_block_size : 1; unsigned has_descendant_that_depends_on_percentage_block_size : 1;
......
<!DOCTYPE html>
<link rel="help" href="https://crbug.com/982403" />
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<meta name="assert" content="Tests the static position of an inline-level absolute-positioned element." />
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="overflow: hidden;">
<div style="float: left; width: 100px; height: 50px; background: green;"></div>
<div style="clear: both; width: 100px; height: 50px; background: red;">
<div style="position: absolute; display: inline; width: 100px; height: 50px; background: green;"></div>
</div>
</div>
<!DOCTYPE html>
<link rel="help" href="https://crbug.com/982403" />
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<meta name="assert" content="Tests the static position of an inline-level absolute-positioned element." />
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="overflow: hidden;">
<div style="float: left; width: 100px; height: 50px; background: green;"></div>
<div style="clear: both; width: 100px; height: 50px; background: red;">
<div></div>
<div style="position: absolute; display: inline; width: 100px; height: 50px; background: green;"></div>
</div>
</div>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment