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

Added SimpleRadial camera model to colmap.cpp #73

Merged
merged 2 commits into from
Apr 12, 2024

Conversation

Jonathan-Gore
Copy link
Contributor

Added the SimpleRadial camera model to colmap.cpp from https://github.com/colmap/colmap/blob/0ea2d5ceee1360bba427b2ef61f1351e59a46f91/src/colmap/sensor/models.h#L235

Copied syntax found in SimplePinhole, copying fy=fx for the models with only one focal length parameter, like SimpleRadial.

I'm new to c++ so I'm not 100% sure how to test the code.

Added the SimpleRadial camera model to colmap.cpp from https://github.com/colmap/colmap/blob/0ea2d5ceee1360bba427b2ef61f1351e59a46f91/src/colmap/sensor/models.h#L235

Copied syntax found in SimplePinhole, copying fy=fx for the models with only one focal length parameter.
colmap.cpp Outdated
cam->fy = cam->fx;
cam->cx = readBinary<double>(camf);
cam->cy = readBinary<double>(camf);
cam->k = readBinary<double>(camf);
Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@Jonathan-Gore Jonathan-Gore Apr 11, 2024

Choose a reason for hiding this comment

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

Understood, I was incorrectly copying verbatim from the colmap parameter list. Would 'k1' be a reasonable substitute in this situation for what I designated as 'k'? I don't fully understand what the other parameters represent past the 'fx' and 'fy' focal length parameters.

Or would it be more of how 'f' = 'fy' and 'fx'? Would 'k' = 'k1', 'k2', and 'k3'?

@pierotofy
Copy link
Owner

Thanks for the PR @Jonathan-Gore ! See the "Build" section from the README to compile and test the changes.

@Jonathan-Gore
Copy link
Contributor Author

Jonathan-Gore commented Apr 11, 2024

On it

Changed k -> k1 so it would be a valid parameter.
@Jonathan-Gore
Copy link
Contributor Author

Jonathan-Gore commented Apr 12, 2024

Updated k -> k1 and successfully built the application!

Ran the newly built .exe on the banana dataset with both PINHOLE and SIMPLE_RADIAL camera models and both were successful!
simple_radial_splat.zip

@pierotofy
Copy link
Owner

Fantastic, thanks for testing and the useful addition! 🙏

@pierotofy pierotofy merged commit 915afbf into pierotofy:main Apr 12, 2024
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.

2 participants