Bug 208659

Summary: Remove the optimization for discarding no operation DisplayList items between Save and Restore items
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: CanvasAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, jacob_uphoff, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=225381
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Said Abou-Hallawa
Reported 2020-03-05 12:35:27 PST
The restoreIndex was an opportunity of optimization. If all no drawing between the Save and Restore happens, this means these items are all void and can be deleted from the DisplayList. With the canvas drawing and with batching the DisplayList flushing, this causes a serious problem when we try to remove the items at and after currentState().saveItemIndex.
Attachments
Patch (7.88 KB, patch)
2020-03-05 12:36 PST, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Patch (7.90 KB, patch)
2020-03-05 13:22 PST, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Patch (10.91 KB, patch)
2020-03-05 15:35 PST, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Said Abou-Hallawa
Comment 1 2020-03-05 12:36:59 PST
Created attachment 392607 [details] Patch
Simon Fraser (smfr)
Comment 2 2020-03-05 13:01:08 PST
Comment on attachment 392607 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=392607&action=review Please file a bug to track adding this optimization back. > Source/WebCore/ChangeLog:3 > + DisplayList::Save item should not track the index of the corresponding Restore item This title should be "remove the optimization for..."
Said Abou-Hallawa
Comment 3 2020-03-05 13:22:12 PST
Created attachment 392614 [details] Patch
WebKit Commit Bot
Comment 4 2020-03-05 14:22:16 PST
The commit-queue encountered the following flaky tests while processing attachment 392614 [details]: editing/spelling/spellcheck-paste-continuous-disabled.html bug 208016 (authors: g.czajkowski@samsung.com and mark.lam@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 5 2020-03-05 14:22:52 PST
Comment on attachment 392614 [details] Patch Clearing flags on attachment: 392614 Committed r257945: <https://trac.webkit.org/changeset/257945>
WebKit Commit Bot
Comment 6 2020-03-05 14:22:53 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7 2020-03-05 14:23:15 PST
<rdar://problem/60100226>
Jacob Uphoff
Comment 8 2020-03-05 14:33:01 PST
Reverted r257945 for reason: This causes tests to fail Committed r257947: <https://trac.webkit.org/changeset/257947>
Simon Fraser (smfr)
Comment 9 2020-03-05 14:50:00 PST
Namely: displaylists/canvas-display-list.html displaylists/extent-includes-shadow.html displaylists/extent-includes-transforms.html
Said Abou-Hallawa
Comment 10 2020-03-05 15:08:59 PST
(In reply to Simon Fraser (smfr) from comment #9) > Namely: > displaylists/canvas-display-list.html > displaylists/extent-includes-shadow.html > displaylists/extent-includes-transforms.html Yes because DisplayList::Restore has no parameters now and operator<<(TextStream& ts, const Item&) should not call operator<<(TextStream& ts, const Save&) because this is going to call itself and we end up in infinite recursion.
Said Abou-Hallawa
Comment 11 2020-03-05 15:35:19 PST
Created attachment 392635 [details] Patch
WebKit Commit Bot
Comment 12 2020-03-05 16:30:44 PST
Comment on attachment 392635 [details] Patch Clearing flags on attachment: 392635 Committed r257958: <https://trac.webkit.org/changeset/257958>
WebKit Commit Bot
Comment 13 2020-03-05 16:30:46 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.