Skip to content

Commit

Permalink
Simplify repainting logic in RenderLayer::recursiveUpdateLayerPositio…
Browse files Browse the repository at this point in the history
…ns()

https://bugs.webkit.org/show_bug.cgi?id=264461
rdar://118151709

Reviewed by Alan Baradlay.

The `NeedsFullRepaint` case previously duplicate some logic that exists in `RenderElement::repaintAfterLayoutIfNeeded()`,
which is that if we're doing a full repaint, we just need to paint the clippedOverflowRects.

This logic was optimized in 270370@main to turn two repaints into one sometimes, and we can re-use it here. We know
that in the RequiresFullRepaint::Yes case, `RenderElement::repaintAfterLayoutIfNeeded()` only consults the clippedOverflowRects,
so we can fold this logic with the second `shouldRepaintAfterLayout()` clause. It's fine to pass empty oldRects if we didn't
have any.

`repaintAfterLayoutIfNeeded()` does the check for printing, so we can remove that. Also the comment duplicates comments
in that function too, so remove it.

* LayoutTests/fast/repaint/hidpi-box-with-subpixel-height-inflates-expected.txt:
* Source/WebCore/rendering/RenderLayer.cpp:
(WebCore::RenderLayer::recursiveUpdateLayerPositions):
(WebCore::RenderLayer::shouldRepaintAfterLayout const):

Canonical link: https://commits.webkit.org/270454@main
  • Loading branch information
smfr committed Nov 9, 2023
1 parent b6917b9 commit 9dc96cf
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ TEST COMPLETE
(rect 0 48.50 22 1)
(rect 0 49.50 800 152.50)
(rect 0 49.50 800 152.50)
(rect 1 1 1 47.50)
(rect 1 1 1 200)
)

27 changes: 7 additions & 20 deletions Source/WebCore/rendering/RenderLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1037,27 +1037,14 @@ void RenderLayer::recursiveUpdateLayerPositions(RenderGeometryMap* geometryMap,

auto oldRects = repaintRects();
computeRepaintRects(repaintContainer.get(), geometryMap);

auto newRects = repaintRects();

// FIXME: Should ASSERT that value calculated for m_outlineBox using the cached offset is the same
// as the value not using the cached offset, but we can't due to https://bugs.webkit.org/show_bug.cgi?id=37048
if ((flags & CheckForRepaint) && newRects) {
if (!renderer().view().printing()) {
if (m_repaintStatus == RepaintStatus::NeedsFullRepaint) {
if (oldRects)
renderer().repaintUsingContainer(repaintContainer.get(), oldRects->clippedOverflowRect);

if (!oldRects || newRects->clippedOverflowRect != oldRects->clippedOverflowRect)
renderer().repaintUsingContainer(repaintContainer.get(), newRects->clippedOverflowRect);

} else if (shouldRepaintAfterLayout()) {
// FIXME: We will convert this to just take the old and new RepaintLayoutRects once
// we change other callers to use RepaintLayoutRects.
auto resolvedOldRects = valueOrDefault(oldRects);
renderer().repaintAfterLayoutIfNeeded(repaintContainer.get(), RenderElement::RequiresFullRepaint::No, resolvedOldRects.clippedOverflowRect, resolvedOldRects.outlineBoundsRect,
&newRects->clippedOverflowRect, &newRects->outlineBoundsRect);
}
if (flags.contains(CheckForRepaint) && newRects) {
ASSERT(isSelfPaintingLayer());
if (shouldRepaintAfterLayout()) {
auto needsFullRepaint = m_repaintStatus == RepaintStatus::NeedsFullRepaint ? RenderElement::RequiresFullRepaint::Yes : RenderElement::RequiresFullRepaint::No;
auto resolvedOldRects = valueOrDefault(oldRects);
renderer().repaintAfterLayoutIfNeeded(repaintContainer.get(), needsFullRepaint, resolvedOldRects.clippedOverflowRect, resolvedOldRects.outlineBoundsRect, &newRects->clippedOverflowRect, &newRects->outlineBoundsRect);
}
}
} else
Expand Down Expand Up @@ -2047,7 +2034,7 @@ inline bool RenderLayer::shouldRepaintAfterLayout() const
return false;
#endif

if (m_repaintStatus == RepaintStatus::NeedsNormalRepaint)
if (m_repaintStatus == RepaintStatus::NeedsNormalRepaint || m_repaintStatus == RepaintStatus::NeedsFullRepaint)
return true;

// Composited layers that were moved during a positioned movement only
Expand Down

0 comments on commit 9dc96cf

Please sign in to comment.