[webkit-changes] [229315] trunk/Source/WebKit

Tue, 06 Mar 2018 03:35:23 -0800

Title: [229315] trunk/Source/WebKit
Revision
229315
Author
zandober...@gmail.com
Date
2018-03-06 03:34:19 -0800 (Tue, 06 Mar 2018)

Log Message

CoordinatedGraphicsScene: properly limit data specific to state commit operation
https://bugs.webkit.org/show_bug.cgi?id=183326

Reviewed by Sergio Villar Senin.

In the process of state commit in CoordinatedGraphicsScene, the released
image backings and backing stores with pending updates are stored in
Vector and HashSet objects, respectively. Instead of these two objects
being member variables on the CoordinatedGraphicsScene class, keep them
in the CommitScope structure that's limited to the operations done in
the commitSceneState() method.

The two member variables are dropped, and the CommitScope object is
passed by reference to any helper method that needs to append either
kind of object to the respective container. At the end of the state
commit, backing stores with pending updates have those updates applied,
and the two containers are cleared out as the CommitScope object is
destroyed.

* Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:
(WebKit::CoordinatedGraphicsScene::setLayerState):
(WebKit::CoordinatedGraphicsScene::prepareContentBackingStore):
(WebKit::CoordinatedGraphicsScene::resetBackingStoreSizeToLayerSize):
(WebKit::CoordinatedGraphicsScene::removeTilesIfNeeded):
(WebKit::CoordinatedGraphicsScene::updateTilesIfNeeded):
(WebKit::CoordinatedGraphicsScene::syncImageBackings):
(WebKit::CoordinatedGraphicsScene::updateImageBacking):
(WebKit::CoordinatedGraphicsScene::clearImageBackingContents):
(WebKit::CoordinatedGraphicsScene::removeImageBacking):
(WebKit::CoordinatedGraphicsScene::commitSceneState):
(WebKit::CoordinatedGraphicsScene::purgeGLResources):
(WebKit::CoordinatedGraphicsScene::removeReleasedImageBackingsIfNeeded): Deleted.
(WebKit::CoordinatedGraphicsScene::commitPendingBackingStoreOperations): Deleted.
* Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (229314 => 229315)


--- trunk/Source/WebKit/ChangeLog	2018-03-06 11:31:50 UTC (rev 229314)
+++ trunk/Source/WebKit/ChangeLog	2018-03-06 11:34:19 UTC (rev 229315)
@@ -1,3 +1,40 @@
+2018-03-06  Zan Dobersek  <zdober...@igalia.com>
+
+        CoordinatedGraphicsScene: properly limit data specific to state commit operation
+        https://bugs.webkit.org/show_bug.cgi?id=183326
+
+        Reviewed by Sergio Villar Senin.
+
+        In the process of state commit in CoordinatedGraphicsScene, the released
+        image backings and backing stores with pending updates are stored in
+        Vector and HashSet objects, respectively. Instead of these two objects
+        being member variables on the CoordinatedGraphicsScene class, keep them
+        in the CommitScope structure that's limited to the operations done in
+        the commitSceneState() method.
+
+        The two member variables are dropped, and the CommitScope object is
+        passed by reference to any helper method that needs to append either
+        kind of object to the respective container. At the end of the state
+        commit, backing stores with pending updates have those updates applied,
+        and the two containers are cleared out as the CommitScope object is
+        destroyed.
+
+        * Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:
+        (WebKit::CoordinatedGraphicsScene::setLayerState):
+        (WebKit::CoordinatedGraphicsScene::prepareContentBackingStore):
+        (WebKit::CoordinatedGraphicsScene::resetBackingStoreSizeToLayerSize):
+        (WebKit::CoordinatedGraphicsScene::removeTilesIfNeeded):
+        (WebKit::CoordinatedGraphicsScene::updateTilesIfNeeded):
+        (WebKit::CoordinatedGraphicsScene::syncImageBackings):
+        (WebKit::CoordinatedGraphicsScene::updateImageBacking):
+        (WebKit::CoordinatedGraphicsScene::clearImageBackingContents):
+        (WebKit::CoordinatedGraphicsScene::removeImageBacking):
+        (WebKit::CoordinatedGraphicsScene::commitSceneState):
+        (WebKit::CoordinatedGraphicsScene::purgeGLResources):
+        (WebKit::CoordinatedGraphicsScene::removeReleasedImageBackingsIfNeeded): Deleted.
+        (WebKit::CoordinatedGraphicsScene::commitPendingBackingStoreOperations): Deleted.
+        * Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h:
+
 2018-03-05  Yusuke Suzuki  <utatane....@gmail.com>
 
         Fix std::make_unique / new[] using system malloc

Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp (229314 => 229315)


--- trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp	2018-03-06 11:31:50 UTC (rev 229314)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp	2018-03-06 11:34:19 UTC (rev 229315)
@@ -210,7 +210,7 @@
     layer->setFilters(state.filters);
 }
 
-void CoordinatedGraphicsScene::setLayerState(CoordinatedLayerID id, const CoordinatedGraphicsLayerState& layerState)
+void CoordinatedGraphicsScene::setLayerState(CoordinatedLayerID id, const CoordinatedGraphicsLayerState& layerState, CommitScope& commitScope)
 {
     ASSERT(m_rootLayerID != InvalidCoordinatedLayerID);
     TextureMapperLayer* layer = layerByID(id);
@@ -281,13 +281,13 @@
     if (layerState.committedScrollOffsetChanged)
         layer->didCommitScrollOffset(layerState.committedScrollOffset);
 
-    prepareContentBackingStore(layer);
+    prepareContentBackingStore(layer, commitScope);
 
     // Apply Operations.
     setLayerChildrenIfNeeded(layer, layerState);
     createTilesIfNeeded(layer, layerState);
-    removeTilesIfNeeded(layer, layerState);
-    updateTilesIfNeeded(layer, layerState);
+    removeTilesIfNeeded(layer, layerState, commitScope);
+    updateTilesIfNeeded(layer, layerState, commitScope);
     setLayerFiltersIfNeeded(layer, layerState);
     setLayerAnimationsIfNeeded(layer, layerState);
     syncPlatformLayerIfNeeded(layer, layerState);
@@ -344,7 +344,7 @@
     m_rootLayer->addChild(layer);
 }
 
-void CoordinatedGraphicsScene::prepareContentBackingStore(TextureMapperLayer* layer)
+void CoordinatedGraphicsScene::prepareContentBackingStore(TextureMapperLayer* layer, CommitScope& commitScope)
 {
     if (!layerShouldHaveBackingStore(layer)) {
         removeBackingStoreIfNeeded(layer);
@@ -352,7 +352,7 @@
     }
 
     createBackingStoreIfNeeded(layer);
-    resetBackingStoreSizeToLayerSize(layer);
+    resetBackingStoreSizeToLayerSize(layer, commitScope);
 }
 
 void CoordinatedGraphicsScene::createBackingStoreIfNeeded(TextureMapperLayer* layer)
@@ -374,12 +374,12 @@
     layer->setBackingStore(nullptr);
 }
 
-void CoordinatedGraphicsScene::resetBackingStoreSizeToLayerSize(TextureMapperLayer* layer)
+void CoordinatedGraphicsScene::resetBackingStoreSizeToLayerSize(TextureMapperLayer* layer, CommitScope& commitScope)
 {
     RefPtr<CoordinatedBackingStore> backingStore = m_backingStores.get(layer);
     ASSERT(backingStore);
     backingStore->setSize(layer->size());
-    m_backingStoresWithPendingBuffers.add(backingStore);
+    commitScope.backingStoresWithPendingBuffers.add(backingStore);
 }
 
 void CoordinatedGraphicsScene::createTilesIfNeeded(TextureMapperLayer* layer, const CoordinatedGraphicsLayerState& state)
