[Webkit-unassigned] [Bug 222270] Implement WebXR getViewport

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=222270

Sergio Villar Senin <svillar at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |svillar at igalia.com

--- Comment #3 from Sergio Villar Senin <svillar at igalia.com> ---
Comment on attachment 421197
  --> https://bugs.webkit.org/attachment.cgi?id=421197
Patch

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

Looking good, just a few changes here and there required.

> Source/WebCore/ChangeLog:9
> +        https://bugs.webkit.org/show_bug.cgi?id=222270

Duplicate entry

> Source/WebCore/ChangeLog:13
> +        * Modules/webxr/WebXRFrame.cpp: set WebXRView viewport modifiable value.

Nit, the comment is added in the line having the method name not in the line with the source file (unless it's a general comment for the file)

> Source/WebCore/Modules/webxr/WebXRSession.cpp:313
> +    // Only immersive sessions viewport scaling.

Maybe "support" is missing between "sessions" and "viewport"

> Source/WebCore/Modules/webxr/WebXRView.cpp:38
> +static constexpr double kMinViewportScale = 0.2;

Does it come from specs? If so please add a link for reference.

> Source/WebCore/Modules/webxr/WebXRViewport.h:50
> +    WebXRViewport(const IntRect&);

explicit

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:217
> +    // Compute the scaled viewport.

>From the specs it looks like this computation should be only done if 6. is true, so I guess you have to move it all to the if block above.

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:235
> +    else /// XREye::None

better write this as 

else {
 // XREye::None
 viewportRect.
}

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210222/da847c2f/attachment.htm>