Summary: | [GPU Process] fast/canvas/canvas-composite-canvas.html and fast/canvas/canvas-composite-image.html fail | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
Component: | Canvas | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | darin, dino, sabouhallawa, simon.fraser, thorton, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Wenson Hsieh
2020-09-25 11:00:35 PDT
Created attachment 409735 [details]
Patch
Created attachment 409743 [details]
Update ChangeLog entry to include ImageBufferCGBitmapBackend.h
Created attachment 409754 [details]
Patch
Committed r267616: <https://trac.webkit.org/changeset/267616> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409754 [details]. 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(). |