Some matrix clarifications #654
Conversation
|
While I was working on this I realized the fundamental issue was that we're thinking about these transforms as transforms "between spaces", which is what's ambiguous (since you can transform the origins of the spaces, or you can transform the coordinates of objects in the spaces). This uniformly makes us talk of these as positions and orientations floating in space, among other things. |
|
When I first wanted to do this PR it had a lot more things that needed doing, fortunately all the recent spec work has pinned down most of the ambiguous bits around this already. |
index.bs
Outdated
| </pre> | ||
|
|
||
| The <dfn attribute for="XRPose">transform</dfn> attribute describes the {{XRRigidTransform}} between two {{XRSpace}}. | ||
| The <dfn attribute for="XRPose">transform</dfn> attribute describes the position and orientation relative to the underlying {{XRSpace}}. |
There was a problem hiding this comment.
There's some potential confusion here since the getPose definition uses sourceSpace and destinationSpace in its arguments, and it's not obvious which of these is the "underlying" space.
How about using relative to the reference {{XRSpace}} here, and changing the getPose definition to match? Something like this?
XRPose? getPose(XRSpace space, XRSpace referenceSpace);
...
When the <dfn method for="XRFrame">getPose(|space|, |referenceSpace|)</dfn> method is invoked, the user agent MUST run the following steps:
...
1. [=Populate the pose=] of |space| in |referenceSpace| at the time represented by |frame| into |pose|.
Not sure what the best name is for the first argument. The proposal here is roughly following the explainer which uses getPose(xrSpace, xrReferenceSpace). Other options would be posedSpace, objectSpace, trackedSpace?
There was a problem hiding this comment.
Oh, I like this. This better clarifies the source/destination thing too.
|
Addressed |
| <div class="algorithm unstable" data-algorithm="get-pose"> | ||
|
|
||
| When the <dfn method for="XRFrame">getPose(|sourceSpace|, |destinationSpace|)</dfn> method is invoked, the user agent MUST run the following steps: | ||
| When the <dfn method for="XRFrame">getPose(|space|, |referenceSpace|)</dfn> method is invoked, the user agent MUST run the following steps: |
There was a problem hiding this comment.
I'm slightly concerned that using "referenceSpace" here may erroneously imply that the input can ONLY be an XRReferenceSpace. That said, naming is hard and there's not a lot a great options that I can think of here that address what you and Klaus were talking about. So maybe it's a non-issue?
There was a problem hiding this comment.
Yeah, that's fair. I think ultimately it's clear from the types, though.
shrug
There was a problem hiding this comment.
Hey, I jumped the gun on this and merged it before Nell had a chance to sign off, and she shares my concern about using referenceSpace as a name here. As a result I'm going to do a follow up to adjust the name again. I'll post a PR when I have something so we can make sure the name is agreeable all around.
There was a problem hiding this comment.
Sounds good. You can just revert a856300
|
[bikeshed]
How about baseSpace, coordinateSpace, or underlyingSpace?
[/bkeshed]
…On Fri, May 17, 2019, 15:28 Manish Goregaokar ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In index.bs
<#654 (comment)>:
> @@ -743,11 +743,11 @@ When the <dfn method for="XRFrame">getViewerPose(|referenceSpace|)</dfn> method
<div class="algorithm unstable" data-algorithm="get-pose">
-When the <dfn method for="XRFrame">getPose(|sourceSpace|, |destinationSpace|)</dfn> method is invoked, the user agent MUST run the following steps:
+When the <dfn method for="XRFrame">getPose(|space|, |referenceSpace|)</dfn> method is invoked, the user agent MUST run the following steps:
Sounds good. You can just revert a856300
<a856300>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#654?email_source=notifications&email_token=AAKT5X32WPAIAZW5BUTSICDPV4WRHA5CNFSM4HNRBD6KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZAUEAA#discussion_r285306670>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAKT5X4RCAZMS5OLFF2G533PV4WRHANCNFSM4HNRBD6A>
.
|
|
Great suggestions, thanks! |
|
This is a great change! Once we start to include offset spaces, anchor spaces, etc., I've found it far more straightforward to reason about the pose of one space "in" another. Aligning to that model within our API naming should help the developers using WebXR as well! At the risk of Brandon calling me out for aligning WebXR to OpenXR 😄, I'll put in my vote for We can then describe
|
|
Thanks all! PR to change the name is now up at #659 |
5 participants
We've been circling around whether transforms are "transforms from" or "transforms to" a bunch, see #582 (comment) , #649 (comment) for the latest examples.
These are some clarifications that may help.
I have a potential fix for #582 ready as well, but I want to land that separately since that actually adds new semantics as opposed to clarifying existing semantics.
r? @toji @NellWaliczek
cc @klausw @thetuvix