Removing views in ImageMatchingMultiSfM is now an option.#2136
Removing views in ImageMatchingMultiSfM is now an option.#2136servantftransperfect wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new boolean parameter removeDuplicates to the ImageMatchingMultiSfM node and the main_imageMatching command-line tool, allowing users to control whether duplicate views from dataset A are removed if they exist in dataset B. This feature addition is accompanied by version updates across several .mg pipeline files. The feedback suggests wrapping a long command-line option definition in main_imageMatching.cpp to improve code readability and adhere to line length standards.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ("outputCombinedSfM", po::value<std::string>(&outputCombinedSfM)->default_value(outputCombinedSfM), | ||
| "Output file path for the combined SfMData file (if empty, don't combine)."); | ||
| "Output file path for the combined SfMData file (if empty, don't combine).") | ||
| ("removeDuplicates", po::value<bool>(&removeDuplicates)->default_value(removeDuplicates), "Do we remove views from A if they belong to B ?"); |
There was a problem hiding this comment.
The added command-line option line is extremely long (154 characters), which violates standard line length limits and reduces readability. Please wrap the description string to the next line to match the formatting of the other options.
("removeDuplicates", po::value<bool>(&removeDuplicates)->default_value(removeDuplicates),
"Do we remove views from A if they belong to B ?");
|
| desc.BoolParam( | ||
| name="removeDuplicates", | ||
| label="Remove duplicates", | ||
| description="Do we remove views from A if they belong to B ?", |
There was a problem hiding this comment.
| description="Do we remove views from A if they belong to B ?", | |
| description="If enabled, views that belong to both A and B will be ignored in A.", |
2 participants
In ImageMatchingMultiSfm, all views of A which were also in B were removed (not those of B if in A).
I added a flag (default to True) to be able to deactivate this heuristic.
P.S. This looks very messy to me. While it may had an interest to the author for its use case, this looks very ad hoc.