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

Per-eye distortion correction #377

Merged
merged 10 commits into from Feb 14, 2020
Merged

Per-eye distortion correction #377

merged 10 commits into from Feb 14, 2020

Conversation

@russell-taylor
Copy link
Contributor

russell-taylor commented Aug 15, 2019

Added the ability to override the per-HMD distortion parameters with per-eye distortion parameters to support a vendor that has custom calibration per eye on their display.

Decided to add a per-eye optional distortion override based on the following analysis:

  • Make the hmd distortion entry in always an array: - Would break all existing configuration files.
  • Make the hmd distortion entry into an optional array: + Only one place in the configuration file to specify distortion; + Backwards compatible with earlier config files; - May be mismatch between # eyes and # distortion (maybe more distortion than eyes...).
  • Add an optional override distortion entry in each eye description: + Would have as many places to specify as there are eyes, so no mismatch; + Backwards compatible with earlier config files; - Would provide two different places to specify the distortion (hmd and eye).

Verified that this was backwards compatible with existing config and that it works with different distortion per eye on a new config.

Using Gitlab private repos rather than sensics ones
…ration file that will override th global distortion correction description if it is present. This enables a different distortion parameter in each eye, which is needed for displays that have a custom calibration per eye.
Copy link
Member

rpavlik left a comment

Look reasonable enough to me. I'm assuming you've tested that it actually builds and runs :) I do like several of the changes i see (things moved to lambda captures instead of class members, etc), and fixing regressions is always good. Builds on Linux. Thanks!

@rpavlik rpavlik merged commit 56f9db6 into sensics:master Feb 14, 2020
@russell-taylor

This comment has been minimized.

Copy link
Contributor Author

russell-taylor commented Feb 14, 2020

Thanks for the review and merge! Indeed, of the changes in the request were verified on Windows, Linux, and Mac.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.