GraphicsContextGLCocoa manages EGL native displays manually bug 236030 > > EGL_GetPlatformDisplayEXT should key on all passed in attributes that affect the display behaviour. > The ANGLE-side fix is ready to be downstreamed: https://chromium-review.googlesource.com/c/angle/angle/+/3440780 Note: there are WK1 behavior where we need to iterate all the existing native displays and release thread on them. If we move away from manual native display handling, we must cache all the known displays for this use-case.
Created attachment 453961 [details] Patch
Created attachment 453962 [details] Patch
Comment on attachment 453962 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453962&action=review > Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.cpp:64 > +static HashSet<GCGLDisplay>& usedDisplays() Is any locking needed around this data structure? In WK1 mode GraphicsContextGLANGLE is used from multiple threads, though exclusively by construction. Slight concern that other ports may start using this in an unsafe manner - for example, for OffscreenCanvas on web workers.
Thanks for looking! (In reply to Kenneth Russell from comment #3) > Comment on attachment 453962 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=453962&action=review > > > Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.cpp:64 > > +static HashSet<GCGLDisplay>& usedDisplays() > > Is any locking needed around this data structure? In WK1 mode > GraphicsContextGLANGLE is used from multiple threads, though exclusively by > construction. WK1 is protected by Web thread lock. I've added comments for that in other cases, but declined here due to below.. > Slight concern that other ports may start using this in an > unsafe manner - for example, for OffscreenCanvas on web workers. It would be concerning, if ANGLE was multithreaded. Currently I think the verdict still is that ANGLE is single threaded, and as such the it would be more confusing than reassuring if ANGLE-using code would contain locking.
<rdar://problem/89957264>
> > Slight concern that other ports may start using this in an > > unsafe manner - for example, for OffscreenCanvas on web workers. BTW: previous patch that guarded offscreencanvas use with a lock in WebGLRenderingContextBase.cpp assumed that maybe GTK is interested in running OffscreenCanvas on top of WebGL on real OpenGL. And perhaps future GPUP WebGL.
Comment on attachment 453962 [details] Patch OK. We do hope to have full multithreading support in ANGLE's Metal backend in the not too distant future. In the meantime this looks fine. r+
(In reply to Kenneth Russell from comment #7) > Comment on attachment 453962 [details] > Patch > > OK. We do hope to have full multithreading support in ANGLE's Metal backend > in the not too distant future. In the meantime this looks fine. r+ Sure, but ANGLE frontend isn't thread-safe either. Like GetANGLEPlatformDisplayMap() just returning a static that is then mutated.
Committed r291035 (248209@main): <https://commits.webkit.org/248209@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 453962 [details].