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

Consensus #2

Closed
wants to merge 5 commits into from
Closed

Consensus #2

wants to merge 5 commits into from

Conversation

vadixidav
Copy link
Member

This implements traits from sample-consensus and adds a test that shows that it is possible to estimate the pose using arrsac. It also fixes the degenerate case checking that is being used with quickcheck so that it succeeds seemingly 100% of the time for use in CI testing (which is now enabled).

@vadixidav
Copy link
Member Author

This isn't ready yet. I just found that it should be possible to perform lambda twist with 2d camera points, but I am unable to reproduce this. I am currently working on making this possible. Trying to follow some direction from openMVG/openMVG#1500.

@mpizenberg
Copy link
Member

mpizenberg commented Dec 16, 2019

Hi @vadixidav, I just read quickly through the email. Thank you for the test fixes! Regarding sample consensus, I'm wondering if it would be better in a separate pnp crate that would depend on this one. I'll make a more thorough review when I find some time in the day or tomorrow.

@vadixidav
Copy link
Member Author

@mpizenberg As we talked about offline, it wasn't ready to merge yet. I fixed the issue I was encountering when trying to make it work with normalized image coordinates. The issue was that I assumed Vector2::to_homogeneous appended a 1.0 to the coordinate, when it actually appended a 0.0. Now it actually does work with normalized image coordinates. So long as the coordinates are passed as if the focal length is 1.0 and the origin is actually at the "camera center", it will perform lambda twist p3p with just the 2d coordinates.

While I was trying to debug my code, I also cross checked the entire codebase, including every potential typo, with the OpenMVG source. I found absolutely no differences, so that should help corroborate that the implementation has no errors in C++ -> Rust translation 👍.

If we should separate the sample consensus stuff into a separate crate, just let me know.

@mpizenberg
Copy link
Member

mpizenberg commented Dec 21, 2019 via email

@vadixidav
Copy link
Member Author

No worries. I am not in any rush. I will have some time off during the holidays so you will see me pop up doing some CV stuff here and there. The traits implemented here, which can be turned on with the consensus feature:

  • the pose
    • the pose implements Model, allowing the residual error of samples to be computed
  • lambda twist
    • the lambda twist algorithm has its own struct called Nordberg that implements the Estimator trait, allowing the estimation of a model from three points

Note that in the unit test, sample consensus allows the removal of estimated models that do not fit all the points. It emits all four solutions as-is and the sample consensus algorithm (in this case arrsac) takes care of returning only the correct model.

@vadixidav
Copy link
Member Author

I have refactored some of the types and logic into the cv-core crate. I am going to refactor this crate to utilize that and sample-consensus so that everything works together. Due to that, I will open a separate PR for that.

@vadixidav vadixidav closed this Dec 27, 2019
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.

None yet

2 participants