Skip to content

Precompute pinhole target coordinates in image warps - #4930

Closed
fringehush-gif wants to merge 1 commit into
colmap:mainfrom
fringehush-gif:codex/pinhole-warp-coordinates
Closed

fringehush-gif wants to merge 1 commit into
colmap:mainfrom
fringehush-gif:codex/pinhole-warp-coordinates

Conversation

@fringehush-gif

Copy link
Copy Markdown
Contributor

Pinhole target unprojection is separable, but the warp loop repeats both coordinate divisions and target-model dispatch for every pixel. Compute normalized x once per column and y once per row, retaining the original arithmetic order and generic path for other targets.

One O(warp-width) vector is added, at most 32 KiB per active warp in these tests. No codec, controller, threading or dependency changes.

Measurements

The same 28 real photo/mask pairs at every setting: recorded OPENCV/PINHOLE cameras, JPEG/PNG photographs, portrait/landscape dimensions and 8-bit grayscale PNG masks, from nine capture groups of one scene. Complete controller timings use mask-undistortion support from #4905; the optimization itself is independent of that feature. Default workers resolve to 24 on this host.

Pixel limit Workers Baseline s Patch s Time change
900 default (24) 2.422 2.154 -11.1%
2400 default (24) 2.244 2.059 -8.3%
4032 default (24) 2.573 2.266 -11.9%
Full worker and pixel-limit matrix
Pixel limit Workers Baseline s Patch s Time change Peak RSS MiB, baseline/patch
900 1 23.924 19.715 -17.6% 362.4 / 362.3
900 2 12.383 10.261 -17.1% 455.8 / 456.6
900 4 6.960 5.673 -18.5% 629.2 / 629.6
900 8 4.172 3.494 -16.3% 974.2 / 974.4
900 default (24) 2.422 2.154 -11.1% 1270.9 / 1273.6
2400 1 24.028 19.786 -17.7% 338.8 / 338.8
2400 2 12.260 10.068 -17.9% 402.9 / 402.2
2400 4 6.449 5.357 -16.9% 522.5 / 534.0
2400 8 3.793 3.194 -15.8% 732.7 / 747.0
2400 default (24) 2.244 2.059 -8.3% 1259.1 / 1230.6
4032 1 33.832 25.991 -23.2% 371.3 / 371.6
4032 2 17.510 13.216 -24.5% 463.2 / 463.9
4032 4 9.565 7.190 -24.8% 647.1 / 647.1
4032 8 5.391 4.194 -22.2% 990.5 / 996.6
4032 default (24) 2.573 2.266 -11.9% 1568.1 / 1548.6

Medians of two runs per version per setting, baseline/patch/patch/baseline order; cell order shuffled, exclusive workstation slot. Full controller timing includes decoding, encoding and buffered writes on warm ext4 inputs. All output JPEG/PNG hashes and sizes matched, including across worker counts at each pixel limit. These private inputs are not included; cold-storage and Windows-native performance remain untested.

At default workers, process CPU time (user + system) also fell by roughly 11–16%.

Concurrency is a larger throughput factor than the patch on this batch and increases peak RAM. Lower pixel limits are not automatically proportional reductions in work: strong downsizing invokes the existing full-resolution intermediate warp and filtered resize. No worker or pixel defaults are changed.

CPU-only, Ryzen 9 3900X, GCC 12 Release under WSL2/ext4. Hardware is tuned: CPU power-limited for thermals.

Validation

16/16 warp tests and 10/10 undistorter tests pass. The new differential test checks SIMPLE_RADIAL, OPENCV and FOV sources, grayscale/RGB, nearest/bilinear interpolation, shifted principal points and direct/intermediate warps. Clang-format, clang-tidy and diff checks pass.

AI disclosure

This was entirely vibe-coded by an LLM. I am not in a position to tell you if this is a useful contribution, unfortunately. Exercise caution accordingly.

Compute normalized x once per column and y once per row for pinhole
warp targets. Preserve the existing subtraction/division order and the
generic path for other target models.

Add byte-exact differential coverage for distorted source cameras,
grayscale/RGB, nearest/bilinear sampling and direct/intermediate warps.

Matched 28-pair full-controller medians with the default 24 workers:
  900 px: 2.422 -> 2.154 s (-11.1%).
  2400 px: 2.244 -> 2.059 s (-8.3%).
  4032 px: 2.573 -> 2.266 s (-11.9%).

CPU-only, Ryzen 9 3900X, GCC 12 Release under WSL2/ext4. Hardware is tuned: CPU power-limited for thermals.

AI disclosure:
This was entirely vibe-coded by an LLM. I am not in a position to tell you if this is a useful contribution, unfortunately. Exercise caution accordingly.
@sarlinpe

Copy link
Copy Markdown
Member

Thanks for identifying this bottleneck in WarpImageBetweenCameras!

While the speedup is worthwhile, special-casing PinholeCameraModel directly in src/colmap/image/warp.cc is not ideal for maintainability and generality:

  1. Breaks camera-model encapsulation: Reimplementing pinhole unprojection and indexing warp_target_camera.params[0..3] in warp.cc leaks low-level camera math and parameter layout outside src/colmap/sensor/models/.
  2. Narrow scope: It only accelerates CamFromImg when the target is PINHOLE in WarpImageBetweenCameras — leaving the other 18 camera models (e.g., SIMPLE_PINHOLE), per-pixel runtime dispatch on source_camera.ImgFromCam, and WarpImageWithHomographyBetweenCameras unoptimized.

