Bug 219468

Summary: [GPU Process] Record changes in GraphicsContextState as individual DisplayList items
Product: WebKit Reporter: Tim Horton <thorton>
Component: CanvasAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED WONTFIX    
Severity: Normal CC: annulen, bfulgham, changseok, dino, esprehn+autocc, ews-watchlist, fmalita, fred.wang, glenn, gyuyoung.kim, hi, joepeck, kondapallykalyan, pdr, ryuan.choi, sabouhallawa, schenney, sergio, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh, zalan
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=221550
Bug Depends on: 220079, 221550, 221637    
Bug Blocks:    
Attachments:
Description Flags
WIP
none
WIP
none
Patch
none
WIP
ews-feeder: commit-queue-
WIP
none
WIP
none
WIP
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Tim Horton 2020-12-02 17:55:16 PST 
SHOULD NEVER BE REACHED
Source/WebKit/Platform/IPC/Encoder.h(110) : static RefPtr<WebCore::SharedBuffer> IPC::Encoder::encodeSingleObject(const T &) [T = WebCore::DisplayList::SetState]
WTFCrash
WTFCrashWithInfo(int, char const*, char const*, int)
WTF::RefPtr<WebCore::SharedBuffer, WTF::RawPtrTraits<WebCore::SharedBuffer>, WTF::DefaultRefDerefTraits<WebCore::SharedBuffer> > IPC::Encoder::encodeSingleObject<WebCore::DisplayList::SetState>(WebCore::DisplayList::SetState const&)
WebKit::RemoteImageBufferProxy<WebKit::ImageBufferShareableMappedIOSurfaceBackend>::encodeItem(WebCore::DisplayList::ItemHandle) const
non-virtual thunk to WebKit::RemoteImageBufferProxy<WebKit::ImageBufferShareableMappedIOSurfaceBackend>::encodeItem(WebCore::DisplayList::ItemHandle) const
WebCore::DisplayList::ItemBuffer::appendEncodedData(WebCore::DisplayList::ItemHandle)
void WebCore::DisplayList::ItemBuffer::append<WebCore::DisplayList::SetState, WebCore::GraphicsContextState const&, WTF::OptionSet<WebCore::GraphicsContextState::Change>&>(WebCore::GraphicsContextState const&, WTF::OptionSet<WebCore::GraphicsContextState::Change>&)
void WebCore::DisplayList::DisplayList::append<WebCore::DisplayList::SetState, WebCore::GraphicsContextState const&, WTF::OptionSet<WebCore::GraphicsContextState::Change>&>(WebCore::GraphicsContextState const&, WTF::OptionSet<WebCore::GraphicsContextState::Change>&)
void WebCore::DisplayList::Recorder::append<WebCore::DisplayList::SetState, WebCore::GraphicsContextState const&, WTF::OptionSet<WebCore::GraphicsContextState::Change>&>(WebCore::GraphicsContextState const&, WTF::OptionSet<WebCore::GraphicsContextState::Change>&)
WebCore::DisplayList::Recorder::appendStateChangeItem(WebCore::GraphicsContextStateChange const&, WTF::OptionSet<WebCore::GraphicsContextState::Change>)
Comment 1 Tim Horton 2020-12-02 17:55:32 PST 
For example, canvas/philip/tests/2d.composite.globalAlpha.canvaspattern.html
Comment 2 Tim Horton 2020-12-02 17:56:04 PST 
The problem is, SetState is encoded using encodeSingleObject, but it has things like Pattern hanging off of it.
Comment 3 Radar WebKit Bug Importer 2020-12-02 19:22:48 PST 
<rdar://problem/71921036>
Comment 4 Tim Horton 2020-12-02 19:28:23 PST 
And Pattern has an Image that it encodes with ImageHandle. We either need a subdisplaylist for the pattern or to instead cache that image ahead of time.
Comment 5 Said Abou-Hallawa 2020-12-19 12:06:45 PST 
Created attachment 416572 [details]
WIP
Comment 6 Said Abou-Hallawa 2020-12-20 11:00:50 PST 
Created attachment 416584 [details]
WIP
Comment 7 Said Abou-Hallawa 2020-12-21 01:33:12 PST 
Created attachment 416597 [details]
Patch
Comment 8 Said Abou-Hallawa 2021-02-05 01:48:59 PST 
Created attachment 419374 [details]
WIP
Comment 9 Said Abou-Hallawa 2021-02-05 02:06:37 PST 
Created attachment 419375 [details]
WIP
Comment 10 Said Abou-Hallawa 2021-02-05 11:22:11 PST 
Created attachment 419437 [details]
WIP
Comment 11 Said Abou-Hallawa 2021-02-05 14:48:41 PST 
Created attachment 419468 [details]
WIP
Comment 12 Said Abou-Hallawa 2021-02-05 21:57:26 PST 
Created attachment 419493 [details]
Patch
Comment 13 Said Abou-Hallawa 2021-02-06 00:50:25 PST 
Created attachment 419497 [details]
Patch
Comment 14 Wenson Hsieh 2021-02-08 14:02:14 PST 
Comment on attachment 419497 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=419497&action=review

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:170
> +class SetInlineState {

It looks like SetInlineState doesn't include RGBA (non-extended) stroke color or fill changes, which (IIRC) make up the bulk of state changes in Paths, Lines and Arcs in MotionMark. I think this could encompass changes to fill and stroke color that only use SRGBA<uint8_t> as well, to keep most of the state change items in these fully inline in those three subtests.

Though, perhaps this could be done as a followup.
Comment 15 Said Abou-Hallawa 2021-02-09 12:44:15 PST 
Comment on attachment 419497 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=419497&action=review

>> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:170
>> +class SetInlineState {
> 
> It looks like SetInlineState doesn't include RGBA (non-extended) stroke color or fill changes, which (IIRC) make up the bulk of state changes in Paths, Lines and Arcs in MotionMark. I think this could encompass changes to fill and stroke color that only use SRGBA<uint8_t> as well, to keep most of the state change items in these fully inline in those three subtests.
> 
> Though, perhaps this could be done as a followup.

Yes this is right. I wrote a similar comment in the ChangeLog above. "Future work is to make SetInlineState handle more cases for colors and gradients." But I will try to do this inline changes in the patch of this bug to avoid introducing regression.
Comment 16 Said Abou-Hallawa 2021-02-14 00:50:20 PST 
Created attachment 420236 [details]
Patch
Comment 17 Said Abou-Hallawa 2021-02-14 12:52:29 PST 
Created attachment 420249 [details]
Patch
Comment 18 Said Abou-Hallawa 2021-02-15 11:38:32 PST 
Created attachment 420343 [details]
Patch
Comment 19 Said Abou-Hallawa 2021-02-16 22:53:45 PST 
Created attachment 420601 [details]
Patch
Comment 20 Said Abou-Hallawa 2021-02-17 01:45:58 PST 
Created attachment 420617 [details]
Patch