Bug 227245

Summary: Add support for MTLSharedTextures in WebXR
Product: WebKit Reporter: Dean Jackson <dino>
Component: WebXRAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: adachan, ews-watchlist, kondapallykalyan, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
thorton: review+, ews-feeder: commit-queue-
EWS test
ews-feeder: commit-queue-
EWS test 2 none

Description Dean Jackson 2021-06-21 19:21:15 PDT 
Sometimes the IOSurface sent for WebXR is a MTLSharedTexture, which means it can't be treated like a regular single-plane IOSurface. Using the fix for 226690, we should be able to create a MTLTexture object from the IOSurface and bind that to a GL texture.
Comment 1 Radar WebKit Bug Importer 2021-06-21 19:21:25 PDT 
<rdar://problem/79591620>
Comment 2 Dean Jackson 2021-06-21 19:45:12 PDT 
Created attachment 431940 [details]
Patch
Comment 3 Ada Chan 2021-06-21 20:58:47 PDT 
Comment on attachment 431940 [details]
Patch

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

> Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:143
> +        auto size = data.surface->size();

Should we still do the size check for the single plane IOSurface case?

> Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:198
> +            gCGL->destroyPbufferAndDetachIOSurface(m_ioSurfaceTextureHandle);

Should we set m_ioSurfaceTextureHandle to null and m_ioSurfaceTextureHandleIsShared to false after?
Comment 4 Dean Jackson 2021-06-22 12:45:11 PDT 
Comment on attachment 431940 [details]
Patch

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

>> Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:143
>> +        auto size = data.surface->size();
> 
> Should we still do the size check for the single plane IOSurface case?

Yeah - good point. I'll put it back in.

>> Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:198
>> +            gCGL->destroyPbufferAndDetachIOSurface(m_ioSurfaceTextureHandle);
> 
> Should we set m_ioSurfaceTextureHandle to null and m_ioSurfaceTextureHandleIsShared to false after?

Agreed.

Ultimately we should have a class that wraps these handles and calls the correct destroyer. I started on that but got annoyed that I'd have to keep a reference to the GraphicsContextGL as well as the handle.
Comment 5 Dean Jackson 2021-06-25 13:41:18 PDT 
Created attachment 432288 [details]
EWS test
Comment 6 Dean Jackson 2021-06-25 13:56:25 PDT 
Created attachment 432289 [details]
EWS test 2
Comment 7 Dean Jackson 2021-06-25 15:07:09 PDT 
Committed r279298 (239174@main): <https://commits.webkit.org/239174@main>