@@ -396,7 +396,7 @@
         backingStore->createTile(tile.tileID, tile.scale);
 }
 
-void CoordinatedGraphicsScene::removeTilesIfNeeded(TextureMapperLayer* layer, const CoordinatedGraphicsLayerState& state)
+void CoordinatedGraphicsScene::removeTilesIfNeeded(TextureMapperLayer* layer, const CoordinatedGraphicsLayerState& state, CommitScope& commitScope)
 {
     if (state.tilesToRemove.isEmpty())
         return;
@@ -408,10 +408,10 @@
     for (auto& tile : state.tilesToRemove)
         backingStore->removeTile(tile);
 
-    m_backingStoresWithPendingBuffers.add(backingStore);
+    commitScope.backingStoresWithPendingBuffers.add(backingStore);
 }
 
-void CoordinatedGraphicsScene::updateTilesIfNeeded(TextureMapperLayer* layer, const CoordinatedGraphicsLayerState& state)
+void CoordinatedGraphicsScene::updateTilesIfNeeded(TextureMapperLayer* layer, const CoordinatedGraphicsLayerState& state, CommitScope& commitScope)
 {
     if (state.tilesToUpdate.isEmpty())
         return;
@@ -428,7 +428,7 @@
         ASSERT(surfaceIt != m_surfaces.end());
 
         backingStore->updateTile(tile.tileID, surfaceUpdateInfo.updateRect, tile.tileRect, surfaceIt->value.copyRef(), surfaceUpdateInfo.surfaceOffset);
-        m_backingStoresWithPendingBuffers.add(backingStore);
+        commitScope.backingStoresWithPendingBuffers.add(backingStore);
     }
 }
 
@@ -456,19 +456,19 @@
         removeUpdateAtlas(atlas);
 }
 
-void CoordinatedGraphicsScene::syncImageBackings(const CoordinatedGraphicsState& state)
+void CoordinatedGraphicsScene::syncImageBackings(const CoordinatedGraphicsState& state, CommitScope& commitScope)
 {
     for (auto& image : state.imagesToRemove)
-        removeImageBacking(image);
+        removeImageBacking(image, commitScope);
 
     for (auto& image : state.imagesToCreate)
         createImageBacking(image);
 
     for (auto& image : state.imagesToUpdate)
-        updateImageBacking(image.first, image.second.copyRef());
+        updateImageBacking(image.first, image.second.copyRef(), commitScope);
 
     for (auto& image : state.imagesToClear)
-        clearImageBackingContents(image);
+        clearImageBackingContents(image, commitScope);
 }
 
 void CoordinatedGraphicsScene::createImageBacking(CoordinatedImageBackingID imageID)
@@ -477,7 +477,7 @@
     m_imageBackings.add(imageID, CoordinatedBackingStore::create());
 }
 
-void CoordinatedGraphicsScene::updateImageBacking(CoordinatedImageBackingID imageID, RefPtr<Nicosia::Buffer>&& buffer)
+void CoordinatedGraphicsScene::updateImageBacking(CoordinatedImageBackingID imageID, RefPtr<Nicosia::Buffer>&& buffer, CommitScope& commitScope)
 {
     ASSERT(m_imageBackings.contains(imageID));
     auto it = m_imageBackings.find(imageID);
@@ -491,24 +491,24 @@
     backingStore->setSize(rect.size());
     backingStore->updateTile(1 /* id */, rect, rect, WTFMove(buffer), rect.location());
 
-    m_backingStoresWithPendingBuffers.add(backingStore);
+    commitScope.backingStoresWithPendingBuffers.add(backingStore);
 }
 
-void CoordinatedGraphicsScene::clearImageBackingContents(CoordinatedImageBackingID imageID)
+void CoordinatedGraphicsScene::clearImageBackingContents(CoordinatedImageBackingID imageID, CommitScope& commitScope)
 {
     ASSERT(m_imageBackings.contains(imageID));
     auto it = m_imageBackings.find(imageID);
     RefPtr<CoordinatedBackingStore> backingStore = it->value;
     backingStore->removeAllTiles();
-    m_backingStoresWithPendingBuffers.add(backingStore);
+    commitScope.backingStoresWithPendingBuffers.add(backingStore);
 }
 
