Skip to content

Made it explicit which realm each object gets created in#1023

Merged
toji merged 1 commit intoimmersive-web:masterfrom
asajeffrey:master
May 11, 2020
Merged

Made it explicit which realm each object gets created in#1023
toji merged 1 commit intoimmersive-web:masterfrom
asajeffrey:master

Conversation

@asajeffrey
Copy link
Copy Markdown

@asajeffrey asajeffrey commented May 11, 2020

Added text for each object creation saying which realm they get created in.


Preview | Diff

@asajeffrey
Copy link
Copy Markdown
Author

Fixes part of #497, in that every XR object now has a well-defined relevant realm.

@asajeffrey
Copy link
Copy Markdown
Author

Mostly this was routine, but there were a few odd cases:

  • When creating a XRWebGLLayer there's a corner case where the GL context comes from a different realm than the XR session.
  • When a user calls the XRRigidTransform constructor, we don't have an XR session, so we just have to use the current realm, which might not be associated with a session.

Copy link
Copy Markdown
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.

This is great! Thank you so much for taking the time to do this!

Comment thread index.bs
1. Let |transform| be a new {{XRRigidTransform}}.
1. If |position| is not a {{DOMPointInit}} initialize |transform|'s {{XRRigidTransform/position}} to <code>{ x: 0.0, y: 0.0, z: 0.0, w: 1.0 }</code>.
1. Let |transform| be a [=new=] {{XRRigidTransform}} in the [=current realm=].
1. Let |transform|'s {{XRRigidTransform/position}} be a [=new=] {{DOMPointReadOnly}} in the [=current realm=].
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Was worried that we lost something in this section, but since the constructor defaults line up with what we had originally specified this is a nice cleanup. Thanks!

@toji toji merged commit 08ed34a into immersive-web:master May 11, 2020
Comment thread index.bs
Comment thread index.bs
1. Set |result|'s {{XRRigidTransform/orientation}} to the quaternion that describes the rotation indicated by the top left 3x3 sub-matrix of |result|'s {{XRRigidTransform/matrix}}.
1. Set |result|'s {{XRRigidTransform/position}} to the vector given by the fourth column of |result|'s {{XRRigidTransform/matrix}}.
1. Let |result| be a [=new=] {{XRRigidTransform}} object in |realm|.
1. Set |result|'s {{XRRigidTransform/matrix}} to a [=new=] {{Float32Array}} in |realm|, the result of premultiplying |B|'s {{XRRigidTransform/matrix}} from the left onto |A|'s {{XRRigidTransform/matrix}}.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tbh it feels like this step should instead use an abstract [=matrix=] variable since XRRigidTransform lazy-inits matrices

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, not really worth it.

I wish I had the time to do the matrix infra spec so that this stuff wouldn't be necessary. We'd also be able to directly spec rigid transforms so there's no decomp-recomp going on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants