Skip to content

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.

Sign up for GitHub

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

Merged
lpanaf merged 3 commits into from
Jan 15, 2026
Merged

fix the id error in retriangulation #230

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e436386
fix the id error in retriangulation
lpanaf Jan 12, 2026
43423ae
f
lpanaf Jan 12, 2026
59bdf39
Update glomap/io/colmap_converter.cc
lpanaf Jan 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions glomap/io/colmap_converter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,11 @@ void ConvertDatabaseToGlomap(const colmap::Database& database,
for (const auto& [camera_id, camera] : cameras) {
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)
Copy link

Copilot AI Jan 12, 2026

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.

Suggested change
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())

Copilot uses AI. Check for mistakes.
rig.SetRigId(++max_rig_id);
else
rig.SetRigId(camera_id);

rig.AddRefSensor(camera.SensorId());
rigs[rig.RigId()] = rig;
cameras_id_to_rig_id[camera_id] = rig.RigId();
Expand All @@ -326,7 +330,7 @@ void ConvertDatabaseToGlomap(const colmap::Database& database,
// For images without frames, initialize trivial frames
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;
Copy link

Copilot AI Jan 12, 2026

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.

Copilot uses AI. Check for mistakes.

CreateFrameForImage(Rigid3d(),
image,
Expand Down
Loading