-
Notifications
You must be signed in to change notification settings - Fork 171
fix the id error in retriangulation #230
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This pull request addresses ID mismatch issues when converting old COLMAP databases to GLOMAP format. The fix ensures that when frame_id or rig_id don't exist in the database, they are assigned values consistent with image_id or camera_id respectively, rather than being randomly incremented.
Changes:
- Modified rig ID assignment logic to use camera_id when no existing rigs are present
- Modified frame ID assignment logic to use image_id when no existing frames are present
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
glomap/io/colmap_converter.cc
Outdated
| if (cameras_id_to_rig_id.find(camera_id) == cameras_id_to_rig_id.end()) { | ||
| Rig rig; | ||
| rig.SetRigId(++max_rig_id); | ||
| if (max_rig_id != 0) |
Copilot
AI
Jan 12, 2026
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.
ID collision bug: When max_rig_id equals 0 but a rig with ID 0 exists in the database, using camera_id directly can cause a collision. For example, if there's an existing rig with ID 0, and a new camera has camera_id=0, this will create a duplicate rig ID. The condition should check whether any rigs exist, not just whether max_rig_id is non-zero. Consider using rigs.empty() or always incrementing from max_rig_id.
| if (max_rig_id != 0) | |
| // If any rigs already exist, assign a new unique rig ID by incrementing | |
| // the current maximum. Otherwise, for the very first rig, reuse the | |
| // camera_id as the rig ID to preserve original behavior. | |
| if (!rigs.empty()) |
glomap/io/colmap_converter.cc
Outdated
| for (auto& [image_id, image] : images) { | ||
| if (image.frame_id == colmap::kInvalidFrameId) { | ||
| frame_t frame_id = ++max_frame_id; | ||
| frame_t frame_id = (max_frame_id > 0)? ++max_frame_id : image_id; |
Copilot
AI
Jan 12, 2026
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.
ID collision bug: When max_frame_id equals 0 but a frame with ID 0 exists in the database, using image_id directly can cause a collision. For example, if there's an existing frame with ID 0, and an image has image_id=0, this will create a duplicate frame ID. Consider checking frames.empty() or always incrementing from max_frame_id to avoid collisions.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
A heuristic fix for the id mismatch for the old colmap database. Previously, frame_id / rig_id are randomly assigned when they do not exist in database, now set it to be consistent with the image_id / camera_id.