Title: [225115] trunk/Source
Revision
225115
Author
zandober...@gmail.com
Date
2017-11-23 06:19:52 -0800 (Thu, 23 Nov 2017)

Log Message

[CoordGraphics] Simplify CoordinatedGraphicsLayer's content buffer updates
https://bugs.webkit.org/show_bug.cgi?id=179972

Reviewed by Carlos Garcia Campos.

Source/WebCore:

During layer flushes, when a CoordinatedGraphicsLayer's tiled backing store
has dirty tiles that need to be updated, the following chain of calls would
be invoked to perform the painting, starting in
CoordinatedGraphicsLayer::updateContentBuffers():
|- CoordinatedGraphicsLayer::updateContentBuffers()
 |- TiledBackingStore::updateTileBuffers()
  |- Tile::updateBackBuffer()
   |- CoordinatedGraphicsLayer::paintToSurface()
    |- CompositingCoordinator::paintToSurface()
     |- UpdateAtlas::paintOnAvailableBuffer()
      |- ThreadSafeCoordinatedSurface::paintToSurface()
       |- Tile::paintToSurfaceContext()
        |- CoordinatedGraphicsLayer::tiledBackingStorePaint()
         |- GraphicsLayer::paintGraphicsLayerContents()

That's a bit much.

In CoordinatedGraphicsLayer::updateContentBuffers(), we now first retrieve
all the dirty layers in our TiledBackingStore. We then iterate through them,
first establishing their ID and then retrieving an available CoordinatedBuffer
that we can use for painting. The CoordinatedBuffer is retrieved through
CompositingCoordinator, which is still caching these buffers via UpdateAtlas
objects.

With a CoordinatedBuffer available, we can then proceed with painting. The
painting context has to be properly set up to paint into the correct area of
the given buffer, and the alpha channel, if necessary, has to be addressed.
After properly positioning and scaling ourselves on the given context, we
can proceed with the GraphicsLayer::paintGraphicsLayerContents() call.

What's left is filling out the missing SurfaceUpdateInfo information which is
then passed to the updateTile() method that properly registers the tile
update we've just performed. The tile itself is marked clean. After the
iteration, we call the didUpdateTileBuffers() method in case any successful
tile update was indeed performed, incrementing the repaint counter.

That's it -- we clipped 8 calls from the call chain. We can now also remove
the CoordinatedBuffer::Client interface, as well as slim down the
TiledBackingStore interface. The Tile class is cleaned up a bit, with methods
shifted into a more sensible order.

No new tests -- no change in functionality.

* platform/graphics/texmap/coordinated/CoordinatedBuffer.cpp:
* platform/graphics/texmap/coordinated/CoordinatedBuffer.h:
* platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
(WebCore::CoordinatedGraphicsLayer::didUpdateTileBuffers):
(WebCore::CoordinatedGraphicsLayer::updateContentBuffers):
(WebCore::CoordinatedGraphicsLayer::tiledBackingStorePaint): Deleted.
(WebCore::CoordinatedGraphicsLayer::paintToSurface): Deleted.
* platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:
* platform/graphics/texmap/coordinated/Tile.cpp:
(WebCore::Tile::ensureTileID):
(WebCore::Tile::isDirty const):
(WebCore::Tile::invalidate):
(WebCore::Tile::markClean):
(WebCore::Tile::updateBackBuffer): Deleted.
(WebCore::Tile::paintToSurfaceContext): Deleted.
* platform/graphics/texmap/coordinated/Tile.h:
(WebCore::Tile::tileID const):
(WebCore::Tile::dirtyRect const):
* platform/graphics/texmap/coordinated/TiledBackingStore.cpp:
(WebCore::TiledBackingStore::dirtyTiles):
(WebCore::TiledBackingStore::updateTileBuffers): Deleted.
* platform/graphics/texmap/coordinated/TiledBackingStore.h:
* platform/graphics/texmap/coordinated/TiledBackingStoreClient.h:

Source/WebKit:

In CompositingCoordinator, add the getCoordinatedBuffer() method, replacing
paintToSurface(). The new method traverses the list of UpdateAtlases and
returns any CoordinatedBuffer object that was free to use. If none exist,
a new UpdateAtlas object is created, and its CoordinatedBuffer is returned.

