Skip to content

GitLab

T
tangled
Commit 64f7f8c0 authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot
Browse files
Options

[LayoutNG] Check for solid edges for layout opportunities. 

Previously we weren't checking for "solid" edges on layout opportunities
within the NGExclusionSpace.

This had the effect of us thinking a particular layout opportunity was
valid (when one edge wasn't against any floats). This caused us to place
the element in such a way that it was "avoiding" a float higher up in
the document (when it shouldn't have).

This introduces a "NGClosedArea" struct, which keeps track of the
"NGEdges" vectors so that before considering a layout opportunity as
valid, we check it has solid edges.

Bug: 959567
Change-Id: If6481a23279eee7a1b188ec4cf5129b503935cc7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1603303
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658748}
parent 65886285
Hide whitespace changes
Inline Side-by-side
Showing with 140 additions and 54 deletions
third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion_space.cc
View file @ 64f7f8c0
......@@ -16,29 +16,33 @@ namespace {
//
// We don't explicitly check the inline-size/block-size of the opportunity as
// they are always produced in the order.
void InsertOpportunity(const NGLayoutOpportunity& opportunity,
Vector<NGLayoutOpportunity, 4>* opportunities) {
if (opportunities->IsEmpty()) {
opportunities->emplace_back(opportunity);
void InsertClosedArea(
const NGExclusionSpaceInternal::NGClosedArea area,
Vector<NGExclusionSpaceInternal::NGClosedArea, 4>* areas) {
if (areas->IsEmpty()) {
areas->emplace_back(area);
return;
}
// We go backwards through the list as there is a higher probability that a
// new opportunity will be at the end of the list.
for (wtf_size_t j = opportunities->size() - 1; j >= 0; --j) {
const NGLayoutOpportunity& other = opportunities->at(j);
if (other.rect.BlockStartOffset() <= opportunity.rect.BlockStartOffset()) {
// new area will be at the end of the list.
for (wtf_size_t j = areas->size() - 1; j >= 0; --j) {
const NGExclusionSpaceInternal::NGClosedArea& other = areas->at(j);
if (other.opportunity.rect.BlockStartOffset() <=
area.opportunity.rect.BlockStartOffset()) {
#if DCHECK_IS_ON()
// If we have the same block-start offset ensure that the size of the
// opportunity doesn't violate the order.
if (other.rect.BlockStartOffset() ==
opportunity.rect.BlockStartOffset()) {
DCHECK_LE(other.rect.BlockSize(), opportunity.rect.BlockSize());
DCHECK_GE(other.rect.InlineSize(), opportunity.rect.InlineSize());
if (other.opportunity.rect.BlockStartOffset() ==
area.opportunity.rect.BlockStartOffset()) {
DCHECK_LE(other.opportunity.rect.BlockSize(),
area.opportunity.rect.BlockSize());
DCHECK_GE(other.opportunity.rect.InlineSize(),
area.opportunity.rect.InlineSize());
}
#endif
opportunities->insert(j + 1, opportunity);
areas->insert(j + 1, area);
return;
}
}
......@@ -397,7 +401,9 @@ void NGExclusionSpaceInternal::DerivedGeometry::Add(
*shelf.shape_exclusions))
: nullptr);
InsertOpportunity(opportunity, &opportunities_);
InsertClosedArea(NGClosedArea(opportunity, shelf.line_left_edges,
shelf.line_right_edges),
&areas_);
}
}
......@@ -579,12 +585,12 @@ void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities(
const LayoutUnit available_inline_size,
const LambdaFunc& lambda) const {
auto* shelves_it = shelves_.begin();
auto* opps_it = opportunities_.begin();
auto* areas_it = areas_.begin();
auto* const shelves_end = shelves_.end();
auto* const opps_end = opportunities_.end();
auto* const areas_end = areas_.end();
while (shelves_it != shelves_end || opps_it != opps_end) {
while (shelves_it != shelves_end || areas_it != areas_end) {
// We should never exhaust the opportunities list before the shelves list,
// as there is always an infinitely sized shelf at the very end.
DCHECK_NE(shelves_it, shelves_end);
......@@ -595,22 +601,35 @@ void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities(
continue;
}
if (opps_it != opps_end) {
const NGLayoutOpportunity& opportunity = *opps_it;
if (areas_it != areas_end) {
const NGClosedArea& area = *areas_it;
if (!Intersects(opportunity, offset, available_inline_size)) {
++opps_it;
if (!Intersects(area.opportunity, offset, available_inline_size)) {
++areas_it;
continue;
}
// We always prefer the closed-off opportunity, instead of the shelf
LayoutUnit block_start_offset = std::max(
area.opportunity.rect.BlockStartOffset(), offset.block_offset);
// We always prefer the closed-off area opportunity, instead of the shelf
// opportunity if they exist at the some offset.
if (opportunity.rect.BlockStartOffset() <= shelf.block_offset) {
if (lambda(CreateLayoutOpportunity(opportunity, offset,
available_inline_size)))
return;
if (block_start_offset <=
std::max(shelf.block_offset, offset.block_offset)) {
LayoutUnit block_end_offset = area.opportunity.rect.BlockEndOffset();
bool has_solid_edges =
HasSolidEdges(area.line_left_edges, block_start_offset,
block_end_offset) &&
HasSolidEdges(area.line_right_edges, block_start_offset,
block_end_offset);
if (has_solid_edges) {
if (lambda(CreateLayoutOpportunity(area.opportunity, offset,
available_inline_size)))
return;
}
++opps_it;
++areas_it;
continue;
}
}
......
third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion_space.h
View file @ 64f7f8c0
......@@ -226,6 +226,49 @@ class CORE_EXPORT NGExclusionSpaceInternal {
bool has_shape_exclusions;
};
// The closed-off area is an internal data-structure representing an area
// above a float. It contains a layout opportunity, and two vectors of
// |NGShelfEdge|. E.g.
//
// 0 1 2 3 4 5 6 7 8
// 0 +---+. .+---+
// |xxx|. .|xxx|
// 10 |xxx|. .|xxx|
// +---+. .+---+
// 20 ........
// +---+
// 30 |xxx|
// |xxx|
// 40 +---+
//
// In the above example the closed-off area is represented with the dotted
// line.
//
// It has the internal values of:
// {
// opportunity: {
// start_offset: {20, LayoutUnit::Min()},
// end_offset: {65, 25},
// }
// line_left_edges: [{0, 15}],
// line_right_edges: [{0, 15}],
// }
//
// Once a closed-off area has been created, it can never be changed due to
// the property that floats always align their block-start edges.
struct NGClosedArea {
NGClosedArea(NGLayoutOpportunity opportunity,
const Vector<NGShelfEdge, 1>& line_left_edges,
const Vector<NGShelfEdge, 1>& line_right_edges)
: opportunity(opportunity),
line_left_edges(line_left_edges),
line_right_edges(line_right_edges) {}
const NGLayoutOpportunity opportunity;
const Vector<NGShelfEdge, 1> line_left_edges;
const Vector<NGShelfEdge, 1> line_right_edges;
};
private:
// In order to reduce the amount of Vector copies, instances of a
// NGExclusionSpaceInternal can share the same exclusions_ Vector. See the
......@@ -296,40 +339,22 @@ class CORE_EXPORT NGExclusionSpaceInternal {
const LayoutUnit available_inline_size,
const LambdaFunc&) const;
// See NGShelf for a broad description of what shelves are. We always begin
// with one, which has the internal value of:
// See |NGShelf| for a broad description of what shelves are. We always
// begin with one, which has the internal value of:
// {
// block_offset: LayoutUnit::Min(),
// line_left: LayoutUnit::Min(),
// line_right: LayoutUnit::Max(),
// }
//
// The list of opportunities represent "closed-off" areas. E.g.
//
// 0 1 2 3 4 5 6 7 8
// 0 +---+. .+---+
// |xxx|. .|xxx|
// 10 |xxx|. .|xxx|
// +---+. .+---+
// 20 ........
// +---+
// 30 |xxx|
// |xxx|
// 40 +---+
//
// In the above example the opportunity is represented with the dotted line.
// It has the internal values of:
// {
// start_offset: {20, LayoutUnit::Min()},
// end_offset: {65, 25},
// }
// Once an opportunity has been created, it can never been changed due to
// the property that floats always align their block-start edges.
//
// We exploit this property by keeping this list of "closed-off" areas, and
// removing shelves to make insertion faster.
Vector<NGShelf, 4> shelves_;
Vector<NGLayoutOpportunity, 4> opportunities_;
// See |NGClosedArea| for a broad description of what closed-off areas are.
//
// Floats always align their block-start edges. We exploit this property by
// keeping a list of closed-off areas. Once a closed-off area has been
// created, it can never change.
Vector<NGClosedArea, 4> areas_;
bool track_shape_exclusions_;
};
......
third_party/blink/web_tests/external/wpt/css/CSS2/floats/floats-placement-001.html 0 → 100644
View file @ 64f7f8c0
<!DOCTYPE html>
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<link rel="help" href="https://drafts.csswg.org/css2/visuren.html#float-position">
<meta name="assert" content="This test checks placement of inflow content with floats present." />
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width: 100px; height: 100px; background: red; line-height: 0; position: relative;">
<div style="float: left; width: 20px;"></div>
<div style="float: right; width: 30px; height: 50px; background: green;"></div>
<div style="float: right; clear: right; width: 100px; height: 50px; background: green;"></div>
<div style="display: inline-block; width: 50px; height: 50px; background: green;"></div>
<div style="position: absolute; width: 20px; height: 50px; top: 0; left: 50px; background: green;"></div>
</div>
third_party/blink/web_tests/external/wpt/css/CSS2/floats/floats-placement-002.html 0 → 100644
View file @ 64f7f8c0
<!DOCTYPE html>
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<link rel="help" href="https://drafts.csswg.org/css2/visuren.html#float-position">
<meta name="assert" content="This test checks placement of inflow content with floats present." />
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width: 100px; height: 100px; background: red; position: relative;">
<div style="float: left; width: 20px;"></div>
<div style="line-height: 0;">
<div style="display: inline-block; width: 100px; height: 20px; background: green;"></div>
</div>
<div style="float: right; width: 20px; height: 80px; background: green;"></div>
<div style="float: right; clear: right; width: 30px; clear: right;"></div>
<div style="display: inline-block; width: 60px; height: 60px; background: green;"></div>
<div style="position: absolute; width: 20px; height: 80px; background: green; top: 20px; right: 20px;"></div>
<div style="position: absolute; width: 60px; height: 20px; background: green; bottom: 0; left: 0;"></div>
</div>
third_party/blink/web_tests/external/wpt/css/CSS2/floats/floats-placement-003.html 0 → 100644
View file @ 64f7f8c0
<!DOCTYPE html>
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<link rel="help" href="https://drafts.csswg.org/css2/visuren.html#float-position">
<meta name="assert" content="This test checks placement of inflow content with floats present." />
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width: 100px; height: 100px; line-height: 0; background: red; position: relative;">
<div style="float: left; width: 20px; height: 20px;"></div>
<div style="height: 30px; background: green;"></div>
<div style="float: right; width: 40px; height: 20px; background: green;"></div>
<div style="float: right; clear: right; width: 50px; height: 50px; background: green;"></div>
<div style="float: left; width: 50px; height: 50px; background: green;"></div>
<span style="display: inline-block; width: 40px; height: 20px; background: green;"></span>
<div style="position: absolute; width: 20px; height: 20px; background: green; left: 40px; top: 30px;"></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