Commit 2ad2d2e6 authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[LayoutNG] Allow floats to be placed "before" other floats of the same type.

Floats have some "interesting" behaviour which allows a left (or right)
float to be placed left (or right) of a previous float. This typically
isn't allowed.

This occurs when a float has "negative size" e.g. it has a large
negative margin, which is larger than its width. E.g.
<div style="width: 50px; margin-left: -150px; float: left;"></div>

In such cases the float doesn't respect the normal rules, and is
placed in a typically invalid area.

This patch also changes "const LogicalSize& minimum_size" to
"const LayoutUnit minimum_inline_size" for "FindLayoutOpportunity".

Bug: 1005437
Change-Id: I4c91a53911b2651346487f71c4b31ec6e0240202
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1819356
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699390}
parent 39831318
...@@ -109,33 +109,6 @@ bool Intersects(const NGLayoutOpportunity& opportunity, ...@@ -109,33 +109,6 @@ bool Intersects(const NGLayoutOpportunity& opportunity,
opportunity.rect.BlockEndOffset() > offset.block_offset; opportunity.rect.BlockEndOffset() > offset.block_offset;
} }
// Returns true if the area defined by the given offset and inline_size
// intersects with the shelfs area.
//
// No checks for the block direction are needed as the given area (defined by
// offset and inline_size) extends to a block-end of infinity, and a shelf also
// has a block-end of infinity.
//
// If the shelf is at -Infinity or +Infinity at either end, the given area
// always intersects.
bool Intersects(const NGExclusionSpaceInternal::NGShelf& shelf,
const NGBfcOffset& offset,
const LayoutUnit inline_size,
bool is_inline_level) {
if (shelf.line_right >= offset.line_offset &&
shelf.line_left <= offset.line_offset + inline_size)
return true;
// Negative available space creates a zero-width opportunity at the inline-end
// of the shelf. Consider such shelf intersects.
if (UNLIKELY(is_inline_level &&
(shelf.line_left > offset.line_offset ||
shelf.line_right < offset.line_offset + inline_size)))
return true;
return false;
}
// Creates a new layout opportunity. The given layout opportunity *must* // Creates a new layout opportunity. The given layout opportunity *must*
// intersect with the given area (defined by offset and inline_size). // intersect with the given area (defined by offset and inline_size).
NGLayoutOpportunity CreateLayoutOpportunity(const NGLayoutOpportunity& other, NGLayoutOpportunity CreateLayoutOpportunity(const NGLayoutOpportunity& other,
...@@ -163,10 +136,7 @@ NGLayoutOpportunity CreateLayoutOpportunity(const NGLayoutOpportunity& other, ...@@ -163,10 +136,7 @@ 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) {
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));
...@@ -547,22 +517,23 @@ NGLayoutOpportunity ...@@ -547,22 +517,23 @@ NGLayoutOpportunity
NGExclusionSpaceInternal::DerivedGeometry::FindLayoutOpportunity( NGExclusionSpaceInternal::DerivedGeometry::FindLayoutOpportunity(
const NGBfcOffset& offset, const NGBfcOffset& offset,
const LayoutUnit available_inline_size, const LayoutUnit available_inline_size,
const LogicalSize& minimum_size) const { const LayoutUnit minimum_inline_size) const {
// TODO(ikilpatrick): Determine what to do for a -ve available_inline_size. // TODO(ikilpatrick): Determine what to do for a -ve available_inline_size.
NGLayoutOpportunity return_opportunity; NGLayoutOpportunity return_opportunity;
IterateAllLayoutOpportunities( IterateAllLayoutOpportunities(
offset, available_inline_size, false /* is_inline_level */, offset, available_inline_size,
[&return_opportunity, &minimum_size, [&return_opportunity, &offset, &available_inline_size,
&available_inline_size](const NGLayoutOpportunity opportunity) -> bool { &minimum_inline_size](const NGLayoutOpportunity opportunity) -> bool {
// Determine if this opportunity will fit the given size. // Determine if this opportunity will fit the given size.
// //
// NOTE: There are cases where the available_inline_size may be smaller // NOTE: There are cases where the |available_inline_size| may be
// than the minimum_size.inline_size. In such cases if the opportunity // smaller than the |minimum_inline_size|. In such cases if the
// is the same as the available_inline_size, it pretends that it "fits". // opportunity is the same as the |available_inline_size|, it pretends
if ((opportunity.rect.InlineSize() >= minimum_size.inline_size || // that it "fits".
opportunity.rect.InlineSize() == available_inline_size) && if (opportunity.rect.InlineSize() >= minimum_inline_size ||
opportunity.rect.BlockSize() >= minimum_size.block_size) { (opportunity.rect.InlineSize() == available_inline_size &&
opportunity.rect.LineStartOffset() == offset.line_offset)) {
return_opportunity = std::move(opportunity); return_opportunity = std::move(opportunity);
return true; return true;
} }
...@@ -581,7 +552,7 @@ NGExclusionSpaceInternal::DerivedGeometry::AllLayoutOpportunities( ...@@ -581,7 +552,7 @@ NGExclusionSpaceInternal::DerivedGeometry::AllLayoutOpportunities(
// This method is only used for determining the position of line-boxes. // This method is only used for determining the position of line-boxes.
IterateAllLayoutOpportunities( IterateAllLayoutOpportunities(
offset, available_inline_size, true /* is_inline_level */, offset, available_inline_size,
[&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;
...@@ -594,7 +565,6 @@ template <typename LambdaFunc> ...@@ -594,7 +565,6 @@ 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();
...@@ -608,11 +578,6 @@ void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities( ...@@ -608,11 +578,6 @@ 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, is_inline_level)) {
++shelves_it;
continue;
}
if (areas_it != areas_end) { if (areas_it != areas_end) {
const NGClosedArea& area = *areas_it; const NGClosedArea& area = *areas_it;
...@@ -653,8 +618,7 @@ void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities( ...@@ -653,8 +618,7 @@ 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;
......
...@@ -40,7 +40,7 @@ class CORE_EXPORT NGExclusionSpaceInternal { ...@@ -40,7 +40,7 @@ class CORE_EXPORT NGExclusionSpaceInternal {
NGLayoutOpportunity FindLayoutOpportunity( NGLayoutOpportunity FindLayoutOpportunity(
const NGBfcOffset& offset, const NGBfcOffset& offset,
const LayoutUnit available_inline_size, const LayoutUnit available_inline_size,
const LogicalSize& minimum_size) const { const LayoutUnit minimum_inline_size) const {
// If the area clears all floats, we can just return the layout opportunity // If the area clears all floats, we can just return the layout opportunity
// which matches the available space. // which matches the available space.
if (offset.block_offset >= if (offset.block_offset >=
...@@ -52,7 +52,7 @@ class CORE_EXPORT NGExclusionSpaceInternal { ...@@ -52,7 +52,7 @@ class CORE_EXPORT NGExclusionSpaceInternal {
} }
return GetDerivedGeometry().FindLayoutOpportunity( return GetDerivedGeometry().FindLayoutOpportunity(
offset, available_inline_size, minimum_size); offset, available_inline_size, minimum_inline_size);
} }
LayoutOpportunityVector AllLayoutOpportunities( LayoutOpportunityVector AllLayoutOpportunities(
...@@ -351,7 +351,7 @@ class CORE_EXPORT NGExclusionSpaceInternal { ...@@ -351,7 +351,7 @@ class CORE_EXPORT NGExclusionSpaceInternal {
NGLayoutOpportunity FindLayoutOpportunity( NGLayoutOpportunity FindLayoutOpportunity(
const NGBfcOffset& offset, const NGBfcOffset& offset,
const LayoutUnit available_inline_size, const LayoutUnit available_inline_size,
const LogicalSize& minimum_size) const; const LayoutUnit minimum_inline_size) const;
LayoutOpportunityVector AllLayoutOpportunities( LayoutOpportunityVector AllLayoutOpportunities(
const NGBfcOffset& offset, const NGBfcOffset& offset,
...@@ -360,7 +360,6 @@ class CORE_EXPORT NGExclusionSpaceInternal { ...@@ -360,7 +360,6 @@ 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
...@@ -424,12 +423,12 @@ class CORE_EXPORT NGExclusionSpace { ...@@ -424,12 +423,12 @@ class CORE_EXPORT NGExclusionSpace {
// Returns a layout opportunity, within the BFC. // Returns a layout opportunity, within the BFC.
// The area to search for layout opportunities is defined by the given offset, // The area to search for layout opportunities is defined by the given offset,
// and available_inline_size. The layout opportunity must be greater than the // and |available_inline_size|. The layout opportunity must be greater than
// given minimum_size. // the given |minimum_inline_size|.
NGLayoutOpportunity FindLayoutOpportunity( NGLayoutOpportunity FindLayoutOpportunity(
const NGBfcOffset& offset, const NGBfcOffset& offset,
const LayoutUnit available_inline_size, const LayoutUnit available_inline_size,
const LogicalSize& minimum_size) const { const LayoutUnit minimum_inline_size = LayoutUnit()) const {
if (!exclusion_space_) { if (!exclusion_space_) {
NGBfcOffset end_offset( NGBfcOffset end_offset(
offset.line_offset + available_inline_size.ClampNegativeToZero(), offset.line_offset + available_inline_size.ClampNegativeToZero(),
...@@ -437,7 +436,7 @@ class CORE_EXPORT NGExclusionSpace { ...@@ -437,7 +436,7 @@ class CORE_EXPORT NGExclusionSpace {
return NGLayoutOpportunity(NGBfcRect(offset, end_offset), nullptr); return NGLayoutOpportunity(NGBfcRect(offset, end_offset), nullptr);
} }
return exclusion_space_->FindLayoutOpportunity( return exclusion_space_->FindLayoutOpportunity(
offset, available_inline_size, minimum_size); offset, available_inline_size, minimum_inline_size);
} }
// If possible prefer FindLayoutOpportunity over this function. // If possible prefer FindLayoutOpportunity over this function.
......
...@@ -1422,7 +1422,7 @@ void NGLineBreaker::HandleFloat(const NGInlineItem& item, ...@@ -1422,7 +1422,7 @@ void NGLineBreaker::HandleFloat(const NGInlineItem& item,
NGLayoutOpportunity opportunity = exclusion_space_->FindLayoutOpportunity( NGLayoutOpportunity opportunity = exclusion_space_->FindLayoutOpportunity(
{constraint_space_.BfcOffset().line_offset, bfc_block_offset}, {constraint_space_.BfcOffset().line_offset, bfc_block_offset},
constraint_space_.AvailableSize().inline_size, LogicalSize()); constraint_space_.AvailableSize().inline_size);
DCHECK_EQ(bfc_block_offset, opportunity.rect.BlockStartOffset()); DCHECK_EQ(bfc_block_offset, opportunity.rect.BlockStartOffset());
......
...@@ -170,8 +170,8 @@ LayoutUnit NGUnpositionedListMarker::ComputeIntrudedFloatOffset( ...@@ -170,8 +170,8 @@ LayoutUnit NGUnpositionedListMarker::ComputeIntrudedFloatOffset(
border_scrollbar_padding.inline_start - border_scrollbar_padding.inline_start -
border_scrollbar_padding.inline_end; border_scrollbar_padding.inline_end;
NGLayoutOpportunity opportunity = NGLayoutOpportunity opportunity =
space.ExclusionSpace().FindLayoutOpportunity( space.ExclusionSpace().FindLayoutOpportunity(origin_offset,
origin_offset, available_size, LogicalSize()); available_size);
DCHECK(marker_layout_object_); DCHECK(marker_layout_object_);
const TextDirection direction = marker_layout_object_->StyleRef().Direction(); const TextDirection direction = marker_layout_object_->StyleRef().Direction();
if (direction == TextDirection::kLtr) { if (direction == TextDirection::kLtr) {
......
...@@ -18,8 +18,7 @@ LayoutUnit CalculateOutOfFlowStaticInlineLevelOffset( ...@@ -18,8 +18,7 @@ LayoutUnit CalculateOutOfFlowStaticInlineLevelOffset(
// Find a layout opportunity, where we would have placed a zero-sized line. // Find a layout opportunity, where we would have placed a zero-sized line.
NGLayoutOpportunity opportunity = exclusion_space.FindLayoutOpportunity( NGLayoutOpportunity opportunity = exclusion_space.FindLayoutOpportunity(
origin_bfc_offset, child_available_inline_size, origin_bfc_offset, child_available_inline_size);
/* minimum_size */ LogicalSize());
LayoutUnit child_line_offset = IsLtr(direction) LayoutUnit child_line_offset = IsLtr(direction)
? opportunity.rect.LineStartOffset() ? opportunity.rect.LineStartOffset()
......
...@@ -52,10 +52,9 @@ NGLayoutOpportunity FindLayoutOpportunityForFloat( ...@@ -52,10 +52,9 @@ NGLayoutOpportunity FindLayoutOpportunityForFloat(
AdjustToClearance(clearance_offset, &adjusted_origin_point); AdjustToClearance(clearance_offset, &adjusted_origin_point);
LogicalSize float_size(inline_size + fragment_margins.InlineSum(),
LayoutUnit());
return exclusion_space.FindLayoutOpportunity( return exclusion_space.FindLayoutOpportunity(
adjusted_origin_point, float_available_size.inline_size, float_size); adjusted_origin_point, float_available_size.inline_size,
inline_size + fragment_margins.InlineSum() /* minimum_inline_size */);
} }
// Creates a constraint space for an unpositioned float. origin_block_offset // Creates a constraint space for an unpositioned float. origin_block_offset
......
<!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: 100px;">
<div style="float: left; width: 50px; height: 100px; margin-left: 50px; margin-right: 50px;background: green;"></div>
<div style="float: left; width: 50px; height: 100px; margin-left: -150px; 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