Bug 213022 – [WebXR] Add a preliminary implementation of XRWebGLLayer

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sergio Villar Senin]

[WebXR] Add a preliminary implementation of XRWebGLLayer

[Sergio Villar Senin]

Created attachment 401538 [details]
Patch

[Sergio Villar Senin]

Ping reviewers

[Carlos Garcia Campos]

Comment on attachment 401538 [details]
Patch

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

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:66
> +

Extra line here

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:78
> +                // TODO: Initialize layer's resources or issue an OperationError.

Use FIXME instead of TODO

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:94
> +    // TODO: implement this

Ditto.

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:120
> +    m_isCompositionDisabled = m_session->mode() == XRSessionMode::Inline ? true : false;

I guess you don't need the ? true : false

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:125
> +        m_antialias = init.antialias;

This is duplicated, I would probably remove the previous one.

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:129
> +        computeRecommendedWebGLFramebufferResolution(recommendedSize);

computeRecommendedWebGLFramebufferResolution could return IntSize instead of using a parameter.

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:137
> +        // TODO: create a proper opaque framebuffer.

FIXME

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:207
> +    IntSize nativeSize;
> +    computeNativeWebGLFramebufferResolution(nativeSize);

Better use return value instead of parameter.

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:210
> +    IntSize recommendedSize;
> +    computeRecommendedWebGLFramebufferResolution(recommendedSize);

And here too. Unless those they really need to receive an existing IntSize to be modified.

[Sergio Villar Senin]

Comment on attachment 401538 [details]
Patch

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

Thanks for the review!

>> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:78
>> +                // TODO: Initialize layer's resources or issue an OperationError.
> 
> Use FIXME instead of TODO

OK

>> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:120
>> +    m_isCompositionDisabled = m_session->mode() == XRSessionMode::Inline ? true : false;
> 
> I guess you don't need the ? true : false

Stupid mistake indeed.

>> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:125
>> +        m_antialias = init.antialias;
> 
> This is duplicated, I would probably remove the previous one.

Right. I'll also fix the weird characters in the comment.

>> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:129
>> +        computeRecommendedWebGLFramebufferResolution(recommendedSize);
> 
> computeRecommendedWebGLFramebufferResolution could return IntSize instead of using a parameter.

Yes, compiler must be able to do the return value optimization so I agree it's better to return it.

[Sergio Villar Senin]

Committed r263346: <https://trac.webkit.org/changeset/263346>

[Radar WebKit Bug Importer]

<rdar://problem/64593229>