Bug 217735

Created attachment 411382 [details] Patch

Created attachment 411386 [details] Patch

Created attachment 411389 [details] Patch

Created attachment 411408 [details] Patch

Created attachment 411416 [details] Patch

Comment on attachment 411416 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411416&action=review > Source/WebCore/html/ImageBitmap.cpp:84 > + auto document = scriptExecutionContext.isDocument() ? &downcast<Document>(scriptExecutionContext) : nullptr; > + auto hostWindow = (document && document->view() && document->view()->root()) ? document->view()->root()->hostWindow() : nullptr; ImageBitmap is: Exposed=(Window,Worker), so this needs to work in Workers too, where scriptExecutionContext.isDocument() will be false.

Comment on attachment 411416 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411416&action=review >> Source/WebCore/html/ImageBitmap.cpp:84 >> + auto hostWindow = (document && document->view() && document->view()->root()) ? document->view()->root()->hostWindow() : nullptr; > > ImageBitmap is: > > Exposed=(Window,Worker), > > so this needs to work in Workers too, where scriptExecutionContext.isDocument() will be false. Yes - are there wpt or layout tests covering creation and use of ImageBitmaps on workers? If not, please add one.

<rdar://problem/70548262>

Created attachment 413003 [details] Patch

Created attachment 413006 [details] Patch

Comment on attachment 411416 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411416&action=review >>> Source/WebCore/html/ImageBitmap.cpp:84 >>> + auto hostWindow = (document && document->view() && document->view()->root()) ? document->view()->root()->hostWindow() : nullptr; >> >> ImageBitmap is: >> >> Exposed=(Window,Worker), >> >> so this needs to work in Workers too, where scriptExecutionContext.isDocument() will be false. > > Yes - are there wpt or layout tests covering creation and use of ImageBitmaps on workers? If not, please add one. Getting the the worker ImageBitmap to create a remote ImageBuffer will need many changes in different areas of the code: 1. We need to have a way to create an ImageBuffer without having to have a HostWindow, for example PlatformStrategies::createImageBuffer(). 2. The RemoteImageBufferProxy needs to be created on the main thread. WebProcess::ensureGPUProcessConnection() asserts RELEASE_ASSERT(RunLoop::isMain()); 3. Executing any operation on this RemoteImageBufferProxy has to be on the main thread also because it creates the WeakPtr<RemoteRenderingBackendProxy> on the main thread. So I would suggest making this bug for the document based ImageBitmap. Enabling the GPU rendering for the worker ImageBitmap can be tracked by a separate bug. These tests use ImageBitmaps on workers: imported/w3c/web-platform-tests/html/canvas/element/imagebitmap/createImageBitmap-in-worker-transfer.html imported/w3c/web-platform-tests/html/canvas/element/imagebitmap/createImageBitmap-serializable.html

See bug 218482

Comment on attachment 413006 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413006&action=review > Source/WebCore/ChangeLog:3 > + [GPU Process] Enable Document based ImageBitmap and OffscreenCanvas drawing to go through GPU "to go through GPU" -> "to use GPU Process rendering" > Source/WebCore/html/ImageBitmap.cpp:84 > + auto document = scriptExecutionContext.isDocument() ? &downcast<Document>(scriptExecutionContext) : nullptr; > + auto hostWindow = (document && document->view() && document->view()->root()) ? document->view()->root()->hostWindow() : nullptr; Does this code ever run in a Worker thread? If so, this is very non-threadsafe. > Source/WebKit/WebProcess/WebPage/WebPage.cpp:3808 > WebProcess::singleton().setUseGPUProcessForMedia(settings.useGPUProcessForMediaEnabled()); > + WebProcess::singleton().setUseGPUProcessForCanvas(m_shouldRenderCanvasInGPUProcess); Why are these two lines different?

Created attachment 413143 [details] Patch

Comment on attachment 413006 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413006&action=review >> Source/WebCore/ChangeLog:3 >> + [GPU Process] Enable Document based ImageBitmap and OffscreenCanvas drawing to go through GPU > > "to go through GPU" -> "to use GPU Process rendering" Title was changed. >> Source/WebCore/html/ImageBitmap.cpp:84 >> + auto hostWindow = (document && document->view() && document->view()->root()) ? document->view()->root()->hostWindow() : nullptr; > > Does this code ever run in a Worker thread? If so, this is very non-threadsafe. This code was cleaned up. >> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3808 >> + WebProcess::singleton().setUseGPUProcessForCanvas(m_shouldRenderCanvasInGPUProcess); > > Why are these two lines different? This part was removed from this bug and is being tracked by bug 218549.

Committed r269381: <https://trac.webkit.org/changeset/269381> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413143 [details].