-
Notifications
You must be signed in to change notification settings - Fork 51
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
start colmap export #131
start colmap export #131
Conversation
gtsfm/utils/io.py
Outdated
file_path = os.path.join(save_dir, "images.txt") | ||
with open(file_path, "w") as f: | ||
f.write("# Number of cameras: {}\n".format(gtsfm_data.number_images())) | ||
f.write("# Image list with two lines of data per image:") | ||
f.write("# IMAGE_ID, QW, QX, QY, QZ, TX, TY, TZ, CAMERA_ID, NAME") |
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.
Technically, camera ID and image ID can be different. How will we handle that? Maybe we need the camera-id image-id map in our pipeline too.
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.
i think that is a more conceptual change that will require some re-thinking. I added it to an issue #130
# TODO: assign unique indices to all keypoints (2d points) | ||
point2d_idx = 0 |
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.
These indices will be 0-n per image, right?
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.
I don't think so.
POINT2D_IDX defines the zero-based index of the keypoint in the images.txt file
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.
Actually, maybe you are right. It's a bit unclear.
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.
Maybe we should do a test run with COLMAP to check that
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.
@adisingh50 would you mind looking into this issue here about what the 2d keypoint/point index means?
Thanks for adding all this. Once this PR is merged into master, the point clouds we render will look much cooler now that we can render colors and camera poses. |
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 minor comments
Dump
images.txt
cameras.txt
points3D.txt
per the COLMAP file specification