-
Notifications
You must be signed in to change notification settings - Fork 802
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
spherical camera #861
spherical camera #861
Conversation
… generalize to other cameras
- remove default error - leverage full range of spherical camera in triangulation
- fix jacobian for reprojection error of the spherical camera - need to improve DLT to fully leverage range of spherical camera
Awesome. I was hoping to add spherical camera support in the future (did a lot of learning about rectification and the double sphere model this past summer). @lucacarlone Would it be possible to reduce the noise by not having the extra unrelated commits? |
thanks @varunagrawal ! I suggest reviewing PRs in this order:
|
I'll take a look at #827, which seems to be blocked on my templating comment. I'll check it out on my side and then merge if I can't fix it. |
perfect- thanks! |
I removed @varunagrawal as reviewer. He (should have) no free cycles :-) |
# Conflicts: # gtsam/geometry/CameraSet.h # gtsam_unstable/slam/SmartProjectionPoseFactorRollingShutter.h # gtsam_unstable/slam/tests/testSmartProjectionPoseFactorRollingShutter.cpp
…sphericalCamera # Conflicts: # gtsam/slam/SmartProjectionFactorP.h # gtsam/slam/tests/smartFactorScenarios.h # gtsam/slam/tests/testSmartProjectionRigFactor.cpp # gtsam_unstable/slam/SmartProjectionPoseFactorRollingShutter.h # gtsam_unstable/slam/tests/testSmartProjectionPoseFactorRollingShutter.cpp
…sphericalCamera # Conflicts: # gtsam_unstable/slam/SmartProjectionPoseFactorRollingShutter.h
…sphericalCamera # Conflicts: # gtsam/slam/tests/testSmartProjectionRigFactor.cpp
@lucacarlone The CI error seems to be a missing |
woohoo! this helps a lot!! Feel free to push the fix if you want (I may not be able to get to this before the weekend) |
@ProfFan could you help out with CI? |
@ProfFan : I feel the problem is that there is a missing template in some function call that gets misinterpreted during compilation: If you look at the log, the error refers to using "[with CAMERA = gtsam::Cal3_S2]" which does not make sense since gtsam::Cal3_S2 is a CALIBRATION, not a CAMERA. |
@lucacarlone Let me take a look :) |
@lucacarlone , @ProfFan and I diagnosed and fixed the template substitution issue. Now there is only one remaining failing test:
on some platforms. Maybe two solutions are possible but the choice is not deterministic? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small nits left, otherwise: amazing!
Added spherical camera to model generic wide-field of view cameras.
Generalized triangulation functions to allow the development of arbitrary cameras.
Adjusted smart factors to remove pinhole assumption.
@dellaert : I suggest merging the PR "smart factors for arbitrary cameras" first. If possible, can you also add my student Marcus Abate [email protected] to GTSAM and add as reviewer here?