Skip to content

GitLab

T
tangled
Commit 04848af0 authored by jchaffraix@chromium.org's avatar jchaffraix@chromium.org
Browse files
Options

Fix NeedsFullRepaintForPositionedMovementLayout 

The logic was just plain wrong: during a position movement layout,
we have to invalidate the old and new position as we are shifted.
The current code would generate incremental invalidation as it
fell down the wrong path.

Refactored the code to do what it's supposed to do and added
an invalidation test to cover a case where incremental
invalidation failed.

BUG=352050

Review URL: https://codereview.chromium.org/202433003

git-svn-id: svn://svn.chromium.org/blink/trunk@169505 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 3209be02
Hide whitespace changes
Inline Side-by-side
Showing with 62 additions and 6 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 04848af0
...@@ -111,6 +111,10 @@ crbug.com/310679 [ Mac ] fast/frames/frame-set-scaling-hit.html [ ImageOnlyFailu ...@@ -111,6 +111,10 @@ crbug.com/310679 [ Mac ] fast/frames/frame-set-scaling-hit.html [ ImageOnlyFailu
crbug.com/318434 virtual/gpu/compositedscrolling/scrollbars/custom-scrollbar-with-incomplete-style.html [ ImageOnlyFailure ] crbug.com/318434 virtual/gpu/compositedscrolling/scrollbars/custom-scrollbar-with-incomplete-style.html [ ImageOnlyFailure ]
crbug.com/346134 [ Linux ] virtual/gpu/compositedscrolling/overflow/invisible-descendants-should-not-affect-opt-in.html [ Pass Failure ] crbug.com/346134 [ Linux ] virtual/gpu/compositedscrolling/overflow/invisible-descendants-should-not-affect-opt-in.html [ Pass Failure ]
crbug.com/352050 [ Mac ] fast/repaint/abspos-shift-image-incorrect-repaint.html [ NeedsRebaseline ]
crbug.com/352050 virtual/softwarecompositing/repaint/repaint-into-ancestor-after-layout.html [ NeedsRebaseline ]
crbug.com/352050 compositing/repaint/repaint-into-ancestor-after-layout.html [ NeedsRebaseline ]
# Tests failing since applyPageScaleFactorInCompositor enabled # Tests failing since applyPageScaleFactorInCompositor enabled
crbug.com/225184 fast/repaint/relayout-fixed-position-after-scale.html [ ImageOnlyFailure ] crbug.com/225184 fast/repaint/relayout-fixed-position-after-scale.html [ ImageOnlyFailure ]
......
third_party/WebKit/LayoutTests/fast/repaint/abspos-shift-image-incorrect-repaint-expected.txt 0 → 100644
View file @ 04848af0
This test checks that shifting an abspos container will properly repaint its descendants.
The images below should be correctly repainted.
(repaint rects
(rect 0 0 1500 237)
(rect -250 0 1500 237)
(rect -250 0 250 237)
(rect 450 0 250 237)
(rect 454 0 250 237)
(rect 1154 0 250 237)
)
third_party/WebKit/LayoutTests/fast/repaint/abspos-shift-image-incorrect-repaint.html 0 → 100644
View file @ 04848af0
<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml" lang="en">
<head>
<style>
.imageWrapper
{
position: relative;
display: inline-block;
width: 700px;
}
#shiftMe
{
position: absolute;
top: 0px;
left: 0px;
width: 1500px;
}
</style>
<script src="resources/text-based-repaint.js"></script>
<script>
function repaintTest()
{
shiftMe.style.left = "-250px";
}
</script>
</head>
<body onload="runRepaintTest();">
<div>This test checks that shifting an abspos container will properly repaint its descendants.</div>
<div>The images below should be correctly repainted.</div>
<div id="shiftMe">
<div class="imageWrapper">
<img src="resources/apple.jpg">
</div>
<div class="imageWrapper">
<img src="resources/apple.jpg">
</div>
</div>
</body>
</html>
third_party/WebKit/Source/core/rendering/RenderLayerRepainter.cpp
View file @ 04848af0
...@@ -74,6 +74,7 @@ void RenderLayerRepainter::repaintAfterLayout(RenderGeometryMap* geometryMap, bo ...@@ -74,6 +74,7 @@ void RenderLayerRepainter::repaintAfterLayout(RenderGeometryMap* geometryMap, bo
LayoutRect oldRepaintRect = m_repaintRect; LayoutRect oldRepaintRect = m_repaintRect;
LayoutRect oldOutlineBox = m_outlineBox; LayoutRect oldOutlineBox = m_outlineBox;
computeRepaintRects(repaintContainer, geometryMap); computeRepaintRects(repaintContainer, geometryMap);
shouldCheckForRepaint &= shouldRepaintLayer();
// FIXME: Should ASSERT that value calculated for m_outlineBox using the cached offset is the same // 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 // as the value not using the cached offset, but we can't due to https://bugs.webkit.org/show_bug.cgi?id=37048
...@@ -83,7 +84,7 @@ void RenderLayerRepainter::repaintAfterLayout(RenderGeometryMap* geometryMap, bo ...@@ -83,7 +84,7 @@ void RenderLayerRepainter::repaintAfterLayout(RenderGeometryMap* geometryMap, bo
m_renderer->repaintUsingContainer(repaintContainer, pixelSnappedIntRect(oldRepaintRect)); m_renderer->repaintUsingContainer(repaintContainer, pixelSnappedIntRect(oldRepaintRect));
if (m_repaintRect != oldRepaintRect) if (m_repaintRect != oldRepaintRect)
m_renderer->repaintUsingContainer(repaintContainer, pixelSnappedIntRect(m_repaintRect)); m_renderer->repaintUsingContainer(repaintContainer, pixelSnappedIntRect(m_repaintRect));
} else if (shouldRepaintAfterLayout()) { } else {
m_renderer->repaintAfterLayoutIfNeeded(repaintContainer, m_renderer->selfNeedsLayout(), oldRepaintRect, oldOutlineBox, &m_repaintRect, &m_outlineBox); m_renderer->repaintAfterLayoutIfNeeded(repaintContainer, m_renderer->selfNeedsLayout(), oldRepaintRect, oldOutlineBox, &m_repaintRect, &m_outlineBox);
} }
} }
...@@ -121,14 +122,13 @@ void RenderLayerRepainter::computeRepaintRectsIncludingDescendants() ...@@ -121,14 +122,13 @@ void RenderLayerRepainter::computeRepaintRectsIncludingDescendants()
layer->repainter().computeRepaintRectsIncludingDescendants(); layer->repainter().computeRepaintRectsIncludingDescendants();
} }
inline bool RenderLayerRepainter::shouldRepaintAfterLayout() const inline bool RenderLayerRepainter::shouldRepaintLayer() const
{ {
if (m_repaintStatus == NeedsNormalRepaint) if (m_repaintStatus != NeedsFullRepaintForPositionedMovementLayout)
return true; return true;
// Composited layers that were moved during a positioned movement only // Composited layers that were moved during a positioned movement only
// layout, don't need to be repainted. They just need to be recomposited. // layout, don't need to be repainted. They just need to be recomposited.
ASSERT(m_repaintStatus == NeedsFullRepaintForPositionedMovementLayout);
return m_renderer->compositingState() != PaintsIntoOwnBacking; return m_renderer->compositingState() != PaintsIntoOwnBacking;
} }
......
third_party/WebKit/Source/core/rendering/RenderLayerRepainter.h
View file @ 04848af0
...@@ -53,7 +53,7 @@ namespace WebCore { ...@@ -53,7 +53,7 @@ namespace WebCore {
enum RepaintStatus { enum RepaintStatus {
NeedsNormalRepaint = 0, NeedsNormalRepaint = 0,
NeedsFullRepaint = 1 << 0, NeedsFullRepaint = 1 << 0,
NeedsFullRepaintForPositionedMovementLayout = 1 << 1 NeedsFullRepaintForPositionedMovementLayout = NeedsFullRepaint | 1 << 1
}; };
class RenderGeometryMap; class RenderGeometryMap;
...@@ -86,7 +86,7 @@ public: ...@@ -86,7 +86,7 @@ public:
void setFilterBackendNeedsRepaintingInRect(const LayoutRect&); void setFilterBackendNeedsRepaintingInRect(const LayoutRect&);
private: private:
bool shouldRepaintAfterLayout() const; bool shouldRepaintLayer() const;
void clearRepaintRects(); void clearRepaintRects();
......
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