Commit f189b068 authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[LayoutNG] Fix placing floats within zero available space.

There was special logic added for zero width inlines in:
https://chromium-review.googlesource.com/c/chromium/src/+/870950/

This logic allowed for zero-width layout opportunities beside floats for
inline-level content.

This also added a TODO saying this was probably wrong for floats.

.... it turned out this was wrong for floats. :)

This adds a flag to switch between these two behaviours.

Bug: 988505
Change-Id: I2db5e0b9cb6f90cb131821ad205728289d9098ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725309Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682265}
parent ace6a6eb
...@@ -120,18 +120,19 @@ bool Intersects(const NGLayoutOpportunity& opportunity, ...@@ -120,18 +120,19 @@ bool Intersects(const NGLayoutOpportunity& opportunity,
// always intersects. // always intersects.
bool Intersects(const NGExclusionSpaceInternal::NGShelf& shelf, bool Intersects(const NGExclusionSpaceInternal::NGShelf& shelf,
const NGBfcOffset& offset, const NGBfcOffset& offset,
const LayoutUnit inline_size) { const LayoutUnit inline_size,
bool is_inline_level) {
if (shelf.line_right >= offset.line_offset && if (shelf.line_right >= offset.line_offset &&
shelf.line_left <= offset.line_offset + inline_size) shelf.line_left <= offset.line_offset + inline_size)
return true; return true;
// Negative available space creates a zero-width opportunity at the inline-end // Negative available space creates a zero-width opportunity at the inline-end
// of the shelf. Consider such shelf intersects. // of the shelf. Consider such shelf intersects.
// TODO(kojii): This is correct to find layout opportunities for zero-width if (UNLIKELY(is_inline_level &&
// in-flow inline or block objects (e.g., <br>,) but not correct for (shelf.line_left > offset.line_offset ||
// zero-width floats. shelf.line_right < offset.line_offset + inline_size)))
if (UNLIKELY(shelf.line_left > offset.line_offset ||
shelf.line_right < offset.line_offset + inline_size))
return true; return true;
return false; return false;
} }
...@@ -162,8 +163,9 @@ NGLayoutOpportunity CreateLayoutOpportunity(const NGLayoutOpportunity& other, ...@@ -162,8 +163,9 @@ NGLayoutOpportunity CreateLayoutOpportunity(const NGLayoutOpportunity& other,
NGLayoutOpportunity CreateLayoutOpportunity( NGLayoutOpportunity CreateLayoutOpportunity(
const NGExclusionSpaceInternal::NGShelf& shelf, const NGExclusionSpaceInternal::NGShelf& shelf,
const NGBfcOffset& offset, const NGBfcOffset& offset,
const LayoutUnit inline_size) { const LayoutUnit inline_size,
DCHECK(Intersects(shelf, offset, inline_size)); bool is_inline_level) {
DCHECK(Intersects(shelf, offset, inline_size, is_inline_level));
NGBfcOffset start_offset(std::max(shelf.line_left, offset.line_offset), NGBfcOffset start_offset(std::max(shelf.line_left, offset.line_offset),
std::max(shelf.block_offset, offset.block_offset)); std::max(shelf.block_offset, offset.block_offset));
...@@ -550,7 +552,7 @@ NGExclusionSpaceInternal::DerivedGeometry::FindLayoutOpportunity( ...@@ -550,7 +552,7 @@ NGExclusionSpaceInternal::DerivedGeometry::FindLayoutOpportunity(
NGLayoutOpportunity return_opportunity; NGLayoutOpportunity return_opportunity;
IterateAllLayoutOpportunities( IterateAllLayoutOpportunities(
offset, available_inline_size, offset, available_inline_size, false /* is_inline_level */,
[&return_opportunity, &minimum_size, [&return_opportunity, &minimum_size,
&available_inline_size](const NGLayoutOpportunity opportunity) -> bool { &available_inline_size](const NGLayoutOpportunity opportunity) -> bool {
// Determine if this opportunity will fit the given size. // Determine if this opportunity will fit the given size.
...@@ -577,8 +579,9 @@ NGExclusionSpaceInternal::DerivedGeometry::AllLayoutOpportunities( ...@@ -577,8 +579,9 @@ NGExclusionSpaceInternal::DerivedGeometry::AllLayoutOpportunities(
const LayoutUnit available_inline_size) const { const LayoutUnit available_inline_size) const {
LayoutOpportunityVector opportunities; LayoutOpportunityVector opportunities;
// This method is only used for determining the position of line-boxes.
IterateAllLayoutOpportunities( IterateAllLayoutOpportunities(
offset, available_inline_size, offset, available_inline_size, true /* is_inline_level */,
[&opportunities](const NGLayoutOpportunity opportunity) -> bool { [&opportunities](const NGLayoutOpportunity opportunity) -> bool {
opportunities.push_back(std::move(opportunity)); opportunities.push_back(std::move(opportunity));
return false; return false;
...@@ -591,6 +594,7 @@ template <typename LambdaFunc> ...@@ -591,6 +594,7 @@ template <typename LambdaFunc>
void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities( void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities(
const NGBfcOffset& offset, const NGBfcOffset& offset,
const LayoutUnit available_inline_size, const LayoutUnit available_inline_size,
bool is_inline_level,
const LambdaFunc& lambda) const { const LambdaFunc& lambda) const {
auto* shelves_it = shelves_.begin(); auto* shelves_it = shelves_.begin();
auto* areas_it = areas_.begin(); auto* areas_it = areas_.begin();
...@@ -604,7 +608,7 @@ void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities( ...@@ -604,7 +608,7 @@ void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities(
DCHECK_NE(shelves_it, shelves_end); DCHECK_NE(shelves_it, shelves_end);
const NGShelf& shelf = *shelves_it; const NGShelf& shelf = *shelves_it;
if (!Intersects(shelf, offset, available_inline_size)) { if (!Intersects(shelf, offset, available_inline_size, is_inline_level)) {
++shelves_it; ++shelves_it;
continue; continue;
} }
...@@ -649,7 +653,8 @@ void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities( ...@@ -649,7 +653,8 @@ void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities(
HasSolidEdges(shelf.line_right_edges, offset.block_offset, HasSolidEdges(shelf.line_right_edges, offset.block_offset,
LayoutUnit::Max()); LayoutUnit::Max());
if (has_solid_edges) { if (has_solid_edges) {
if (lambda(CreateLayoutOpportunity(shelf, offset, available_inline_size))) if (lambda(CreateLayoutOpportunity(shelf, offset, available_inline_size,
is_inline_level)))
return; return;
} }
++shelves_it; ++shelves_it;
......
...@@ -360,6 +360,7 @@ class CORE_EXPORT NGExclusionSpaceInternal { ...@@ -360,6 +360,7 @@ class CORE_EXPORT NGExclusionSpaceInternal {
template <typename LambdaFunc> template <typename LambdaFunc>
void IterateAllLayoutOpportunities(const NGBfcOffset& offset, void IterateAllLayoutOpportunities(const NGBfcOffset& offset,
const LayoutUnit available_inline_size, const LayoutUnit available_inline_size,
bool is_inline_level,
const LambdaFunc&) const; const LambdaFunc&) const;
// See |NGShelf| for a broad description of what shelves are. We always // See |NGShelf| for a broad description of what shelves are. We always
......
<!DOCTYPE html>
<link rel="help" href="https://www.w3.org/TR/CSS22/visudet.html#float-width" title="10.3.5 Floating, non-replaced elements">
<link rel="match" href="../../reference/ref-filled-green-100px-square-only.html">
<p>Test passes if there is a filled green square.</p>
<div style="width: 0px;">
<div style="float: left; width: 100px; height: 50px; background: green;"></div>
<div style="float: left; width: 100px; height: 50px; background: green;"></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