Skip to content

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

Merged
merged 2 commits into from
May 1, 2019

Conversation

Manishearth
Copy link
Contributor

moved from #611 which was accidentally closed

r? @NellWaliczek

Copy link
Member

@toji toji left a 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=].
Copy link
Member

@toji toji Apr 29, 2019

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.

Copy link
Contributor Author

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)

Copy link
Contributor Author

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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"

Copy link
Member

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

Copy link
Contributor

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=] ?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

@Manishearth Manishearth force-pushed the boundsgeometry-origin branch 3 times, most recently from 37c58c2 to b61172a Compare April 29, 2019 20:54
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.
Copy link
Member

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=].
Copy link
Member

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}}?

@Manishearth Manishearth force-pushed the boundsgeometry-origin branch from b61172a to d80759a Compare April 30, 2019 00:09
@Manishearth
Copy link
Contributor Author

Oops, fixed.

Copy link
Member

@NellWaliczek NellWaliczek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cwilso cwilso added this to the April 2019 milestone May 1, 2019
@Manishearth Manishearth force-pushed the boundsgeometry-origin branch from d80759a to 95ce2ff Compare May 1, 2019 19:05
@Manishearth
Copy link
Contributor Author

Rebased, addressed @klausw's feedback

@Manishearth Manishearth force-pushed the boundsgeometry-origin branch from 38db050 to ebaebac Compare May 1, 2019 19:10
Copy link
Member

@toji toji left a 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!

@toji toji merged commit d3ac4c4 into immersive-web:master May 1, 2019
@AdaRoseCannon AdaRoseCannon added the ed:spec Include in newsletter, spec change label May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ed:spec Include in newsletter, spec change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants