Bug 226422 – [WebXR] invalidateFramebuffer is not the same as clearing contents

• Component: New Bugs
• Status: RESOLVED FIXED

[Dean Jackson]

[WebXR] invalidateFramebuffer is not the same as clearing contents

[Dean Jackson]

The WebXR specification says that buffer contents must be cleared before each frame. The code currently does glInvalidateFramebuffer to wipe the attachments, which isn't the same thing (and produces an error since it tries to invalidate attachments that don't exist).

[Radar WebKit Bug Importer]

<rdar://problem/78652351>

[Dean Jackson]

Created attachment 430104 [details]
Patch

[Sam Weinig]

Comment on attachment 430104 [details]
Patch

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

> Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:112
> -    gl.bindFramebuffer(GL::FRAMEBUFFER, m_framebuffer->object());
> +    gl.bindFramebuffer(GraphicsContextGL::FRAMEBUFFER, m_framebuffer->object());
>      // https://immersive-web.github.io/webxr/#opaque-framebuffer
> -    // The buffers attached to an opaque framebuffer MUST be cleared to the values in the table below when first created,
> +    // The buffers attached to an opaque framebuffer MUST be cleared to the values in the provided table when first created,
>      // or prior to the processing of each XR animation frame.
> -    std::array<const GCGLenum, 3> attachments = { GL::COLOR_ATTACHMENT0, GL::STENCIL_ATTACHMENT, GL::DEPTH_ATTACHMENT };
> -    gl.invalidateFramebuffer(GL::FRAMEBUFFER, makeGCGLSpan(attachments.data(), attachments.size()));
> +    // FIXME: Actually do the clearing (not using invalidateFramebuffer). This will have to be done after we've attached
> +    // the textures/renderbuffers.

We should also probably abstract this away from specific GL at some point so it can support WebGPU framebuffers as well.

[Dean Jackson]

Comment on attachment 430104 [details]
Patch

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

>> Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:112
>> +    // the textures/renderbuffers.
> 
> We should also probably abstract this away from specific GL at some point so it can support WebGPU framebuffers as well.

Agreed.

[Dean Jackson]

Committed r278256 (238293@main): <https://commits.webkit.org/238293@main>

[Imanol Fernandez]

>
> We should also probably abstract this away from specific GL at some point so
> it can support WebGPU framebuffers as well.

In theory the Opaque Framebuffer approach is kind of deprecated and the path forward is the WebXR Layers API, specifically the XRProjectionLayer for this use case. The UAs will provide "Opaque Textures" and the framebuffer creation complexity will me moved to JS and WebGL/WebGPU side.

So it's probably better to focus on that path since I dont expect that a WebGPU Opaque Framebuffer will be added to the spec

[Sam Weinig]

(In reply to Imanol Fernandez from comment #7)
> >
> > We should also probably abstract this away from specific GL at some point so
> > it can support WebGPU framebuffers as well.
> 
> In theory the Opaque Framebuffer approach is kind of deprecated and the path
> forward is the WebXR Layers API, specifically the XRProjectionLayer for this
> use case. The UAs will provide "Opaque Textures" and the framebuffer
> creation complexity will me moved to JS and WebGL/WebGPU side.
> 
> So it's probably better to focus on that path since I dont expect that a
> WebGPU Opaque Framebuffer will be added to the spec

Gotcha. Can you point me wo where XRProjectionLayer is being spec'd? I couldn't find it in https://immersive-web.github.io/webxr/.

[Imanol Fernandez]

> 
> Gotcha. Can you point me wo where XRProjectionLayer is being spec'd? I
> couldn't find it in https://immersive-web.github.io/webxr/.

It's spec'd here https://immersive-web.github.io/layers/#xrprojectionlayertype