Skip to content

GitLab

T
tangled
Commit fc5997e3 authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot
Browse files
Options

[LayoutNG] Inline static position and OOFDescendant propagation 

This patch computes static position in inline layout, and propagates
OOFDescendants up the fragment tree.

kojii: I've added NGInlineLayoutAlgorithm::CurrentDirection method. 
Rename suggestions welcome.

Interesting bits:

1) LayoutBlock::LayoutPositionedObject 
LayoutBlock::LayoutPositionedObject had to be modified because it would
relayout already positioned OOF. The way it decided relayout was 
needed was by positioning the element, and then comparing its position
to position it already had. NG and Legacy would disagree here by a
fraction of a pixel, and this would trigger relayout.

My fix skips this check if LayoutNGBlockFlow is a containing block.

2) Failing tests: there are 23 new failures, and 159 passes.

I've examined the failures. Will need help from inline team on 2a, 2b
and 2c.

2a) DCHECK inline_size < 0
animations/interpolation/letter-spacing-interpolation.html
animations/interpolation/word-spacing-interpolation.html
fast/inline/empty-inline-create-linebox.html
fast/text/text-large-negative-letter-spacing-with-opacity.html
fast/text/text-letter-spacing.html
fast/text/international/rtl-negative-letter-spacing.html
css2.1/t0803-c5502-imrgn-r-03-b-a.html
css2.1/t0804-c5507-ipadn-r-03-b-a.html

2b) bad static position. Block is being positioned by inline-layout-algorithm,
instead of block-layout-algorithm. Should start a new line
external/wpt/css/CSS2/normal-flow/block-non-replaced-width-001.xht
external/wpt/css/CSS2/normal-flow/inline-block-replaced-height-004.xht
external/wpt/css/CSS2/normal-flow/inline-block-replaced-height-005.xht
external/wpt/css/CSS2/normal-flow/inline-block-replaced-height-007.xht
external/wpt/css/CSS2/normal-flow/inline-replaced-height-004.xht
external/wpt/css/CSS2/normal-flow/inline-replaced-height-005.xht
external/wpt/css/CSS2/normal-flow/inline-replaced-height-007.xht
fast/css/sticky/sticky-vertically-overconstrained.html

2c) <span> has many <br>. NG includes <br> in line height, Legacy does not. FF
  does not at all
fast/replaced/absolute-position-percentage-height.html

2d) passes on reload. Content might not be laid out when document.onload() fires
Fully correct (like FF), unlike Legacy which fails 2 tests.
fast/box-sizing/replaced.html

2e) some relayout multicol weirdness
fast/multicol/dynamic/relayout-abspos-in-relpos-spanner.html
ietestcenter/css3/multicolumn/column-containing-block-001.htm
ietestcenter/css3/multicolumn/column-width-applies-to-008.htm
ietestcenter/css3/multicolumn/column-width-applies-to-014.htm

2f) unusual test: svg, shadow roots, i am not sure what is going on
svg/foreign-object-under-shadow-root-under-hidden.html



