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

Add new auto_mapper command #1167

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

anmatako
Copy link
Contributor

@anmatako anmatako commented Mar 9, 2021

The new auto_mapper command is a convenience command that combines mapper, triangulator, and bundle adjuster as needed based on the provided options. It can do the following:

  1. Standard mapper with optional point triangulator and rig BA if a rig config path is provided
  2. Hierarchical mapper followed by point triangulator and rig or standard BA
  3. Read existing reconstruction followed by point triangulator and rig or standard BA

Also, added new option skip_color_extraction in both auto_mapper and point_triangulator that avoids the call to Reconstruction::ExtractColorsForAllImages after triangulation since that's a pretty slow part of the process (single-threaded only) and is not necessary when we don't care about colors of the sparse point clouds

// frame002.png
// ...
//
std::vector<CameraRig> ReadCameraRigConfig(const std::string& rig_config_path,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No code changes here; just moved it from below to group all helper functions together.

@ahojnnes
Copy link
Contributor

ahojnnes commented Mar 9, 2021

I like the skip_color_extraction but I am not 100% sure about the logic in the auto_mapper command to be honest. I don't want to encourage people to rely too much on the hierarchical_mapper, which will not work too well when the input consists of internet images, etc.

@anmatako
Copy link
Contributor Author

I like the skip_color_extraction but I am not 100% sure about the logic in the auto_mapper command to be honest. I don't want to encourage people to rely too much on the hierarchical_mapper, which will not work too well when the input consists of internet images, etc.

I can see the concern about relying too much on auto_mapper. Has worked well enough for me so far since our datasets are more cohesive, but that may not generalize well enough to be broadly used.

I can leave the refactoring and the new skip_color_extraction for point triangulator and remove the auto_mapper command from this PR. Another alternative would be to make the default hierarchical mapper threshold really high (for the auto_mapper command only) to prevent accidental usage, such that auto_mapper would default to mapper unless options are explicitly set to enable the hierarchical_mapper.

Either approach is fine by me.

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.

None yet

2 participants