Bug 223560

Summary: [GPU Process] Replace ItemHandle with a const Variant&
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: NEW ---    
Severity: Normal CC: annulen, changseok, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, kondapallykalyan, pdr, ryuan.choi, sergio, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 224314, 225579, 225582, 225589, 223849, 223863, 224145, 224146, 224148, 224270    
Bug Blocks:    
Attachments:
Description Flags
WIP
none
WIP
none
WIP
none
Tests pass but needs ChangeLog
none
Patch
none
WIP
none
Rebased
none
WIP
none
WIP
none
Patch
none
Patch ews-feeder: commit-queue-

Description Myles C. Maxfield 2021-03-21 02:20:07 PDT 
[GPU Process] Migrate DisplayList::Iterator to use a Variant instead of ItemHandle
Comment 1 Myles C. Maxfield 2021-03-21 02:21:28 PDT 
Created attachment 423822 [details]
WIP
Comment 2 Myles C. Maxfield 2021-03-21 04:48:55 PDT 
Created attachment 423824 [details]
WIP
Comment 3 Myles C. Maxfield 2021-03-21 14:22:02 PDT 
Created attachment 423836 [details]
WIP
Comment 4 Myles C. Maxfield 2021-03-21 16:27:16 PDT 
Created attachment 423840 [details]
Tests pass but needs ChangeLog
Comment 5 Myles C. Maxfield 2021-03-21 21:51:34 PDT 
Created attachment 423854 [details]
Patch
Comment 6 Wenson Hsieh 2021-03-21 22:52:53 PDT 
Comment on attachment 423854 [details]
Patch

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

(Just a couple of quick comments so far)

> Source/WebCore/ChangeLog:11
> +        were just and object, placed there by placement new.

"just an object"

> Source/WebCore/ChangeLog:50
> +               display list items. These clients were always specified in WebKit - they were only optional for
> +               testing. However, the code could be significantly simplified if we don't have to handle the

Does this statement still apply to the in-memory display list case? (i.e. to cache DrawGlyph items when painting text frequently)
Comment 7 Wenson Hsieh 2021-03-21 23:25:30 PDT 
Comment on attachment 423854 [details]
Patch

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

> Source/WebCore/ChangeLog:100
> +                   which is no problem becasue we were always specifying one in WebKit anyway.

"becasue" => "because"
Comment 8 Myles C. Maxfield 2021-03-22 18:28:42 PDT 
Created attachment 423974 [details]
WIP
Comment 9 Myles C. Maxfield 2021-03-22 19:46:38 PDT 
Created attachment 423981 [details]
Rebased
Comment 10 Myles C. Maxfield 2021-03-22 23:45:11 PDT 
Created attachment 423991 [details]
WIP
Comment 11 Myles C. Maxfield 2021-03-22 23:57:31 PDT 
Created attachment 423992 [details]
WIP
Comment 12 Myles C. Maxfield 2021-03-24 00:58:05 PDT 
Created attachment 424103 [details]
Patch
Comment 13 Myles C. Maxfield 2021-03-24 11:24:00 PDT 
Created attachment 424161 [details]
Patch
Comment 14 Radar WebKit Bug Importer 2021-03-28 02:21:14 PDT 
<rdar://problem/75929228>
Comment 15 Simon Fraser (smfr) 2021-03-29 09:43:15 PDT 
Is this a good time to do this refactoring?
Comment 16 Myles C. Maxfield 2021-04-03 01:21:54 PDT 
We should also do whatever is necessary to move WritingClient and ReadingClient from InMemoryDisplayList.h to InMemoryDisplayList.cpp.
Comment 17 Myles C. Maxfield 2021-04-07 21:53:04 PDT 
Remaining pieces:
- After 224270 and 224314, make InMemoryDisplayList not use ItemHandle
- Swap out the implementation of DisplayList::Iterator, but keep the same API signatures
- Update Iterator's signature, and update callers
- Hunting down all the last places that use ItemHandle
Comment 18 Myles C. Maxfield 2021-04-21 14:03:41 PDT 
We should also make ItemBuffer::createItemBuffer() stop manually calling malloc()