- Revision
- 166296
- Author
- timothy_hor...@apple.com
- Date
- 2014-03-26 08:25:28 -0700 (Wed, 26 Mar 2014)
Log Message
[iOS WebKit2] Flush all surfaces after painting into all of them, instead of after painting into each one
https://bugs.webkit.org/show_bug.cgi?id=130768
<rdar://problem/16421471>
Reviewed by Simon Fraser.
* Shared/mac/RemoteLayerBackingStore.h:
Add flush(), which synchronously flushes painting operations on the underlying backing store.
Factor clearBackingStore() out of ensureBackingStore/display, which releases our reference to underlying backing store.
Add two members for storing the back surface and front buffer context until flush() is called.
- We need to keep the back surface alive because the CGImageRef created from it is referenced by
the front surface's drawing queue, and won't be freed until said queue is flushed. If we release
the back surface (and its associated CGContextRef) *before* the CGImageRef is freed, we will
do an expensive readback of the surface.
- When not using accelerated drawing, we need to keep the front buffer's CGContextRef around
until the flush occurs so that we can avoid re-creating it in order to perform the flush. This
happens automatically in the accelerated drawing case via WebCore::IOSurface.
* Shared/mac/RemoteLayerBackingStore.mm:
(RemoteLayerBackingStore::ensureBackingStore):
(RemoteLayerBackingStore::clearBackingStore):
(RemoteLayerBackingStore::display):
Factor clearBackingStore() out of ensureBackingStore() and display().
Update a comment about the above performance gotcha.
Store the current back surface/front buffer context.
(RemoteLayerBackingStore::drawInContext):
Don't flush the context immediately after painting.
(RemoteLayerBackingStore::applyBackingStoreToLayer):
Move things around to reduce duplicated code.
(RemoteLayerBackingStore::flush):
Flush the current front surface/buffer's context.
Clear the new pending-flush members.
* WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:
(WebKit::flushBackingStoreChangesInTransaction):
(WebKit::RemoteLayerTreeDrawingArea::flushLayers):
Crawl through all of the valid changed backing stores in the transaction and flush them.
Remove a completely useless assertion.
* platform/graphics/cocoa/IOSurface.h:
Add a non-ensuring platformContext() getter.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (166295 => 166296)
--- trunk/Source/WebCore/ChangeLog 2014-03-26 15:18:30 UTC (rev 166295)
+++ trunk/Source/WebCore/ChangeLog 2014-03-26 15:25:28 UTC (rev 166296)
@@ -1,3 +1,14 @@
+2014-03-26 Tim Horton <timothy_hor...@apple.com>
+
+ [iOS WebKit2] Flush all surfaces after painting into all of them, instead of after painting into each one
+ https://bugs.webkit.org/show_bug.cgi?id=130768
+ <rdar://problem/16421471>
+
+ Reviewed by Simon Fraser.
+
+ * platform/graphics/cocoa/IOSurface.h:
+ Add a non-ensuring platformContext() getter.
+
2014-03-26 James Craig <jcr...@apple.com>
Web Inspector: AXI: crash when inspecting "bar" text node in getAccessibilityPropertiesForNode layout test
Modified: trunk/Source/WebCore/platform/graphics/cocoa/IOSurface.h (166295 => 166296)
--- trunk/Source/WebCore/platform/graphics/cocoa/IOSurface.h 2014-03-26 15:18:30 UTC (rev 166295)
+++ trunk/Source/WebCore/platform/graphics/cocoa/IOSurface.h 2014-03-26 15:25:28 UTC (rev 166296)
@@ -52,6 +52,7 @@
IOSurfaceRef surface() const { return m_surface.get(); }
GraphicsContext& ensureGraphicsContext();
CGContextRef ensurePlatformContext();
+ CGContextRef platformContext() { return m_cgContext.get(); }
enum class SurfaceState {
Valid,
Modified: trunk/Source/WebKit2/ChangeLog (166295 => 166296)
--- trunk/Source/WebKit2/ChangeLog 2014-03-26 15:18:30 UTC (rev 166295)
+++ trunk/Source/WebKit2/ChangeLog 2014-03-26 15:25:28 UTC (rev 166296)
@@ -1,3 +1,47 @@
+2014-03-26 Tim Horton <timothy_hor...@apple.com>
+
+ [iOS WebKit2] Flush all surfaces after painting into all of them, instead of after painting into each one
+ https://bugs.webkit.org/show_bug.cgi?id=130768
+ <rdar://problem/16421471>
+
+ Reviewed by Simon Fraser.
+
+ * Shared/mac/RemoteLayerBackingStore.h:
+ Add flush(), which synchronously flushes painting operations on the underlying backing store.
+ Factor clearBackingStore() out of ensureBackingStore/display, which releases our reference to underlying backing store.
+ Add two members for storing the back surface and front buffer context until flush() is called.
+ - We need to keep the back surface alive because the CGImageRef created from it is referenced by
+ the front surface's drawing queue, and won't be freed until said queue is flushed. If we release
+ the back surface (and its associated CGContextRef) *before* the CGImageRef is freed, we will
+ do an expensive readback of the surface.
+ - When not using accelerated drawing, we need to keep the front buffer's CGContextRef around
+ until the flush occurs so that we can avoid re-creating it in order to perform the flush. This
+ happens automatically in the accelerated drawing case via WebCore::IOSurface.
+
+ * Shared/mac/RemoteLayerBackingStore.mm:
+ (RemoteLayerBackingStore::ensureBackingStore):
+ (RemoteLayerBackingStore::clearBackingStore):
+ (RemoteLayerBackingStore::display):
+ Factor clearBackingStore() out of ensureBackingStore() and display().
+ Update a comment about the above performance gotcha.
+ Store the current back surface/front buffer context.
+
+ (RemoteLayerBackingStore::drawInContext):
+ Don't flush the context immediately after painting.
+
+ (RemoteLayerBackingStore::applyBackingStoreToLayer):
+ Move things around to reduce duplicated code.
+
+ (RemoteLayerBackingStore::flush):
+ Flush the current front surface/buffer's context.
+ Clear the new pending-flush members.
+
+ * WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:
+ (WebKit::flushBackingStoreChangesInTransaction):
+ (WebKit::RemoteLayerTreeDrawingArea::flushLayers):
+ Crawl through all of the valid changed backing stores in the transaction and flush them.
+ Remove a completely useless assertion.
+
2014-03-25 Simon Fraser <simon.fra...@apple.com>
Add a new type of scrolling tree node for overflow scrolling
Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h (166295 => 166296)
--- trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h 2014-03-26 15:18:30 UTC (rev 166295)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h 2014-03-26 15:25:28 UTC (rev 166296)
@@ -75,10 +75,14 @@
#endif
return !!m_frontBuffer;
}
+
+ void flush();
+
private:
void drawInContext(WebCore::GraphicsContext&, CGImageRef frontImage);
RetainPtr<CGImageRef> createImageForFrontBuffer() const;
+ void clearBackingStore();
PlatformCALayerRemote* m_layer;
@@ -89,8 +93,10 @@
WebCore::Region m_dirtyRegion;
RefPtr<ShareableBitmap> m_frontBuffer;
+ RetainPtr<CGContextRef> m_bufferContextPendingFlush;
#if USE(IOSURFACE)
RefPtr<WebCore::IOSurface> m_frontSurface;
+ RefPtr<WebCore::IOSurface> m_backSurfacePendingFlush;
#endif
bool m_acceleratesDrawing;
Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm (166295 => 166296)
--- trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm 2014-03-26 15:18:30 UTC (rev 166295)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm 2014-03-26 15:25:28 UTC (rev 166296)
@@ -68,9 +68,16 @@
m_acceleratesDrawing = acceleratesDrawing;
m_isOpaque = isOpaque;
+ clearBackingStore();
+}
+
+void RemoteLayerBackingStore::clearBackingStore()
+{
#if USE(IOSURFACE)
m_frontSurface = nullptr;
+ m_backSurfacePendingFlush = nullptr;
#endif
+ m_bufferContextPendingFlush = nullptr;
m_frontBuffer = nullptr;
}
@@ -152,6 +159,8 @@
bool RemoteLayerBackingStore::display()
{
+ ASSERT(!m_bufferContextPendingFlush);
+
if (!m_layer)
return false;
@@ -159,12 +168,7 @@
// to note that our backing store has been cleared.
if (!m_layer->owner() || !m_layer->owner()->platformCALayerDrawsContent()) {
bool previouslyDrewContents = hasFrontBuffer();
-
- m_frontBuffer = nullptr;
-#if USE(IOSURFACE)
- m_frontSurface = nullptr;
-#endif
-
+ clearBackingStore();
return previouslyDrewContents;
}
@@ -184,6 +188,7 @@
IntSize expandedScaledSize = expandedIntSize(scaledSize);
#if USE(IOSURFACE)
+ ASSERT(!m_backSurfacePendingFlush);
if (m_acceleratesDrawing) {
RefPtr<IOSurface> backSurface = IOSurface::create(expandedScaledSize, ColorSpaceDeviceRGB);
GraphicsContext& context = backSurface->ensureGraphicsContext();
@@ -196,10 +201,11 @@
frontImage = m_frontSurface->createImage();
drawInContext(context, frontImage.get());
- // If our frontImage is derived from an IOSurface, we need to
- // destroy the image before the CGContext it's derived from,
- // so that the context doesn't make a CPU copy of the surface data.
- frontImage = nullptr;
+ // If our frontImage is derived from an IOSurface, we need to destroy the image before the
+ // CGContext it's derived from, so that the context doesn't make a CPU copy of the surface data.
+ // Since the flush will happen later, this means we need to hold on to the IOSurface
+ // (and thus its context) until after the flush completes.
+ m_backSurfacePendingFlush = m_frontSurface;
m_frontSurface = backSurface;
return true;
@@ -211,6 +217,7 @@
RetainPtr<CGImageRef> frontImage = createImageForFrontBuffer();
m_frontBuffer = ShareableBitmap::createShareable(expandedScaledSize, m_isOpaque ? ShareableBitmap::NoFlags : ShareableBitmap::SupportsAlpha);
std::unique_ptr<GraphicsContext> context = m_frontBuffer->createGraphicsContext();
+ m_bufferContextPendingFlush = context->platformContext();
drawInContext(*context, frontImage.get());
return true;
@@ -288,8 +295,6 @@
m_dirtyRegion = Region();
m_paintingRects.clear();
-
- CGContextFlush(context.platformContext());
}
void RemoteLayerBackingStore::enumerateRectsBeingDrawn(CGContextRef context, void (^block)(CGRect))
@@ -309,15 +314,34 @@
void RemoteLayerBackingStore::applyBackingStoreToLayer(CALayer *layer)
{
+ layer.contentsOpaque = m_isOpaque;
+
#if USE(IOSURFACE)
- if (acceleratesDrawing())
+ if (acceleratesDrawing()) {
layer.contents = (id)m_frontSurface->surface();
- else
- layer.contents = (id)createImageForFrontBuffer().get();
-#else
+ return;
+ }
+#endif
+
ASSERT(!acceleratesDrawing());
layer.contents = (id)createImageForFrontBuffer().get();
+}
+
+void RemoteLayerBackingStore::flush()
+{
+#if USE(IOSURFACE)
+ if (acceleratesDrawing()) {
+ CGContextRef platformContext = m_frontSurface->platformContext();
+ ASSERT(platformContext);
+ ASSERT(m_backSurfacePendingFlush);
+ CGContextFlush(platformContext);
+ m_backSurfacePendingFlush = nullptr;
+ return;
+ }
#endif
- layer.contentsOpaque = m_isOpaque;
+ ASSERT(!acceleratesDrawing());
+ ASSERT(m_bufferContextPendingFlush);
+ CGContextFlush(m_bufferContextPendingFlush.get());
+ m_bufferContextPendingFlush = nullptr;
}
Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm (166295 => 166296)
--- trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm 2014-03-26 15:18:30 UTC (rev 166295)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm 2014-03-26 15:25:28 UTC (rev 166296)
@@ -302,6 +302,17 @@
flushLayers();
}
+static void flushBackingStoreChangesInTransaction(RemoteLayerTreeTransaction& transaction)
+{
+ for (auto& layerProperties : transaction.changedLayers().values()) {
+ if (!layerProperties->changedProperties & RemoteLayerTreeTransaction::BackingStoreChanged)
+ return;
+
+ if (RemoteLayerBackingStore* backingStore = layerProperties->backingStore.get())
+ backingStore->flush();
+ }
+}
+
void RemoteLayerTreeDrawingArea::flushLayers()
{
if (!m_rootLayer)
@@ -317,8 +328,6 @@
m_remoteLayerTreeContext->flushOutOfTreeLayers();
- ASSERT(m_rootLayer);
-
// FIXME: minize these transactions if nothing changed.
RemoteLayerTreeTransaction layerTransaction;
m_remoteLayerTreeContext->buildTransaction(layerTransaction, *m_rootLayer);
@@ -330,6 +339,9 @@
toRemoteScrollingCoordinator(m_webPage->scrollingCoordinator())->buildTransaction(scrollingTransaction);
#endif
+ // FIXME: Move flushing backing store and sending CommitLayerTree onto a background thread.
+ flushBackingStoreChangesInTransaction(layerTransaction);
+
m_webPage->send(Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTree(layerTransaction, scrollingTransaction));
}
