Skip to content
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

Fix camera transforms in VR #13414

Merged
merged 3 commits into from
Mar 14, 2018
Merged

Fix camera transforms in VR #13414

merged 3 commits into from
Mar 14, 2018

Conversation

brianpeiris
Copy link
Contributor

Fixes #13236


} else {

standingMatrix.makeTranslation( 0, scope.userHeight, 0 );
Copy link
Contributor

Choose a reason for hiding this comment

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

Not using userHeight anymore is intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The diff is misleading. This if block was moved up to line 113

Copy link
Contributor

Choose a reason for hiding this comment

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

I see the standingMatrix and scope.userHeight is accounted for above. If position is null I think this else branch would reset the height back to 0 overriding the scope.userHeight, doesn’t it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you're right. Good catch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing the else would do I think

@brianpeiris
Copy link
Contributor Author

This also fixes the transform of objects attached to the camera. E.g. the crosshair in the webvr_cubes example.

@dmarcos
Copy link
Contributor

dmarcos commented Feb 24, 2018

I just tested on Vive and I cannot reproduce the culling issues. @brianpeiris 🏅

@mrdoob mrdoob added this to the r91 milestone Feb 24, 2018
@mrdoob mrdoob merged commit 0c866a8 into mrdoob:dev Mar 14, 2018
@mrdoob
Copy link
Owner

mrdoob commented Mar 14, 2018

Thanks!

@RSpace
Copy link

RSpace commented Mar 24, 2018

I just tested this fix with my own A-Frame / Three.js app where I started to see clipping issues for the right eye on Oculus Rift after updating to A-Frame 0.8.1 / Three.js r90.

When using latest version from mrdoob#dev where this has PR been merged in, I unfortunately still have exactly the same clipping issue for the right eye as before. So this PR hasn't fixed the issue for me, but I unfortunately lack the detailed knowledge of how transforms and culling works to fix it myself.

Anything I can do to help to fully resolve this issue?

@brianpeiris
Copy link
Contributor Author

@RSpace Do you have a link to repro?

@dmarcos
Copy link
Contributor

dmarcos commented Mar 28, 2018

@brianpeiris Your PR solved the problem of objects being incorrectly culled due to the bad construction of the camera matrix. The issue described in #13236 has been always present since the VR logic switched to use just the left eye frustum for culling. Any example should reproduce the problem where objects are incorrectly culled on the right side of the right eye field of view. More info on aframevr/aframe#3488

@brianpeiris
Copy link
Contributor Author

Ah, thanks for clearing that up.

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.

4 participants