[Webkit-unassigned] [Bug 221225] Implement WebXR getViewerPose and getPose

[bugzilla-daemon at webkit.org]

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

--- Comment #19 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 420521
  --> https://bugs.webkit.org/attachment.cgi?id=420521
Patch

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

> Source/WebCore/Modules/webxr/WebXRBoundedReferenceSpace.cpp:78
> +    ASSERT(is<Document>(scriptExecutionContext()));

I guess you could do something like:
auto* document = downcast<Document>(scriptExecutionContext());
if (!document)
   ...

> Source/WebCore/Modules/webxr/WebXRBoundedReferenceSpace.h:40
>      WTF_MAKE_ISO_ALLOCATED(WebXRBoundedReferenceSpace);

Can WebXRBoundedReferenceSpace be set as final?

> Source/WebCore/Modules/webxr/WebXRFrame.cpp:39
> +

Unnecessary

> Source/WebCore/Modules/webxr/WebXRFrame.cpp:142
> +    if (limit) {

We could probably compute limit closer to its actual use.

> Source/WebCore/Modules/webxr/WebXRFrame.h:68
> +    explicit WebXRFrame(Ref<WebXRSession>&&, IsAnimationFrame);

explici not needed.

> Source/WebCore/Modules/webxr/WebXRFrame.h:69
> +    WebXRFrame(WebXRSession&, bool isAnimationFrame);

To remove?

> Source/WebCore/Modules/webxr/WebXRFrame.idl:37
> +    [MayThrowException, CallWith=Document] WebXRPose? getPose(WebXRSpace space, WebXRSpace baseSpace);

Should we use the Document here, or the document that created the WebXRSession?

> Source/WebCore/Modules/webxr/WebXRReferenceSpace.cpp:102
>      ASSERT(is<Document>(scriptExecutionContext()));

ditto here, we can probably simplify a tiny bit.

> Source/WebCore/Modules/webxr/WebXRReferenceSpace.cpp:126
> +

not needed

> Source/WebCore/Modules/webxr/WebXRReferenceSpace.h:53
> +    WebXRReferenceSpace(Document&, Ref<WebXRSession>&&, Ref<WebXRRigidTransform>&&, XRReferenceSpaceType);

Should be either protected or private, not public though probably since we have create static method

> Source/WebCore/Modules/webxr/WebXRSpace.h:86
> +    WebXRSession& m_session;

A ref here is probably not safe since WebXRViewerSpace is refcounted, it could potentially outlive its WebXRSession.
If we cannot make WebXRViewerSpace a unique_ptr, let's have a WeakPtr there.
The alternative would be for WebXRSpace to be RefCounted.
Or to introduce a WebXRSpaceBase which is not refcounted from which would derive WebXRViewerSpace and WebXRSpace.

-- 
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/20210217/8ab71c52/attachment.htm>