WebKit should query EGL_BIND_TO_TEXTURE_TARGET_ANGLE via eglGetConfigAttrib in order to determine whether to bind its IOSurfaces to 2D or rectangular textures, instead of hardcoding platform knowledge in GraphicsContextGLOpenGL::drawingBufferTextureTarget, GraphicsContextGLOpenGL::drawingBufferTextureTargetQuery, createPbufferAndAttachIOSurface, and other places. This will allow ANGLE's Metal backend to drop support for binding IOSurfaces to rectangular textures. OpenGL on macOS only supported the rectangular texture path, but Metal allows IOSurfaces to be bound to ordinary 2D textures. The support for rectangular textures that was added to ANGLE's Metal backend in WebKit's fork was incomplete, and broke a lot of unit and end-to-end tests. It's been removed upstream in ANGLE. Rolling upstream ANGLE back into WebKit is predicated on removing usage of rectangular textures with the Metal backend.
Taking this one into my queue :)
Created attachment 441265 [details] Patch
Comment on attachment 441265 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441265&action=review Good work overall. Please rebase and let's see green EWS runs before approving. > Source/WebCore/ChangeLog:6 > + Refactor GrraphicsContextGLCocoa to hold a reference to Grr -> Gr :) But: GraphicsContextGLOpenGLCocoa Also, please indent to match. > Source/WebCore/platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:473 > + EGL_GetConfigAttrib(m_displayObj, m_configObj, EGL_BIND_TO_TEXTURE_TARGET_ANGLE, &m_drawingBufferTextureTarget); Are we guaranteed to only call this when m_displayObj and m_configObj are valid? If there's any confusion then let's consider doing this upon display initialization and have any asserts possible about not calling this on a destroyed GraphicsContextGLOpenGL.
Created attachment 441412 [details] Patch
Comment on attachment 441412 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441412&action=review Looks good to me. Couple of small comments about the asserts. r+ > Source/WebCore/platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:471 > + ASSERT(!"Invalid enum returned from EGL_GetConfigAttrib"); This is the first I've seen this trick used to incorporate a message. Consider ASSERT_WITH_MESSAGE(false, ...). > Source/WebCore/platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:484 > + ASSERT(!"Invalid drawing target"); Same here. > Source/WebCore/platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:497 > + assert(!"Invalid drawing target"); Please use WebKit's ASSERT macro.
<rdar://problem/84324684>
Created attachment 441660 [details] Patch
Comment on attachment 441660 [details] Patch Still looks good. r+ again
Created attachment 441693 [details] Patch for landing
Committed r284438 (243200@main): <https://commits.webkit.org/243200@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441693 [details].