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

Combine field-of-vision and projection matrix, to fix Oculus distortions #134

Merged
merged 1 commit into from
May 30, 2022

Conversation

felipeerias
Copy link
Collaborator

This PR is another step along the road to fix distortions on the Oculus.

The main change is that the call to CameraEye::SetPerspective() receives a combination of the field-of-vision projection and the projection matrix provided by the Oculus SDK. Furthermore, this now happens on each frame.

This change is able to fix the distortions in WebXR that happen when the user sets the inter-eye distance to 1 or 3 and then starts a WebXR experience.

The only remaining problem happens when the user changes the inter-eye distance while in the middle of a WebXR experience. My suspicion is that Gecko gets the correct values when WebXR is launched, but those values are not updated afterwards.

@felipeerias felipeerias requested review from svillar and elima May 30, 2022 14:08
@felipeerias felipeerias changed the title Combine field-of-vision with the projection matrix provided by the device Combine field-of-vision and projection matrix, to fix Oculus distortions May 30, 2022
Copy link
Contributor

@elima elima left a comment

Choose a reason for hiding this comment

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

Looks good!
This is already a great improvement. Changing the PD mid-experience is arguably a rare use case.
Anyway, I suppose this is all we can do to fix this on Wolvic side.

Thanks!

Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

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

Really awesome change

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