-void CoordinatedGraphicsScene::removeImageBacking(CoordinatedImageBackingID imageID)
+void CoordinatedGraphicsScene::removeImageBacking(CoordinatedImageBackingID imageID, CommitScope& commitScope)
 {
     ASSERT(m_imageBackings.contains(imageID));
 
     // We don't want TextureMapperLayer refers a dangling pointer.
-    m_releasedImageBackings.append(m_imageBackings.take(imageID));
+    commitScope.releasedImageBackings.append(m_imageBackings.take(imageID));
 }
 
 void CoordinatedGraphicsScene::assignImageBackingToLayer(TextureMapperLayer* layer, CoordinatedImageBackingID imageID)
@@ -523,24 +523,13 @@
     layer->setContentsLayer(it->value.get());
 }
 
-void CoordinatedGraphicsScene::removeReleasedImageBackingsIfNeeded()
-{
-    m_releasedImageBackings.clear();
-}
-
-void CoordinatedGraphicsScene::commitPendingBackingStoreOperations()
-{
-    for (auto& backingStore : m_backingStoresWithPendingBuffers)
-        backingStore->commitTileOperations(*m_textureMapper);
-
-    m_backingStoresWithPendingBuffers.clear();
-}
-
 void CoordinatedGraphicsScene::commitSceneState(const CoordinatedGraphicsState& state)
 {
     if (!m_client)
         return;
 
+    CommitScope commitScope;
+
     m_renderedContentsScrollPosition = state.scrollPosition;
 
     createLayers(state.layersToCreate);
@@ -549,14 +538,17 @@
     if (state.rootCompositingLayer != m_rootLayerID)
         setRootLayerID(state.rootCompositingLayer);
 
-    syncImageBackings(state);
+    syncImageBackings(state, commitScope);
     syncUpdateAtlases(state);
 
     for (auto& layer : state.layersToUpdate)
-        setLayerState(layer.first, layer.second);
+        setLayerState(layer.first, layer.second, commitScope);
 
-    commitPendingBackingStoreOperations();
-    removeReleasedImageBackingsIfNeeded();
+    for (auto& backingStore : commitScope.backingStoresWithPendingBuffers)
+        backingStore->commitTileOperations(*m_textureMapper);
+
+    commitScope.releasedImageBackings.clear();
+    commitScope.backingStoresWithPendingBuffers.clear();
 }
 
 void CoordinatedGraphicsScene::renderNextFrame()
@@ -591,7 +583,6 @@
     ASSERT(!m_client);
 
     m_imageBackings.clear();
-    m_releasedImageBackings.clear();
 #if USE(COORDINATED_GRAPHICS_THREADED)
     for (auto& proxy : m_platformLayerProxies.values())
         proxy->invalidate();
@@ -605,7 +596,6 @@
     m_fixedLayers.clear();
     m_textureMapper = nullptr;
     m_backingStores.clear();
-    m_backingStoresWithPendingBuffers.clear();
 }
 
 void CoordinatedGraphicsScene::commitScrollOffset(uint32_t layerID, const IntSize& offset)

Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h (229314 => 229315)


--- trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h	2018-03-06 11:31:50 UTC (rev 229314)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h	2018-03-06 11:34:19 UTC (rev 229315)
@@ -97,14 +97,23 @@
     void releaseUpdateAtlases(const Vector<uint32_t>&);
 
 private:
