Changeset 257945 in webkit

Timestamp:

Mar 5, 2020 2:22:47 PM (4 years ago)

Author:

commit-queue@webkit.org

Message:

Remove the optimization for discarding no operation DisplayList items between Save and Restore items
​https://bugs.webkit.org/show_bug.cgi?id=208659

Patch by Said Abou-Hallawa <​sabouhallawa@apple.com> on 2020-03-05
Reviewed by Simon Fraser.

This optimization is wrong in the case of drawing a canvas in general.
The original implementation of the DisplayList assumes balanced Save/
Restore GraphicsContext. In canvas a GraphicsConext 'save' can be issued
in a frame and the corresponding restore is issued many frames later.

• platform/graphics/displaylists/DisplayList.cpp:

(WebCore::DisplayList::DisplayList::removeItemsFromIndex): Deleted.

• platform/graphics/displaylists/DisplayList.h:
• platform/graphics/displaylists/DisplayListItems.cpp:
• platform/graphics/displaylists/DisplayListItems.h:

(WebCore::DisplayList::Save::encode const):
(WebCore::DisplayList::Save::decode):
(WebCore::DisplayList::Save::restoreIndex const): Deleted.
(WebCore::DisplayList::Save::setRestoreIndex): Deleted.

• platform/graphics/displaylists/DisplayListRecorder.cpp:

(WebCore::DisplayList::Recorder::save):
(WebCore::DisplayList::Recorder::restore):

• platform/graphics/displaylists/DisplayListRecorder.h:

(WebCore::DisplayList::Recorder::ContextState::cloneForSave const):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/displaylists/DisplayList.cpp (modified) (1 diff)
• platform/graphics/displaylists/DisplayList.h (modified) (1 diff)
• platform/graphics/displaylists/DisplayListItems.cpp (modified) (1 diff)
• platform/graphics/displaylists/DisplayListItems.h (modified) (2 diffs)
• platform/graphics/displaylists/DisplayListRecorder.cpp (modified) (3 diffs)
• platform/graphics/displaylists/DisplayListRecorder.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-03-05  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Remove the optimization for discarding no operation DisplayList items between Save and Restore items
+        https://bugs.webkit.org/show_bug.cgi?id=208659
+
+        Reviewed by Simon Fraser.
+
+        This optimization is wrong in the case of drawing a canvas in general.
+        The original implementation of the DisplayList assumes balanced Save/
+        Restore GraphicsContext. In canvas a GraphicsConext 'save' can be issued
+        in a frame and the corresponding restore is issued many frames later.
+
+        * platform/graphics/displaylists/DisplayList.cpp:
+        (WebCore::DisplayList::DisplayList::removeItemsFromIndex): Deleted.
+        * platform/graphics/displaylists/DisplayList.h:
+        * platform/graphics/displaylists/DisplayListItems.cpp:
+        * platform/graphics/displaylists/DisplayListItems.h:
+        (WebCore::DisplayList::Save::encode const):
+        (WebCore::DisplayList::Save::decode):
+        (WebCore::DisplayList::Save::restoreIndex const): Deleted.
+        (WebCore::DisplayList::Save::setRestoreIndex): Deleted.
+        * platform/graphics/displaylists/DisplayListRecorder.cpp:
+        (WebCore::DisplayList::Recorder::save):
+        (WebCore::DisplayList::Recorder::restore):
+        * platform/graphics/displaylists/DisplayListRecorder.h:
+        (WebCore::DisplayList::Recorder::ContextState::cloneForSave const):
+
 2020-03-05  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.cpp

@@ -52 +52 @@
 {
     m_list.clear();
-}
-
-void DisplayList::removeItemsFromIndex(size_t index)
-{
-    m_list.resize(index);
 }
 

trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.h

@@ -179 +179 @@
 
     WEBCORE_EXPORT void clear();
-    void removeItemsFromIndex(size_t);
 
     size_t itemCount() const { return m_list.size(); }

trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp

@@ -195 +195 @@
 }
 
-static TextStream& operator<<(TextStream& ts, const Save& item)
-{
-    ts.dumpProperty("restore-index", item.restoreIndex());
-    return ts;
-}
-
 Restore::Restore()
     : Item(ItemType::Restore)

trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h

@@ -76 +76 @@
     WEBCORE_EXPORT virtual ~Save();
 
-    // Index in the display list of the corresponding Restore item. 0 if unmatched.
-    size_t restoreIndex() const { return m_restoreIndex; }
-    void setRestoreIndex(size_t index) { m_restoreIndex = index; }
-
     template<class Encoder> void encode(Encoder&) const;
     template<class Decoder> static Optional<Ref<Save>> decode(Decoder&);
@@ -87 +83 @@
 
     void apply(GraphicsContext&) const override;
-    
-    size_t m_restoreIndex { 0 };
-};
-
-template<class Encoder>
-void Save::encode(Encoder& encoder) const
-{
-    encoder << static_cast<uint64_t>(m_restoreIndex);
-}
-
-template<class Decoder>
-Optional<Ref<Save>> Save::decode(Decoder& decoder)
-{
-    Optional<uint64_t> restoreIndex;
-    decoder >> restoreIndex;
-    if (!restoreIndex)
-        return WTF::nullopt;
-
-    // FIXME: Validate restoreIndex? But we don't have the list context here.
-    auto save = Save::create();
-    save->setRestoreIndex(static_cast<size_t>(*restoreIndex));
-    return save;
+};
+
+template<class Encoder>
+void Save::encode(Encoder&) const
+{
+}
+
+template<class Decoder>
+Optional<Ref<Save>> Save::decode(Decoder&)
+{
+    return Save::create();
 }
 

trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp

@@ -144 +144 @@
 {
     appendItem(Save::create());
-    m_stateStack.append(m_stateStack.last().cloneForSave(m_displayList.itemCount() - 1));
+    m_stateStack.append(m_stateStack.last().cloneForSave());
 }
 
@@ -153 +153 @@
 
     bool stateUsedForDrawing = currentState().wasUsedForDrawing;
-    size_t saveIndex = currentState().saveItemIndex;
 
     m_stateStack.removeLast();
@@ -159 +158 @@
     currentState().wasUsedForDrawing |= stateUsedForDrawing;
 
-    if (!stateUsedForDrawing && saveIndex) {
-        // This Save/Restore didn't contain any drawing items. Roll back to just before the last save.
-        m_displayList.removeItemsFromIndex(saveIndex);
-        return;
-    }
-
     appendItem(Restore::create());
-
-    if (saveIndex) {
-        Save& saveItem = downcast<Save>(m_displayList.itemAt(saveIndex));
-        saveItem.setRestoreIndex(m_displayList.itemCount() - 1);
-    }
 }
 

trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h

@@ -146 +146 @@
         GraphicsContextState lastDrawingState;
         bool wasUsedForDrawing { false };
-        size_t saveItemIndex { 0 };
         
         ContextState(const GraphicsContextState& state, const AffineTransform& transform, const FloatRect& clip)
@@ -155 +154 @@
         }
         
-        ContextState cloneForSave(size_t saveIndex) const
+        ContextState cloneForSave() const
         {
             ContextState state(lastDrawingState, ctm, clipBounds);
             state.stateChange = stateChange;
-            state.saveItemIndex = saveIndex;
             return state;
         }