Bug 183162 – [WebVR] Convert VRPlatformDisplayInfo into a class

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

[Sergio Villar Senin]

[WebVR] Convert VRPlatformDisplayInfo into a class

[Sergio Villar Senin]

Created attachment 334686 [details]
Patch

[Sergio Villar Senin]

Created attachment 334687 [details]
Patch

[Zan Dobersek]

Comment on attachment 334687 [details]
Patch

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

> Source/WebCore/platform/vr/VRPlatformDisplay.h:49
> +    String displayName() const { return m_displayName; }

[1]: Should return a const reference.

> Source/WebCore/platform/vr/VRPlatformDisplay.h:50
> +    void setDisplayName(String&& displayName) { m_displayName = WTFMove(displayName); }

For later reference, this works as intended.

> Source/WebCore/platform/vr/VRPlatformDisplay.h:65
> +    FloatPoint3D eyeTranslation(Eye eye) const { return m_eyeTranslation[eye]; }

Ditto [1].

> Source/WebCore/platform/vr/VRPlatformDisplay.h:66
> +    void setEyeTranslation(Eye eye, FloatPoint3D&& translation) { m_eyeTranslation[eye] = translation; }

[2]: Rvalue references are only useful in these cases for classes that support move semantics, i.e. have a move constructor and/or move assignment operator. This works with String, but is not the case for FloatPoint3D, so I'd recommend using a simple const reference to the FloatPoint3D object as the parameter.

> Source/WebCore/platform/vr/VRPlatformDisplay.h:74
> +    FieldOfView eyeFieldOfView(Eye eye) const { return m_eyeFieldOfView[eye]; }

Ditto [1].

> Source/WebCore/platform/vr/VRPlatformDisplay.h:75
> +    void setEyeFieldOfView(Eye eye, FieldOfView&& fieldOfView) { m_eyeFieldOfView[eye] = WTFMove(fieldOfView); }

Ditto [2] for FieldOfView. WTFMove() doesn't help.

> Source/WebCore/platform/vr/VRPlatformDisplay.h:81
> +    RenderSize renderSize() const { return m_renderSize; }

Ditto [1].

> Source/WebCore/platform/vr/VRPlatformDisplay.h:82
> +    void setRenderSize(RenderSize&& renderSize) { m_renderSize = WTFMove(renderSize); }

Ditto [2] for RenderSize. WTFMove() doesn't help.

> Source/WebCore/platform/vr/VRPlatformDisplay.h:84
> +    void setPlayAreaBounds(FloatSize&& playAreaBounds) { m_playAreaBounds = playAreaBounds; }

Ditto [2] for FloatSize.

> Source/WebCore/platform/vr/VRPlatformDisplay.h:85
> +    std::optional<FloatSize> playAreaBounds() const { return m_playAreaBounds; }

Ditto [1].

> Source/WebCore/platform/vr/VRPlatformDisplay.h:87
> +    void setSittingToStandingTransform(TransformationMatrix&& transform) { m_sittingToStandingTransform = transform; }

Ditto [2] for TransformationMatrix.

> Source/WebCore/platform/vr/VRPlatformDisplay.h:88
> +    std::optional<TransformationMatrix> sittingToStandingTransform() const { return m_sittingToStandingTransform; }

Ditto [1].

[Sergio Villar Senin]

(In reply to Zan Dobersek from comment #3)
> Comment on attachment 334687 [details]
> > Source/WebCore/platform/vr/VRPlatformDisplay.h:66
> > +    void setEyeTranslation(Eye eye, FloatPoint3D&& translation) { m_eyeTranslation[eye] = translation; }
> 
> [2]: Rvalue references are only useful in these cases for classes that
> support move semantics, i.e. have a move constructor and/or move assignment
> operator. This works with String, but is not the case for FloatPoint3D, so
> I'd recommend using a simple const reference to the FloatPoint3D object as
> the parameter.

For some reason I don't know I thought they all had them (the move constructor). Yeah I'll replace them all with const references.

[Sergio Villar Senin]

Created attachment 334691 [details]
Patch

Patch for landing

[Sergio Villar Senin]

Committed r229089: <https://trac.webkit.org/changeset/229089>

[Radar WebKit Bug Importer]

<rdar://problem/37974126>