I opened #4935 to address this more generally by hoisting CAMERA_MODEL_SWITCH_CASES to the scanline level (CamFromImgRow and ImgFromCamRow). It keeps all projection math and parameter indexing encapsulated in sensor/models/, unifies WarpImageBetweenCameras and WarpImageWithHomographyBetweenCameras, and accelerates both CamFromImg and ImgFromCam across all camera models (~34–62% faster on 12 MP warps).

@fringehush-gif

Copy link
Copy Markdown
Contributor Author

Glad I could help somehow.

Finding opportunities and fixing them outside of my little bubble is clearly not within my skill set.

Any general suggestions I can take with me, keeping in mind that I am not a seasoned developer? I'd like to contribute without machine gunning AI slop all over the place.

Developing on my local builds seemingly works, but generalizing that without fast forwarding the universe into an early heat death by adding complexity is way harder.

@sarlinpe

sarlinpe commented Nov 1, 2026

Copy link
Copy Markdown
Member

Consider auditing your changes before pushing them using an agent with a fresh context and the review skill in #4934. Contributions are always welcome :)

@sarlinpe sarlinpe closed this Nov 1, 2026
sarlinpe added a commit that referenced this pull request Nov 2, 2026
Supersedes #4930.

## Problem
`WarpImageBetweenCameras` and `WarpImageWithHomographyBetweenCameras`
call `Camera::CamFromImg` and `Camera::ImgFromCam` on every pixel. Both
go through the 19-way runtime `switch (model_id)` in
`sensor/models/runtime.h` (~24M dispatches for a 12 MP image), which
prevents the compiler from inlining the concrete camera model across a
scanline, hoisting row-invariant terms, constant-folding unit depth (`w
= 1`), and vectorizing the inner loop.

## Fix
Hoist the `CAMERA_MODEL_SWITCH_CASES` dispatch out of the inner `x` loop
into two scanline helpers (`CamFromImgRow` and `ImgFromCamRow`) in
`src/colmap/image/warp.cc`, and unify both warp functions into a single
`WarpBetweenCamerasImpl` (+30 net lines in `warp.cc`):
- Optimizes both `WarpImageBetweenCameras` and
`WarpImageWithHomographyBetweenCameras`, while deduplicating their
setup, scanline loop, interpolation dispatch, and rescaling.
- Accelerates both `CamFromImg` (target) and `ImgFromCam` (source) for
**all** camera models without leaking camera math or parameter indices
outside `sensor/models/`.
- Kept local to `warp.cc` rather than `sensor/models/runtime.h`: other
`CamFromImg` / `ImgFromCam` loops in the codebase
(`P3PEstimator::Residuals`, `HomographyMatrixRayEstimator::Residuals`,
`UndistortCamera`, `UndistortReconstruction`) operate on 100–10k points
(<0.5–3 ms) or 3D rays with `w != 1`, whereas scanline generation (`x +
0.5` with scalar `y`, unit depth `w = 1`, and separate input/output row
buffers) is significantly faster than generic in-place point-array
batching.

### Comparison with #4930
#4930 special-cases `PinholeCameraModel` in `WarpImageBetweenCameras` by
indexing `params[0..3]` directly. Compared to #4930, this PR:
- Keeps all camera parameter layout and projection math inside
`sensor/models/`.
- Works for all 19 camera models on both source and target cameras (not
only `PINHOLE` targets).
- Covers `WarpImageWithHomographyBetweenCameras` (stereo rectification)
as well.
- Delivers larger speedups even on `PINHOLE` targets by also eliminating
per-pixel dispatch and `w = 1` division in `ImgFromCam`.

## Benchmark
4032×3024 RGB image (12.2 MP), Release build (`g++ 15.2 -O3 -DNDEBUG`)
on Intel Xeon @ 2.20 GHz, best of 5 runs after warmup; all outputs are
bit-identical to `main`:

| Function | Cameras (`source -> target`) | Interpolation | `main` |
#4930 | **This PR** |
|---|---|---|---|---|---|
| `WarpImageBetweenCameras` | `OPENCV -> PINHOLE` | Bilinear | 1175.9 ms
| 908.7 ms (−22.7%) | **723.9 ms (−38.4%)** |
| `WarpImageBetweenCameras` | `OPENCV -> PINHOLE` | Nearest | 726.3 ms |
447.4 ms (−38.4%) | **274.4 ms (−62.2%)** |
| `WarpImageBetweenCameras` | `SIMPLE_RADIAL -> PINHOLE` | Bilinear |
1109.1 ms | 844.7 ms (−23.8%) | **660.3 ms (−40.5%)** |
| `WarpImageBetweenCameras` | `OPENCV -> SIMPLE_PINHOLE` | Bilinear |
1217.4 ms | 1180.9 ms (−3.0%) | **669.5 ms (−45.0%)** |
| `WarpImageWithHomographyBetweenCameras` | `OPENCV -> PINHOLE` |
Bilinear | 1159.3 ms | 1159.3 ms (0.0%) | **767.6 ms (−33.8%)** |

## Tests
- `Warp.CameraModelsMatchPerPixelProjection`
(`src/colmap/image/warp_test.cc`) checks bit-exact output against
per-pixel `Camera::CamFromImg` and `Camera::ImgFromCam` across `OPENCV
-> PINHOLE`, `SIMPLE_DIVISION -> FOV` (partial `ImgFromCam` failure),
and `SIMPLE_RADIAL -> EQUIRECTANGULAR` (partial `CamFromImg` failure)
for both interpolation modes, both direct and intermediate-resolution
warp paths, and both `WarpImageBetweenCameras` and
`WarpImageWithHomographyBetweenCameras`.
- `image/warp_test`, `image/undistortion_test`, `image/line_test`, and
`controllers/undistorters_test` pass.
@fringehush-gif
fringehush-gif deleted the codex/pinhole-warp-coordinates branch November 3, 2026 19:52
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.

2 participants