-
Notifications
You must be signed in to change notification settings - Fork 401
Handle detached arrays #684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Given that there's now an explicit algorithm for obtaining XRRigidTransform.matrix it may also be worth fleshing it out by saying exactly how this matrix is to be obtained, the same way we do for XRRay already. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go ahead and file an issue to track expanding on the XRRigidTransform.matrix populating algorithm.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
filed #686 |
Fix algorithm for obtaining XRRay.matrix - copy-paste error introduced by PR immersive-web#684.
Fix algorithm for obtaining XRRay.matrix - copy-paste error introduced by PR #684.
@@ -1106,10 +1111,21 @@ The <dfn attribute for="XRRigidTransform">position</dfn> attribute is a 3-dimens | |||
|
|||
The <dfn attribute for="XRRigidTransform">orientation</dfn> attribute is a quaternion describing the rotational component of the transform. The {{XRRigidTransform/orientation}} MUST be normalized to have a length of <code>1.0</code>. | |||
|
|||
The <dfn attribute for="XRRigidTransform">matrix</dfn> attribute returns the transform described by the {{XRRigidTransform/position}} and {{XRRigidTransform/orientation}} attributes as a [=matrix=]. This attribute SHOULD be lazily evaluated. | |||
The <dfn attribute for="XRRigidTransform">matrix</dfn> attribute returns the transform described by the {{XRRigidTransform/position}} and {{XRRigidTransform/orientation}} attributes as a [=matrix=]. This attribute MUST be computed by [=XRRigidTransform/obtain the matrix|obtaining the matrix=] for the {{XRRigidTransform}}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Manishearth @toji Sorry for the post-merge comment here:
The "lazy evaluated" part originally introduced in #560 disappeared here. As it is viable to use a DualQuaternion as view transform directly, it would make sense to keep the matrix computation optional. It's just a nit-optimization for a currently less common use case, of course.
Or does "MUST be computed" imply it's computed on access?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, awesome, thanks for the clarification!
Implements Option 2 from #630 (comment)
fixes #630
r? @toji @NellWaliczek