Changeset 257958 in webkit


Ignore:
Timestamp:
Mar 5, 2020 4:30:37 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.

Source/WebCore:

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:

(WebCore::DisplayList::operator<<):

  • 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):

LayoutTests:

Re-baseline the DisplayList tests.

  • displaylists/canvas-display-list-expected.txt:
  • displaylists/extent-includes-shadow-expected.txt:
  • displaylists/extent-includes-transforms-expected.txt:
Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r257951 r257958  
     12020-03-05  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        Remove the optimization for discarding no operation DisplayList items between Save and Restore items
     4        https://bugs.webkit.org/show_bug.cgi?id=208659
     5
     6        Reviewed by Simon Fraser.
     7
     8        Re-baseline the DisplayList tests.
     9
     10        * displaylists/canvas-display-list-expected.txt:
     11        * displaylists/extent-includes-shadow-expected.txt:
     12        * displaylists/extent-includes-transforms-expected.txt:
     13
    1142020-03-05  Zalan Bujtas  <zalan@apple.com>
    215
  • trunk/LayoutTests/displaylists/canvas-display-list-expected.txt

    r257156 r257958  
    11 
    2 (save
    3   (restore-index 0))
     2(save)
    43(set-state
    54  (change-flags 1050912)
  • trunk/LayoutTests/displaylists/extent-includes-shadow-expected.txt

    r233869 r257958  
    33  (x 0.00)
    44  (y 0.00))
    5 (save
    6   (restore-index 4))
     5(save)
    76(set-state
    87  (change-flags 1024)
  • trunk/LayoutTests/displaylists/extent-includes-transforms-expected.txt

    r253417 r257958  
    55(concatentate-ctm
    66  (ctm {m=((0.87,0.50)(-0.50,0.87)) t=(81.70,-18.30)}))
    7 (save
    8   (restore-index 5))
     7(save)
    98(set-state
    109  (change-flags 1024)
  • trunk/Source/WebCore/ChangeLog

    r257955 r257958  
     12020-03-05  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        Remove the optimization for discarding no operation DisplayList items between Save and Restore items
     4        https://bugs.webkit.org/show_bug.cgi?id=208659
     5
     6        Reviewed by Simon Fraser.
     7
     8        This optimization is wrong in the case of drawing a canvas in general.
     9        The original implementation of the DisplayList assumes balanced Save/
     10        Restore GraphicsContext. In canvas a GraphicsConext 'save' can be issued
     11        in a frame and the corresponding restore is issued many frames later.
     12
     13        * platform/graphics/displaylists/DisplayList.cpp:
     14        (WebCore::DisplayList::DisplayList::removeItemsFromIndex): Deleted.
     15        * platform/graphics/displaylists/DisplayList.h:
     16        * platform/graphics/displaylists/DisplayListItems.cpp:
     17        (WebCore::DisplayList::operator<<):
     18        * platform/graphics/displaylists/DisplayListItems.h:
     19        (WebCore::DisplayList::Save::encode const):
     20        (WebCore::DisplayList::Save::decode):
     21        (WebCore::DisplayList::Save::restoreIndex const): Deleted.
     22        (WebCore::DisplayList::Save::setRestoreIndex): Deleted.
     23        * platform/graphics/displaylists/DisplayListRecorder.cpp:
     24        (WebCore::DisplayList::Recorder::save):
     25        (WebCore::DisplayList::Recorder::restore):
     26        * platform/graphics/displaylists/DisplayListRecorder.h:
     27        (WebCore::DisplayList::Recorder::ContextState::cloneForSave const):
     28
    1292020-03-05  Ben Nham  <nham@apple.com>
    230
  • trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.cpp

    r257947 r257958  
    5252{
    5353    m_list.clear();
    54 }
    55 
    56 void DisplayList::removeItemsFromIndex(size_t index)
    57 {
    58     m_list.resize(index);
    5954}
    6055
  • trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.h

    r257947 r257958  
    179179
    180180    WEBCORE_EXPORT void clear();
    181     void removeItemsFromIndex(size_t);
    182181
    183182    size_t itemCount() const { return m_list.size(); }
  • trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp

    r257947 r257958  
    195195}
    196196
    197 static TextStream& operator<<(TextStream& ts, const Save& item)
    198 {
    199     ts.dumpProperty("restore-index", item.restoreIndex());
    200     return ts;
    201 }
    202 
    203197Restore::Restore()
    204198    : Item(ItemType::Restore)
     
    14051399    // FIXME: Make a macro which takes a macro for all these enumeration switches
    14061400    switch (item.type()) {
    1407     case ItemType::Save:
    1408         ts << downcast<Save>(item);
    1409         break;
    14101401    case ItemType::Translate:
    14111402        ts << downcast<Translate>(item);
     
    15381529
    15391530    // Items with no additional data.
     1531    case ItemType::Save:
    15401532    case ItemType::Restore:
    15411533    case ItemType::EndTransparencyLayer:
  • trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h

    r257947 r257958  
    7676    WEBCORE_EXPORT virtual ~Save();
    7777
    78     // Index in the display list of the corresponding Restore item. 0 if unmatched.
    79     size_t restoreIndex() const { return m_restoreIndex; }
    80     void setRestoreIndex(size_t index) { m_restoreIndex = index; }
    81 
    8278    template<class Encoder> void encode(Encoder&) const;
    8379    template<class Decoder> static Optional<Ref<Save>> decode(Decoder&);
     
    8783
    8884    void apply(GraphicsContext&) const override;
    89    
    90     size_t m_restoreIndex { 0 };
    91 };
    92 
    93 template<class Encoder>
    94 void Save::encode(Encoder& encoder) const
    95 {
    96     encoder << static_cast<uint64_t>(m_restoreIndex);
    97 }
    98 
    99 template<class Decoder>
    100 Optional<Ref<Save>> Save::decode(Decoder& decoder)
    101 {
    102     Optional<uint64_t> restoreIndex;
    103     decoder >> restoreIndex;
    104     if (!restoreIndex)
    105         return WTF::nullopt;
    106 
    107     // FIXME: Validate restoreIndex? But we don't have the list context here.
    108     auto save = Save::create();
    109     save->setRestoreIndex(static_cast<size_t>(*restoreIndex));
    110     return save;
     85};
     86
     87template<class Encoder>
     88void Save::encode(Encoder&) const
     89{
     90}
     91
     92template<class Decoder>
     93Optional<Ref<Save>> Save::decode(Decoder&)
     94{
     95    return Save::create();
    11196}
    11297
  • trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp

    r257947 r257958  
    144144{
    145145    appendItem(Save::create());
    146     m_stateStack.append(m_stateStack.last().cloneForSave(m_displayList.itemCount() - 1));
     146    m_stateStack.append(m_stateStack.last().cloneForSave());
    147147}
    148148
     
    153153
    154154    bool stateUsedForDrawing = currentState().wasUsedForDrawing;
    155     size_t saveIndex = currentState().saveItemIndex;
    156155
    157156    m_stateStack.removeLast();
     
    159158    currentState().wasUsedForDrawing |= stateUsedForDrawing;
    160159
    161     if (!stateUsedForDrawing && saveIndex) {
    162         // This Save/Restore didn't contain any drawing items. Roll back to just before the last save.
    163         m_displayList.removeItemsFromIndex(saveIndex);
    164         return;
    165     }
    166 
    167160    appendItem(Restore::create());
    168 
    169     if (saveIndex) {
    170         Save& saveItem = downcast<Save>(m_displayList.itemAt(saveIndex));
    171         saveItem.setRestoreIndex(m_displayList.itemCount() - 1);
    172     }
    173161}
    174162
  • trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h

    r257947 r257958  
    146146        GraphicsContextState lastDrawingState;
    147147        bool wasUsedForDrawing { false };
    148         size_t saveItemIndex { 0 };
    149148       
    150149        ContextState(const GraphicsContextState& state, const AffineTransform& transform, const FloatRect& clip)
     
    155154        }
    156155       
    157         ContextState cloneForSave(size_t saveIndex) const
     156        ContextState cloneForSave() const
    158157        {
    159158            ContextState state(lastDrawingState, ctm, clipBounds);
    160159            state.stateChange = stateChange;
    161             state.saveItemIndex = saveIndex;
    162160            return state;
    163161        }
Note: See TracChangeset for help on using the changeset viewer.