Bug 216982 - [GPU Process] fast/canvas/canvas-composite-canvas.html and fast/canvas/canvas-composite-image.html fail
Summary: [GPU Process] fast/canvas/canvas-composite-canvas.html and fast/canvas/canvas...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-09-25 11:00 PDT by Wenson Hsieh
Modified: 2020-09-26 03:18 PDT (History)
6 users (show)

See Also:


Attachments
Patch (11.20 KB, patch)
2020-09-25 14:03 PDT, Wenson Hsieh
simon.fraser: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Update ChangeLog entry to include ImageBufferCGBitmapBackend.h (11.25 KB, patch)
2020-09-25 14:48 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (12.85 KB, patch)
2020-09-25 16:10 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2020-09-25 11:00:35 PDT
SSIA
Comment 1 Wenson Hsieh 2020-09-25 14:03:09 PDT
Created attachment 409735 [details]
Patch
Comment 2 Wenson Hsieh 2020-09-25 14:48:04 PDT
Created attachment 409743 [details]
Update ChangeLog entry to include ImageBufferCGBitmapBackend.h
Comment 3 Wenson Hsieh 2020-09-25 16:10:43 PDT
Created attachment 409754 [details]
Patch
Comment 4 EWS 2020-09-25 19:44:06 PDT
Committed r267616: <https://trac.webkit.org/changeset/267616>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409754 [details].
Comment 5 Radar WebKit Bug Importer 2020-09-25 19:45:42 PDT
<rdar://problem/69607603>
Comment 6 Said Abou-Hallawa 2020-09-26 03:18:04 PDT
I think this patch is not the right approach. These tests are also broken when "Enable Display List Drawing" is turned on. So it is a display-list rendering issue.

The LayoutTests change suggests that the display-list CPU drawing for canvas is now broken and/or the coordinates are flipped. Either way this does not seem right.

I think the problem is the code in setupContext() is not recorded in the display list. If we succeed in recording them, getCTM() will return the correct CTM always and the compositing bug is fixed. To do that, I would suggest the following:

1. Remove the call to setupContext() from the constructor of ImageBufferCGBitmapBackend and ImageBufferIOSurfaceBackend.

2. Make setupContext() a virtual empty method of ImageBufferBackend and override it only in ImageBufferCGBackend.

3. Make setupContext() takes a GraphicsContext argument and use it instead of using ImageBufferBackend::context(). We need to pass ImageBuffer::context() always to setupContext(). ImageBuffer::context() can be either the backend or the display list context. So if the ImageBuffer is backed by a display list, these commands will be recorded before they are applied to the backend.

2. For CPU based ImageBuffers, make the call to setupContext() in ConcreteImageBuffer::create() after creating the ImageBuffer.

5. For GPU based ImageBuffer, the call to setupContext() should happen in the WebProcess after the backend is created (i.e. RemoteImageBuffer::createBackend()). For the GPU side (i.e. RemoteImageBufferProxy), we should never call setupContext(). Flipping the coordinates should happen while replaying back the display list. So we need to implement RemoteImageBufferProxy::create() instead of calling the ConcreteImageBuffer::create().