In  UpdateAtlas, the paintOnAvailableBuffer() method is replaced with
getCoordinatedBuffer(). The latter allocates the necessary area but then
returns a reference to the CoordinatedBuffer buffer, along with the atlas ID
and the allocated rectangle information, and does not invoke any painting
operation like paintOnAvailableBuffer() did.

* WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp:
(WebKit::CompositingCoordinator::getCoordinatedBuffer):
(WebKit::CompositingCoordinator::paintToSurface): Deleted.
* WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h:
* WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.cpp:
(WebKit::UpdateAtlas::getCoordinatedBuffer):
(WebKit::UpdateAtlas::paintOnAvailableBuffer): Deleted.
* WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (225114 => 225115)


--- trunk/Source/WebCore/ChangeLog	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebCore/ChangeLog	2017-11-23 14:19:52 UTC (rev 225115)
@@ -1,5 +1,79 @@
 2017-11-23  Zan Dobersek  <zdober...@igalia.com>
 
+        [CoordGraphics] Simplify CoordinatedGraphicsLayer's content buffer updates
+        https://bugs.webkit.org/show_bug.cgi?id=179972
+
+        Reviewed by Carlos Garcia Campos.
+
+        During layer flushes, when a CoordinatedGraphicsLayer's tiled backing store
+        has dirty tiles that need to be updated, the following chain of calls would
+        be invoked to perform the painting, starting in
+        CoordinatedGraphicsLayer::updateContentBuffers():
+        |- CoordinatedGraphicsLayer::updateContentBuffers()
+         |- TiledBackingStore::updateTileBuffers()
+          |- Tile::updateBackBuffer()
+           |- CoordinatedGraphicsLayer::paintToSurface()
+            |- CompositingCoordinator::paintToSurface()
+             |- UpdateAtlas::paintOnAvailableBuffer()
+              |- ThreadSafeCoordinatedSurface::paintToSurface()
+               |- Tile::paintToSurfaceContext()
+                |- CoordinatedGraphicsLayer::tiledBackingStorePaint()
+                 |- GraphicsLayer::paintGraphicsLayerContents()
+
+        That's a bit much.
+
+        In CoordinatedGraphicsLayer::updateContentBuffers(), we now first retrieve
+        all the dirty layers in our TiledBackingStore. We then iterate through them,
+        first establishing their ID and then retrieving an available CoordinatedBuffer
+        that we can use for painting. The CoordinatedBuffer is retrieved through
+        CompositingCoordinator, which is still caching these buffers via UpdateAtlas
+        objects.
+
+        With a CoordinatedBuffer available, we can then proceed with painting. The
+        painting context has to be properly set up to paint into the correct area of
+        the given buffer, and the alpha channel, if necessary, has to be addressed.
+        After properly positioning and scaling ourselves on the given context, we
+        can proceed with the GraphicsLayer::paintGraphicsLayerContents() call.
+
+        What's left is filling out the missing SurfaceUpdateInfo information which is
+        then passed to the updateTile() method that properly registers the tile
+        update we've just performed. The tile itself is marked clean. After the
+        iteration, we call the didUpdateTileBuffers() method in case any successful
+        tile update was indeed performed, incrementing the repaint counter.
+
+        That's it -- we clipped 8 calls from the call chain. We can now also remove
+        the CoordinatedBuffer::Client interface, as well as slim down the
+        TiledBackingStore interface. The Tile class is cleaned up a bit, with methods
+        shifted into a more sensible order.
+
+        No new tests -- no change in functionality.
+
+        * platform/graphics/texmap/coordinated/CoordinatedBuffer.cpp:
+        * platform/graphics/texmap/coordinated/CoordinatedBuffer.h:
+        * platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
+        (WebCore::CoordinatedGraphicsLayer::didUpdateTileBuffers):
+        (WebCore::CoordinatedGraphicsLayer::updateContentBuffers):
+        (WebCore::CoordinatedGraphicsLayer::tiledBackingStorePaint): Deleted.
+        (WebCore::CoordinatedGraphicsLayer::paintToSurface): Deleted.
+        * platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:
+        * platform/graphics/texmap/coordinated/Tile.cpp:
+        (WebCore::Tile::ensureTileID):
+        (WebCore::Tile::isDirty const):
+        (WebCore::Tile::invalidate):
+        (WebCore::Tile::markClean):
+        (WebCore::Tile::updateBackBuffer): Deleted.
+        (WebCore::Tile::paintToSurfaceContext): Deleted.
+        * platform/graphics/texmap/coordinated/Tile.h:
+        (WebCore::Tile::tileID const):
+        (WebCore::Tile::dirtyRect const):
+        * platform/graphics/texmap/coordinated/TiledBackingStore.cpp:
+        (WebCore::TiledBackingStore::dirtyTiles):
+        (WebCore::TiledBackingStore::updateTileBuffers): Deleted.
+        * platform/graphics/texmap/coordinated/TiledBackingStore.h:
+        * platform/graphics/texmap/coordinated/TiledBackingStoreClient.h:
+
+2017-11-23  Zan Dobersek  <zdober...@igalia.com>
+
         [CoordGraphics] Remove relay CoordinatedBuffer::Client implementations
         https://bugs.webkit.org/show_bug.cgi?id=179970
 

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBuffer.cpp (225114 => 225115)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBuffer.cpp	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBuffer.cpp	2017-11-23 14:19:52 UTC (rev 225115)
@@ -32,7 +32,6 @@
 #if USE(COORDINATED_GRAPHICS)
 
 #include "ImageBuffer.h"
