[webkit-reviews] review granted: [Bug 184265] [OpenVR][WebVR] Retrieve FrameData in WebVR's rAF : [Attachment 337490] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 9 04:43:32 PDT 2018


Zan Dobersek <zan at falconsigh.net> has granted Sergio Villar Senin
<svillar at igalia.com>'s request for review:
Bug 184265: [OpenVR][WebVR] Retrieve FrameData in WebVR's rAF
https://bugs.webkit.org/show_bug.cgi?id=184265

Attachment 337490: Patch

https://bugs.webkit.org/attachment.cgi?id=337490&action=review




--- Comment #13 from Zan Dobersek <zan at falconsigh.net> ---
Comment on attachment 337490
  --> https://bugs.webkit.org/attachment.cgi?id=337490
Patch

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

> Source/WebCore/Modules/webvr/VRFrameData.h:59
> +    double m_timestamp;

This too should be default-initialized to 0.

> Source/WebCore/Modules/webvr/VRPose.cpp:43
> +    float positionData[3] = { m_trackingInfo.position->x(),
m_trackingInfo.position->y(), m_trackingInfo.position->z() };

Nit: IMO it's cleaner to retrieve the reference of the m_trackingInfo.position
object and then access x/y/z values through that:
```
    auto& position = *m_trackingInfo.position;
    float positionData[3] = { position.x(), position.y(), position.z() };
```

> Source/WebCore/Modules/webvr/VRPose.cpp:62
> +    float orientationData[4] = { m_trackingInfo.orientation->x,
m_trackingInfo.orientation->y, m_trackingInfo.orientation->z,
m_trackingInfo.orientation->w };

Nit: similarly:
```
    auto& orientation = *m_trackingInfo.orientation;
    float orientationData[4] = { orientation.x, orientation.y, orientation.z,
orientation.w };
```


More information about the webkit-reviews mailing list