226422 2021-05-29 12:38:34 -0700 [WebXR] invalidateFramebuffer is not the same as clearing contents 2021-06-01 14:12:26 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 dino dino adachan ifernandez sam webkit-bug-importer oldest_to_newest 1765106 0 dino 2021-05-29 12:38:34 -0700 [WebXR] invalidateFramebuffer is not the same as clearing contents 1765107 1 dino 2021-05-29 12:40:06 -0700 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). 1765108 2 webkit-bug-importer 2021-05-29 12:42:44 -0700 <rdar://problem/78652351> 1765109 3 430104 dino 2021-05-29 12:46:48 -0700 Created attachment 430104 Patch 1765118 4 430104 sam 2021-05-29 13:41:36 -0700 Comment on attachment 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. 1765239 5 430104 dino 2021-05-30 12:43:48 -0700 Comment on attachment 430104 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. 1765240 6 dino 2021-05-30 12:45:29 -0700 Committed r278256 (238293@main): <https://commits.webkit.org/238293@main> 1765366 7 ifernandez 2021-05-31 02:32:18 -0700 > > 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 1765645 8 sam 2021-06-01 09:43:09 -0700 (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/. 1765752 9 ifernandez 2021-06-01 14:12:26 -0700 > > 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 430104 2021-05-29 12:46:48 -0700 2021-05-30 12:43:48 -0700 Patch bug-226422-20210530054646.patch text/plain 2928 dino U3VidmVyc2lvbiBSZXZpc2lvbjogMjc4MjQwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNzFmZDZkYmU5MDNlMGVk ZDU4ZmFmZTg3M2U4MGEzYjgxNTJlOTFhZi4uOGM1Njg4MjFmODkzMDMwNDk4OWNjYWRiZmVhYjU1 ZjBhYTc3YzUzNyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1IEBACisyMDIxLTA1LTI5ICBEZWFu IEphY2tzb24gIDxkaW5vQGFwcGxlLmNvbT4KKworICAgICAgICBbV2ViWFJdIGludmFsaWRhdGVG cmFtZWJ1ZmZlciBpcyBub3QgdGhlIHNhbWUgYXMgY2xlYXJpbmcgY29udGVudHMKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIyNjQyMgorICAgICAgICA8 cmRhcjovL3Byb2JsZW0vNzg2NTIzNTE+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgVGhlIFdlYlhSIHNwZWNpZmljYXRpb24gc2F5cyB0aGF0IGJ1ZmZl ciBjb250ZW50cyBtdXN0IGJlIGNsZWFyZWQKKyAgICAgICAgYmVmb3JlIGVhY2ggZnJhbWUuIFRo ZSBjb2RlIGN1cnJlbnRseSBkb2VzIGdsSW52YWxpZGF0ZUZyYW1lYnVmZmVyCisgICAgICAgIHRv IHdpcGUgdGhlIGF0dGFjaG1lbnRzLCB3aGljaCBpc24ndCB0aGUgc2FtZSB0aGluZyAoYW5kIHBy b2R1Y2VzCisgICAgICAgIGFuIGVycm9yIHNpbmNlIGl0IHRyaWVzIHRvIGludmFsaWRhdGUgYXR0 YWNobWVudHMgdGhhdCBkb24ndCBleGlzdCkuCisKKyAgICAgICAgUmVtb3ZlIHRoZSBjYWxsIHRv IGludmFsaWRhdGVGcmFtZWJ1ZmZlciBmb3IgdGhlIG1vbWVudCBzaW5jZSBpdAorICAgICAgICBj YXVzZXMgYSBnbEVycm9yLiBBZGQgYSBGSVhNRSB0byByZXBsYWNlIGl0IHdpdGggZXhwbGljaXQg Y2FsbHMKKyAgICAgICAgdG8gZ2xDbGVhci4gSSBleHBlY3QgbW9zdCBjb250ZW50IGRvZXMgdGhp cyBhbHJlYWR5LCBzbyBpdCBpcyB1bmxpa2VseQorICAgICAgICBhbnl0aGluZyB3aWxsIGJyZWFr LgorCisgICAgICAgICogTW9kdWxlcy93ZWJ4ci9XZWJYUk9wYXF1ZUZyYW1lYnVmZmVyLmNwcDoK KyAgICAgICAgKFdlYkNvcmU6OldlYlhST3BhcXVlRnJhbWVidWZmZXI6OnN0YXJ0RnJhbWUpOiBS ZW1vdmUgdGhlIGNhbGwgdG8KKyAgICAgICAgaW52YWxpZGF0ZUZyYW1lYnVmZmVyLgorCiAyMDIx LTA1LTI5ICBBbGFuIEJ1anRhcyAgPHphbGFuQGFwcGxlLmNvbT4KIAogICAgICAgICBbTEZDXVtU RkNdIERvIG5vdCBpbmNsdWRlIHZlcnRpY2FsIHNwYWNpbmcgd2hlbiByZXNvbHZpbmcgcGVyY2Vu dCBoZWlnaHQgZm9yIHRhYmxlIGNvbnRlbnQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL01v ZHVsZXMvd2VieHIvV2ViWFJPcGFxdWVGcmFtZWJ1ZmZlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9N b2R1bGVzL3dlYnhyL1dlYlhST3BhcXVlRnJhbWVidWZmZXIuY3BwCmluZGV4IGI5NDdkODIxNGY1 Y2Q0ZGZiNDIzYmJkMTY3MGMxYzVmNjhiMTAwMTIuLmZiZGIwYzlmOGRlYmM0Zjg4ZjVlYzUzMmIx Y2U1YzJiODkxOWVmNDggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvd2VieHIv V2ViWFJPcGFxdWVGcmFtZWJ1ZmZlci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvTW9kdWxlcy93 ZWJ4ci9XZWJYUk9wYXF1ZUZyYW1lYnVmZmVyLmNwcApAQCAtMTA0LDEyICsxMDQsMTIgQEAgdm9p ZCBXZWJYUk9wYXF1ZUZyYW1lYnVmZmVyOjpzdGFydEZyYW1lKGNvbnN0IFBsYXRmb3JtWFI6OkRl dmljZTo6RnJhbWVEYXRhOjpMYXkKICAgICAgICAgZ2wuYmluZEZyYW1lYnVmZmVyKEdMOjpGUkFN RUJVRkZFUiwgYm91bmRGQk8pOwogICAgIH0pOwogCi0gICAgZ2wuYmluZEZyYW1lYnVmZmVyKEdM OjpGUkFNRUJVRkZFUiwgbV9mcmFtZWJ1ZmZlci0+b2JqZWN0KCkpOworICAgIGdsLmJpbmRGcmFt ZWJ1ZmZlcihHcmFwaGljc0NvbnRleHRHTDo6RlJBTUVCVUZGRVIsIG1fZnJhbWVidWZmZXItPm9i amVjdCgpKTsKICAgICAvLyBodHRwczovL2ltbWVyc2l2ZS13ZWIuZ2l0aHViLmlvL3dlYnhyLyNv cGFxdWUtZnJhbWVidWZmZXIKLSAgICAvLyBUaGUgYnVmZmVycyBhdHRhY2hlZCB0byBhbiBvcGFx dWUgZnJhbWVidWZmZXIgTVVTVCBiZSBjbGVhcmVkIHRvIHRoZSB2YWx1ZXMgaW4gdGhlIHRhYmxl IGJlbG93IHdoZW4gZmlyc3QgY3JlYXRlZCwKKyAgICAvLyBUaGUgYnVmZmVycyBhdHRhY2hlZCB0 byBhbiBvcGFxdWUgZnJhbWVidWZmZXIgTVVTVCBiZSBjbGVhcmVkIHRvIHRoZSB2YWx1ZXMgaW4g dGhlIHByb3ZpZGVkIHRhYmxlIHdoZW4gZmlyc3QgY3JlYXRlZCwKICAgICAvLyBvciBwcmlvciB0 byB0aGUgcHJvY2Vzc2luZyBvZiBlYWNoIFhSIGFuaW1hdGlvbiBmcmFtZS4KLSAgICBzdGQ6OmFy cmF5PGNvbnN0IEdDR0xlbnVtLCAzPiBhdHRhY2htZW50cyA9IHsgR0w6OkNPTE9SX0FUVEFDSE1F TlQwLCBHTDo6U1RFTkNJTF9BVFRBQ0hNRU5ULCBHTDo6REVQVEhfQVRUQUNITUVOVCB9OwotICAg IGdsLmludmFsaWRhdGVGcmFtZWJ1ZmZlcihHTDo6RlJBTUVCVUZGRVIsIG1ha2VHQ0dMU3Bhbihh dHRhY2htZW50cy5kYXRhKCksIGF0dGFjaG1lbnRzLnNpemUoKSkpOworICAgIC8vIEZJWE1FOiBB Y3R1YWxseSBkbyB0aGUgY2xlYXJpbmcgKG5vdCB1c2luZyBpbnZhbGlkYXRlRnJhbWVidWZmZXIp LiBUaGlzIHdpbGwgaGF2ZSB0byBiZSBkb25lIGFmdGVyIHdlJ3ZlIGF0dGFjaGVkCisgICAgLy8g dGhlIHRleHR1cmVzL3JlbmRlcmJ1ZmZlcnMuCiAKICNpZiBVU0UoT1BFTkdMX0VTKQogICAgIGF1 dG8mIGV4dGVuc2lvbnMgPSByZWludGVycHJldF9jYXN0PEV4dGVuc2lvbnNHTE9wZW5HTEVTJj4o Z2wuZ2V0RXh0ZW5zaW9ucygpKTsK