RESOLVED FIXED 224014
[Win][WK2] Implement IPC::Semaphore to run WebGL in GPU process
https://bugs.webkit.org/show_bug.cgi?id=224014
Summary [Win][WK2] Implement IPC::Semaphore to run WebGL in GPU process
Fujii Hironori
Reported 2021-03-31 13:42:38 PDT
[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)
Attachments
WIP patch (20.74 KB, patch)
2021-03-31 13:42 PDT, Fujii Hironori
no flags
Patch (24.21 KB, patch)
2021-04-02 00:49 PDT, Fujii Hironori
no flags
Patch (24.30 KB, patch)
2021-04-04 21:30 PDT, Fujii Hironori
no flags
Fujii Hironori
Comment 1 2021-03-31 13:42:55 PDT
Created attachment 424814 [details] WIP patch
Kimmo Kinnunen
Comment 2 2021-04-01 01:40:59 PDT
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
Fujii Hironori
Comment 3 2021-04-01 13:45:12 PDT
Thank you very much. All sound good ideas. Will fix.
Fujii Hironori
Comment 4 2021-04-01 14:45:45 PDT
(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.
Fujii Hironori
Comment 5 2021-04-02 00:49:37 PDT
Fujii Hironori
Comment 6 2021-04-04 21:29:32 PDT
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.
Fujii Hironori
Comment 7 2021-04-04 21:30:41 PDT
Don Olmstead
Comment 8 2021-04-05 10:07:37 PDT
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`
Fujii Hironori
Comment 9 2021-04-05 13:22:51 PDT
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.
Fujii Hironori
Comment 10 2021-04-05 17:50:26 PDT
Comment on attachment 425140 [details] Patch Clearing flags on attachment: 425140 Committed r275463 (236123@main): <https://commits.webkit.org/236123@main>
Fujii Hironori
Comment 11 2021-04-05 17:50:30 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 12 2021-04-05 17:51:38 PDT
Myles C. Maxfield
Comment 13 2021-04-06 10:14:18 PDT
*** Bug 223764 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.