Skip to content

Towards removing custom glomap image class#3805

Merged
ahojnnes merged 20 commits intomainfrom
user/jsch/glomap-image
Dec 15, 2025
Merged

Towards removing custom glomap image class#3805
ahojnnes merged 20 commits intomainfrom
user/jsch/glomap-image

Conversation

@ahojnnes
Copy link
Contributor

@ahojnnes ahojnnes commented Dec 7, 2025

First step in removing the custom class. Next steps will then remove the features/features_undist fields as well as refactor the current "misuse" of "IsRegistered/HasPose" for flagging images in the largest connected component.

Copy link
Contributor

@B1ueber2y B1ueber2y left a comment

Choose a reason for hiding this comment

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

LGTM. From reading the code there seem to be no behavior changes.

We can largely benefit from a benchmarking similar to the incremental mapper of colmap to ensure stability. Or alternatively, a faster option is that we can have some reference reconstructions on some scenes from the original glomap and check reconstruction statistics to ensure equivalence.

@ahojnnes
Copy link
Contributor Author

ahojnnes commented Dec 7, 2025

Agreed, see other PR.

@B1ueber2y
Copy link
Contributor

Next steps will then remove the features/features_undist fields as well as refactor the current "misuse" of "IsRegistered/HasPose" for flagging images in the largest connected component.

Does it make sense to refactor the misuse in this PR? After merging it would be difficult to locate as IsRegistered / HasPose will be mixed up in the codebase.

Base automatically changed from user/jsch/glomap-gravity-prior to main December 12, 2025 02:19
@ahojnnes
Copy link
Contributor Author

Next steps will then remove the features/features_undist fields as well as refactor the current "misuse" of "IsRegistered/HasPose" for flagging images in the largest connected component.

Does it make sense to refactor the misuse in this PR? After merging it would be difficult to locate as IsRegistered / HasPose will be mixed up in the codebase.

I am not sure this PR makes it more difficult. You can just grep all usage of HasPose() in the src/glomap subfolder to refactor.

@ahojnnes ahojnnes enabled auto-merge (squash) December 15, 2025 09:02
@ahojnnes ahojnnes merged commit 374128b into main Dec 15, 2025
14 checks passed
@ahojnnes ahojnnes deleted the user/jsch/glomap-image branch December 15, 2025 09:21
B1ueber2y added a commit that referenced this pull request Dec 27, 2025
…ation. (#3877)

Did some regression tests of global pipeline, and found a bug introduced
in #3805.

From this commit hash the global positioning throws when not all images
are successfully registered. This PR fixes the issue.
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.

3 participants