Commit 0f4ea08c authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[LayoutNG] Fix shape-outside-{circle-048,polygon-020}.html tests.

This was two subtle bugs in one.

1) We need to copy back data to the ShapeOutsideInfo side
   data-structure, even if its an "intermediate" layout, (during the
   min/max pass).

   This was causing the incorrect size to be calculated for body, as
   it thought the shape was "empty" (its size was 0,0).

   Eventually this should be fixed by moving the shape to the layout
   result, removing this side data-structure.

2) The static-position for vertical-rl was broken.
   When accounting for flipped-blocks, we need to use the containing
   block writing-mode, instead of the direct parent.

   Static position doesn't really work cross writing modes in the
   existing layout system, so this isn't perfect, is closer to the old
   system.

Bug: 635619
Change-Id: Ia7f8144cc1c9515b161bf3502c58fee66e59f3c3
Reviewed-on: https://chromium-review.googlesource.com/c/1315390
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarAleks Totic <atotic@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605345}
parent ce2ed51a
...@@ -205,7 +205,6 @@ crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftest ...@@ -205,7 +205,6 @@ crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftest
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-border-box-border-radius-004.html [ Pass ] crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-border-box-border-radius-004.html [ Pass ]
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-circle-042.html [ Pass ] crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-circle-042.html [ Pass ]
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-circle-044.html [ Pass ] crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-circle-044.html [ Pass ]
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-circle-048.html [ Failure ]
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-circle-050.html [ Pass ] crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-circle-050.html [ Pass ]
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-circle-055.html [ Pass ] crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-circle-055.html [ Pass ]
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-content-box-border-radius-002.html [ Pass ] crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-content-box-border-radius-002.html [ Pass ]
...@@ -218,7 +217,6 @@ crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftest ...@@ -218,7 +217,6 @@ crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftest
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-margin-box-border-radius-004.html [ Pass ] crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-margin-box-border-radius-004.html [ Pass ]
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-margin-box-border-radius-008.html [ Pass ] crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-margin-box-border-radius-008.html [ Pass ]
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-padding-box-border-radius-002.html [ Pass ] crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-padding-box-border-radius-002.html [ Pass ]
crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-polygon-020.html [ Failure ]
crbug.com/591099 external/wpt/editing/run/bold.html [ Pass ] crbug.com/591099 external/wpt/editing/run/bold.html [ Pass ]
crbug.com/591099 external/wpt/editing/run/fontname.html [ Pass ] crbug.com/591099 external/wpt/editing/run/fontname.html [ Pass ]
crbug.com/591099 external/wpt/editing/run/formatblock.html [ Pass ] crbug.com/591099 external/wpt/editing/run/formatblock.html [ Pass ]
......
...@@ -210,7 +210,7 @@ void LayoutNGBlockFlow::UpdateOutOfFlowBlockLayout() { ...@@ -210,7 +210,7 @@ void LayoutNGBlockFlow::UpdateOutOfFlowBlockLayout() {
// border-box relative, flip it around the size of the border box, rather than // border-box relative, flip it around the size of the border box, rather than
// the size of the containing block (padding box). // the size of the containing block (padding box).
LayoutUnit block_top_or_left = LayoutUnit block_top_or_left =
parent_style->IsFlippedBlocksWritingMode() container_style->IsFlippedBlocksWritingMode()
? container_border_box_logical_height - static_block ? container_border_box_logical_height - static_block
: static_block; : static_block;
...@@ -221,7 +221,7 @@ void LayoutNGBlockFlow::UpdateOutOfFlowBlockLayout() { ...@@ -221,7 +221,7 @@ void LayoutNGBlockFlow::UpdateOutOfFlowBlockLayout() {
? NGPhysicalOffset(inline_left_or_top, block_top_or_left) ? NGPhysicalOffset(inline_left_or_top, block_top_or_left)
: NGPhysicalOffset(block_top_or_left, inline_left_or_top); : NGPhysicalOffset(block_top_or_left, inline_left_or_top);
NGStaticPosition static_position = NGStaticPosition static_position =
NGStaticPosition::Create(parent_style->GetWritingMode(), NGStaticPosition::Create(container_style->GetWritingMode(),
parent_style->Direction(), static_location); parent_style->Direction(), static_location);
// Set correct container for inline containing blocks. // Set correct container for inline containing blocks.
......
...@@ -190,6 +190,7 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout( ...@@ -190,6 +190,7 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout(
// TODO(layoutng): Figure out why these two call can't be inside the // TODO(layoutng): Figure out why these two call can't be inside the
// !constraint_space.IsIntermediateLayout() block below. // !constraint_space.IsIntermediateLayout() block below.
UpdateShapeOutsideInfoIfNeeded( UpdateShapeOutsideInfoIfNeeded(
*layout_result,
constraint_space.PercentageResolutionSize().inline_size); constraint_space.PercentageResolutionSize().inline_size);
// We may need paint invalidation even if we can reuse layout, as our // We may need paint invalidation even if we can reuse layout, as our
// paint offset/visual rect may have changed due to relative // paint offset/visual rect may have changed due to relative
...@@ -263,6 +264,19 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout( ...@@ -263,6 +264,19 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout(
FinishLayout(block_flow, constraint_space, break_token, layout_result); FinishLayout(block_flow, constraint_space, break_token, layout_result);
} }
// We always need to update the ShapeOutsideInfo even if the layout is
// intermediate (e.g. called during a min/max pass).
//
// If a shape-outside float is present in an orthogonal flow, when
// calculating the min/max-size (by performing an intermediate layout), we
// might calculate this incorrectly, as the layout won't take into account the
// shape-outside area.
//
// TODO(ikilpatrick): This should be fixed by moving the shape-outside data
// to the NGLayoutResult, removing this "side" data-structure.
UpdateShapeOutsideInfoIfNeeded(
*layout_result, constraint_space.PercentageResolutionSize().inline_size);
return layout_result; return layout_result;
} }
...@@ -585,9 +599,6 @@ void NGBlockNode::CopyFragmentDataToLayoutBox( ...@@ -585,9 +599,6 @@ void NGBlockNode::CopyFragmentDataToLayoutBox(
box_->UpdateAfterLayout(); box_->UpdateAfterLayout();
box_->ClearNeedsLayout(); box_->ClearNeedsLayout();
UpdateShapeOutsideInfoIfNeeded(
constraint_space.PercentageResolutionSize().inline_size);
// Overflow computation depends on this being set. // Overflow computation depends on this being set.
if (box_->IsLayoutBlockFlow()) { if (box_->IsLayoutBlockFlow()) {
LayoutBlockFlow* block_flow = ToLayoutBlockFlow(box_); LayoutBlockFlow* block_flow = ToLayoutBlockFlow(box_);
...@@ -877,9 +888,12 @@ scoped_refptr<NGLayoutResult> NGBlockNode::RunOldLayout( ...@@ -877,9 +888,12 @@ scoped_refptr<NGLayoutResult> NGBlockNode::RunOldLayout(
builder.SetPadding(padding); builder.SetPadding(padding);
CopyBaselinesFromOldLayout(constraint_space, &builder); CopyBaselinesFromOldLayout(constraint_space, &builder);
scoped_refptr<NGLayoutResult> layout_result = builder.ToBoxFragment();
UpdateShapeOutsideInfoIfNeeded( UpdateShapeOutsideInfoIfNeeded(
constraint_space.PercentageResolutionSize().inline_size); *layout_result, constraint_space.PercentageResolutionSize().inline_size);
return builder.ToBoxFragment();
return layout_result;
} }
void NGBlockNode::CopyBaselinesFromOldLayout( void NGBlockNode::CopyBaselinesFromOldLayout(
...@@ -942,19 +956,24 @@ LayoutUnit NGBlockNode::AtomicInlineBaselineFromOldLayout( ...@@ -942,19 +956,24 @@ LayoutUnit NGBlockNode::AtomicInlineBaselineFromOldLayout(
// current shape machinery requires setting the size of the float after layout // current shape machinery requires setting the size of the float after layout
// in the parents writing mode. // in the parents writing mode.
void NGBlockNode::UpdateShapeOutsideInfoIfNeeded( void NGBlockNode::UpdateShapeOutsideInfoIfNeeded(
const NGLayoutResult& layout_result,
LayoutUnit percentage_resolution_inline_size) { LayoutUnit percentage_resolution_inline_size) {
if (!box_->IsFloating() || !box_->GetShapeOutsideInfo()) if (!box_->IsFloating() || !box_->GetShapeOutsideInfo())
return; return;
// The box_ may not have a valid size yet (due to an intermediate layout),
// use the fragment's size instead.
DCHECK(layout_result.PhysicalFragment());
LayoutSize box_size = layout_result.PhysicalFragment()->Size().ToLayoutSize();
// TODO(ikilpatrick): Ideally this should be moved to a NGLayoutResult // TODO(ikilpatrick): Ideally this should be moved to a NGLayoutResult
// computing the shape area. There may be an issue with the new fragmentation // computing the shape area. There may be an issue with the new fragmentation
// model and computing the correct sizes of shapes. // model and computing the correct sizes of shapes.
ShapeOutsideInfo* shape_outside = box_->GetShapeOutsideInfo(); ShapeOutsideInfo* shape_outside = box_->GetShapeOutsideInfo();
LayoutBlock* containing_block = box_->ContainingBlock(); LayoutBlock* containing_block = box_->ContainingBlock();
shape_outside->SetReferenceBoxLogicalSize( shape_outside->SetReferenceBoxLogicalSize(
containing_block->IsHorizontalWritingMode() containing_block->IsHorizontalWritingMode() ? box_size
? box_->Size() : box_size.TransposedSize());
: box_->Size().TransposedSize());
shape_outside->SetPercentageResolutionInlineSize( shape_outside->SetPercentageResolutionInlineSize(
percentage_resolution_inline_size); percentage_resolution_inline_size);
} }
......
...@@ -136,6 +136,7 @@ class CORE_EXPORT NGBlockNode final : public NGLayoutInputNode { ...@@ -136,6 +136,7 @@ class CORE_EXPORT NGBlockNode final : public NGLayoutInputNode {
const NGConstraintSpace&); const NGConstraintSpace&);
void UpdateShapeOutsideInfoIfNeeded( void UpdateShapeOutsideInfoIfNeeded(
const NGLayoutResult&,
LayoutUnit percentage_resolution_inline_size); LayoutUnit percentage_resolution_inline_size);
}; };
......
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