-#include "IntRect.h"
 
 namespace WebCore {
 

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBuffer.h (225114 => 225115)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBuffer.h	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBuffer.h	2017-11-23 14:19:52 UTC (rev 225115)
@@ -39,7 +39,6 @@
 class GraphicsContext;
 class Image;
 class ImageBuffer;
-class IntRect;
 
 class CoordinatedBuffer : public ThreadSafeRefCounted<CoordinatedBuffer> {
 public:
@@ -49,12 +48,6 @@
     };
     using Flags = unsigned;
 
-    class Client {
-    public:
-        virtual ~Client() = default;
-        virtual void paintToSurfaceContext(GraphicsContext&) = 0;
-    };
-
     static Ref<CoordinatedBuffer> create(const IntSize&, Flags);
     ~CoordinatedBuffer();
 

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp (225114 => 225115)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp	2017-11-23 14:19:52 UTC (rev 225115)
@@ -94,6 +94,15 @@
     notifyFlushRequired();
 }
 
+void CoordinatedGraphicsLayer::didUpdateTileBuffers()
+{
+    if (!isShowingRepaintCounter())
+        return;
+
+    m_layerState.repaintCount = incrementRepaintCount();
+    m_layerState.repaintCountChanged = true;
+}
+
 void CoordinatedGraphicsLayer::setShouldUpdateVisibleRect()
 {
     m_shouldUpdateVisibleRect = true;
@@ -842,22 +851,6 @@
     m_mainBackingStore->setSupportsAlpha(!contentsOpaque());
 }
 
-void CoordinatedGraphicsLayer::tiledBackingStorePaint(GraphicsContext& context, const IntRect& rect)
-{
-    if (rect.isEmpty())
-        return;
-    paintGraphicsLayerContents(context, rect);
-}
-
-void CoordinatedGraphicsLayer::didUpdateTileBuffers()
-{
-    if (!isShowingRepaintCounter())
-        return;
-
-    m_layerState.repaintCount = incrementRepaintCount();
-    m_layerState.repaintCountChanged = true;
-}
-
 void CoordinatedGraphicsLayer::tiledBackingStoreHasPendingTileCreation()
 {
     setNeedsVisibleRectAdjustment();
@@ -892,13 +885,6 @@
     return enclosingIntRect(rect);
 }
 
