Skip to content

GitLab

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

Revert "Move some scroll invalidations to the paint invalidation phase" 

This change partly reverts
https://codereview.chromium.org/498773007

This is blocked on fixing issues with incorrect paint
invalidation containers. Those are complicated matters
and require a lot of thinking to get right. It's also not
clear the amount of work required to fix this more fundamental
issue and we need something to fix the regression.

This change doesn't re-introduce the
updateFixedElementPaintInvalidationRectsAfterScroll
logic as it should be taken care by the paint
invalidation logic (or covered by some test case).

BUG=426507

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

git-svn-id: svn://svn.chromium.org/blink/trunk@185463 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 19f7d409
Hide whitespace changes
Inline Side-by-side
Showing with 125 additions and 4 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 22eadb07
......@@ -1350,6 +1350,12 @@ crbug.com/426543 fast/js/mozilla/strict/15.4.4.13.html [ Pass Failure ]
crbug.com/401377 [ Win Release ] http/tests/serviceworker/fetch-mixed-content-to-inscope.html [ Pass Timeout ]
crbug.com/401377 [ Win Release ] http/tests/serviceworker/fetch-mixed-content-to-outscope.html [ Pass Timeout ]
crbug.com/426507 fast/layers/scroll-descendant-with-cached-cliprects.html [ NeedsRebaseline ]
crbug.com/426507 fast/repaint/fixed-scroll-simple.html [ NeedsRebaseline ]
crbug.com/426507 fast/repaint/scroll-stacking-context-backface-visiblity-leaves-traces.html [ NeedsRebaseline ]
crbug.com/426507 compositing/repaint/fixed-pos-inside-composited-intermediate-layer.html [ NeedsRebaseline ]
crbug.com/426507 compositing/repaint/fixed-pos-with-abs-pos-child-scroll.html [ NeedsRebaseline ]
crbug.com/426078 [ Win7 ] http/tests/security/img-crossorigin-no-credentials-prompt.html [ Pass Timeout ]
crbug.com/430557 [ Win Linux Mac ] inspector/layers/layer-canvas-log.html [ Failure ]
......
third_party/WebKit/LayoutTests/fast/repaint/scroll-stacking-context-backface-visiblity-leaves-traces-expected.txt 0 → 100644
View file @ 22eadb07
{
"bounds": [785, 5056],
"children": [
{
"bounds": [785, 5056],
"contentsOpaque": true,
"drawsContent": true,
"children": [
{
"position": [8, 28],
"bounds": [769, 2122],
"drawsContent": true,
"backfaceVisibility": "hidden",
"repaintRects": [
[10, 1572, 150, 550]
]
}
]
}
]
}
third_party/WebKit/LayoutTests/fast/repaint/scroll-stacking-context-backface-visiblity-leaves-traces.html 0 → 100644
View file @ 22eadb07
<!DOCTYPE html>
<style>
#searchbar {
bottom: 0; /* Useful for seeing the issue visually. */
position: fixed;
}
#recentlink {
background: purple;
position: absolute;
width: 150px;
height: 150px;
}
header {
padding: 10px;
position: relative;
z-index: 50;
-webkit-backface-visibility: hidden;
}
</style>
To manually test, just scroll down, there should be no purple trace.
<header>
<div id="searchbar">
<div id="recentlink"></div>
</div>
</header>
<!-- Used to have some overflowing content to scroll -->
<div style="height: 5000px"></div>
<script src="resources/text-based-repaint.js" type="text/javascript"></script>
<script>
window.scrollTo(0, 1000);
window.testIsAsync = true;
if (window.testRunner)
testRunner.waitUntilDone();
function repaintTest()
{
window.scrollBy(0, 400);
finishRepaintTest();
}
// We need to skip 2 frames for the bug to show under DRT.
window.requestAnimationFrame(function() {
window.requestAnimationFrame(function() {
runRepaintTest();
});
});
</script>
third_party/WebKit/Source/core/frame/FrameView.cpp
View file @ 22eadb07
......@@ -1344,6 +1344,19 @@ static void setShouldDoFullPaintInvalidationIncludingNonCompositingDescendants(c
}
}
LayoutRect paintInvalidationRectIncludingNonCompositingDescendants(RenderLayer* layer)
{
LayoutRect paintInvalidationRect = layer->renderer()->previousPaintInvalidationRect();
for (RenderLayer* child = layer->firstChild(); child; child = child->nextSibling()) {
// Don't include paint invalidation rects for composited child layers; they will paint themselves and have a different origin.
if (child->isPaintInvalidationContainer())
continue;
paintInvalidationRect.unite(paintInvalidationRectIncludingNonCompositingDescendants(child));
}
return paintInvalidationRect;
}
bool FrameView::scrollContentsFastPath(const IntSize& scrollDelta)
{
if (!contentsInCompositedLayer() || hasSlowRepaintObjects())
......@@ -1354,6 +1367,7 @@ bool FrameView::scrollContentsFastPath(const IntSize& scrollDelta)
return true;
}
Region regionToUpdate;
for (const auto& viewportConstrainedObject : *m_viewportConstrainedObjects) {
RenderObject* renderer = viewportConstrainedObject;
ASSERT(renderer->style()->hasViewportConstrainedPosition());
......@@ -1371,10 +1385,39 @@ bool FrameView::scrollContentsFastPath(const IntSize& scrollDelta)
if (layer->hasAncestorWithFilterOutsets())
return false;
setShouldDoFullPaintInvalidationIncludingNonCompositingDescendants(layer);
IntRect updateRect = pixelSnappedIntRect(paintInvalidationRectIncludingNonCompositingDescendants(layer));
const RenderLayerModelObject* repaintContainer = layer->renderer()->containerForPaintInvalidation();
if (repaintContainer && !repaintContainer->isRenderView()) {
// Invalidate the old and new locations of fixed position elements that are not drawn into the RenderView.
updateRect.moveBy(scrollPosition());
IntRect previousRect = updateRect;
previousRect.move(scrollDelta);
// FIXME: Rather than uniting the rects, we should just issue both invalidations.
updateRect.unite(previousRect);
layer->renderer()->invalidatePaintUsingContainer(repaintContainer, updateRect, PaintInvalidationScroll);
} else {
// Coalesce the paint invalidations that will be issued to the renderView.
updateRect = contentsToRootView(updateRect);
if (!updateRect.isEmpty())
regionToUpdate.unite(updateRect);
}
}
InspectorInstrumentation::didScroll(page());
// Invalidate the old and new locations of fixed position elements that are drawn into the RenderView.
Vector<IntRect> subRectsToUpdate = regionToUpdate.rects();
size_t viewportConstrainedObjectsCount = subRectsToUpdate.size();
for (size_t i = 0; i < viewportConstrainedObjectsCount; ++i) {
IntRect updateRect = subRectsToUpdate[i];
IntRect scrolledRect = updateRect;
scrolledRect.move(-scrollDelta);
updateRect.unite(scrolledRect);
// FIXME: We should be able to issue these invalidations separately and before we actually scroll.
renderView()->setBackingNeedsPaintInvalidationInRect(rootViewToContents(updateRect), PaintInvalidationScroll);
}
return true;
}
......@@ -2557,12 +2600,11 @@ void FrameView::updateLayoutAndStyleForPainting()
updateCompositedSelectionBoundsIfNeeded();
InspectorInstrumentation::didUpdateLayerTree(m_frame.get());
}
scrollContentsIfNeededRecursive();
scrollContentsIfNeededRecursive();
if (view)
invalidateTreeIfNeededRecursive();
}
ASSERT(lifecycle().state() == DocumentLifecycle::PaintInvalidationClean);
}
......
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