- Revision
- 154470
- Author
- [email protected]
- Date
- 2013-08-22 17:28:48 -0700 (Thu, 22 Aug 2013)
Log Message
compositing/geometry/bounds-ignores-hidden-dynamic.html has incorrect initial rendering
https://bugs.webkit.org/show_bug.cgi?id=119825
Source/WebCore:
Reviewed by Tim Horton.
r137526 and some earlier commits attempted to avoid unconditionally
repainting layers when their size changes, because this was causing
TiledBacking layers to repaint when the document size changed.
However, the approach required that we have good information about
whether size changes require a repaint, which in some cases is hard
to determine, especially when RenderLayer changes affect our
decisions about which layers are composited.
Fix by pushing the decision about whether to repaint on size change
into GraphicsLayer. The default is to repaint on size change,
but GraphicsLayer provides a function that can be overridden to
modify this behavior; GraphicsLayerCA does so to avoid repaints
when layers with TiledBackings get resized.
Test: compositing/repaint/repaint-on-layer-grouping-change.html
* WebCore.exp.in: WebKit2 needs GraphicsLayer::setSize, which is no longer inline.
* platform/graphics/GraphicsLayer.cpp:
(WebCore::GraphicsLayer::setOffsetFromRenderer):
(WebCore::GraphicsLayer::setSize):
* platform/graphics/GraphicsLayer.h:
(WebCore::GraphicsLayer::shouldRepaintOnSizeChange):
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::shouldRepaintOnSizeChange):
* platform/graphics/ca/GraphicsLayerCA.h:
* rendering/RenderLayerBacking.h: No longer need m_boundsConstrainedByClipping
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::RenderLayerBacking):
(WebCore::RenderLayerBacking::updateCompositedBounds):
(WebCore::RenderLayerBacking::updateGraphicsLayerGeometry): setSize takes
care of repainting for us now, so we can remove all the conditional code.
LayoutTests:
Reviewed by Tim Horton.
Test that dumps repaint rects on layers after visibility changes affect
the layer hierarchy.
* compositing/repaint/repaint-on-layer-grouping-change-expected.txt: Added.
* compositing/repaint/repaint-on-layer-grouping-change.html: Added.
* compositing/repaint/resize-repaint-expected.txt: Update result.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (154469 => 154470)
--- trunk/LayoutTests/ChangeLog 2013-08-23 00:28:45 UTC (rev 154469)
+++ trunk/LayoutTests/ChangeLog 2013-08-23 00:28:48 UTC (rev 154470)
@@ -1,3 +1,17 @@
+2013-08-22 Simon Fraser <[email protected]>
+
+ compositing/geometry/bounds-ignores-hidden-dynamic.html has incorrect initial rendering
+ https://bugs.webkit.org/show_bug.cgi?id=119825
+
+ Reviewed by Tim Horton.
+
+ Test that dumps repaint rects on layers after visibility changes affect
+ the layer hierarchy.
+
+ * compositing/repaint/repaint-on-layer-grouping-change-expected.txt: Added.
+ * compositing/repaint/repaint-on-layer-grouping-change.html: Added.
+ * compositing/repaint/resize-repaint-expected.txt: Update result.
+
2013-08-22 Christophe Dumez <[email protected]>
[SVG2] Merge SVGStyledElement and SVGElement
Added: trunk/LayoutTests/compositing/repaint/repaint-on-layer-grouping-change-expected.txt (0 => 154470)
--- trunk/LayoutTests/compositing/repaint/repaint-on-layer-grouping-change-expected.txt (rev 0)
+++ trunk/LayoutTests/compositing/repaint/repaint-on-layer-grouping-change-expected.txt 2013-08-23 00:28:48 UTC (rev 154470)
@@ -0,0 +1,33 @@
+
+(GraphicsLayer
+ (bounds 800.00 600.00)
+ (children 1
+ (GraphicsLayer
+ (bounds 800.00 600.00)
+ (contentsOpaque 1)
+ (children 2
+ (GraphicsLayer
+ (position 10.00 10.00)
+ (anchor -0.02 -0.04)
+ (bounds 540.00 240.00)
+ (drawsContent 1)
+ (repaint rects
+ (rect 0.00 0.00 540.00 240.00)
+ )
+ )
+ (GraphicsLayer
+ (position 10.00 260.00)
+ (anchor -0.20 -0.20)
+ (bounds 50.00 50.00)
+ (contentsOpaque 1)
+ (drawsContent 1)
+ (repaint rects
+ (rect 490.00 190.00 50.00 50.00)
+ (rect 0.00 0.00 50.00 50.00)
+ )
+ )
+ )
+ )
+ )
+)
+
Property changes on: trunk/LayoutTests/compositing/repaint/repaint-on-layer-grouping-change-expected.txt
___________________________________________________________________
Added: svn:mime-type
Added: svn:keywords
Added: svn:eol-style
Added: trunk/LayoutTests/compositing/repaint/repaint-on-layer-grouping-change.html (0 => 154470)
--- trunk/LayoutTests/compositing/repaint/repaint-on-layer-grouping-change.html (rev 0)
+++ trunk/LayoutTests/compositing/repaint/repaint-on-layer-grouping-change.html 2013-08-23 00:28:48 UTC (rev 154470)
@@ -0,0 +1,68 @@
+<style>
+ .composited {
+ -webkit-transform: translateZ(0);
+ }
+
+ .box {
+ width: 100px;
+ height: 100px;
+ background-color: blue;
+ }
+
+ img {
+ background-color: black;
+ width: 50px;
+ height: 50px;
+ }
+
+ img.to-visible {
+ visibility: hidden;
+ }
+ body.changed img.to-visible {
+ visibility: visible;
+ }
+
+ img.to-hidden {
+ visibility: visible;
+ }
+ body.changed img.to-hidden {
+ visibility: hidden;
+ }
+
+ #layers {
+ opacity: 0; /* hide from pixel result */
+ }
+</style>
+<script>
+ if (window.testRunner) {
+ testRunner.dumpAsText(true);
+ testRunner.waitUntilDone();
+ }
+
+ function doTest()
+ {
+ document.body.offsetWidth;
+ window.setTimeout(function() {
+ if (window.internals)
+ window.internals.startTrackingRepaints(document);
+ document.body.classList.toggle('changed');
+ if (window.testRunner) {
+ document.getElementById('layers').innerText = window.internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_REPAINT_RECTS);
+ testRunner.notifyDone();
+ }
+ }, 0);
+ }
+ window.addEventListener('load', doTest, false);
+</script>
+<body>
+ <div style="position: absolute; left: 0px; top: 0px; z-index: 106; " class="composited">
+ <img style="position: absolute; left: 10px; top: 10px; z-index: 0;">
+ <img style="position: absolute; left: 500px; top: 200px; z-index: 0;" class="to-visible">
+ </div>
+
+ <div style="position: absolute; left: 0px; top: 250px; z-index: 106; " class="composited">
+ <img style="position: absolute; left: 10px; top: 10px; z-index: 0;">
+ <img style="position: absolute; left: 500px; top: 200px; z-index: 0;" class="to-hidden">
+ </div>
+<pre id="layers">Layer tree goes here in DRT</pre>
+</body>
Property changes on: trunk/LayoutTests/compositing/repaint/repaint-on-layer-grouping-change.html
___________________________________________________________________
Added: svn:mime-type
Added: svn:keywords
Added: svn:eol-style
Modified: trunk/LayoutTests/compositing/repaint/resize-repaint-expected.txt (154469 => 154470)
--- trunk/LayoutTests/compositing/repaint/resize-repaint-expected.txt 2013-08-23 00:28:45 UTC (rev 154469)
+++ trunk/LayoutTests/compositing/repaint/resize-repaint-expected.txt 2013-08-23 00:28:48 UTC (rev 154470)
@@ -14,8 +14,7 @@
(bounds 402.00 207.00)
(drawsContent 1)
(repaint rects
- (rect 2.00 104.00 398.00 1.00)
- (rect 0.00 104.00 402.00 1.00)
+ (rect 0.00 0.00 402.00 207.00)
)
)
)
Modified: trunk/Source/WebCore/ChangeLog (154469 => 154470)
--- trunk/Source/WebCore/ChangeLog 2013-08-23 00:28:45 UTC (rev 154469)
+++ trunk/Source/WebCore/ChangeLog 2013-08-23 00:28:48 UTC (rev 154470)
@@ -1,5 +1,45 @@
2013-08-22 Simon Fraser <[email protected]>
+ compositing/geometry/bounds-ignores-hidden-dynamic.html has incorrect initial rendering
+ https://bugs.webkit.org/show_bug.cgi?id=119825
+
+ Reviewed by Tim Horton.
+
+ r137526 and some earlier commits attempted to avoid unconditionally
+ repainting layers when their size changes, because this was causing
+ TiledBacking layers to repaint when the document size changed.
+
+ However, the approach required that we have good information about
+ whether size changes require a repaint, which in some cases is hard
+ to determine, especially when RenderLayer changes affect our
+ decisions about which layers are composited.
+
+ Fix by pushing the decision about whether to repaint on size change
+ into GraphicsLayer. The default is to repaint on size change,
+ but GraphicsLayer provides a function that can be overridden to
+ modify this behavior; GraphicsLayerCA does so to avoid repaints
+ when layers with TiledBackings get resized.
+
+ Test: compositing/repaint/repaint-on-layer-grouping-change.html
+
+ * WebCore.exp.in: WebKit2 needs GraphicsLayer::setSize, which is no longer inline.
+ * platform/graphics/GraphicsLayer.cpp:
+ (WebCore::GraphicsLayer::setOffsetFromRenderer):
+ (WebCore::GraphicsLayer::setSize):
+ * platform/graphics/GraphicsLayer.h:
+ (WebCore::GraphicsLayer::shouldRepaintOnSizeChange):
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::shouldRepaintOnSizeChange):
+ * platform/graphics/ca/GraphicsLayerCA.h:
+ * rendering/RenderLayerBacking.h: No longer need m_boundsConstrainedByClipping
+ * rendering/RenderLayerBacking.cpp:
+ (WebCore::RenderLayerBacking::RenderLayerBacking):
+ (WebCore::RenderLayerBacking::updateCompositedBounds):
+ (WebCore::RenderLayerBacking::updateGraphicsLayerGeometry): setSize takes
+ care of repainting for us now, so we can remove all the conditional code.
+
+2013-08-22 Simon Fraser <[email protected]>
+
Repaint counters are sometimes not in the corner of the compositing layer
https://bugs.webkit.org/show_bug.cgi?id=120176
Modified: trunk/Source/WebCore/WebCore.exp.in (154469 => 154470)
--- trunk/Source/WebCore/WebCore.exp.in 2013-08-23 00:28:45 UTC (rev 154469)
+++ trunk/Source/WebCore/WebCore.exp.in 2013-08-23 00:28:48 UTC (rev 154470)
@@ -1372,6 +1372,7 @@
__ZNK7WebCore12TextIterator5rangeEv
__ZNK7WebCore13ContainerNode14childNodeCountEv
__ZNK7WebCore13ContainerNode9childNodeEj
+__ZN7WebCore13GraphicsLayer7setSizeERKNS_9FloatSizeE
__ZNK7WebCore13GraphicsLayer18accumulatedOpacityEv
__ZNK7WebCore13GraphicsLayer18getDebugBorderInfoERNS_5ColorERf
__ZNK7WebCore13GraphicsLayer26backingStoreMemoryEstimateEv
Modified: trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp (154469 => 154470)
--- trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp 2013-08-23 00:28:45 UTC (rev 154469)
+++ trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp 2013-08-23 00:28:48 UTC (rev 154470)
@@ -307,6 +307,17 @@
setNeedsDisplay();
}
+void GraphicsLayer::setSize(const FloatSize& size)
+{
+ if (size == m_size)
+ return;
+
+ m_size = size;
+
+ if (shouldRepaintOnSizeChange())
+ setNeedsDisplay();
+}
+
void GraphicsLayer::setBackgroundColor(const Color& color)
{
m_backgroundColor = color;
Modified: trunk/Source/WebCore/platform/graphics/GraphicsLayer.h (154469 => 154470)
--- trunk/Source/WebCore/platform/graphics/GraphicsLayer.h 2013-08-23 00:28:45 UTC (rev 154469)
+++ trunk/Source/WebCore/platform/graphics/GraphicsLayer.h 2013-08-23 00:28:48 UTC (rev 154470)
@@ -289,7 +289,7 @@
// The size of the layer.
const FloatSize& size() const { return m_size; }
- virtual void setSize(const FloatSize& size) { m_size = size; }
+ virtual void setSize(const FloatSize&);
// The boundOrigin affects the offset at which content is rendered, and sublayers are positioned.
const FloatPoint& boundsOrigin() const { return m_boundsOrigin; }
@@ -501,6 +501,8 @@
// rotations of >= 180 degrees
static int validateTransformOperations(const KeyframeValueList&, bool& hasBigRotation);
+ virtual bool shouldRepaintOnSizeChange() const { return drawsContent(); }
+
virtual void setOpacityInternal(float) { }
// The layer being replicated.
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (154469 => 154470)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2013-08-23 00:28:45 UTC (rev 154469)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2013-08-23 00:28:48 UTC (rev 154470)
@@ -672,6 +672,11 @@
noteLayerPropertyChanged(ContentsRectChanged);
}
+bool GraphicsLayerCA::shouldRepaintOnSizeChange() const
+{
+ return drawsContent() && !tiledBacking();
+}
+
bool GraphicsLayerCA::addAnimation(const KeyframeValueList& valueList, const IntSize& boxSize, const Animation* anim, const String& animationName, double timeOffset)
{
ASSERT(!animationName.isEmpty());
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h (154469 => 154470)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h 2013-08-23 00:28:45 UTC (rev 154469)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h 2013-08-23 00:28:48 UTC (rev 154470)
@@ -176,6 +176,8 @@
virtual double backingStoreMemoryEstimate() const;
+ virtual bool shouldRepaintOnSizeChange() const OVERRIDE;
+
void updateOpacityOnLayer();
#if ENABLE(CSS_FILTERS)
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (154469 => 154470)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2013-08-23 00:28:45 UTC (rev 154469)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2013-08-23 00:28:48 UTC (rev 154470)
@@ -110,7 +110,6 @@
: m_owningLayer(layer)
, m_scrollLayerID(0)
, m_artificiallyInflatedBounds(false)
- , m_boundsConstrainedByClipping(false)
, m_isMainFrameRenderViewLayer(false)
, m_usingTiledCacheLayer(false)
, m_requiresOwnBackingStore(true)
@@ -448,9 +447,7 @@
clippingBounds.move(-delta.x(), -delta.y());
layerBounds.intersect(clippingBounds);
- m_boundsConstrainedByClipping = true;
- } else
- m_boundsConstrainedByClipping = false;
+ }
// If the element has a transform-origin that has fixed lengths, and the renderer has zero size,
// then we need to ensure that the compositing layer has non-zero size so that we can apply
@@ -705,17 +702,9 @@
}
m_graphicsLayer->setPosition(FloatPoint(relativeCompositingBounds.location() - graphicsLayerParentLocation));
+ m_graphicsLayer->setSize(contentsSize);
m_graphicsLayer->setOffsetFromRenderer(toIntSize(localCompositingBounds.location()));
- FloatSize oldSize = m_graphicsLayer->size();
- if (oldSize != contentsSize) {
- m_graphicsLayer->setSize(contentsSize);
- // Usually invalidation will happen via layout etc, but if we've affected the layer
- // size by constraining relative to a clipping ancestor or the viewport, we
- // have to invalidate to avoid showing stretched content.
- if (m_boundsConstrainedByClipping)
- m_graphicsLayer->setNeedsDisplay();
- }
if (!m_isMainFrameRenderViewLayer) {
// For non-root layers, background is always painted by the primary graphics layer.
ASSERT(!m_backgroundLayer);
@@ -732,10 +721,7 @@
}
if (m_maskLayer) {
- if (m_maskLayer->size() != m_graphicsLayer->size()) {
- m_maskLayer->setSize(m_graphicsLayer->size());
- m_maskLayer->setNeedsDisplay();
- }
+ m_maskLayer->setSize(m_graphicsLayer->size());
m_maskLayer->setPosition(FloatPoint());
m_maskLayer->setOffsetFromRenderer(m_graphicsLayer->offsetFromRenderer());
}
@@ -792,10 +778,7 @@
}
m_foregroundLayer->setPosition(foregroundPosition);
- if (foregroundSize != m_foregroundLayer->size()) {
- m_foregroundLayer->setSize(foregroundSize);
- m_foregroundLayer->setNeedsDisplay();
- }
+ m_foregroundLayer->setSize(foregroundSize);
m_foregroundLayer->setOffsetFromRenderer(foregroundOffset);
}
@@ -808,10 +791,7 @@
backgroundSize = frameView->visibleContentRect().size();
}
m_backgroundLayer->setPosition(backgroundPosition);
- if (backgroundSize != m_backgroundLayer->size()) {
- m_backgroundLayer->setSize(backgroundSize);
- m_backgroundLayer->setNeedsDisplay();
- }
+ m_backgroundLayer->setSize(backgroundSize);
m_backgroundLayer->setOffsetFromRenderer(m_graphicsLayer->offsetFromRenderer());
}
@@ -855,9 +835,7 @@
m_scrollingContentsLayer->setOffsetFromRenderer(scrollingContentsOffset, GraphicsLayer::DontSetNeedsDisplay);
if (m_foregroundLayer) {
- if (m_foregroundLayer->size() != m_scrollingContentsLayer->size())
- m_foregroundLayer->setSize(m_scrollingContentsLayer->size());
- m_foregroundLayer->setNeedsDisplay();
+ m_foregroundLayer->setSize(m_scrollingContentsLayer->size());
m_foregroundLayer->setOffsetFromRenderer(m_scrollingContentsLayer->offsetFromRenderer());
}
}
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.h (154469 => 154470)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.h 2013-08-23 00:28:45 UTC (rev 154469)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.h 2013-08-23 00:28:48 UTC (rev 154470)
@@ -301,7 +301,6 @@
LayoutSize m_subpixelAccumulation; // The accumulated subpixel offset of the compositedBounds compared to absolute coordinates.
bool m_artificiallyInflatedBounds; // bounds had to be made non-zero to make transform-origin work
- bool m_boundsConstrainedByClipping;
bool m_isMainFrameRenderViewLayer;
bool m_usingTiledCacheLayer;
bool m_requiresOwnBackingStore;