-bool CoordinatedGraphicsLayer::paintToSurface(const IntSize& size, uint32_t& atlas, IntPoint& offset, CoordinatedBuffer::Client& client)
-{
-    ASSERT(m_coordinator);
-    ASSERT(m_coordinator->isFlushingLayerChanges());
-    return m_coordinator->paintToSurface(size, contentsOpaque() ? CoordinatedBuffer::NoFlags : CoordinatedBuffer::SupportsAlpha, atlas, offset, client);
-}
-
 void CoordinatedGraphicsLayer::createTile(uint32_t tileID, float scaleFactor)
 {
     ASSERT(m_coordinator);
@@ -968,8 +954,61 @@
         m_mainBackingStore->createTilesIfNeeded(transformedVisibleRect(), IntRect(0, 0, size().width(), size().height()));
     }
 
-    m_mainBackingStore->updateTileBuffers();
+    ASSERT(m_coordinator && m_coordinator->isFlushingLayerChanges());
 
+    auto dirtyTiles = m_mainBackingStore->dirtyTiles();
+    if (!dirtyTiles.isEmpty()) {
+        bool didUpdateTiles = false;
+
+        for (auto& tileReference : dirtyTiles) {
+            auto& tile = tileReference.get();
+            tile.ensureTileID();
+
+            auto& tileRect = tile.rect();
+            auto& dirtyRect = tile.dirtyRect();
+
+            SurfaceUpdateInfo updateInfo;
+            IntRect targetRect;
+            auto coordinatedBuffer = m_coordinator->getCoordinatedBuffer(dirtyRect.size(),
+                contentsOpaque() ? CoordinatedBuffer::NoFlags : CoordinatedBuffer::SupportsAlpha,
+                updateInfo.atlasID, targetRect);
+            if (!coordinatedBuffer)
+                continue;
+
+            {
+                GraphicsContext& context = coordinatedBuffer->context();
+                context.save();
+                context.clip(targetRect);
+                context.translate(targetRect.x(), targetRect.y());
+
+                if (coordinatedBuffer->supportsAlpha()) {
+                    context.setCompositeOperation(CompositeCopy);
+                    context.fillRect(IntRect(IntPoint::zero(), dirtyRect.size()), Color::transparent);
+                    context.setCompositeOperation(CompositeSourceOver);
+                }
+
+                context.translate(-dirtyRect.x(), -dirtyRect.y());
+                float backingStoreScale = m_mainBackingStore->contentsScale();
+                context.scale(FloatSize(backingStoreScale, backingStoreScale));
+
+                paintGraphicsLayerContents(context, m_mainBackingStore->mapToContents(dirtyRect));
+
+                context.restore();
+            }
+
+            updateInfo.surfaceOffset = targetRect.location();
+            updateInfo.updateRect = dirtyRect;
+            updateInfo.updateRect.move(-tileRect.x(), -tileRect.y());
+            updateTile(tile.tileID(), updateInfo, tileRect);
+
+            tile.markClean();
+            didUpdateTiles |= true;
+        }
+
+        if (didUpdateTiles)
+            didUpdateTileBuffers();
+    }
+
     // The previous backing store is kept around to avoid flickering between
     // removing the existing tiles and painting the new ones. The first time
     // the visibleRect is full painted we remove the previous backing store.

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h (225114 => 225115)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h	2017-11-23 14:19:52 UTC (rev 225115)
@@ -48,7 +48,7 @@
     virtual FloatRect visibleContentsRect() const = 0;
     virtual Ref<CoordinatedImageBacking> createImageBackingIfNeeded(Image&) = 0;
     virtual void detachLayer(CoordinatedGraphicsLayer*) = 0;
-    virtual bool paintToSurface(const IntSize&, CoordinatedBuffer::Flags, uint32_t& atlasID, IntPoint&, CoordinatedBuffer::Client&) = 0;
+    virtual RefPtr<CoordinatedBuffer> getCoordinatedBuffer(const IntSize&, CoordinatedBuffer::Flags, uint32_t&, IntRect&) = 0;
 
     virtual void syncLayerState(CoordinatedLayerID, CoordinatedGraphicsLayerState&) = 0;
 };
@@ -130,13 +130,10 @@
     IntRect transformedVisibleRect();
 
     // TiledBackingStoreClient
