<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[166296] trunk/Source</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/166296">166296</a></dd>
<dt>Author</dt> <dd>timothy_horton@apple.com</dd>
<dt>Date</dt> <dd>2014-03-26 08:25:28 -0700 (Wed, 26 Mar 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>[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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicscocoaIOSurfaceh">trunk/Source/WebCore/platform/graphics/cocoa/IOSurface.h</a></li>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2SharedmacRemoteLayerBackingStoreh">trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h</a></li>
<li><a href="#trunkSourceWebKit2SharedmacRemoteLayerBackingStoremm">trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebPagemacRemoteLayerTreeDrawingAreamm">trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (166295 => 166296)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,14 @@
</span><ins>+2014-03-26 Tim Horton <timothy_horton@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.
+
</ins><span class="cx"> 2014-03-26 James Craig <jcraig@apple.com>
</span><span class="cx">
</span><span class="cx"> Web Inspector: AXI: crash when inspecting "bar" text node in getAccessibilityPropertiesForNode layout test
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicscocoaIOSurfaceh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/cocoa/IOSurface.h (166295 => 166296)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -52,6 +52,7 @@
</span><span class="cx"> IOSurfaceRef surface() const { return m_surface.get(); }
</span><span class="cx"> GraphicsContext& ensureGraphicsContext();
</span><span class="cx"> CGContextRef ensurePlatformContext();
</span><ins>+ CGContextRef platformContext() { return m_cgContext.get(); }
</ins><span class="cx">
</span><span class="cx"> enum class SurfaceState {
</span><span class="cx"> Valid,
</span></span></pre></div>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (166295 => 166296)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,47 @@
</span><ins>+2014-03-26 Tim Horton <timothy_horton@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.
+
</ins><span class="cx"> 2014-03-25 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Add a new type of scrolling tree node for overflow scrolling
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedmacRemoteLayerBackingStoreh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h (166295 => 166296)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -75,10 +75,14 @@
</span><span class="cx"> #endif
</span><span class="cx"> return !!m_frontBuffer;
</span><span class="cx"> }
</span><ins>+
+ void flush();
+
</ins><span class="cx"> private:
</span><span class="cx"> void drawInContext(WebCore::GraphicsContext&, CGImageRef frontImage);
</span><span class="cx">
</span><span class="cx"> RetainPtr<CGImageRef> createImageForFrontBuffer() const;
</span><ins>+ void clearBackingStore();
</ins><span class="cx">
</span><span class="cx"> PlatformCALayerRemote* m_layer;
</span><span class="cx">
</span><span class="lines">@@ -89,8 +93,10 @@
</span><span class="cx"> WebCore::Region m_dirtyRegion;
</span><span class="cx">
</span><span class="cx"> RefPtr<ShareableBitmap> m_frontBuffer;
</span><ins>+ RetainPtr<CGContextRef> m_bufferContextPendingFlush;
</ins><span class="cx"> #if USE(IOSURFACE)
</span><span class="cx"> RefPtr<WebCore::IOSurface> m_frontSurface;
</span><ins>+ RefPtr<WebCore::IOSurface> m_backSurfacePendingFlush;
</ins><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> bool m_acceleratesDrawing;
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedmacRemoteLayerBackingStoremm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm (166295 => 166296)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -68,9 +68,16 @@
</span><span class="cx"> m_acceleratesDrawing = acceleratesDrawing;
</span><span class="cx"> m_isOpaque = isOpaque;
</span><span class="cx">
</span><ins>+ clearBackingStore();
+}
+
+void RemoteLayerBackingStore::clearBackingStore()
+{
</ins><span class="cx"> #if USE(IOSURFACE)
</span><span class="cx"> m_frontSurface = nullptr;
</span><ins>+ m_backSurfacePendingFlush = nullptr;
</ins><span class="cx"> #endif
</span><ins>+ m_bufferContextPendingFlush = nullptr;
</ins><span class="cx"> m_frontBuffer = nullptr;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -152,6 +159,8 @@
</span><span class="cx">
</span><span class="cx"> bool RemoteLayerBackingStore::display()
</span><span class="cx"> {
</span><ins>+ ASSERT(!m_bufferContextPendingFlush);
+
</ins><span class="cx"> if (!m_layer)
</span><span class="cx"> return false;
</span><span class="cx">
</span><span class="lines">@@ -159,12 +168,7 @@
</span><span class="cx"> // to note that our backing store has been cleared.
</span><span class="cx"> if (!m_layer->owner() || !m_layer->owner()->platformCALayerDrawsContent()) {
</span><span class="cx"> bool previouslyDrewContents = hasFrontBuffer();
</span><del>-
- m_frontBuffer = nullptr;
-#if USE(IOSURFACE)
- m_frontSurface = nullptr;
-#endif
-
</del><ins>+ clearBackingStore();
</ins><span class="cx"> return previouslyDrewContents;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -184,6 +188,7 @@
</span><span class="cx"> IntSize expandedScaledSize = expandedIntSize(scaledSize);
</span><span class="cx">
</span><span class="cx"> #if USE(IOSURFACE)
</span><ins>+ ASSERT(!m_backSurfacePendingFlush);
</ins><span class="cx"> if (m_acceleratesDrawing) {
</span><span class="cx"> RefPtr<IOSurface> backSurface = IOSurface::create(expandedScaledSize, ColorSpaceDeviceRGB);
</span><span class="cx"> GraphicsContext& context = backSurface->ensureGraphicsContext();
</span><span class="lines">@@ -196,10 +201,11 @@
</span><span class="cx"> frontImage = m_frontSurface->createImage();
</span><span class="cx"> drawInContext(context, frontImage.get());
</span><span class="cx">
</span><del>- // 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;
</del><ins>+ // 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;
</ins><span class="cx"> m_frontSurface = backSurface;
</span><span class="cx">
</span><span class="cx"> return true;
</span><span class="lines">@@ -211,6 +217,7 @@
</span><span class="cx"> RetainPtr<CGImageRef> frontImage = createImageForFrontBuffer();
</span><span class="cx"> m_frontBuffer = ShareableBitmap::createShareable(expandedScaledSize, m_isOpaque ? ShareableBitmap::NoFlags : ShareableBitmap::SupportsAlpha);
</span><span class="cx"> std::unique_ptr<GraphicsContext> context = m_frontBuffer->createGraphicsContext();
</span><ins>+ m_bufferContextPendingFlush = context->platformContext();
</ins><span class="cx"> drawInContext(*context, frontImage.get());
</span><span class="cx">
</span><span class="cx"> return true;
</span><span class="lines">@@ -288,8 +295,6 @@
</span><span class="cx">
</span><span class="cx"> m_dirtyRegion = Region();
</span><span class="cx"> m_paintingRects.clear();
</span><del>-
- CGContextFlush(context.platformContext());
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void RemoteLayerBackingStore::enumerateRectsBeingDrawn(CGContextRef context, void (^block)(CGRect))
</span><span class="lines">@@ -309,15 +314,34 @@
</span><span class="cx">
</span><span class="cx"> void RemoteLayerBackingStore::applyBackingStoreToLayer(CALayer *layer)
</span><span class="cx"> {
</span><ins>+ layer.contentsOpaque = m_isOpaque;
+
</ins><span class="cx"> #if USE(IOSURFACE)
</span><del>- if (acceleratesDrawing())
</del><ins>+ if (acceleratesDrawing()) {
</ins><span class="cx"> layer.contents = (id)m_frontSurface->surface();
</span><del>- else
- layer.contents = (id)createImageForFrontBuffer().get();
-#else
</del><ins>+ return;
+ }
+#endif
+
</ins><span class="cx"> ASSERT(!acceleratesDrawing());
</span><span class="cx"> layer.contents = (id)createImageForFrontBuffer().get();
</span><ins>+}
+
+void RemoteLayerBackingStore::flush()
+{
+#if USE(IOSURFACE)
+ if (acceleratesDrawing()) {
+ CGContextRef platformContext = m_frontSurface->platformContext();
+ ASSERT(platformContext);
+ ASSERT(m_backSurfacePendingFlush);
+ CGContextFlush(platformContext);
+ m_backSurfacePendingFlush = nullptr;
+ return;
+ }
</ins><span class="cx"> #endif
</span><span class="cx">
</span><del>- layer.contentsOpaque = m_isOpaque;
</del><ins>+ ASSERT(!acceleratesDrawing());
+ ASSERT(m_bufferContextPendingFlush);
+ CGContextFlush(m_bufferContextPendingFlush.get());
+ m_bufferContextPendingFlush = nullptr;
</ins><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPagemacRemoteLayerTreeDrawingAreamm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm (166295 => 166296)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -302,6 +302,17 @@
</span><span class="cx"> flushLayers();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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();
+ }
+}
+
</ins><span class="cx"> void RemoteLayerTreeDrawingArea::flushLayers()
</span><span class="cx"> {
</span><span class="cx"> if (!m_rootLayer)
</span><span class="lines">@@ -317,8 +328,6 @@
</span><span class="cx">
</span><span class="cx"> m_remoteLayerTreeContext->flushOutOfTreeLayers();
</span><span class="cx">
</span><del>- ASSERT(m_rootLayer);
-
</del><span class="cx"> // FIXME: minize these transactions if nothing changed.
</span><span class="cx"> RemoteLayerTreeTransaction layerTransaction;
</span><span class="cx"> m_remoteLayerTreeContext->buildTransaction(layerTransaction, *m_rootLayer);
</span><span class="lines">@@ -330,6 +339,9 @@
</span><span class="cx"> toRemoteScrollingCoordinator(m_webPage->scrollingCoordinator())->buildTransaction(scrollingTransaction);
</span><span class="cx"> #endif
</span><span class="cx">
</span><ins>+ // FIXME: Move flushing backing store and sending CommitLayerTree onto a background thread.
+ flushBackingStoreChangesInTransaction(layerTransaction);
+
</ins><span class="cx"> m_webPage->send(Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTree(layerTransaction, scrollingTransaction));
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>