[webkit-reviews] review granted: [Bug 183162] [WebVR] Convert VRPlatformDisplayInfo into a class : [Attachment 334687] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 27 07:01:21 PST 2018
Zan Dobersek <zan at falconsigh.net> has granted Sergio Villar Senin
<svillar at igalia.com>'s request for review:
Bug 183162: [WebVR] Convert VRPlatformDisplayInfo into a class
https://bugs.webkit.org/show_bug.cgi?id=183162
Attachment 334687: Patch
https://bugs.webkit.org/attachment.cgi?id=334687&action=review
--- Comment #3 from Zan Dobersek <zan at falconsigh.net> ---
Comment on attachment 334687
--> https://bugs.webkit.org/attachment.cgi?id=334687
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].
More information about the webkit-reviews
mailing list