-    void tiledBackingStorePaint(GraphicsContext&, const IntRect&) override;
-    void didUpdateTileBuffers() override;
     void tiledBackingStoreHasPendingTileCreation() override;
     void createTile(uint32_t tileID, float) override;
     void updateTile(uint32_t tileID, const SurfaceUpdateInfo&, const IntRect&) override;
     void removeTile(uint32_t tileID) override;
-    bool paintToSurface(const IntSize&, uint32_t& /* atlasID */, IntPoint&, CoordinatedBuffer::Client&) override;
 
     void setCoordinator(CoordinatedGraphicsLayerClient*);
 
@@ -165,6 +162,7 @@
     void didChangeChildren();
     void didChangeFilters();
     void didChangeImageBacking();
+    void didUpdateTileBuffers();
 
     void resetLayerState();
     void syncLayerState();

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/Tile.cpp (225114 => 225115)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/Tile.cpp	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/Tile.cpp	2017-11-23 14:19:52 UTC (rev 225115)
@@ -27,7 +27,7 @@
 #include "Tile.h"
 
 #if USE(COORDINATED_GRAPHICS)
-#include "GraphicsContext.h"
+
 #include "SurfaceUpdateInfo.h"
 #include "TiledBackingStore.h"
 #include "TiledBackingStoreClient.h"
@@ -38,9 +38,9 @@
 
 Tile::Tile(TiledBackingStore& tiledBackingStore, const Coordinate& tileCoordinate)
     : m_tiledBackingStore(tiledBackingStore)
+    , m_ID(InvalidTileID)
     , m_coordinate(tileCoordinate)
     , m_rect(tiledBackingStore.tileRectForCoordinate(tileCoordinate))
-    , m_ID(InvalidTileID)
     , m_dirtyRect(m_rect)
 {
 }
@@ -51,33 +51,8 @@
         m_tiledBackingStore.client().removeTile(m_ID);
 }
 
-bool Tile::isDirty() const
+void Tile::ensureTileID()
 {
-    return !m_dirtyRect.isEmpty();
-}
-
-void Tile::invalidate(const IntRect& dirtyRect)
-{
-    IntRect tileDirtyRect = intersection(dirtyRect, m_rect);
-    if (tileDirtyRect.isEmpty())
-        return;
-
-    m_dirtyRect.unite(tileDirtyRect);
-}
-
-bool Tile::updateBackBuffer()
-{
-    if (!isDirty())
-        return false;
-
-    SurfaceUpdateInfo updateInfo;
-
-    if (!m_tiledBackingStore.client().paintToSurface(m_dirtyRect.size(), updateInfo.atlasID, updateInfo.surfaceOffset, *this))
-        return false;
-
-    updateInfo.updateRect = m_dirtyRect;
-    updateInfo.updateRect.move(-m_rect.x(), -m_rect.y());
-
     static uint32_t id = 1;
     if (m_ID == InvalidTileID) {
         m_ID = id++;
@@ -86,18 +61,11 @@
             m_ID = id++;
         m_tiledBackingStore.client().createTile(m_ID, m_tiledBackingStore.contentsScale());
     }
-    m_tiledBackingStore.client().updateTile(m_ID, updateInfo, m_rect);
-
-    m_dirtyRect = IntRect();
-
-    return true;
 }
 
-void Tile::paintToSurfaceContext(GraphicsContext& context)
+bool Tile::isDirty() const
 {
-    context.translate(-m_dirtyRect.x(), -m_dirtyRect.y());
-    context.scale(FloatSize(m_tiledBackingStore.contentsScale(), m_tiledBackingStore.contentsScale()));
-    m_tiledBackingStore.client().tiledBackingStorePaint(context, m_tiledBackingStore.mapToContents(m_dirtyRect));
+    return !m_dirtyRect.isEmpty();
 }
 
 bool Tile::isReadyToPaint() const
@@ -105,6 +73,18 @@
     return m_ID != InvalidTileID;
 }
 
