[webkit-reviews] review granted: [Bug 208597] Canvas drawing commands have to be flushed to the GPUProcess in batches : [Attachment 392479] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Mar 7 00:20:16 PST 2020
Myles C. Maxfield <mmaxfield at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 208597: Canvas drawing commands have to be flushed to the GPUProcess in
batches
https://bugs.webkit.org/show_bug.cgi?id=208597
Attachment 392479: Patch
https://bugs.webkit.org/attachment.cgi?id=392479&action=review
--- Comment #3 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 392479
--> https://bugs.webkit.org/attachment.cgi?id=392479
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=392479&action=review
Elegant.
> Source/WebCore/platform/graphics/displaylists/DisplayListObserver.h:31
> +class Observer {
Calling this by the general name DisplayList::Observer seems misleading as it
really only observes the recorder. Please consider either giving it a longer
name, or making it an inner class (DisplayList::Recorder::Observer).
Personally, I prefer DisplayList::Recorder::Observer because it keeps the two
classes close together, rather than being spread to two separate files.
> Source/WebKit/WebProcess/GPU/graphics/RemoteImageBuffer.h:85
> +
RemoteImageBufferMessageHandler::flushDrawingContextAndWaitCommit(displayList);
I like the design of this better than what I was doing in
https://bugs.webkit.org/show_bug.cgi?id=208560. I've updated that patch to
match this design.
> Source/WebKit/WebProcess/GPU/graphics/RemoteImageBuffer.h:94
> + if (displayList.itemCount() >= DisplayListBatchSize) {
This doesn't really matter much, but shouldn't this condition be checked after
the item gets appended? The action of appending the item is the trigger which
can cause this condition to become true.
More information about the webkit-reviews
mailing list