<!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>[166541] trunk/Source/WebKit2</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/166541">166541</a></dd>
<dt>Author</dt> <dd>timothy_horton@apple.com</dd>
<dt>Date</dt> <dd>2014-03-31 16:00:30 -0700 (Mon, 31 Mar 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>Double-buffer RemoteLayerBackingStore
https://bugs.webkit.org/show_bug.cgi?id=130990

Reviewed by Simon Fraser.

We'll keep a front and back buffer for each surface; the front is generally currently
being displayed in the UI process, and the back is the one we'll paint into.

Swap the two surfaces each time we paint; since we synchronize with the UI process,
the old front surface will generally be out-of-use by the render server by the time
we paint again. However, since render server commits are asynchronous and we have 
no way to syncronize with them yet, we have to check if the (swapped to front) back buffer is in use,
and create a new front buffer if it is.

Triple-buffering would solve this problem, as would synchronization with the render server,
as would a pool of surfaces - we will revisit these solutions in future patches.

* Shared/mac/RemoteLayerBackingStore.h:
* Shared/mac/RemoteLayerBackingStore.mm:
(RemoteLayerBackingStore::ensureBackingStore):
(RemoteLayerBackingStore::clearBackingStore):
Factor clearBackingStore() out of ensureBackingStore() and display().

(RemoteLayerBackingStore::display):
Swap buffers. Since m_backSurface will hold on to the back surface's CGContext,
we don't need to worry about tearing down the image first anymore.
Don't worry about creating a back image (nor copying it into the front image)
if we're going to paint the whole layer.

(RemoteLayerBackingStore::drawInContext):
Fix some names.

(RemoteLayerBackingStore::applyBackingStoreToLayer):
Reduce duplication.

(RemoteLayerBackingStore::createImageForFrontBuffer): Deleted.</pre>

<h3>Modified Paths</h3>
<ul>
<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>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (166540 => 166541)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog        2014-03-31 22:52:31 UTC (rev 166540)
+++ trunk/Source/WebKit2/ChangeLog        2014-03-31 23:00:30 UTC (rev 166541)
</span><span class="lines">@@ -1,5 +1,44 @@
</span><span class="cx"> 2014-03-31  Tim Horton  &lt;timothy_horton@apple.com&gt;
</span><span class="cx"> 
</span><ins>+        Double-buffer RemoteLayerBackingStore
+        https://bugs.webkit.org/show_bug.cgi?id=130990
+
+        Reviewed by Simon Fraser.
+
+        We'll keep a front and back buffer for each surface; the front is generally currently
+        being displayed in the UI process, and the back is the one we'll paint into.
+
+        Swap the two surfaces each time we paint; since we synchronize with the UI process,
+        the old front surface will generally be out-of-use by the render server by the time
+        we paint again. However, since render server commits are asynchronous and we have 
+        no way to syncronize with them yet, we have to check if the (swapped to front) back buffer is in use,
+        and create a new front buffer if it is.
+
+        Triple-buffering would solve this problem, as would synchronization with the render server,
+        as would a pool of surfaces - we will revisit these solutions in future patches.
+
+        * Shared/mac/RemoteLayerBackingStore.h:
+        * Shared/mac/RemoteLayerBackingStore.mm:
+        (RemoteLayerBackingStore::ensureBackingStore):
+        (RemoteLayerBackingStore::clearBackingStore):
+        Factor clearBackingStore() out of ensureBackingStore() and display().
+
+        (RemoteLayerBackingStore::display):
+        Swap buffers. Since m_backSurface will hold on to the back surface's CGContext,
+        we don't need to worry about tearing down the image first anymore.
+        Don't worry about creating a back image (nor copying it into the front image)
+        if we're going to paint the whole layer.
+
+        (RemoteLayerBackingStore::drawInContext):
+        Fix some names.
+
+        (RemoteLayerBackingStore::applyBackingStoreToLayer):
+        Reduce duplication.
+
+        (RemoteLayerBackingStore::createImageForFrontBuffer): Deleted.
+
+2014-03-31  Tim Horton  &lt;timothy_horton@apple.com&gt;
+
</ins><span class="cx">         Synchronize Web process remote layer tree commits with CoreAnimation commits in the UI process
</span><span class="cx">         https://bugs.webkit.org/show_bug.cgi?id=130984
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedmacRemoteLayerBackingStoreh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h (166540 => 166541)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h        2014-03-31 22:52:31 UTC (rev 166540)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h        2014-03-31 23:00:30 UTC (rev 166541)
</span><span class="lines">@@ -75,11 +75,11 @@
</span><span class="cx"> #endif
</span><span class="cx">         return !!m_frontBuffer;
</span><span class="cx">     }
</span><ins>+
</ins><span class="cx"> private:
</span><del>-    void drawInContext(WebCore::GraphicsContext&amp;, CGImageRef frontImage);
</del><ins>+    void drawInContext(WebCore::GraphicsContext&amp;, CGImageRef backImage);
+    void clearBackingStore();
</ins><span class="cx"> 
</span><del>-    RetainPtr&lt;CGImageRef&gt; createImageForFrontBuffer() const;
-
</del><span class="cx">     PlatformCALayerRemote* m_layer;
</span><span class="cx"> 
</span><span class="cx">     WebCore::IntSize m_size;
</span><span class="lines">@@ -89,8 +89,10 @@
</span><span class="cx">     WebCore::Region m_dirtyRegion;
</span><span class="cx"> 
</span><span class="cx">     RefPtr&lt;ShareableBitmap&gt; m_frontBuffer;
</span><ins>+    RefPtr&lt;ShareableBitmap&gt; m_backBuffer;
</ins><span class="cx"> #if USE(IOSURFACE)
</span><span class="cx">     RefPtr&lt;WebCore::IOSurface&gt; m_frontSurface;
</span><ins>+    RefPtr&lt;WebCore::IOSurface&gt; m_backSurface;
</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 (166540 => 166541)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm        2014-03-31 22:52:31 UTC (rev 166540)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm        2014-03-31 23:00:30 UTC (rev 166541)
</span><span class="lines">@@ -68,10 +68,17 @@
</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_backSurface = nullptr;
</ins><span class="cx"> #endif
</span><span class="cx">     m_frontBuffer = nullptr;
</span><ins>+    m_backBuffer = nullptr;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void RemoteLayerBackingStore::encode(IPC::ArgumentEncoder&amp; encoder) const
</span><span class="lines">@@ -141,15 +148,6 @@
</span><span class="cx">     setNeedsDisplay(IntRect(IntPoint(), m_size));
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-RetainPtr&lt;CGImageRef&gt; RemoteLayerBackingStore::createImageForFrontBuffer() const
-{
-    if (!m_frontBuffer || m_acceleratesDrawing)
-        return nullptr;
-
-    // FIXME: Do we need Copy?
-    return m_frontBuffer-&gt;makeCGImageCopy();
-}
-
</del><span class="cx"> bool RemoteLayerBackingStore::display()
</span><span class="cx"> {
</span><span class="cx">     if (!m_layer)
</span><span class="lines">@@ -159,20 +157,16 @@
</span><span class="cx">     // to note that our backing store has been cleared.
</span><span class="cx">     if (!m_layer-&gt;owner() || !m_layer-&gt;owner()-&gt;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="cx">     if (m_dirtyRegion.isEmpty() || m_size.isEmpty())
</span><span class="cx">         return false;
</span><span class="cx"> 
</span><ins>+    IntRect layerBounds(IntPoint(), m_size);
</ins><span class="cx">     if (!hasFrontBuffer())
</span><del>-        m_dirtyRegion.unite(IntRect(IntPoint(), m_size));
</del><ins>+        m_dirtyRegion.unite(layerBounds);
</ins><span class="cx"> 
</span><span class="cx">     if (m_layer-&gt;owner()-&gt;platformCALayerShowRepaintCounter(m_layer)) {
</span><span class="cx">         IntRect indicatorRect(0, 0, 52, 27);
</span><span class="lines">@@ -183,40 +177,49 @@
</span><span class="cx">     scaledSize.scale(m_scale);
</span><span class="cx">     IntSize expandedScaledSize = expandedIntSize(scaledSize);
</span><span class="cx"> 
</span><ins>+    bool willPaintEntireBackingStore = m_dirtyRegion.bounds().contains(layerBounds);
</ins><span class="cx"> #if USE(IOSURFACE)
</span><span class="cx">     if (m_acceleratesDrawing) {
</span><del>-        RefPtr&lt;IOSurface&gt; backSurface = IOSurface::create(expandedScaledSize, ColorSpaceDeviceRGB);
-        GraphicsContext&amp; context = backSurface-&gt;ensureGraphicsContext();
</del><ins>+        std::swap(m_frontSurface, m_backSurface);
+
+        if (!m_frontSurface || m_frontSurface-&gt;isInUse()) {
+            // FIXME: Instead of discarding it, put the unusable in-use surface into a pool for future use.
+            m_frontSurface = IOSurface::create(expandedScaledSize, ColorSpaceDeviceRGB);
+        }
+
+        RetainPtr&lt;CGImageRef&gt; backImage;
+        if (m_backSurface &amp;&amp; !willPaintEntireBackingStore)
+            backImage = m_backSurface-&gt;createImage();
+
+        GraphicsContext&amp; context = m_frontSurface-&gt;ensureGraphicsContext();
</ins><span class="cx">         context.clearRect(FloatRect(FloatPoint(), expandedScaledSize));
</span><span class="cx">         context.scale(FloatSize(1, -1));
</span><span class="cx">         context.translate(0, -expandedScaledSize.height());
</span><ins>+        drawInContext(context, backImage.get());
</ins><span class="cx"> 
</span><del>-        RetainPtr&lt;CGImageRef&gt; frontImage;
-        if (m_frontSurface)
-            frontImage = m_frontSurface-&gt;createImage();
-        drawInContext(context, frontImage.get());
</del><ins>+        m_frontSurface-&gt;clearGraphicsContext();
</ins><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;
-        m_frontSurface = backSurface;
-
</del><span class="cx">         return true;
</span><span class="cx">     }
</span><span class="cx"> #else
</span><span class="cx">     ASSERT(!m_acceleratesDrawing);
</span><span class="cx"> #endif
</span><span class="cx"> 
</span><del>-    RetainPtr&lt;CGImageRef&gt; frontImage = createImageForFrontBuffer();
-    m_frontBuffer = ShareableBitmap::createShareable(expandedScaledSize, m_isOpaque ? ShareableBitmap::NoFlags : ShareableBitmap::SupportsAlpha);
</del><ins>+    std::swap(m_frontBuffer, m_backBuffer);
+    if (!m_frontBuffer)
+        m_frontBuffer = ShareableBitmap::createShareable(expandedScaledSize, m_isOpaque ? ShareableBitmap::NoFlags : ShareableBitmap::SupportsAlpha);
</ins><span class="cx">     std::unique_ptr&lt;GraphicsContext&gt; context = m_frontBuffer-&gt;createGraphicsContext();
</span><del>-    drawInContext(*context, frontImage.get());
</del><ins>+
+    RetainPtr&lt;CGImageRef&gt; backImage;
+    if (m_backBuffer &amp;&amp; !willPaintEntireBackingStore)
+        backImage = m_backBuffer-&gt;makeCGImage();
+
+    drawInContext(*context, backImage.get());
</ins><span class="cx">     
</span><span class="cx">     return true;
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void RemoteLayerBackingStore::drawInContext(GraphicsContext&amp; context, CGImageRef frontImage)
</del><ins>+void RemoteLayerBackingStore::drawInContext(GraphicsContext&amp; context, CGImageRef backImage)
</ins><span class="cx"> {
</span><span class="cx">     IntRect layerBounds(IntPoint(), m_size);
</span><span class="cx">     IntRect scaledLayerBounds(IntPoint(), expandedIntSize(m_size * m_scale));
</span><span class="lines">@@ -227,6 +230,7 @@
</span><span class="cx">     // than webLayerWastedSpaceThreshold of the total dirty area, we'll repaint each rect separately.
</span><span class="cx">     // Otherwise, repaint the entire bounding box of the dirty region.
</span><span class="cx">     IntRect dirtyBounds = m_dirtyRegion.bounds();
</span><ins>+
</ins><span class="cx">     Vector&lt;IntRect&gt; dirtyRects = m_dirtyRegion.rects();
</span><span class="cx">     if (dirtyRects.size() &gt; webLayerMaxRectsToPaint || m_dirtyRegion.totalArea() &gt; webLayerWastedSpaceThreshold * dirtyBounds.width() * dirtyBounds.height()) {
</span><span class="cx">         dirtyRects.clear();
</span><span class="lines">@@ -248,7 +252,7 @@
</span><span class="cx">         cgPaintingRects[i] = enclosingIntRect(scaledPaintingRect);
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    if (frontImage) {
</del><ins>+    if (backImage) {
</ins><span class="cx">         CGContextSaveGState(cgContext);
</span><span class="cx">         CGContextSetBlendMode(cgContext, kCGBlendModeCopy);
</span><span class="cx"> 
</span><span class="lines">@@ -258,7 +262,7 @@
</span><span class="cx"> 
</span><span class="cx">         CGContextTranslateCTM(cgContext, 0, scaledLayerBounds.height());
</span><span class="cx">         CGContextScaleCTM(cgContext, 1, -1);
</span><del>-        CGContextDrawImage(cgContext, scaledLayerBounds, frontImage);
</del><ins>+        CGContextDrawImage(cgContext, scaledLayerBounds, backImage);
</ins><span class="cx">         CGContextRestoreGState(cgContext);
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="lines">@@ -309,15 +313,15 @@
</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-&gt;surface();
</span><del>-    else
-        layer.contents = (id)createImageForFrontBuffer().get();
-#else
-    ASSERT(!acceleratesDrawing());
-    layer.contents = (id)createImageForFrontBuffer().get();
</del><ins>+        return;
+    }
</ins><span class="cx"> #endif
</span><span class="cx"> 
</span><del>-    layer.contentsOpaque = m_isOpaque;
</del><ins>+    ASSERT(!acceleratesDrawing());
+    layer.contents = (id)m_frontBuffer-&gt;makeCGImageCopy().get();
</ins><span class="cx"> }
</span></span></pre>
</div>
</div>

</body>
</html>