Bug 220126

Summary: [GPU Process]: Don't call flushDrawingContext() in the middle of recording drawing commands
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: Layout and RenderingAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, changseok, esprehn+autocc, ews-watchlist, gyuyoung.kim, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
simon.fraser: review+
Patch none

Description Said Abou-Hallawa 2020-12-23 11:18:42 PST 
Before using a single shared memory buffer for sending the DL items from WebP to GPUP, calling flushDrawingContext() was necessary to synchronize multiple ImageBuffers drawing. The single shared memory buffer guarantees the order of drawing is preserved even when an ImageBuffer is drawn into another ImageBuffer. So flushDrawingContext() should be called only when the drawing is finished and the backend of the ImageBuffer is about to be used.
Comment 1 Said Abou-Hallawa 2020-12-23 11:19:16 PST 
<rdar://problem/72394086>
Comment 2 Said Abou-Hallawa 2020-12-23 11:29:35 PST 
Created attachment 416720 [details]
Patch
Comment 3 Wenson Hsieh 2020-12-23 11:33:41 PST 
Comment on attachment 416720 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Avoid calling lushDrawingContext() except when the drawing is finished

Nit - flushDrawingContext()

> Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:-185
> -    imageBuffer.flushDrawingContext();

Just to confirm — we don't need to call CGContextFlush for the previous canvas's remote image buffer either?

> Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h:-387
> -    size_t m_itemCountInCurrentDisplayList { 0 };

Nice.
Comment 4 Simon Fraser (smfr) 2020-12-23 11:42:57 PST 
Comment on attachment 416720 [details]
Patch

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

>> Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:-185
>> -    imageBuffer.flushDrawingContext();
> 
> Just to confirm — we don't need to call CGContextFlush for the previous canvas's remote image buffer either?

CG should take care of flushing when drawing one IOSurface-based CGContext into another.
Comment 5 Said Abou-Hallawa 2020-12-23 12:09:15 PST 
Created attachment 416722 [details]
Patch
Comment 6 Said Abou-Hallawa 2020-12-23 12:11:59 PST 
*** Bug 219558 has been marked as a duplicate of this bug. ***
Comment 7 EWS 2020-12-23 13:58:03 PST 
Committed r271079: <https://trac.webkit.org/changeset/271079>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416722 [details].