[webkit-reviews] review granted: [Bug 226422] [WebXR] invalidateFramebuffer is not the same as clearing contents : [Attachment 430104] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Dean Jackson <dino at apple.com>'s request
for review:
Bug 226422: [WebXR] invalidateFramebuffer is not the same as clearing contents
https://bugs.webkit.org/show_bug.cgi?id=226422

Attachment 430104: Patch

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




--- Comment #4 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 430104
  --> https://bugs.webkit.org/attachment.cgi?id=430104
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.