Bug: 740993
Change-Id: I0eab6ec0ee87bdb10c9a69df0e18f19bcaafef68
Reviewed-on: https://chromium-review.googlesource.com/738870
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513087}
parent a7743377
Hide whitespace changes
Inline Side-by-side
Showing with 85 additions and 28 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ fc5997e3
......@@ -371,6 +371,15 @@ crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/block-in-in
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/block-in-inline-remove-004.xht [ Failure Pass ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/block-in-inline-remove-005.xht [ Failure Pass ]
# Inline: Abspos should start new block layout to compute static position
crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/block-non-replaced-width-001.xht [ Failure ]
crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-block-replaced-height-004.xht [ Failure ]
crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-block-replaced-height-005.xht [ Failure ]
crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-block-replaced-height-007.xht [ Failure ]
crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-height-004.xht [ Failure ]
crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-height-005.xht [ Failure ]
crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-height-007.xht [ Failure ]
# Block: margin collapsing.
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/root-box-001.xht [ Failure ]
......@@ -551,9 +560,7 @@ crbug.com/635619 virtual/layout_ng/fast/block/margin-collapse/webkit-margin-coll
crbug.com/635619 virtual/layout_ng/fast/block/margin-collapse/webkit-margin-collapse-siblings.html [ Failure ]
### virtual/layout_ng/overflow
crbug.com/728378 virtual/layout_ng/overflow/overflow-basic-003.html [ Failure ]
crbug.com/724701 virtual/layout_ng/overflow/overflow-basic-004.html [ Failure ]
crbug.com/728378 virtual/layout_ng/overflow/overflow-bug-chrome-ng-001.html [ Failure ]
crbug.com/728378 virtual/layout_ng/overflow/overflow-position-003.html [ Failure ]
# ====== LayoutNG-only failures until here ======
......
third_party/WebKit/Source/core/layout/LayoutBlock.cpp
View file @ fc5997e3
......@@ -794,7 +794,8 @@ void LayoutBlock::LayoutPositionedObject(LayoutBox* positioned_object,
if (!positioned_object->NormalChildNeedsLayout() &&
(relayout_children || height_available_to_children_changed_ ||
NeedsLayoutDueToStaticPosition(positioned_object)))
(!IsLayoutNGBlockFlow() &&
NeedsLayoutDueToStaticPosition(positioned_object))))
layout_scope.SetChildNeedsLayout(positioned_object);
LayoutUnit logical_top_estimate;
......
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc
View file @ fc5997e3
......@@ -71,6 +71,13 @@ void NGInlineLayoutAlgorithm::CreateLine(NGLineInfo* line_info,
PlaceItems(line_info, *exclusion_space);
}
TextDirection NGInlineLayoutAlgorithm::CurrentDirection(
TextDirection default_direction) const {
if (!box_states_)
return default_direction;
return box_states_->LineBoxState().style->Direction();
}
void NGInlineLayoutAlgorithm::BidiReorder(NGInlineItemResults* line_items) {
// TODO(kojii): UAX#9 L1 is not supported yet. Supporting L1 may change
// embedding levels of parts of runs, which requires to split items.
......@@ -209,16 +216,10 @@ void NGInlineLayoutAlgorithm::PlaceItems(
PlaceListMarker(item, &item_result, *line_info);
DCHECK_GT(line_box_.size(), list_marker_index.value());
} else if (item.Type() == NGInlineItem::kOutOfFlowPositioned) {
// TODO(layout-dev): Report the correct static position for the out of
// flow descendant. We can't do this here yet as it doesn't know the
// size of the line box.
container_builder_.AddOutOfFlowDescendant(
// Absolute positioning blockifies the box's display type.
// https://drafts.csswg.org/css-display/#transformations
{NGBlockNode(ToLayoutBox(item.GetLayoutObject())),
NGStaticPosition::Create(ConstraintSpace().WritingMode(),
ConstraintSpace().Direction(),
NGPhysicalOffset())});
NGBlockNode node(ToLayoutBox(item.GetLayoutObject()));
container_builder_.AddOutOfFlowChildCandidate(
node, NGLogicalOffset(position, LayoutUnit()),
CurrentDirection(line_info->BaseDirection()));
continue;
} else {
continue;
......@@ -571,6 +572,11 @@ scoped_refptr<NGLayoutResult> NGInlineLayoutAlgorithm::Layout() {
container_builder_.SwapPositionedFloats(&positioned_floats_);
container_builder_.SetExclusionSpace(std::move(exclusion_space));
Vector<NGOutOfFlowPositionedDescendant> descendant_candidates;
container_builder_.GetAndClearOutOfFlowDescendantCandidates(
&descendant_candidates);
for (auto& descendant : descendant_candidates)
container_builder_.AddOutOfFlowDescendant(descendant);
return container_builder_.ToLineBoxFragment();
}
......@@ -589,14 +595,9 @@ scoped_refptr<NGLayoutResult> NGInlineLayoutAlgorithm::LayoutEmptyInline() {
ConstraintSpace().PercentageResolutionSize(), bfc_line_offset,
bfc_line_offset, margins, node, /* break_token */ nullptr));
} else if (item.Type() == NGInlineItem::kOutOfFlowPositioned) {
// TODO(layout-dev): Report the correct static position for the out of
// flow descendant. We can't do this here yet as it doesn't know the
// size of the line box.
NGBlockNode node(ToLayoutBox(item.GetLayoutObject()));
container_builder_.AddOutOfFlowDescendant(
{node, NGStaticPosition::Create(ConstraintSpace().WritingMode(),
ConstraintSpace().Direction(),
NGPhysicalOffset())});
container_builder_.AddOutOfFlowChildCandidate(
node, NGLogicalOffset(), CurrentDirection(node.Style().Direction()));
} else {
DCHECK_NE(item.Type(), NGInlineItem::kAtomicInline);
DCHECK_NE(item.Type(), NGInlineItem::kControl);
......@@ -617,6 +618,11 @@ scoped_refptr<NGLayoutResult> NGInlineLayoutAlgorithm::LayoutEmptyInline() {
container_builder_.SetEndMarginStrut(ConstraintSpace().MarginStrut());
container_builder_.SetExclusionSpace(std::move(exclusion_space));
Vector<NGOutOfFlowPositionedDescendant> descendant_candidates;
container_builder_.GetAndClearOutOfFlowDescendantCandidates(
&descendant_candidates);
for (auto& descendant : descendant_candidates)
container_builder_.AddOutOfFlowDescendant(descendant);
return container_builder_.ToLineBoxFragment();
}
......
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.h
View file @ fc5997e3
......@@ -52,6 +52,8 @@ class CORE_EXPORT NGInlineLayoutAlgorithm final
bool IsHorizontalWritingMode() const { return is_horizontal_writing_mode_; }
TextDirection CurrentDirection(TextDirection default_direction) const;
void BidiReorder(NGInlineItemResults*);
void PlaceItems(NGLineInfo*, const NGExclusionSpace&);
......
third_party/WebKit/Source/core/layout/ng/ng_container_fragment_builder.cc
View file @ fc5997e3
......@@ -22,6 +22,7 @@ NGContainerFragmentBuilder::~NGContainerFragmentBuilder() {}
NGContainerFragmentBuilder& NGContainerFragmentBuilder::SetInlineSize(
LayoutUnit inline_size) {
DCHECK_GE(inline_size, LayoutUnit());
inline_size_ = inline_size;
return *this;
}
......@@ -54,10 +55,44 @@ NGContainerFragmentBuilder& NGContainerFragmentBuilder::AddChild(
scoped_refptr<NGLayoutResult> child,
const NGLogicalOffset& child_offset) {
// Collect the child's out of flow descendants.
for (const NGOutOfFlowPositionedDescendant& descendant :
child->OutOfFlowPositionedDescendants()) {
oof_positioned_candidates_.push_back(
NGOutOfFlowPositionedCandidate{descendant, child_offset});
// child_offset is offset of inline_start/block_start vertex.
// Candidates need offset of top/left vertex.
const auto& ouf_of_flow_descendants = child->OutOfFlowPositionedDescendants();
if (!ouf_of_flow_descendants.IsEmpty()) {
NGLogicalOffset top_left_offset;
NGPhysicalSize child_size = child->PhysicalFragment()->Size();
switch (WritingMode()) {
case kHorizontalTopBottom:
top_left_offset =
(IsRtl(Direction()))
? NGLogicalOffset{child_offset.inline_offset + child_size.width,
child_offset.block_offset}
: child_offset;
break;
case kVerticalRightLeft:
case kSidewaysRightLeft:
top_left_offset =
(IsRtl(Direction()))
? NGLogicalOffset{child_offset.inline_offset +
child_size.height,
child_offset.block_offset + child_size.width}
: NGLogicalOffset{child_offset.inline_offset,
child_offset.block_offset + child_size.width};
break;
case kVerticalLeftRight:
case kSidewaysLeftRight:
top_left_offset = (IsRtl(Direction()))
? NGLogicalOffset{child_offset.inline_offset +
child_size.height,
child_offset.block_offset}
: child_offset;
break;
}
for (const NGOutOfFlowPositionedDescendant& descendant :
ouf_of_flow_descendants) {
oof_positioned_candidates_.push_back(
NGOutOfFlowPositionedCandidate{descendant, top_left_offset});
}
}
return AddChild(child->PhysicalFragment(), child_offset);
......@@ -74,12 +109,14 @@ NGContainerFragmentBuilder& NGContainerFragmentBuilder::AddChild(
NGContainerFragmentBuilder&
NGContainerFragmentBuilder::AddOutOfFlowChildCandidate(
NGBlockNode child,
const NGLogicalOffset& child_offset) {
const NGLogicalOffset& child_offset,
Optional<TextDirection> direction) {
DCHECK(child);
oof_positioned_candidates_.push_back(NGOutOfFlowPositionedCandidate{
NGOutOfFlowPositionedDescendant{
child, NGStaticPosition::Create(WritingMode(), Direction(),
child, NGStaticPosition::Create(WritingMode(),
direction ? *direction : Direction(),
NGPhysicalOffset())},
child_offset});
......
third_party/WebKit/Source/core/layout/ng/ng_container_fragment_builder.h
View file @ fc5997e3
......@@ -84,9 +84,13 @@ class CORE_EXPORT NGContainerFragmentBuilder : public NGBaseFragmentBuilder {
// NGOutOfFlowLayoutPart(container_style, builder).Run();
//
// See layout part for builder interaction.
//
// @param direction: default candidate direction is builder's direction.
// Pass in direction if candidates direction does not match.
NGContainerFragmentBuilder& AddOutOfFlowChildCandidate(
NGBlockNode,
const NGLogicalOffset&);
const NGLogicalOffset& child_offset,
Optional<TextDirection> direction = WTF::nullopt);
NGContainerFragmentBuilder& AddOutOfFlowDescendant(
NGOutOfFlowPositionedDescendant);
......@@ -112,7 +116,7 @@ class CORE_EXPORT NGContainerFragmentBuilder : public NGBaseFragmentBuilder {
// physical size the fragment builder.
struct NGOutOfFlowPositionedCandidate {
NGOutOfFlowPositionedDescendant descendant;
NGLogicalOffset child_offset;
NGLogicalOffset child_offset; // Logical offset of child's top left vertex.
};
NGContainerFragmentBuilder(scoped_refptr<const ComputedStyle>,
......
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