Changeset 229315 in webkit


Ignore:
Timestamp:
Mar 6, 2018 3:34:19 AM (6 years ago)
Author:
zandobersek@gmail.com
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:
Location:
trunk/Source/WebKit
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r229309 r229315  
     12018-03-06  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        CoordinatedGraphicsScene: properly limit data specific to state commit operation
     4        https://bugs.webkit.org/show_bug.cgi?id=183326
     5
     6        Reviewed by Sergio Villar Senin.
     7
     8        In the process of state commit in CoordinatedGraphicsScene, the released
     9        image backings and backing stores with pending updates are stored in
     10        Vector and HashSet objects, respectively. Instead of these two objects
     11        being member variables on the CoordinatedGraphicsScene class, keep them
     12        in the CommitScope structure that's limited to the operations done in
     13        the commitSceneState() method.
     14
     15        The two member variables are dropped, and the CommitScope object is
     16        passed by reference to any helper method that needs to append either
     17        kind of object to the respective container. At the end of the state
     18        commit, backing stores with pending updates have those updates applied,
     19        and the two containers are cleared out as the CommitScope object is
     20        destroyed.
     21
     22        * Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:
     23        (WebKit::CoordinatedGraphicsScene::setLayerState):
     24        (WebKit::CoordinatedGraphicsScene::prepareContentBackingStore):
     25        (WebKit::CoordinatedGraphicsScene::resetBackingStoreSizeToLayerSize):
     26        (WebKit::CoordinatedGraphicsScene::removeTilesIfNeeded):
     27        (WebKit::CoordinatedGraphicsScene::updateTilesIfNeeded):
     28        (WebKit::CoordinatedGraphicsScene::syncImageBackings):
     29        (WebKit::CoordinatedGraphicsScene::updateImageBacking):
     30        (WebKit::CoordinatedGraphicsScene::clearImageBackingContents):
     31        (WebKit::CoordinatedGraphicsScene::removeImageBacking):
     32        (WebKit::CoordinatedGraphicsScene::commitSceneState):
     33        (WebKit::CoordinatedGraphicsScene::purgeGLResources):
     34        (WebKit::CoordinatedGraphicsScene::removeReleasedImageBackingsIfNeeded): Deleted.
     35        (WebKit::CoordinatedGraphicsScene::commitPendingBackingStoreOperations): Deleted.
     36        * Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h:
     37
    1382018-03-05  Yusuke Suzuki  <utatane.tea@gmail.com>
    239

  • trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp

    r228373 r229315  
    211211}
    212212
    213 void CoordinatedGraphicsScene::setLayerState(CoordinatedLayerID id, const CoordinatedGraphicsLayerState& layerState)
     213void CoordinatedGraphicsScene::setLayerState(CoordinatedLayerID id, const CoordinatedGraphicsLayerState& layerState, CommitScope& commitScope)
    214214{
    215215    ASSERT(m_rootLayerID != InvalidCoordinatedLayerID);
    … …  
    282282        layer->didCommitScrollOffset(layerState.committedScrollOffset);
    283283
    284     prepareContentBackingStore(layer);
     284    prepareContentBackingStore(layer, commitScope);
    285285
    286286    // Apply Operations.
    287287    setLayerChildrenIfNeeded(layer, layerState);
    288288    createTilesIfNeeded(layer, layerState);
    289     removeTilesIfNeeded(layer, layerState);
    290     updateTilesIfNeeded(layer, layerState);
     289    removeTilesIfNeeded(layer, layerState, commitScope);
     290    updateTilesIfNeeded(layer, layerState, commitScope);
    291291    setLayerFiltersIfNeeded(layer, layerState);
    292292    setLayerAnimationsIfNeeded(layer, layerState);
    … …  
    345345}
    346346
    347 void CoordinatedGraphicsScene::prepareContentBackingStore(TextureMapperLayer* layer)
     347void CoordinatedGraphicsScene::prepareContentBackingStore(TextureMapperLayer* layer, CommitScope& commitScope)
    348348{
    349349    if (!layerShouldHaveBackingStore(layer)) {
    … …  
    353353
    354354    createBackingStoreIfNeeded(layer);
    355     resetBackingStoreSizeToLayerSize(layer);
     355    resetBackingStoreSizeToLayerSize(layer, commitScope);
    356356}
    357357
    … …  
    375375}
    376376
    377 void CoordinatedGraphicsScene::resetBackingStoreSizeToLayerSize(TextureMapperLayer* layer)
     377void CoordinatedGraphicsScene::resetBackingStoreSizeToLayerSize(TextureMapperLayer* layer, CommitScope& commitScope)
    378378{
    379379    RefPtr<CoordinatedBackingStore> backingStore = m_backingStores.get(layer);
    380380    ASSERT(backingStore);
    381381    backingStore->setSize(layer->size());
    382     m_backingStoresWithPendingBuffers.add(backingStore);
     382    commitScope.backingStoresWithPendingBuffers.add(backingStore);
    383383}
    384384
    … …  
    397397}
    398398
    399 void CoordinatedGraphicsScene::removeTilesIfNeeded(TextureMapperLayer* layer, const CoordinatedGraphicsLayerState& state)
     399void CoordinatedGraphicsScene::removeTilesIfNeeded(TextureMapperLayer* layer, const CoordinatedGraphicsLayerState& state, CommitScope& commitScope)
    400400{
    401401    if (state.tilesToRemove.isEmpty())
    … …  
    409409        backingStore->removeTile(tile);
    410410
    411     m_backingStoresWithPendingBuffers.add(backingStore);
    412 }
    413 
    414 void CoordinatedGraphicsScene::updateTilesIfNeeded(TextureMapperLayer* layer, const CoordinatedGraphicsLayerState& state)
     411    commitScope.backingStoresWithPendingBuffers.add(backingStore);
     412}
     413
     414void CoordinatedGraphicsScene::updateTilesIfNeeded(TextureMapperLayer* layer, const CoordinatedGraphicsLayerState& state, CommitScope& commitScope)
    415415{
    416416    if (state.tilesToUpdate.isEmpty())
    … …  
    429429
    430430        backingStore->updateTile(tile.tileID, surfaceUpdateInfo.updateRect, tile.tileRect, surfaceIt->value.copyRef(), surfaceUpdateInfo.surfaceOffset);
    431         m_backingStoresWithPendingBuffers.add(backingStore);
     431        commitScope.backingStoresWithPendingBuffers.add(backingStore);
    432432    }
    433433}
    … …  
    457457}
    458458
    459 void CoordinatedGraphicsScene::syncImageBackings(const CoordinatedGraphicsState& state)
     459void CoordinatedGraphicsScene::syncImageBackings(const CoordinatedGraphicsState& state, CommitScope& commitScope)
    460460{
    461461    for (auto& image : state.imagesToRemove)
    462         removeImageBacking(image);
     462        removeImageBacking(image, commitScope);
    463463
    464464    for (auto& image : state.imagesToCreate)
    … …  
    466466
    467467    for (auto& image : state.imagesToUpdate)
    468         updateImageBacking(image.first, image.second.copyRef());
     468        updateImageBacking(image.first, image.second.copyRef(), commitScope);
    469469
    470470    for (auto& image : state.imagesToClear)
    471         clearImageBackingContents(image);
     471        clearImageBackingContents(image, commitScope);
    472472}
    473473
    … …  
    478478}
    479479
    480 void CoordinatedGraphicsScene::updateImageBacking(CoordinatedImageBackingID imageID, RefPtr<Nicosia::Buffer>&& buffer)
     480void CoordinatedGraphicsScene::updateImageBacking(CoordinatedImageBackingID imageID, RefPtr<Nicosia::Buffer>&& buffer, CommitScope& commitScope)
    481481{
    482482    ASSERT(m_imageBackings.contains(imageID));
    … …  
    492492    backingStore->updateTile(1 /* id */, rect, rect, WTFMove(buffer), rect.location());
    493493
    494     m_backingStoresWithPendingBuffers.add(backingStore);
    495 }
    496 
    497 void CoordinatedGraphicsScene::clearImageBackingContents(CoordinatedImageBackingID imageID)
     494    commitScope.backingStoresWithPendingBuffers.add(backingStore);
     495}
     496
     497void CoordinatedGraphicsScene::clearImageBackingContents(CoordinatedImageBackingID imageID, CommitScope& commitScope)
    498498{
    499499    ASSERT(m_imageBackings.contains(imageID));
    … …  
    501501    RefPtr<CoordinatedBackingStore> backingStore = it->value;
    502502    backingStore->removeAllTiles();
    503     m_backingStoresWithPendingBuffers.add(backingStore);
    504 }
    505 
    506 void CoordinatedGraphicsScene::removeImageBacking(CoordinatedImageBackingID imageID)
     503    commitScope.backingStoresWithPendingBuffers.add(backingStore);
     504}
     505
     506void CoordinatedGraphicsScene::removeImageBacking(CoordinatedImageBackingID imageID, CommitScope& commitScope)
    507507{
    508508    ASSERT(m_imageBackings.contains(imageID));
    509509
    510510    // We don't want TextureMapperLayer refers a dangling pointer.
    511     m_releasedImageBackings.append(m_imageBackings.take(imageID));
     511    commitScope.releasedImageBackings.append(m_imageBackings.take(imageID));
    512512}
    513513
    … …  
    524524}
    525525
    526 void CoordinatedGraphicsScene::removeReleasedImageBackingsIfNeeded()
    527 {
    528     m_releasedImageBackings.clear();
    529 }
    530 
    531 void CoordinatedGraphicsScene::commitPendingBackingStoreOperations()
    532 {
    533     for (auto& backingStore : m_backingStoresWithPendingBuffers)
    534         backingStore->commitTileOperations(*m_textureMapper);
    535 
    536     m_backingStoresWithPendingBuffers.clear();
    537 }
    538 
    539526void CoordinatedGraphicsScene::commitSceneState(const CoordinatedGraphicsState& state)
    540527{
    541528    if (!m_client)
    542529        return;
     530
     531    CommitScope commitScope;
    543532
    544533    m_renderedContentsScrollPosition = state.scrollPosition;
    … …  
    550539        setRootLayerID(state.rootCompositingLayer);
    551540
    552     syncImageBackings(state);
     541    syncImageBackings(state, commitScope);
    553542    syncUpdateAtlases(state);
    554543
    555544    for (auto& layer : state.layersToUpdate)
    556         setLayerState(layer.first, layer.second);
    557 
    558     commitPendingBackingStoreOperations();
    559     removeReleasedImageBackingsIfNeeded();
     545        setLayerState(layer.first, layer.second, commitScope);
     546
     547    for (auto& backingStore : commitScope.backingStoresWithPendingBuffers)
     548        backingStore->commitTileOperations(*m_textureMapper);
     549
     550    commitScope.releasedImageBackings.clear();
     551    commitScope.backingStoresWithPendingBuffers.clear();
    560552}
    561553
    … …  
    592584
    593585    m_imageBackings.clear();
    594     m_releasedImageBackings.clear();
    595586#if USE(COORDINATED_GRAPHICS_THREADED)
    596587    for (auto& proxy : m_platformLayerProxies.values())
    … …  
    606597    m_textureMapper = nullptr;
    607598    m_backingStores.clear();
    608     m_backingStoresWithPendingBuffers.clear();
    609599}
    610600

  • trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h

    r225266 r229315  
    9898
    9999private:
     100    struct CommitScope {
     101        CommitScope() = default;
     102        CommitScope(CommitScope&) = delete;
     103        CommitScope& operator=(const CommitScope&) = delete;
     104
     105        Vector<RefPtr<CoordinatedBackingStore>> releasedImageBackings;
     106        HashSet<RefPtr<CoordinatedBackingStore>> backingStoresWithPendingBuffers;
     107    };
     108
    100109    void setRootLayerID(WebCore::CoordinatedLayerID);
    101110    void createLayers(const Vector<WebCore::CoordinatedLayerID>&);
    102111    void deleteLayers(const Vector<WebCore::CoordinatedLayerID>&);
    103     void setLayerState(WebCore::CoordinatedLayerID, const WebCore::CoordinatedGraphicsLayerState&);
     112    void setLayerState(WebCore::CoordinatedLayerID, const WebCore::CoordinatedGraphicsLayerState&, CommitScope&);
    104113    void setLayerChildrenIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
    105     void updateTilesIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
     114    void updateTilesIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&, CommitScope&);
    106115    void createTilesIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
    107     void removeTilesIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
     116    void removeTilesIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&, CommitScope&);
    108117    void setLayerFiltersIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
    109118    void setLayerAnimationsIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&);
    … …  
    115124    void removeUpdateAtlas(uint32_t atlasID);
    116125
    117     void syncImageBackings(const WebCore::CoordinatedGraphicsState&);
     126    void syncImageBackings(const WebCore::CoordinatedGraphicsState&, CommitScope&);
    118127    void createImageBacking(WebCore::CoordinatedImageBackingID);
    119     void updateImageBacking(WebCore::CoordinatedImageBackingID, RefPtr<Nicosia::Buffer>&&);
    120     void clearImageBackingContents(WebCore::CoordinatedImageBackingID);
    121     void removeImageBacking(WebCore::CoordinatedImageBackingID);
     128    void updateImageBacking(WebCore::CoordinatedImageBackingID, RefPtr<Nicosia::Buffer>&&, CommitScope&);
     129    void clearImageBackingContents(WebCore::CoordinatedImageBackingID, CommitScope&);
     130    void removeImageBacking(WebCore::CoordinatedImageBackingID, CommitScope&);
    122131
    123132    WebCore::TextureMapperLayer* layerByID(WebCore::CoordinatedLayerID id)
    … …  
    140149
    141150    void assignImageBackingToLayer(WebCore::TextureMapperLayer*, WebCore::CoordinatedImageBackingID);
    142     void removeReleasedImageBackingsIfNeeded();
    143151    void ensureRootLayer();
    144     void commitPendingBackingStoreOperations();
    145152
    146     void prepareContentBackingStore(WebCore::TextureMapperLayer*);
     153    void prepareContentBackingStore(WebCore::TextureMapperLayer*, CommitScope&);
    147154    void createBackingStoreIfNeeded(WebCore::TextureMapperLayer*);
    148155    void removeBackingStoreIfNeeded(WebCore::TextureMapperLayer*);
    149     void resetBackingStoreSizeToLayerSize(WebCore::TextureMapperLayer*);
     156    void resetBackingStoreSizeToLayerSize(WebCore::TextureMapperLayer*, CommitScope&);
    150157
    151158#if USE(COORDINATED_GRAPHICS_THREADED)
    … …  
    157164
    158165    HashMap<WebCore::CoordinatedImageBackingID, RefPtr<CoordinatedBackingStore>> m_imageBackings;
    159     Vector<RefPtr<CoordinatedBackingStore>> m_releasedImageBackings;
    160 
    161166    HashMap<WebCore::TextureMapperLayer*, RefPtr<CoordinatedBackingStore>> m_backingStores;
    162     HashSet<RefPtr<CoordinatedBackingStore>> m_backingStoresWithPendingBuffers;
    163167
    164168#if USE(COORDINATED_GRAPHICS_THREADED)
Note: See TracChangeset for help on using the changeset viewer.