◐ Shell
clean mode source ↗

Change XRRigidTransform inverse from a method to an attribute by toji · Pull Request #560 · immersive-web/webxr

@toji toji added the agenda

Request discussion in the next telecon/FTF

label

Mar 13, 2019

@toji

NellWaliczek

@toji toji mentioned this pull request

Apr 1, 2019

@toji toji deleted the inverse-attrib branch

April 1, 2019 23:00

bors-servo pushed a commit to servo/servo that referenced this pull request

Apr 4, 2019
Update XR code to use rigid transforms and new pose/transform stuff from the spec

This updates our XR code to use euclid's new [RigidTransform3D type](servo/euclid#328), which is more efficent and convenient to work with.

It additionally brings us up to speed with the spec:

 - `XRViewerPose` was made a subclass of `XRPose` (immersive-web/webxr#496)
 - `XRView.viewMatrix` was removed in favor of `XRRigidTransform.inverse.matrix` (immersive-web/webxr#531)
 - `XRRigidTransform.inverse` is an attribute (immersive-web/webxr#560)
 - `XRRigidTransform` now validates positions in its constructor (immersive-web/webxr#568)

Furthermore, it adds support for `XRRigidTransform.matrix`.

While fixing this I also noticed that our view matrix code was incorrect, we calculated view matrices as `pose.to_column_major_array()`, whereas it *should* be `pose.inverse().to_row_major_array()` (since Euclid uses row vectors, whenever the spec says it wants a column major array we should use `.to_row_major_array()` since all web specs implicitly use column vectors). For 3DOF devices poses are mostly rotations anyway, so the effective transpose behaved _like_ an inversion, but was incorrect.

This PR gets rid of `view.viewMatrix` anyway, however I felt like I should mention this discrepancy, since otherwise the replacement of `view.viewMatrix` with `view.transform.inverse.matrix` doesn't make sense

r? @jdm

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23159)
<!-- Reviewable:end -->

bors-servo pushed a commit to servo/servo that referenced this pull request

Apr 4, 2019
Update XR code to use rigid transforms and new pose/transform stuff from the spec

This updates our XR code to use euclid's new [RigidTransform3D type](servo/euclid#328), which is more efficent and convenient to work with.

It additionally brings us up to speed with the spec:

 - `XRViewerPose` was made a subclass of `XRPose` (immersive-web/webxr#496)
 - `XRView.viewMatrix` was removed in favor of `XRRigidTransform.inverse.matrix` (immersive-web/webxr#531)
 - `XRRigidTransform.inverse` is an attribute (immersive-web/webxr#560)
 - `XRRigidTransform` now validates positions in its constructor (immersive-web/webxr#568)

Furthermore, it adds support for `XRRigidTransform.matrix`.

While fixing this I also noticed that our view matrix code was incorrect, we calculated view matrices as `pose.to_column_major_array()`, whereas it *should* be `pose.inverse().to_row_major_array()` (since Euclid uses row vectors, whenever the spec says it wants a column major array we should use `.to_row_major_array()` since all web specs implicitly use column vectors). For 3DOF devices poses are mostly rotations anyway, so the effective transpose behaved _like_ an inversion, but was incorrect.

This PR gets rid of `view.viewMatrix` anyway, however I felt like I should mention this discrepancy, since otherwise the replacement of `view.viewMatrix` with `view.transform.inverse.matrix` doesn't make sense

r? @jdm

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23159)
<!-- Reviewable:end -->

bors-servo pushed a commit to servo/servo that referenced this pull request

Apr 4, 2019
Update XR code to use rigid transforms and new pose/transform stuff from the spec

This updates our XR code to use euclid's new [RigidTransform3D type](servo/euclid#328), which is more efficent and convenient to work with.

It additionally brings us up to speed with the spec:

 - `XRViewerPose` was made a subclass of `XRPose` (immersive-web/webxr#496)
 - `XRView.viewMatrix` was removed in favor of `XRRigidTransform.inverse.matrix` (immersive-web/webxr#531)
 - `XRRigidTransform.inverse` is an attribute (immersive-web/webxr#560)
 - `XRRigidTransform` now validates positions in its constructor (immersive-web/webxr#568)

Furthermore, it adds support for `XRRigidTransform.matrix`.

While fixing this I also noticed that our view matrix code was incorrect, we calculated view matrices as `pose.to_column_major_array()`, whereas it *should* be `pose.inverse().to_row_major_array()` (since Euclid uses row vectors, whenever the spec says it wants a column major array we should use `.to_row_major_array()` since all web specs implicitly use column vectors). For 3DOF devices poses are mostly rotations anyway, so the effective transpose behaved _like_ an inversion, but was incorrect.

This PR gets rid of `view.viewMatrix` anyway, however I felt like I should mention this discrepancy, since otherwise the replacement of `view.viewMatrix` with `view.transform.inverse.matrix` doesn't make sense

r? @jdm

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23159)
<!-- Reviewable:end -->

bors-servo pushed a commit to servo/servo that referenced this pull request

Apr 4, 2019
Update XR code to use rigid transforms and new pose/transform stuff from the spec

This updates our XR code to use euclid's new [RigidTransform3D type](servo/euclid#328), which is more efficent and convenient to work with.

It additionally brings us up to speed with the spec:

 - `XRViewerPose` was made a subclass of `XRPose` (immersive-web/webxr#496)
 - `XRView.viewMatrix` was removed in favor of `XRRigidTransform.inverse.matrix` (immersive-web/webxr#531)
 - `XRRigidTransform.inverse` is an attribute (immersive-web/webxr#560)
 - `XRRigidTransform` now validates positions in its constructor (immersive-web/webxr#568)

Furthermore, it adds support for `XRRigidTransform.matrix`.

While fixing this I also noticed that our view matrix code was incorrect, we calculated view matrices as `pose.to_column_major_array()`, whereas it *should* be `pose.inverse().to_row_major_array()` (since Euclid uses row vectors, whenever the spec says it wants a column major array we should use `.to_row_major_array()` since all web specs implicitly use column vectors). For 3DOF devices poses are mostly rotations anyway, so the effective transpose behaved _like_ an inversion, but was incorrect.

This PR gets rid of `view.viewMatrix` anyway, however I felt like I should mention this discrepancy, since otherwise the replacement of `view.viewMatrix` with `view.transform.inverse.matrix` doesn't make sense

r? @jdm

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23159)
<!-- Reviewable:end -->