Explicitly specify how the views array is populated#614
Conversation
toji
left a comment
toji
left a comment
There was a problem hiding this comment.
This looks really good! Left a few minor comments that I would appreciate if you could address, then I'm happy to merge it.
Reading through this does highlight to me that a structural change to the spec that @NellWaliczek previously suggested could help this read better. I'm happy to address that after this lands, though, since it would mostly involve moving text around rather than changing any verbiage you've got here.
|
Updated! (I'm okay with re-splitting out the initialize algorithm, or splitting out just the loop part if y'all want it) |
toji
left a comment
toji
left a comment
There was a problem hiding this comment.
Thanks! I'll give Nell a chance to approve prior to merging, but it looks good to me.
NellWaliczek
left a comment
NellWaliczek
left a comment
There was a problem hiding this comment.
Just nits left. I'm approving under the assumption that the nits are addressed :)
|
|
||
| Each {{XRSession}} has a <dfn for="XRSession">mode</dfn>, which is one of the values of {{XRSessionMode}}. | ||
|
|
||
| The {{XRSession/viewerSpace}} on an {{XRSession}} has a <dfn for="XRSession/viewerSpace">list of views</dfn>, which is a [=/list=] of [=view=]s corresponding to the views provided by the underlying device. |
There was a problem hiding this comment.
underlying device
Don't we have a variable that defines this?
| 1. Initialize |xrview|'s {{XRView/eye}} to |view|'s [=view/eye=] | ||
| 1. Initialize |xrview|'s {{XRView/projectionMatrix}} to |view|'s [=view/projection matrix=] | ||
| 1. Let |offset| be an {{XRRigidTransform}} equal to the [=view offset=] of |view| | ||
| 1. Set |xrview|'s {{XRView/transform}} property to the result of [=multiply transforms|multiplying=] the |offset| transform by the {{XRViewerPose}}'s {{XRPose/transform}} |
There was a problem hiding this comment.
Someone not me should confirm this shouldn't be an inverse...
There was a problem hiding this comment.
I'm gonna go out on a limb here and say that I'm pretty sure that this shouldn't be an inverse (unlike the handling of the bounds geometry, which did need to be.)
Gonna merge this now, and accept public shaming from @klausw if I'm wrong.
There was a problem hiding this comment.
I'm more concerned about the multiplication order than whether it should be an inverse, but I'm reasonably sure about both.
4 participants
Moved from #610 which got accidentally closed due to a branch being deleted
fixes #565
r? @NellWaliczek