Repository navigation
SfM: Update visibility on feature transitions (CPU perf) - #5006
Merged
ahojnnes merged 1 commit intoNov 10, 2026
Merged
Conversation
## Summary Update the visibility pyramid when a feature gains its first triangulated correspondence or loses its last. Additional correspondences change support but leave the visible feature set unchanged, so repeating every pyramid-level update does unnecessary work. ## Changes - Put SetPoint and ResetPoint beside the existing zero/one visibility-count transitions, preserving occupied-cell scores for colliding features. - Extend the existing visibility-score and AddImage tests to cover repeated support, collisions, partial/final removal, underflow and rebuilding the manager. ## Background The repeated updates are inherited from fe8f82a, "Initial commit" (2016), in src/base/image.cc. The ObservationManager extraction, eaebe3c, "Add ObservationManager (colmap#2575)", retained them. ## Results On the composed-fork version, fixed public Add64/Delete32/Delete32 bookkeeping on a 16-image, 64-track, 120-pair fixture: | Platform | Before | After | | --- | ---: | ---: | | Linux/WSL | 1.395 ms | 0.640 ms | | Windows | 2.776 ms | 1.438 ms | Each value averages four fresh process means per arm, ABBAABBA, with two warmup and ten measured batches per process. Canonical inputs and outputs match within each platform. Whole-command Linux observations, including setup/oracles/serialization: median CPU 0.495 -> 0.443 s, median per-process peak RSS 414.50 -> 414.35 MiB, mean instructions 3.879 -> 3.782 billion. Windows median CPU 2.234 -> 2.211 s and median per-process peak working set 14.54 -> 14.33 MiB have overlapping process ranges. These broader counters describe the probe, not isolated pyramid operations. ## Validation - Clean base and candidate: all 15 maintained manager cases passed in each arm, without skips. The composed-fork version also passed 18 manager and two mapper cases on Linux and Windows. - Configured clang-tidy-18 checked observation_manager.cc in both arms and the maintained test, with no displayed source diagnostics; generated/header warnings were suppressed. Changed-line formatting passed. - Incremental qualification reused retained pristine-upstream support and rebuilt the two selected RANSAC-dependent owners needed for the newer headers. Final source, selected-link and runtime-provider checks passed. AI disclosure: Totally vibe coded
ahojnnes
approved these changes
Nov 9, 2026
ahojnnes
enabled auto-merge (squash)
November 9, 2026 16:07
fringehush-gif
added a commit
to fringehush-gif/colmap
that referenced
this pull request
Nov 10, 2026
## Summary Integrate the upstream visibility-transition optimization into the fork while retaining its observation-manager behavior and both lifecycle checks. ## Changes - Merge upstream 137f216, "SfM: Update visibility on feature transitions (CPU perf) (colmap#5006)", as the second parent; retain fork d896761 and its current production implementation. - Exercise the upstream partial and final observation-removal sequence and the fork registration and deregistration sequence with independently constructed state in the existing AddImage test. Preserve the remaining fork tests. - Attribute the inherited pattern to fe8f82a, "Initial commit", and eaebe3c, "Add ObservationManager (colmap#2575)". ## Validation - The selected Linux observation-manager suite passed 18/18 cases with no skips; the AddImage case executed both independent sequences. The changed test and current reconstruction owner were rebuilt, and selected archive members, source dependencies and runtime providers were checked. - Configured clang-tidy18 on the changed test: no displayed source diagnostics; generated/header warnings suppressed, consistent with retained baseline. Clang-format 18.1.8 left the affected AddImage range unchanged. AI disclosure: Totally vibe coded
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Update the visibility pyramid only when an image feature gains its first triangulated correspondence or loses its last. Extra supporting correspondences leave occupied cells unchanged, so repeating every pyramid-level update does unnecessary work.
This is a small inherited optimization, based directly on official COLMAP 3c1a1b9.
Changes
Background
The update pattern dates to fe8f82a, "Initial commit" (2016), in src/base/image.cc. It was retained by eaebe3c, "Add ObservationManager (#2575)".
Results
The source-equivalent transition change was measured in a composed-fork fixed bookkeeping probe: Add64/Delete32/Delete32 on 16 images, 64 tracks and 120 pairs.
Each table value is the arithmetic mean of four fresh process means per arm (ABBAABBA). Each process mean averages ten measured batches after two warmups. Canonical inputs and outputs agree within each platform. These are fixed bookkeeping timings, not reconstruction throughput measurements.
Validation
AI disclosure: Totally vibe coded