Title: [152548] trunk/Source/WebCore
- Revision
- 152548
- Author
- [email protected]
- Date
- 2013-07-10 14:15:56 -0700 (Wed, 10 Jul 2013)
Log Message
Deferring layer flushes can cause painting without layout being done
https://bugs.webkit.org/show_bug.cgi?id=118493
Reviewed by Simon Fraser.
r147797 added a mechanism for the TileController to inform RenderLayerCompositor
that it had performed an action (setNeedsDisplay, etc.) that would cause CoreAnimation
to call back into us to paint in this runloop, and that it shouldn't throttle the next
layer flush.
However, if tiles were created and left unparented (because the TileController was
out-of-window), when they are later parented, we failed to inform RenderLayerCompositor
that they were about to call back for painting, and so it would happily throttle the
next layer flush, and layout would be out of date, and garbage and corruption could result.
To resolve this, rework the logic surrounding parenting existing-but-unparented tiles
to ensure that they get added to the TileController's dirtyRect.
No new tests; the bug only reproduces under specific timing circumstances, and
manifests itself as an assertion failure or garbage on the screen, so it's quite
hard to make a workable test.
* platform/graphics/ca/mac/TileController.mm:
(WebCore::TileController::revalidateTiles):
- Factor out the code to parent unparented tiles so that it is shared between the
created-new-tile and reparenting-existing-tile cases.
- Keep track of whether we updated the frame of a pre-existing tile.
- If the tile is dirty and needed to be parented (because either it's new
or hadn't been parented before), add it to the dirty rect list.
- If the tile changed size, add it to the dirty rect list.
(WebCore::TileController::ensureTilesForRect):
Ditto everything from revalidateTiles.
(WebCore::TileController::createTileLayer):
- Explicitly mark fresh tiles as needing display, so that we can depend on
[WebTileLayer needsDisplay] in revalidateTiles to tell us that it's dirty.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (152547 => 152548)
--- trunk/Source/WebCore/ChangeLog 2013-07-10 21:03:26 UTC (rev 152547)
+++ trunk/Source/WebCore/ChangeLog 2013-07-10 21:15:56 UTC (rev 152548)
@@ -1,3 +1,43 @@
+2013-07-10 Tim Horton <[email protected]>
+
+ Deferring layer flushes can cause painting without layout being done
+ https://bugs.webkit.org/show_bug.cgi?id=118493
+
+ Reviewed by Simon Fraser.
+
+ r147797 added a mechanism for the TileController to inform RenderLayerCompositor
+ that it had performed an action (setNeedsDisplay, etc.) that would cause CoreAnimation
+ to call back into us to paint in this runloop, and that it shouldn't throttle the next
+ layer flush.
+
+ However, if tiles were created and left unparented (because the TileController was
+ out-of-window), when they are later parented, we failed to inform RenderLayerCompositor
+ that they were about to call back for painting, and so it would happily throttle the
+ next layer flush, and layout would be out of date, and garbage and corruption could result.
+
+ To resolve this, rework the logic surrounding parenting existing-but-unparented tiles
+ to ensure that they get added to the TileController's dirtyRect.
+
+ No new tests; the bug only reproduces under specific timing circumstances, and
+ manifests itself as an assertion failure or garbage on the screen, so it's quite
+ hard to make a workable test.
+
+ * platform/graphics/ca/mac/TileController.mm:
+ (WebCore::TileController::revalidateTiles):
+ - Factor out the code to parent unparented tiles so that it is shared between the
+ created-new-tile and reparenting-existing-tile cases.
+ - Keep track of whether we updated the frame of a pre-existing tile.
+ - If the tile is dirty and needed to be parented (because either it's new
+ or hadn't been parented before), add it to the dirty rect list.
+ - If the tile changed size, add it to the dirty rect list.
+
+ (WebCore::TileController::ensureTilesForRect):
+ Ditto everything from revalidateTiles.
+
+ (WebCore::TileController::createTileLayer):
+ - Explicitly mark fresh tiles as needing display, so that we can depend on
+ [WebTileLayer needsDisplay] in revalidateTiles to tell us that it's dirty.
+
2013-07-10 Eric Carlson <[email protected]>
[Mac] every enabled text track should be listed in the track menu
Modified: trunk/Source/WebCore/platform/graphics/ca/mac/TileController.mm (152547 => 152548)
--- trunk/Source/WebCore/platform/graphics/ca/mac/TileController.mm 2013-07-10 21:03:26 UTC (rev 152547)
+++ trunk/Source/WebCore/platform/graphics/ca/mac/TileController.mm 2013-07-10 21:15:56 UTC (rev 152548)
@@ -686,26 +686,30 @@
IntRect tileRect = rectForTileIndex(tileIndex);
m_primaryTileCoverageRect.unite(tileRect);
+ bool shouldChangeTileLayerFrame = false;
+
TileInfo& tileInfo = m_tiles.add(tileIndex, TileInfo()).iterator->value;
- if (!tileInfo.layer) {
+ if (!tileInfo.layer)
tileInfo.layer = createTileLayer(tileRect);
- if (!m_unparentsOffscreenTiles || m_isInWindow)
- [m_tileContainerLayer.get() addSublayer:tileInfo.layer.get()];
- } else {
- if ((!m_unparentsOffscreenTiles || m_isInWindow) && ![tileInfo.layer.get() superlayer])
- [m_tileContainerLayer.get() addSublayer:tileInfo.layer.get()];
-
+ else {
// We already have a layer for this tile. Ensure that its size is correct.
FloatSize tileLayerSize([tileInfo.layer.get() frame].size);
- if (tileLayerSize == FloatSize(tileRect.size()))
- continue;
+ shouldChangeTileLayerFrame = tileLayerSize != FloatSize(tileRect.size());
- [tileInfo.layer.get() setFrame:tileRect];
+ if (shouldChangeTileLayerFrame)
+ [tileInfo.layer.get() setFrame:tileRect];
}
-
- FloatRect scaledTileRect = tileRect;
- scaledTileRect.scale(1 / m_scale);
- dirtyRects.append(scaledTileRect);
+
+ bool shouldParentTileLayer = (!m_unparentsOffscreenTiles || m_isInWindow) && ![tileInfo.layer.get() superlayer];
+
+ if (shouldParentTileLayer)
+ [m_tileContainerLayer.get() addSublayer:tileInfo.layer.get()];
+
+ if ((shouldParentTileLayer && [tileInfo.layer.get() needsDisplay]) || shouldChangeTileLayerFrame) {
+ FloatRect scaledTileRect = tileRect;
+ scaledTileRect.scale(1 / m_scale);
+ dirtyRects.append(scaledTileRect);
+ }
}
}
@@ -752,6 +756,8 @@
if (dirtyRects.isEmpty())
return;
+
+ // This will ensure we flush compositing state and do layout in this run loop iteration.
platformLayer->owner()->platformCALayerDidCreateTiles(dirtyRects);
}
@@ -836,18 +842,18 @@
IntRect tileRect = rectForTileIndex(tileIndex);
TileInfo& tileInfo = m_tiles.add(tileIndex, TileInfo()).iterator->value;
- if (!tileInfo.layer) {
+
+ bool shouldChangeTileLayerFrame = false;
+
+ if (!tileInfo.layer)
tileInfo.layer = createTileLayer(tileRect);
- [m_tileContainerLayer.get() addSublayer:tileInfo.layer.get()];
- } else {
- if (![tileInfo.layer.get() superlayer])
- [m_tileContainerLayer.get() addSublayer:tileInfo.layer.get()];
-
+ else {
// We already have a layer for this tile. Ensure that its size is correct.
CGSize tileLayerSize = [tileInfo.layer.get() frame].size;
- if (tileLayerSize.width >= tileRect.width() && tileLayerSize.height >= tileRect.height())
- continue;
- [tileInfo.layer.get() setFrame:tileRect];
+ shouldChangeTileLayerFrame = tileLayerSize.width < tileRect.width() || tileLayerSize.height < tileRect.height();
+
+ if (shouldChangeTileLayerFrame)
+ [tileInfo.layer.get() setFrame:tileRect];
}
if (!tileRect.intersects(m_primaryTileCoverageRect)) {
@@ -855,9 +861,16 @@
++tilesInCohort;
}
- FloatRect scaledTileRect = tileRect;
- scaledTileRect.scale(1 / m_scale);
- dirtyRects.append(scaledTileRect);
+ bool shouldParentTileLayer = ![tileInfo.layer.get() superlayer];
+
+ if (shouldParentTileLayer)
+ [m_tileContainerLayer.get() addSublayer:tileInfo.layer.get()];
+
+ if ((shouldParentTileLayer && [tileInfo.layer.get() needsDisplay]) || shouldChangeTileLayerFrame) {
+ FloatRect scaledTileRect = tileRect;
+ scaledTileRect.scale(1 / m_scale);
+ dirtyRects.append(scaledTileRect);
+ }
}
}
@@ -867,6 +880,7 @@
if (m_tiledScrollingIndicatorLayer)
updateTileCoverageMap();
+ // This will ensure we flush compositing state and do layout in this run loop iteration.
if (!dirtyRects.isEmpty())
platformLayer->owner()->platformCALayerDidCreateTiles(dirtyRects);
}
@@ -993,12 +1007,9 @@
RetainPtr<WebTileLayer> TileController::createTileLayer(const IntRect& tileRect)
{
RetainPtr<WebTileLayer> layer = LayerPool::sharedPool()->takeLayerWithSize(tileRect.size());
- if (layer) {
- // If we were able to restore a layer from the LayerPool, we should call setNeedsDisplay to
- // ensure we avoid stale content.
+ if (layer)
[layer resetPaintCount];
- [layer setNeedsDisplay];
- } else
+ else
layer = adoptNS([[WebTileLayer alloc] init]);
[layer.get() setAnchorPoint:CGPointZero];
[layer.get() setFrame:tileRect];
@@ -1016,6 +1027,8 @@
[layer.get() setAcceleratesDrawing:m_acceleratesDrawing];
#endif
+ [layer setNeedsDisplay];
+
return layer;
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
