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

[bugzilla-daemon at webkit.org]

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

--- Comment #4 from Imanol Fernandez <ifernandez 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

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

Fixed

>> 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)

Fixed

>> Source/WebCore/Modules/webxr/WebXRSession.cpp:313
>> +    // Only immersive sessions viewport scaling.
> 
> Maybe "support" is missing between "sessions" and "viewport"

Yes, I wanted to add "support" work in that comment

>> 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.

It's an arbitrary value. I added some comments. Chrome is using around 0.05 now and Gecko around 0.2 IIRC.

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

done

>> 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.

The spec only updates the viewports when dirty while the patch was computing them on each getViewport call. I updated the patch to user dirty checking. We also need to handle canvasResize to do it correctly

>> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:235
>> +    else /// XREye::None
> 
> better write this as 
> 
> else {
>  // XREye::None
>  viewportRect.
> }

done

-- 
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/20210223/634fb439/attachment.htm>