[Win][WK2] Implement IPC::Semaphore to run WebGL in GPU process WebGL canvas tests are failing in WinCairo WK2 layout tests since r274327 enabled GPU process mode for WebKitTestRunner. webgl/1.0.3/conformance/canvas/canvas-test.html [ Failure ] webgl/1.0.3/conformance/canvas/draw-static-webgl-to-multiple-canvas-test.html [ Failure ] webgl/1.0.3/conformance/canvas/draw-webgl-to-canvas-test.html [ Failure ] webgl/1.0.3/conformance/canvas/to-data-url-test.html [ Failure ] webgl/1.0.3/conformance/context/context-attribute-preserve-drawing-buffer.html [ Failure ] webgl/1.0.3/conformance/textures/gl-pixelstorei.html [ Failure ] (snipped)
Created attachment 424814 [details] WIP patch
Comment on attachment 424814 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=424814&action=review Great stuff. I know it's WIP, just some ideas. > Source/WebKit/Platform/IPC/win/IPCSemaphoreWin.cpp:46 > + m_semaphoreHandle = CreateSemaphoreA(nullptr, 0, 1, getSemaphoreName(m_semaphoreNumber).data()); Would be great if you investigated if it's possible to create a nameless semaphore and then duplicate handle it to the other process. I think the support already might exist in AttachmentWin. > Source/WebKit/Platform/IPC/win/IPCSemaphoreWin.cpp:87 > + ReleaseSemaphore(m_semaphoreHandle, 1, nullptr); These could assert maybe that the result is somehow expected > Source/WebKit/Platform/IPC/win/IPCSemaphoreWin.cpp:119 > + CloseHandle(m_semaphoreHandle); need to clear the handle after this
Thank you very much. All sound good ideas. Will fix.
(In reply to Fujii Hironori from comment #0) > [Win][WK2] Implement IPC::Semaphore to run WebGL in GPU process > > WebGL canvas tests are failing in WinCairo WK2 layout tests since r274327 > enabled GPU process mode for WebKitTestRunner. I was wrong. Implementing IPC::Semaphore doesn't solve those test failures. They are different issue. I will fix those test failures in other bug ticket.
Created attachment 424994 [details] Patch
Comment on attachment 424814 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=424814&action=review >> Source/WebKit/Platform/IPC/win/IPCSemaphoreWin.cpp:87 >> + ReleaseSemaphore(m_semaphoreHandle, 1, nullptr); > > These could assert maybe that the result is somehow expected This ReleaseSemaphore can fail as ERROR_TOO_MANY_POSTS because the maximum count of semaphore is 1.
Created attachment 425140 [details] Patch
Comment on attachment 425140 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425140&action=review r=me Just have some comments on whether or not different guards might be more appropriate here. > Source/WebKit/Platform/IPC/StreamClientConnection.h:105 > +#if PLATFORM(COCOA) || PLATFORM(WIN) Maybe a HAVE(IPC_SEMAPHORE) or something like that? > Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:116 > +#if PLATFORM(COCOA) || PLATFORM(WIN) > #include "RemoteGraphicsContextGLProxy.h" > #endif Is there a better guard for this? Maybe an `ENABLE_WEBGL_IN_GPU_PROCESS`
Comment on attachment 425140 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425140&action=review Thank you very much. >> Source/WebKit/Platform/IPC/StreamClientConnection.h:105 >> +#if PLATFORM(COCOA) || PLATFORM(WIN) > > Maybe a HAVE(IPC_SEMAPHORE) or something like that? Will fix in a follow-up patch. >> Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:116 >> #endif > > Is there a better guard for this? Maybe an `ENABLE_WEBGL_IN_GPU_PROCESS` Will consider.
Comment on attachment 425140 [details] Patch Clearing flags on attachment: 425140 Committed r275463 (236123@main): <https://commits.webkit.org/236123@main>
All reviewed patches have been landed. Closing bug.
<rdar://problem/76245142>
*** Bug 223764 has been marked as a duplicate of this bug. ***