2048254 - Support binding IOSurfaces to textures on ANGLE/EGL for WebGL

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, task)

Description

[Jamie Nicol [:jnicol]]

On CGL we bind Mac IOSurfaces to GL textures using the MacIOSurface::BindTexImage helper function, which calls CGLTexImageIOSurface2D().

On ANGLE/EGL we need to use an alternative. The extension we can use is EGL_ANGLE_iosurface_client_buffer.

For running just WebGL on ANGLE/Metal (bug 2027951) we need to use this for SharedSurface_IOSurface, and in GLBlitHelper::BlitImage(MacIOSurface*). (To later support Webrender on ANGLE/Metal we'll need to additionally fix RenderMacIOSurfaceTextureHost and SurfacePoolCA)

There is a slight wrinkle in that the ANGLE extension does not support the GL_RGB_422_APPLE and GL_UNSIGNED_SHORT_8_8_REV_APPLE format and types that we use for kCVPixelFormatType_422YpCbCr8_yuvs and kCVPixelFormatType_422YpCbCr8FullRange format IOSurfaces.

It seems we use these for 8bit 4:2:2 formats in the MacIOSurfaceImage::SetData() path. I think we should be able to work around this by switching to using kCVPixelFormatType_422YpCbCr8BiPlanarVideoRange and kCVPixelFormatType_422YpCbCr8BiPlanarFullRange instead. Brad, Markus, can you think of any problems with doing so? Could we end up with IOSurfaces in the single-plane interleaved format some other way?

Comment 1

[Markus Stange [:mstange]]

I don't know. My guess is that the BiPlanar ones are recommended over the interleaved ones anyway, and that as long as we don't produce interleaved ones ourselves we won't see them. But I'm not really sure how these code paths are exercised. How do you get a video frame into WebGL? By passing a video element to the WebGL texture2D function?

Comment 2

[Brad Werth [:bradwerth]]

Getting rid of the single planar formats seems safe and sensible to me. I assume that the "negative" implcations of doing so are an insignificant bump in memory.

Comment 3

[Jamie Nicol [:jnicol]]

By passing a video element to the WebGL texture2D function?

Yep, precisely. And on arm64 macs you also need to set webgl.disable-DOM-blit-uploads=false to use the blit fast path (I'm not sure why it's true). Otherwise it does a readback and upload (also an important path to test though)

I have a patch to implement this and I've confirmed the readback, BlitHelper, and webrender/compositor paths all appear to work. So if we're confident(-ish) that we won't encounter the single-plane formats from another source, it sounds like I should go ahead and switch to the biplanar formats. See if there's any fallout, and then hopefully we don't have to worry about single-planar formats for ANGLE

Comment 4

[Jamie Nicol [:jnicol]]

Attachment: Bug 2048254 - Implement MacIOSurface::BindTexImage() for EGL/ANGLE. r?#gfx-reviewers

Add support for the EGL_ANGLE_iosurface_client_buffer extension to
GLLibraryEGL.

Add a helper function to MacIOSurface to query which texture target
IOSurfaces should be bound to. On CGL this simply returns
GL_TEXTURE_RECTANGLE, and on EGL we query the config's
EGL_BIND_TO_TEXTURE_TARGET_ANGLE attribute.

Reduce duplication in the internalFormat, format, and type calculation
in BindTexImage(), dropping support for the interleaved single plane
formats no longer used as of bug 2048665. Assert and return failure if
an unrecognised format is encountered, rather than just assuming BGRA.

For CGL contexts, call CGLTexImageIOSurface2D() as before. For EGL
contexts, ensure the ANGLE_iosurface_client_buffer extension is
supported, then create a PBuffer EGLSurface from the IOSurface, and
bind that to the provided texture.

Rather than return the EGLSurface to the caller and force it to manage
the surface's lifetime, we instead call eglDestroySurface()
immediately after binding to the texture. The spec ensures the surface
will remain alive as long as it is bound to the texture or until the
texture is destroyed.

Note the extension requires we always pass EGL_TEXTURE_RGBA as the
format. For the internalFormat and type, we mostly use the same values
as for CGL. The exception is BGRA surfaces, for which we use
GL_UNSIGNED_BYTE as the type, and either GL_BGRA or GL_RGB for the
internalFormat: GL_BGR is not a supported internalFormat as per the
extension spec, but ANGLE does the right thing when passed GL_RGB for
a BGRA IOSurface.

Comment 5

[Jamie Nicol [:jnicol]]

Attachment: Bug 2048254 - Allow SharedSurface_IOSurface to work with EGL/ANGLE. r?#gfx-reviewers

With the MacIOSurface::BindTexImage() changes in the previous patch,
this pretty much just works. We just need to ensure the correct
texture target is used, obtained by calling the new helper function
MacIOSurface::GetTextureTarget().

Comment 6

[Jamie Nicol [:jnicol]]

Attachment: Bug 2048254 - Make GLBlitHelper work with IOSurfaces when using EGL/ANGLE. r?#gfx-reviewers

Again we must ensure the correct texture target is used, and
additionally we must adjust the blit program's texMatrix uniforms
based on said target, to account for GL_TEXTURE_2D requiring
normalized texture coordinates and GL_TEXTURE_RECTANGLE requiring
unnormalized.

Additionally fixes a pre-existing bug using an incorrect texMatrix for
the chroma plane on 4:2:2 formats: only the width should be halved,
not the height.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0dccef5d2c79
https://hg.mozilla.org/integration/autoland/rev/f6c1d3ede362
Implement MacIOSurface::BindTexImage() for EGL/ANGLE. r=gfx-reviewers,lsalzman
https://github.com/mozilla-firefox/firefox/commit/49231c3888cd
https://hg.mozilla.org/integration/autoland/rev/e2cfb64e6c0b
Allow SharedSurface_IOSurface to work with EGL/ANGLE. r=gfx-reviewers,lsalzman
https://github.com/mozilla-firefox/firefox/commit/0b994f80be5a
https://hg.mozilla.org/integration/autoland/rev/b853bffade22
Make GLBlitHelper work with IOSurfaces when using EGL/ANGLE. r=gfx-reviewers,lsalzman

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/f6c1d3ede362
https://hg.mozilla.org/mozilla-central/rev/e2cfb64e6c0b
https://hg.mozilla.org/mozilla-central/rev/b853bffade22