+    struct CommitScope {
+        CommitScope() = default;
+        CommitScope(CommitScope&) = delete;
+        CommitScope& operator=(const CommitScope&) = delete;
+
+        Vector<RefPtr<CoordinatedBackingStore>> releasedImageBackings;
+        HashSet<RefPtr<CoordinatedBackingStore>> backingStoresWithPendingBuffers;
+    };
+
     void setRootLayerID(WebCore::CoordinatedLayerID);
     void createLayers(const Vector<WebCore::CoordinatedLayerID>&);
     void deleteLayers(const Vector<WebCore::CoordinatedLayerID>&);
-    void setLayerState(WebCore::CoordinatedLayerID, const WebCore::CoordinatedGraphicsLayerState&);
+    void setLayerState(WebCore::CoordinatedLayerID, const WebCore::CoordinatedGraphicsLayerState&, CommitScope&);
     void setLayerChildrenIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
-    void updateTilesIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
+    void updateTilesIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&, CommitScope&);
     void createTilesIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
-    void removeTilesIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
+    void removeTilesIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&, CommitScope&);
     void setLayerFiltersIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
     void setLayerAnimationsIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
     void syncPlatformLayerIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
@@ -114,11 +123,11 @@
     void createUpdateAtlas(uint32_t atlasID, RefPtr<Nicosia::Buffer>&&);
     void removeUpdateAtlas(uint32_t atlasID);
 
-    void syncImageBackings(const WebCore::CoordinatedGraphicsState&);
+    void syncImageBackings(const WebCore::CoordinatedGraphicsState&, CommitScope&);
     void createImageBacking(WebCore::CoordinatedImageBackingID);
-    void updateImageBacking(WebCore::CoordinatedImageBackingID, RefPtr<Nicosia::Buffer>&&);
-    void clearImageBackingContents(WebCore::CoordinatedImageBackingID);
-    void removeImageBacking(WebCore::CoordinatedImageBackingID);
+    void updateImageBacking(WebCore::CoordinatedImageBackingID, RefPtr<Nicosia::Buffer>&&, CommitScope&);
+    void clearImageBackingContents(WebCore::CoordinatedImageBackingID, CommitScope&);
+    void removeImageBacking(WebCore::CoordinatedImageBackingID, CommitScope&);
 
     WebCore::TextureMapperLayer* layerByID(WebCore::CoordinatedLayerID id)
     {
@@ -139,14 +148,12 @@
     void deleteLayer(WebCore::CoordinatedLayerID);
 
     void assignImageBackingToLayer(WebCore::TextureMapperLayer*, WebCore::CoordinatedImageBackingID);
-    void removeReleasedImageBackingsIfNeeded();
     void ensureRootLayer();
-    void commitPendingBackingStoreOperations();
 
-    void prepareContentBackingStore(WebCore::TextureMapperLayer*);
+    void prepareContentBackingStore(WebCore::TextureMapperLayer*, CommitScope&);
     void createBackingStoreIfNeeded(WebCore::TextureMapperLayer*);
     void removeBackingStoreIfNeeded(WebCore::TextureMapperLayer*);
-    void resetBackingStoreSizeToLayerSize(WebCore::TextureMapperLayer*);
+    void resetBackingStoreSizeToLayerSize(WebCore::TextureMapperLayer*, CommitScope&);
 
 #if USE(COORDINATED_GRAPHICS_THREADED)
     void onNewBufferAvailable() override;
@@ -156,10 +163,7 @@
     std::unique_ptr<WebCore::TextureMapper> m_textureMapper;
 
     HashMap<WebCore::CoordinatedImageBackingID, RefPtr<CoordinatedBackingStore>> m_imageBackings;
-    Vector<RefPtr<CoordinatedBackingStore>> m_releasedImageBackings;
-
     HashMap<WebCore::TextureMapperLayer*, RefPtr<CoordinatedBackingStore>> m_backingStores;
-    HashSet<RefPtr<CoordinatedBackingStore>> m_backingStoresWithPendingBuffers;
 
 #if USE(COORDINATED_GRAPHICS_THREADED)
     HashMap<WebCore::TextureMapperLayer*, RefPtr<WebCore::TextureMapperPlatformLayerProxy>> m_platformLayerProxies;

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to