Bug 200904

Summary: Implement accelerated video-to-texture upload path for ANGLE backend for WebGL on desktop
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: WebGLAssignee: Kenneth Russell <kbr>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, dino, enga, eric.carlson, ews-watchlist, glenn, graouts, jdarpinian, jer.noble, kainino, kondapallykalyan, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 202780    
Bug Blocks: 198948, 205734    
Attachments:
Description Flags
Patch dino: review+

Description Kenneth Russell 2019-08-19 17:09:33 PDT
The accelerated video-to-WebGL-texture upload path in Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp and related files is currently #ifdef'd out for the ANGLE backend for WebGL. From examining the code and after discussion with Geoff Lang and Jamie Madill from the ANGLE team, in order to implement it, it seems a few changes will be necessary: (these are only for desktop GL; more work will be needed for iOS and OpenGL ES):

1) A new ANGLE extension needs to be defined to expose the CGLContextObj and CGLPixelFormatObj associated with the EGLDisplay.

2) A new ANGLE extension needs to be defined to allow creation of a pbuffer from a client buffer which is actually a driver-level OpenGL texture ID.

3) TextureCacheCV will need to be changed to allocate a separate OpenGL context sharing resources with ANGLE's, and allocate the CVOpenGLTextureCacheCreate with that separate context. textureFromImage will get the native texture from the CVOpenGLTextureCache and import it into ANGLE's context as a pbuffer, then bind it to a texture ANGLE can reference. At that point, the rest of the code in VideoTextureCopierCV should be able to operate against the texture in ANGLE's context. Some of the shaders may need to be ported to use ANGLE's rectangular texture extensions.

This bug might end up being a meta-bug, since at least one ANGLE roll will be needed for the above changes.

There's also an IOSurface code path in VideoTextureCopierCV::copyImageToPlatformTexture; I haven't figured out whether that's usually taken and whether it's important to implement.
Comment 1 Kenneth Russell 2019-11-08 13:24:51 PST
Ran into a slight problem while hooking everything up. It looks like along with the OpenGL deprecation on macOS, CVOpenGLTextureCache has effectively been disabled. The call to CVOpenGLTextureCacheCreateTextureFromImage here:

https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/graphics/cv/TextureCacheCV.mm#L86

returns -6683, which is kCVReturnPixelBufferNotOpenGLCompatible ( /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreVideo.framework/Versions/A/Headers/CVReturn.h ).

It therefore looks like the EGL_ANGLE_opengl_texture_client_buffer proposed extension isn't actually needed, and the existing EGL_ANGLE_iosurface_client_buffer extension can be used for this purpose.
Comment 2 Kenneth Russell 2019-11-09 00:45:53 PST
Created attachment 383207 [details]
Patch
Comment 3 Kenneth Russell 2019-11-09 00:48:45 PST
The EGL_ANGLE_iosurface_client_buffer extension already did everything needed. After only a couple of compile and minor runtime issues, this "just worked"! Tested with the ANGLE backend on macOS with Mozilla's "Animating textures in WebGL" article. The new code path is being taken, and works!

James (jdarpinian@), could you please take this over if any updates to the code are needed? Thanks!
Comment 4 Alex Christensen 2019-11-09 13:53:46 PST
Comment on attachment 383207 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383207&action=review

cool

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:50
> +// Skip the inclusion of ANGLE's explicit context entry points for now.

for now?

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:757
> +    EGLSurface pbuffer = EGL_CreatePbufferFromClientBuffer(display, EGL_IOSURFACE_ANGLE, surface, m_context->platformConfig(), surfaceAttributes);

Is there a way we could return a std::unique_ptr with a custom deleter that to make the memory management model a little more sane?
Comment 5 Kenneth Russell 2019-11-19 18:38:01 PST
Comment on attachment 383207 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383207&action=review

>> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:50
>> +// Skip the inclusion of ANGLE's explicit context entry points for now.
> 
> for now?

We might later try to use those in order to avoid a thread-local storage lookup for every call into ANGLE. Other files with changes to use ANGLE have the same comment.

>> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:757
>> +    EGLSurface pbuffer = EGL_CreatePbufferFromClientBuffer(display, EGL_IOSURFACE_ANGLE, surface, m_context->platformConfig(), surfaceAttributes);
> 
> Is there a way we could return a std::unique_ptr with a custom deleter that to make the memory management model a little more sane?

These are the semantics of the standard EGL APIs. Yes, we could consider adding wrappers but could we do that as a later refactor to more of the code which uses ANGLE? There are multiple other places (like WebGLLayer.mm) which manually create and delete pbuffers which should be changed too.
Comment 6 Dean Jackson 2019-11-21 10:34:25 PST
Comment on attachment 383207 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383207&action=review

I'll land manually.

> Source/WebCore/ChangeLog:13
> +        Uses the EGL_ANGLE_iosurface_client_buffer extension to import
> +        IOSurfaces; reuses all of the existing shader and OpenGL code.

Not important to this patch, but I'll mention to anyone watching that we don't yet have a solution for this in the iOS simulator (where we cannot bind IOSurface to GL).

> Source/WebCore/platform/graphics/GraphicsContext3D.h:748
> +        // Necessary desktop OpenGL constants

Nit: End with period.
Comment 7 Dean Jackson 2019-11-21 15:58:36 PST
Committed r252741: <https://trac.webkit.org/changeset/252741>
Comment 8 Radar WebKit Bug Importer 2019-11-21 15:59:30 PST
<rdar://problem/57411766>