[webkit-reviews] review granted: [Bug 225896] [WebXR] Attach IOSurface to WebXROpaqueFramebuffer : [Attachment 430116] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat May 29 17:07:13 PDT 2021
Sam Weinig <sam at webkit.org> has granted Dean Jackson <dino at apple.com>'s request
for review:
Bug 225896: [WebXR] Attach IOSurface to WebXROpaqueFramebuffer
https://bugs.webkit.org/show_bug.cgi?id=225896
Attachment 430116: Patch
https://bugs.webkit.org/attachment.cgi?id=430116&action=review
--- Comment #6 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 430116
--> https://bugs.webkit.org/attachment.cgi?id=430116
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=430116&action=review
> Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:189
> + if (m_ioSurfaceTextureHandle) {
> + auto gCGL = static_cast<GraphicsContextGLOpenGL*>(&gl);
> + gCGL->destroyPbufferAndDetachIOSurface(m_ioSurfaceTextureHandle);
> + }
Is there a smart pointer we can use for this? This seems extremely easy to get
wrong.
> Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:248
> + bool supportsPackedDepthStencil = true;
Seems like this should probably only be true is useDepthStencil is true? (all
the other #if sections check useDepthStencil here.
> Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:267
> + // Cap the maxiumum multisample count at 4. Any more than this is
likely overkill and will impact performance.
Is this observable and/or in the spec?
More information about the webkit-reviews
mailing list