+void Tile::invalidate(const IntRect& dirtyRect)
+{
+    IntRect tileDirtyRect = intersection(dirtyRect, m_rect);
+    if (!tileDirtyRect.isEmpty())
+        m_dirtyRect.unite(tileDirtyRect);
+}
+
+void Tile::markClean()
+{
+    m_dirtyRect = { };
+}
+
 void Tile::resize(const IntSize& newSize)
 {
     m_rect = IntRect(m_rect.location(), newSize);

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/Tile.h (225114 => 225115)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/Tile.h	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/Tile.h	2017-11-23 14:19:52 UTC (rev 225115)
@@ -35,10 +35,9 @@
 
 namespace WebCore {
 
-class GraphicsContext;
 class TiledBackingStore;
 
-class Tile : public CoordinatedBuffer::Client {
+class Tile {
 public:
     typedef IntPoint Coordinate;
 
@@ -45,23 +44,24 @@
     Tile(TiledBackingStore&, const Coordinate&);
     ~Tile();
 
+    uint32_t tileID() const { return m_ID; }
+    void ensureTileID();
+
+    const Coordinate& coordinate() const { return m_coordinate; }
+    const IntRect& rect() const { return m_rect; }
+    const IntRect& dirtyRect() const { return m_dirtyRect; }
     bool isDirty() const;
-    void invalidate(const IntRect&);
-    bool updateBackBuffer();
     bool isReadyToPaint() const;
 
-    const Coordinate& coordinate() const { return m_coordinate; }
-    const IntRect& rect() const { return m_rect; }
+    void invalidate(const IntRect&);
+    void markClean();
     void resize(const IntSize&);
 
-    void paintToSurfaceContext(GraphicsContext&) override;
-
 private:
     TiledBackingStore& m_tiledBackingStore;
+    uint32_t m_ID;
     Coordinate m_coordinate;
     IntRect m_rect;
-
-    uint32_t m_ID;
     IntRect m_dirtyRect;
 };
 

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp (225114 => 225115)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp	2017-11-23 14:19:52 UTC (rev 225115)
@@ -88,21 +88,15 @@
     }
 }
 
-void TiledBackingStore::updateTileBuffers()
+Vector<std::reference_wrapper<Tile>> TiledBackingStore::dirtyTiles()
 {
-    // FIXME: In single threaded case, tile back buffers could be updated asynchronously 
-    // one by one and then swapped to front in one go. This would minimize the time spent
-    // blocking on tile updates.
-    bool updated = false;
+    Vector<std::reference_wrapper<Tile>> tiles;
     for (auto& tile : m_tiles.values()) {
-        if (!tile->isDirty())
-            continue;
-
-        updated |= tile->updateBackBuffer();
+        if (tile->isDirty())
+            tiles.append(*tile);
     }
 
-    if (updated)
-        m_client.didUpdateTileBuffers();
+    return tiles;
 }
 
 double TiledBackingStore::tileDistance(const IntRect& viewport, const Tile::Coordinate& tileCoordinate) const

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.h (225114 => 225115)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.h	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.h	2017-11-23 14:19:52 UTC (rev 225115)
@@ -47,7 +47,7 @@
 
     float contentsScale() { return m_contentsScale; }
 
-    void updateTileBuffers();
+    Vector<std::reference_wrapper<Tile>> dirtyTiles();
 
     void invalidate(const IntRect& dirtyRect);
 

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStoreClient.h (225114 => 225115)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStoreClient.h	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStoreClient.h	2017-11-23 14:19:52 UTC (rev 225115)
@@ -20,8 +20,6 @@
 #ifndef TiledBackingStoreClient_h
 #define TiledBackingStoreClient_h
 
-#include "CoordinatedBuffer.h"
-
 namespace WebCore {
 
 #if USE(COORDINATED_GRAPHICS)
@@ -32,14 +30,11 @@
 class TiledBackingStoreClient {
 public:
     virtual ~TiledBackingStoreClient() = default;
-    virtual void tiledBackingStorePaint(GraphicsContext&, const IntRect&) = 0;
-    virtual void didUpdateTileBuffers() = 0;
     virtual void tiledBackingStoreHasPendingTileCreation() = 0;
 
     virtual void createTile(uint32_t tileID, float) = 0;
     virtual void updateTile(uint32_t tileID, const SurfaceUpdateInfo&, const IntRect&) = 0;
     virtual void removeTile(uint32_t tileID) = 0;
-    virtual bool paintToSurface(const IntSize&, uint32_t& atlasID, IntPoint&, CoordinatedBuffer::Client&) = 0;
 };
 
 #endif

Modified: trunk/Source/WebKit/ChangeLog (225114 => 225115)


--- trunk/Source/WebKit/ChangeLog	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebKit/ChangeLog	2017-11-23 14:19:52 UTC (rev 225115)
@@ -1,5 +1,32 @@
 2017-11-23  Zan Dobersek  <zdober...@igalia.com>
 
+        [CoordGraphics] Simplify CoordinatedGraphicsLayer's content buffer updates
+        https://bugs.webkit.org/show_bug.cgi?id=179972
+
+        Reviewed by Carlos Garcia Campos.
+
+        In CompositingCoordinator, add the getCoordinatedBuffer() method, replacing
+        paintToSurface(). The new method traverses the list of UpdateAtlases and
+        returns any CoordinatedBuffer object that was free to use. If none exist,
+        a new UpdateAtlas object is created, and its CoordinatedBuffer is returned.
+
+        In  UpdateAtlas, the paintOnAvailableBuffer() method is replaced with
+        getCoordinatedBuffer(). The latter allocates the necessary area but then
+        returns a reference to the CoordinatedBuffer buffer, along with the atlas ID
+        and the allocated rectangle information, and does not invoke any painting
+        operation like paintOnAvailableBuffer() did.
+
+        * WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp:
+        (WebKit::CompositingCoordinator::getCoordinatedBuffer):
+        (WebKit::CompositingCoordinator::paintToSurface): Deleted.
+        * WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h:
+        * WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.cpp:
+        (WebKit::UpdateAtlas::getCoordinatedBuffer):
+        (WebKit::UpdateAtlas::paintOnAvailableBuffer): Deleted.
+        * WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.h:
+
+2017-11-23  Zan Dobersek  <zdober...@igalia.com>
+
         [CoordGraphics] Remove relay CoordinatedBuffer::Client implementations
         https://bugs.webkit.org/show_bug.cgi?id=179970
 

Modified: trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp (225114 => 225115)


--- trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp	2017-11-23 14:19:52 UTC (rev 225115)
@@ -385,21 +385,19 @@
     m_updateAtlases.clear();
 }
 
-bool CompositingCoordinator::paintToSurface(const IntSize& size, CoordinatedBuffer::Flags flags, uint32_t& atlasID, IntPoint& offset, CoordinatedBuffer::Client& client)
+RefPtr<CoordinatedBuffer> CompositingCoordinator::getCoordinatedBuffer(const IntSize& size, CoordinatedBuffer::Flags flags, uint32_t& atlasID, IntRect& allocatedRect)
 {
-    for (auto& updateAtlas : m_updateAtlases) {
-        UpdateAtlas* atlas = updateAtlas.get();
+    for (auto& atlas : m_updateAtlases) {
         if (atlas->supportsAlpha() == (flags & CoordinatedBuffer::SupportsAlpha)) {
-            // This will be false if there is no available buffer space.
-            if (atlas->paintOnAvailableBuffer(size, atlasID, offset, client))
-                return true;
+            if (auto buffer = atlas->getCoordinatedBuffer(size, atlasID, allocatedRect))
+                return buffer;
         }
     }
 
-    static const int ScratchBufferDimension = 1024; // Should be a power of two.
+    static const int ScratchBufferDimension = 1024;
     m_updateAtlases.append(std::make_unique<UpdateAtlas>(*this, ScratchBufferDimension, flags));
     scheduleReleaseInactiveAtlases();
-    return m_updateAtlases.last()->paintOnAvailableBuffer(size, atlasID, offset, client);
+    return m_updateAtlases.last()->getCoordinatedBuffer(size, atlasID, allocatedRect);
 }
 
 const Seconds releaseInactiveAtlasesTimerInterval { 500_ms };

Modified: trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h (225114 => 225115)


--- trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h	2017-11-23 14:19:52 UTC (rev 225115)
@@ -30,6 +30,7 @@
 #if USE(COORDINATED_GRAPHICS)
 
 #include "UpdateAtlas.h"
+#include <WebCore/CoordinatedBuffer.h>
 #include <WebCore/CoordinatedGraphicsLayer.h>
 #include <WebCore/CoordinatedGraphicsState.h>
 #include <WebCore/CoordinatedImageBacking.h>
@@ -116,7 +117,7 @@
     WebCore::FloatRect visibleContentsRect() const override;
     Ref<WebCore::CoordinatedImageBacking> createImageBackingIfNeeded(WebCore::Image&) override;
     void detachLayer(WebCore::CoordinatedGraphicsLayer*) override;
-    bool paintToSurface(const WebCore::IntSize&, WebCore::CoordinatedBuffer::Flags, uint32_t& /* atlasID */, WebCore::IntPoint&, WebCore::CoordinatedBuffer::Client&) override;
+    RefPtr<WebCore::CoordinatedBuffer> getCoordinatedBuffer(const WebCore::IntSize&, WebCore::CoordinatedBuffer::Flags, uint32_t&, WebCore::IntRect&) override;
     void syncLayerState(WebCore::CoordinatedLayerID, WebCore::CoordinatedGraphicsLayerState&) override;
 
     // UpdateAtlas::Client

Modified: trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.cpp (225114 => 225115)


--- trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.cpp	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.cpp	2017-11-23 14:19:52 UTC (rev 225115)
@@ -60,38 +60,17 @@
     m_areaAllocator = nullptr;
 }
 
-bool UpdateAtlas::paintOnAvailableBuffer(const IntSize& size, uint32_t& atlasID, IntPoint& offset, CoordinatedBuffer::Client& client)
+RefPtr<CoordinatedBuffer> UpdateAtlas::getCoordinatedBuffer(const IntSize& size, uint32_t& atlasID, IntRect& allocatedRect)
 {
     m_inactivityInSeconds = 0;
     buildLayoutIfNeeded();
-    IntRect rect = m_areaAllocator->allocate(size);
+    allocatedRect = m_areaAllocator->allocate(size);
 
-    // No available buffer was found.
-    if (rect.isEmpty())
-        return false;
+    if (allocatedRect.isEmpty())
+        return nullptr;
 
     atlasID = m_ID;
-
-    // FIXME: Use tri-state buffers, to allow faster updates.
-    offset = rect.location();
-
-    {
-        GraphicsContext& context = m_buffer->context();
-        context.save();
-        context.clip(rect);
-        context.translate(rect.x(), rect.y());
-
-        if (supportsAlpha()) {
-            context.setCompositeOperation(CompositeCopy);
-            context.fillRect(IntRect(IntPoint::zero(), size), Color::transparent);
-            context.setCompositeOperation(CompositeSourceOver);
-        }
-
-        client.paintToSurfaceContext(context);
-        context.restore();
-    }
-
-    return true;
+    return m_buffer.copyRef();
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.h (225114 => 225115)


--- trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.h	2017-11-23 13:09:26 UTC (rev 225114)
+++ trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.h	2017-11-23 14:19:52 UTC (rev 225115)
@@ -23,14 +23,13 @@
 
 #include "AreaAllocator.h"
 #include <WebCore/CoordinatedBuffer.h>
-#include <WebCore/IntSize.h>
 #include <wtf/RefPtr.h>
 
 #if USE(COORDINATED_GRAPHICS)
 
 namespace WebCore {
-class GraphicsContext;
-class IntPoint;
+class IntRect;
+class IntSize;
 }
 
 namespace WebKit {
@@ -49,8 +48,7 @@
 
     const WebCore::IntSize& size() const { return m_buffer->size(); }
 
-    // Returns false if there is no available buffer.
-    bool paintOnAvailableBuffer(const WebCore::IntSize&, uint32_t& atlasID, WebCore::IntPoint& offset, WebCore::CoordinatedBuffer::Client&);
+    RefPtr<WebCore::CoordinatedBuffer> getCoordinatedBuffer(const WebCore::IntSize&, uint32_t&, WebCore::IntRect&);
     void didSwapBuffers();
     bool supportsAlpha() const { return m_buffer->supportsAlpha(); }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to