Skip to content

Commit

Permalink
Ensure correct paint property state when painting floating objects
Browse files Browse the repository at this point in the history
Before the regression, most of paint state adjustments were done in
PaintLayerPainter. After we fixed the fundamental compositing bug by
not forcing self-painting PaintLayer for some elements and create
PaintOffsetTranslation for non-PaintLayers in more cases, the issue
of missing paint state adjustments was exposed in more cases.

This CL ensures correct paint state when painting floating objects.

(cherry picked from commit 6626576)

Bug: 1298871
Change-Id: Ifc16af5459ad34c30059229b9d75e825c84edb75
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3516735
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#981629}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3538160
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4896@{#704}
Cr-Branched-From: 1f63ff4-refs/heads/main@{#972766}
  • Loading branch information
wangxianzhu authored and Chromium LUCI CQ committed Mar 19, 2022
1 parent 51c1089 commit 0c705fc
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 27 deletions.
47 changes: 26 additions & 21 deletions third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc
Expand Up @@ -358,6 +358,13 @@ bool ShouldPaintCarets(const NGPhysicalBoxFragment& fragment) {
return ShouldPaintCursorCaret(fragment) || ShouldPaintDragCaret(fragment);
}

PaintInfo FloatPaintInfo(const PaintInfo& paint_info) {
PaintInfo float_paint_info(paint_info);
if (paint_info.phase == PaintPhase::kFloat)
float_paint_info.phase = PaintPhase::kForeground;
return float_paint_info;
}

} // anonymous namespace

PhysicalRect NGBoxFragmentPainter::InkOverflowIncludingFilters() const {
Expand Down Expand Up @@ -805,7 +812,6 @@ void NGBoxFragmentPainter::PaintBlockChild(
}

void NGBoxFragmentPainter::PaintFloatingItems(const PaintInfo& paint_info,
const PaintInfo& float_paint_info,
NGInlineCursor* cursor) {
while (*cursor) {
const NGFragmentItem* item = cursor->Current().Item();
Expand All @@ -820,6 +826,7 @@ void NGBoxFragmentPainter::PaintFloatingItems(const PaintInfo& paint_info,
continue;
}
if (child_fragment->IsFloating()) {
PaintInfo float_paint_info = FloatPaintInfo(paint_info);
if (child_fragment->CanTraverse()) {
NGBoxFragmentPainter(*child_fragment).Paint(float_paint_info);
} else {
Expand All @@ -839,17 +846,15 @@ void NGBoxFragmentPainter::PaintFloatingChildren(
const NGPhysicalFragment& container,
const PaintInfo& paint_info) {
DCHECK(container.HasFloatingDescendantsForPaint());
const PaintInfo* local_paint_info = &paint_info;
absl::optional<ScopedPaintState> paint_state;
absl::optional<ScopedBoxContentsPaintState> contents_paint_state;
if (const auto* box = DynamicTo<LayoutBox>(container.GetLayoutObject())) {
paint_state.emplace(container, paint_info);
contents_paint_state.emplace(*paint_state, *box);
local_paint_info = &contents_paint_state->GetPaintInfo();
}

PaintInfo float_paint_info(paint_info);
if (paint_info.phase == PaintPhase::kFloat)
float_paint_info.phase = PaintPhase::kForeground;
PaintFloatingChildren(container, paint_info, float_paint_info);
}

void NGBoxFragmentPainter::PaintFloatingChildren(
const NGPhysicalFragment& container,
const PaintInfo& paint_info,
const PaintInfo& float_paint_info) {
DCHECK(container.HasFloatingDescendantsForPaint());

for (const NGLink& child : container.Children()) {
Expand All @@ -860,7 +865,7 @@ void NGBoxFragmentPainter::PaintFloatingChildren(
if (child_fragment.CanTraverse()) {
if (child_fragment.IsFloating()) {
NGBoxFragmentPainter(To<NGPhysicalBoxFragment>(child_fragment))
.Paint(float_paint_info);
.Paint(FloatPaintInfo(*local_paint_info));
continue;
}

Expand All @@ -878,7 +883,7 @@ void NGBoxFragmentPainter::PaintFloatingChildren(
// the first case when we're more stable.

ObjectPainter(*child_fragment.GetLayoutObject())
.PaintAllPhasesAtomically(float_paint_info);
.PaintAllPhasesAtomically(FloatPaintInfo(*local_paint_info));
continue;
}

Expand All @@ -888,18 +893,18 @@ void NGBoxFragmentPainter::PaintFloatingChildren(

// Drawing in SelectionDragImage phase can result in an exponential
// paint time: crbug.com://1182106
if (paint_info.phase != PaintPhase::kSelectionDragImage &&
if (local_paint_info->phase != PaintPhase::kSelectionDragImage &&
child_fragment.Type() == NGPhysicalFragment::kFragmentBox &&
FragmentRequiresLegacyFallback(child_fragment)) {
child_fragment.GetLayoutObject()->Paint(paint_info);
child_fragment.GetLayoutObject()->Paint(*local_paint_info);
continue;
}
}

// The selection paint traversal is special. We will visit all fragments
// (including floats) in the normal paint traversal. There isn't any point
// performing the special float traversal here.
if (paint_info.phase == PaintPhase::kSelectionDragImage)
if (local_paint_info->phase == PaintPhase::kSelectionDragImage)
continue;

if (!child_fragment.HasFloatingDescendantsForPaint())
Expand All @@ -910,7 +915,7 @@ void NGBoxFragmentPainter::PaintFloatingChildren(
// jumping directly to its children (which is what we normally do when
// looking for floats), in order to set up the clip rectangle.
NGBoxFragmentPainter(To<NGPhysicalBoxFragment>(child_fragment))
.Paint(paint_info);
.Paint(*local_paint_info);
continue;
}

Expand All @@ -922,9 +927,9 @@ void NGBoxFragmentPainter::PaintFloatingChildren(
unsigned identifier = FragmentainerUniqueIdentifier(
To<NGPhysicalBoxFragment>(child_fragment));
ScopedDisplayItemFragment scope(paint_info.context, identifier);
PaintFloatingChildren(child_fragment, paint_info, float_paint_info);
PaintFloatingChildren(child_fragment, *local_paint_info);
} else {
PaintFloatingChildren(child_fragment, paint_info, float_paint_info);
PaintFloatingChildren(child_fragment, *local_paint_info);
}
}

Expand All @@ -936,13 +941,13 @@ void NGBoxFragmentPainter::PaintFloatingChildren(
DynamicTo<NGPhysicalBoxFragment>(&container)) {
if (const NGFragmentItems* items = box->Items()) {
NGInlineCursor cursor(*box, *items);
PaintFloatingItems(paint_info, float_paint_info, &cursor);
PaintFloatingItems(*local_paint_info, &cursor);
return;
}
if (inline_box_cursor_) {
DCHECK(box->IsInlineBox());
NGInlineCursor descendants = inline_box_cursor_->CursorForDescendants();
PaintFloatingItems(paint_info, float_paint_info, &descendants);
PaintFloatingItems(*local_paint_info, &descendants);
return;
}
DCHECK(!box->IsInlineBox());
Expand Down
Expand Up @@ -164,14 +164,9 @@ class CORE_EXPORT NGBoxFragmentPainter : public BoxPainterBase {
const PaintInfo& paint_info,
const PhysicalOffset& paint_offset,
const PhysicalOffset& parent_offset);
void PaintFloatingItems(const PaintInfo& paint_info,
const PaintInfo& float_paint_info,
NGInlineCursor* cursor);
void PaintFloatingItems(const PaintInfo& paint_info, NGInlineCursor* cursor);
void PaintFloatingChildren(const NGPhysicalFragment&,
const PaintInfo& paint_info);
void PaintFloatingChildren(const NGPhysicalFragment&,
const PaintInfo& paint_info,
const PaintInfo& float_paint_info);
void PaintFloats(const PaintInfo&);
void PaintMask(const PaintInfo&, const PhysicalOffset& paint_offset);
void PaintBackground(const PaintInfo&,
Expand Down
@@ -0,0 +1,10 @@
<!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">
<link rel="help" href="https://crbug.com/1298871">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="transform-style: preserve-3d">
<div style="background: red; width: 100px; height: 100px">
<div style="float: left; background: green; width: 100px; height: 100px"></div>
</div>
</div>

0 comments on commit 0c705fc

Please sign in to comment.