Repository navigation
Precompute pinhole target coordinates in image warps - #4930
fringehush-gif wants to merge 1 commit into
Conversation
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.
|
Thanks for identifying this bottleneck in While the speedup is worthwhile, special-casing
I opened #4935 to address this more generally by hoisting |
|
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. |
|
Consider auditing your changes before pushing them using an agent with a fresh context and the review skill in #4934. Contributions are always welcome :) |
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.
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.
Full worker and pixel-limit matrix
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