Bug 217218 - Refactor VideoTextureCopier to be specific to a particular GraphicsContextGL and polymorphic to it
Summary: Refactor VideoTextureCopier to be specific to a particular GraphicsContextGL ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Mac Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on:
Blocks: webglgpup
  Show dependency treegraph
 
Reported: 2020-10-02 04:40 PDT by Kimmo Kinnunen
Modified: 2020-11-11 12:08 PST (History)
14 users (show)

See Also:


Attachments
Patch (127.09 KB, patch)
2020-10-28 05:30 PDT, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (127.03 KB, patch)
2020-11-10 23:44 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2020-10-02 04:40:59 PDT
Refactor VideoTextureCopier to be specific to a particular GraphicsContextGL and polymorphic to it

VideoTextureCopierCV
VideoTextureCopierGStreamer

Solves two problems:
1) No reason to create copier per media object
 -> If context is lost, media object caches stale converter?'
 -> Creates conversion shaders per media object, not per graphics context

2) Not possible to create remote version of this
 -> Need to refactor before being able to not use GraphicsContextGLOpenGL

Should be:

GraphicsContectGL::copyImageToPlatformTexture
GraphicsContextGL::copyVideoTextureToPlatformTexture

OR

GraphicsContectGL::getVideoTextureCopier
Comment 1 Radar WebKit Bug Importer 2020-10-02 04:41:24 PDT
<rdar://problem/69876433>
Comment 2 Kimmo Kinnunen 2020-10-26 06:26:20 PDT
This is needed for web process video drawing to web process webgl canvas after WebGL is using GraphicsContextGL interface.
This is needed for web process video drawing to remote webgl canvas (transitional feature).
Comment 3 Kimmo Kinnunen 2020-10-28 05:30:01 PDT
Created attachment 412520 [details]
Patch
Comment 4 Kimmo Kinnunen 2020-11-10 23:44:54 PST
Created attachment 413789 [details]
Patch
Comment 5 Dean Jackson 2020-11-11 01:09:41 PST
Comment on attachment 413789 [details]
Patch

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

> Source/WebCore/platform/graphics/GraphicsContextGL.h:1300
> +    virtual GraphicsContextGLCV* asCV() = 0;

I wonder if we should just expose copyPixelBufferToTexture on GraphicsContextGL directly, and have the context keep the GraphicsContextGLCV class for internal use?
Comment 6 Kimmo Kinnunen 2020-11-11 01:25:02 PST
Comment on attachment 413789 [details]
Patch

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

>> Source/WebCore/platform/graphics/GraphicsContextGL.h:1300
>> +    virtual GraphicsContextGLCV* asCV() = 0;
> 
> I wonder if we should just expose copyPixelBufferToTexture on GraphicsContextGL directly, and have the context keep the GraphicsContextGLCV class for internal use?

So the idea here is that there could be GraphicsContextGLGStreamer interface for GStreamer specific things, like GStreamer specific copyGStreamerImageToTexture(somespecificaparameters). 
Also if some day there is other platforms with other APIs, those could have their own interface.
This way we solve:
 - avoid having union of all needed parameters in the method calls. E.g. GStreamer has "image orientation" to be passed, where as Cocoa does not have that.
 - avoid having empty functions for platforms not needing a certain function
 - avoid having PlatformNativePixelBuffer typedef abstractions that may conflict (e.g. some platforms might have two such abstractions, other platform might have only one).
Comment 7 Kimmo Kinnunen 2020-11-11 01:28:06 PST
So wrt what we talked off-line about ExtensionsGL, GraphicsContextGLCV is the concept of ExtensionsGL done in a way (that I think) probably originally was sort-of envisioned.
Comment 8 Kimmo Kinnunen 2020-11-11 01:32:26 PST
And to elaborate:
In case we want a generic function call, and eventually we probably should want it, we should add copyImageBufferToTexture() call, and extend the WebCore::ImageBuffer abstraction so that native video objects can be wrapped into the ImageBuffer.
Similar to eventually we should make GraphicsContextGL to draw to WebCore::ImageBuffer, so that then we can draw WebGL context to WebGL context in generic way by drawing ImageBuffer to the WebGL context
Comment 9 EWS 2020-11-11 02:44:48 PST
Committed r269678: <https://trac.webkit.org/changeset/269678>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 413789 [details].
Comment 10 Dean Jackson 2020-11-11 12:08:48 PST
Sounds good!