[webkit-reviews] review denied: [Bug 226423] [WebXR] Provide a way to bind and unbind IOSurfaces to ANGLE Pbuffers : [Attachment 430106] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Dean Jackson <dino at apple.com>'s request
for review:
Bug 226423: [WebXR] Provide a way to bind and unbind IOSurfaces to ANGLE
Pbuffers
https://bugs.webkit.org/show_bug.cgi?id=226423

Attachment 430106: Patch

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




--- Comment #3 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 430106
  --> https://bugs.webkit.org/attachment.cgi?id=430106
Patch

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

> Source/WebCore/platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:651
> +    auto usageHintAngle = [&] () -> EGLint {
> +	   if (usage == PbufferAttachmentUsage::Read)
> +	       return EGL_IOSURFACE_READ_HINT_ANGLE;
> +	   return EGL_IOSURFACE_WRITE_HINT_ANGLE;
> +    }();

Never both?

> Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGL.h:574
> +    friend class WebXROpaqueFramebuffer;

This is a layering violation. Platform level concepts should not be aware of
things like WebXR. Why is this needed? Why not just make this public?

> Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGL.h:578
> +    enum class PbufferAttachmentUsage {
> +	   Read,
> +	   Write
> +    };

I would put this all on one line.