-
Notifications
You must be signed in to change notification settings - Fork 401
Make boundsGeometry work relative to the effective origin #613
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
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.
Sorry about stomping on your branch previously! I'll be more careful about that in the future.
index.bs
Outdated
The <dfn attribute for="XRBoundedReferenceSpace">boundsGeometry</dfn> attribute describes the border around the {{XRBoundedReferenceSpace}}, which the user can expect to safely move within. | ||
Each {{XRBoundedReferenceSpace}} has a <dfn for="XRBoundedReferenceSpace">natural bounds geometry</dfn> describing the border around the {{XRBoundedReferenceSpace}}, which the user can expect to safely move within. The polygonal boundary is given as an array of {{DOMPointReadOnly}}s, which represents a loop of points at the edges of the safe space. The points describe offsets from the [=XRSpace/native origin=] in meters. Points MUST be given in a clockwise order as viewed from above, looking towards the negative end of the Y axis. The {{DOMPointReadOnly/y}} value of each point MUST be <code>0</code> and the {{DOMPointReadOnly/w}} value of each point MUST be <code>1</code>. The bounds can be considered to originate at the floor and extend infinitely high. The shape it describes MAY be convex or concave. | ||
|
||
The <dfn attribute for="XRBoundedReferenceSpace">boundsGeometry</dfn> attribute is an array of {{DOMPointReadOnly}}s such that each entry is equal to the entry in the {{XRBoundedReferenceSpace}}'s [=XRBoundedReferenceSpace/natural bounds geometry=] [=multiply transforms|multiplied=] by the {{XRReferenceSpace/originOffset}}. In other words, it provides the same border around the {{XRBoundedReferenceSpace}} relative to the [=XRSpace/effective origin=]. |
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.
Can @klausw give this a sanity check? The logic reads backwards to me (because we actually want to be negating the effects of the originOffset
) but with my attempt to define the same thing previously it was called out as changing the behavior of originOffset
.
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.
Oh, yeah, it should be multiplied by the inverse of the offset. It's hard to keep these things straight 😄
(need to figure out if this is a pre or post multiplication, brb doing some math)
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.
Oh, wait, these are points, there is no post multiplication
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.
Though this brings up a related problem, spec doesn't have a fixed algorithm for multiplying a transform against a vector.
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.
Updated, but I just use the word "multiplied" here. I can also use "transformed"
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.
If @klausw is happy with the language (and the other minor issues I raised are fixed), I'm good to approve
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 wrote:
Oh, wait, these are points, there is no post multiplication
It's not inherently impossible, so it needs to be specified. (AFAIK D3D interprets points as row vectors, and those would need postmultiplying transforms.)
Though this brings up a related problem, spec doesn't have a fixed algorithm for multiplying a transform against a vector.
We do have that now, https://immersive-web.github.io/webxr/#matrices specifically says the transform matrices "are applied to column vectors by premultiplying the matrix from the left", including a written-out example.
Overall I think this looks correct as written, apart from a few nitpicks:
multiplied by the {{XRRigidTransform/inverse}} of the {{XRReferenceSpace/originOffset}}.
I'd go with "premultiplied" here just to be explicit about it, though it's technically redundant since we have a definition for transforming points.
In other words, it provides the same border around the {{XRBoundedReferenceSpace}} relative to the [=XRSpace/effective origin=].
Technically you can't make a border around an XRSpace since they're infinite in extent. Maybe specifically say in {{XRBoundedReferenceSpace}} coordinates relative to the [=XRSpace/effective origin=]
?
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.
We do have that now, https://immersive-web.github.io/webxr/#matrices specifically says the transform matrices "are applied to column vectors by premultiplying the matrix from the left", including a written-out example.
Ah, I was looking for a linkable algorithm. But this is fine I guess.
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.
It's not inherently impossible, so it needs to be specified. (AFAIK D3D interprets points as row vectors, and those would need postmultiplying transforms.)
Yeah, I was assuming we were talking about column vectors only.
37c58c2
to
b61172a
Compare
index.bs
Outdated
@@ -851,9 +851,9 @@ The origin of a {{XRBoundedReferenceSpace}} MUST be positioned at the floor, suc | |||
|
|||
Note: Other XR platforms sometimes refer to the type of tracking offered by a {{bounded}} reference space as "room scale" tracking. An {{XRBoundedReferenceSpace}} is not intended to describe multi-room spaces, areas with uneven floor levels, or very large open areas. Content that needs to handle those scenarios should use an {{XRUnboundedReferenceSpace}}. | |||
|
|||
The <dfn attribute for="XRBoundedReferenceSpace">boundsGeometry</dfn> attribute describes the border around the {{XRBoundedReferenceSpace}}, which the user can expect to safely move within. | |||
Each {{XRBoundedReferenceSpace}} has a <dfn for="XRBoundedReferenceSpace">natural bounds geometry</dfn> describing the border around the {{XRBoundedReferenceSpace}}, which the user can expect to safely move within. The polygonal boundary is given as an array of {{DOMPointReadOnly}}s, which represents a loop of points at the edges of the safe space. The points describe offsets from the [=XRSpace/native origin=] in meters. Points MUST be given in a clockwise order as viewed from above, looking towards the negative end of the Y axis. The {{DOMPointReadOnly/y}} value of each point MUST be <code>0</code> and the {{DOMPointReadOnly/w}} value of each point MUST be <code>1</code>. The bounds can be considered to originate at the floor and extend infinitely high. The shape it describes MAY be convex or concave. |
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.
natural bounds geometry
Did you mean "native"?
index.bs
Outdated
|
||
The polygonal boundary is given as an array of {{DOMPointReadOnly}}s, which represents a loop of points at the edges of the safe space. The points describe offsets from the {{XRReferenceSpace}} origin in meters. Points MUST be given in a clockwise order as viewed from above, looking towards the negative end of the Y axis. The {{DOMPointReadOnly/y}} value of each point MUST be <code>0</code> and the {{DOMPointReadOnly/w}} value of each point MUST be <code>1</code>. The bounds can be considered to originate at the floor and extend infinitely high. The shape it describes MAY be convex or concave. | ||
The <dfn attribute for="XRBoundedReferenceSpace">boundsGeometry</dfn> attribute is an array of {{DOMPointReadOnly}}s such that each entry is equal to the entry in the {{XRBoundedReferenceSpace}}'s [=XRBoundedReferenceSpace/natural bounds geometry=] multiplied by the {{XRReferenceSpace/inverse}} of the {{XRReferenceSpace/originOffset}}. In other words, it provides the same border around the {{XRBoundedReferenceSpace}} relative to the [=XRSpace/effective origin=]. |
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.
{{XRReferenceSpace/inverse}}
Did you mean {{XRRigidTransform/inverse}}?
b61172a
to
d80759a
Compare
Oops, fixed. |
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
d80759a
to
95ce2ff
Compare
Rebased, addressed @klausw's feedback |
38db050
to
ebaebac
Compare
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 Manish and Klaus!
moved from #611 which was accidentally closed
r? @NellWaliczek