Skip to content

Conversation

@brianpeiris
Copy link
Contributor

@brianpeiris brianpeiris commented Feb 28, 2018

Description:
#3327 changed the way we calculate tracked controller pose:

  • As a side effect, the code no longer updated the individual position and quaternion properties of the controller object. This prevents users and libraries (like networked-aframe) from reading those properties.
  • It also removed use of tracked-control's rotationOffset property, this lead to hand-controls with incorrectly rotated hand models when using the Oculus Rift and Touch, as evident in the tracked-controls example.
  • Lastly, it regressed standing matrix application for 3DoF controls -- i.e. standing matrix should only be applied for 6DoF controls, since the controller position is controlled by the arm model, which bases it off the head position.

Changes proposed:

  • Decompose the tracked object's matrix in to its individual components
  • Reintroduce use of the rotationOffset property.
  • Only apply the standing matrix if the controller is 6DoF
  • Add tests to cover these changes.

object3D.rotateZ(this.data.rotationOffset * THREE.Math.DEG2RAD);

object3D.updateMatrix();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't tested this PR in a Daydream/GearVR with a 3DoF controller yet. We disable matrixAutoUpdate in init, so I'm not sure how this code could have worked for those controllers without an explicit updateMatrix. Perhaps it was actually broken. I'll test with a 3DoF controller when I get a chance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed. Daydream controller does not update without this.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for testing on it.


// Apply transforms, if 6DOF and in VR.
if (vrDisplay) {
if (vrDisplay && pose.position !== null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dmarcos After testing in Daydream, I found that this change was necessary as well. Just wanted to ensure it makes sense to you as well.

Copy link
Member

Choose a reason for hiding this comment

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

It makes sense, we can just make position (vrDisplay && pose.position)

assertVec3(el.getAttribute('position'), [2, 4, 6]);
});

test('applies standing matrix transform', function () {
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the tests

@dmarcos
Copy link
Member

dmarcos commented Feb 28, 2018

Thank you @brianpeiris 👍

@dmarcos dmarcos merged commit 38c73a1 into aframevr:master Feb 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants