Camera model fixes and unit test overhaul by servantftransperfect · Pull Request #2202 · alicevision/AliceVision

servantftransperfect · 2026-10-12T11:01:44Z

[camera] [sfm] Camera model fixes and unit test overhaul

Description

Rewriting the camera unit tests to cover every camera model with and without Distortion and Undistortion objects revealed several bugs. Most of them were in the Jacobians and in how the two distortion objects interact. This PR fixes them, makes the bundle adjustment cost functions consistent for cameras with an Undistortion, and replaces the camera tests.

Several fixes change numerical results. The most visible changes are listed under Behavior changes below.

Fixes

Camera models

Focal length in mm (21078be34): setFocalLength and setInitialFocalLength converted millimeters to pixels using the image width, but the getters and CameraInit use the largest side. On portrait images, saving and reloading an .sfm file scaled the focal length by w/h.
Pinhole(w, h, K) (781ae7a05): the constructor stored the principal point of K directly as the offset, which shifted it by (w/2, h/2). It now subtracts the image center, as setK already did. This affects the Middlebury importer.
Rescaling with an Undistortion (cc6a47bad): rescale() did not update the Undistortion. A rescaled camera with a 3DE or radial undistortion therefore projected tens of pixels off. The undistortion is now rescaled too. Anamorphic desqueezes keep the pixel aspect ratio of the undistortion.
Equidistant image circle (b4290aaab): operator== ignored the circle, and rescale() left it at the original resolution.
Copying intrinsics (eb517f2d9): assign() and copy assignment shared the Distortion and Undistortion objects instead of cloning them, so modifying one camera modified the other.
Equirectangular Jacobians (92c91da93): the point and scale Jacobians ignored the pose, and the atan2 derivative used the wrong denominator. Also, getDerivativetoUnitSphereWrtPoint left a row uninitialized.
Jacobians of removeDistortion (5bcb2ea0b): the radial and fisheye models used a first-order approximation that was off by up to 14% for strong distortions. These are now exact, computed through the inverse function theorem. DistortionBrown used to throw here and now implements them, so Brown cameras can be refined with back-projection residuals.

Distortion and Undistortion (`afca83629`)

addDistortion and removeDistortion applied either the Distortion or the Undistortion, never both. They now compose them:
- addDistortion applies the Distortion in the camera plane, then the inverse Undistortion in pixels.
- removeDistortion applies the same steps in reverse order.
The analytic Jacobians only model the Distortion. They now use addDistortionOnly and removeDistortionOnly, so they are exact even when an Undistortion is present.
New transformProjectDistortionOnly and backProjectUnitDistortionOnly functions expose the projection that these Jacobians differentiate.
New Undistortion::getDerivativeUndistortWrtPoint.

Bundle adjustment (`fe5ca19d6`)

For cameras with an Undistortion, CostIntrinsicsProject and CostIntrinsicsLift evaluated the full projection, which includes the Undistortion, while their Jacobians only covered the Distortion. Since the Undistortion is constant during the adjustment, these cost functions now:

undistort the observation once at construction and compute the residual in undistorted space, which matches the Jacobians;
weight the projection residual by the inverse Jacobian of the undistortion, so the error is still in raw pixels. Where the undistortion folds and its Jacobian is singular, the residual falls back to the plain undistorted pixel error.

Behavior changes

Portrait images: focal lengths in mm set through the setters now match the ones read back. Existing .sfm files are unchanged, but values recomputed from mm will differ.
Cameras with both a Distortion and an Undistortion now project and back-project through both.
Bundle adjustment of cameras with an Undistortion now converges on the correct optimum instead of following inconsistent Jacobians.

Tests

Camera (cc8614888): the per-model tests (pinholeRadial, pinholeBrown, pinholeFisheye, pinholeFisheye1, equidistant) are replaced by tests that run over every camera model. The shared fixtures are in camera_test_common.hpp.
- intrinsicProjection: project/back-project round trips and rescaling.
- intrinsicDerivatives: every analytic Jacobian checked against numerical differentiation.
- intrinsicState: parameters, copies, comparisons, focal length and the Pinhole K matrix.
- cameraCommon: shared IntrinsicBase behavior.
- cameraUndistortImage: image undistortion.
- The distortion and undistortion tests are extended.
sfm (694a00bd4):
- costIntrinsics: the cost function Jacobians, the residual expressed in pixels, and the fallback for a degenerate undistortion.
- bundleAdjustment_undistortion: convergence with an Undistortion, with and without refining the Distortion.

All camera, cost function, bundle adjustment and panorama tests pass locally.

Copilot

🟡 Changes recommended

Fold detection accepts orientation-reversing Jacobians, and zero-sized matrix-constructor call sites remain incorrectly centered.

2 open findings

What changed in this PR

Fixes camera-model projection, distortion, Jacobian, copying, and rescaling behavior while consolidating coverage into model-wide tests.

Changes:

Composes distortion and undistortion consistently and aligns bundle-adjustment residuals with Jacobians.
Corrects focal conversion, matrix construction, equirectangular derivatives, copying, and rescaling.
Replaces model-specific tests with shared comprehensive suites.

File	Description
`src/aliceVision/sfm/CMakeLists.txt`	Registers new SfM tests.
`src/aliceVision/sfm/bundle/costIntrinsics_test.cpp`	Tests intrinsic costs and Jacobians.
`src/aliceVision/sfm/bundle/costfunctions/intrinsicsProject.hpp`	Evaluates projection costs in undistorted space.
`src/aliceVision/sfm/bundle/costfunctions/intrinsicsLift.hpp`	Aligns lifting with distortion-only Jacobians.
`src/aliceVision/sfm/bundle/bundleAdjustment_undistortion_test.cpp`	Tests bundle adjustment with undistortion.
`src/aliceVision/camera/Undistortion.hpp`	Declares point Jacobian API.
`src/aliceVision/camera/Undistortion.cpp`	Implements pixel-space point Jacobian.
`src/aliceVision/camera/undistortion_test.cpp`	Expands undistortion coverage.
`src/aliceVision/camera/pinholeRadial_test.cpp`	Removes superseded radial tests.
`src/aliceVision/camera/pinholeFisheye1_test.cpp`	Removes superseded fisheye1 tests.
`src/aliceVision/camera/pinholeFisheye_test.cpp`	Removes superseded fisheye tests.
`src/aliceVision/camera/pinholeBrown_test.cpp`	Removes superseded Brown tests.
`src/aliceVision/camera/Pinhole.hpp`	Fixes matrix construction and adds distortion-only projection.
`src/aliceVision/camera/Pinhole.cpp`	Implements consistent projection derivatives.
`src/aliceVision/camera/intrinsicState_test.cpp`	Tests state, copying, focal length, and matrices.
`src/aliceVision/camera/IntrinsicScaleOffsetDisto.hpp`	Adds deep assignment and composed distortion APIs.
`src/aliceVision/camera/IntrinsicScaleOffsetDisto.cpp`	Rescales pixel-based undistortion state.
`src/aliceVision/camera/IntrinsicScaleOffset.cpp`	Fixes portrait focal conversion.
`src/aliceVision/camera/intrinsicProjection_test.cpp`	Tests projection and rescaling across models.
`src/aliceVision/camera/intrinsicDerivatives_test.cpp`	Validates analytic Jacobians numerically.
`src/aliceVision/camera/Equirectangular.hpp`	Exposes distortion-only projection.
`src/aliceVision/camera/Equirectangular.cpp`	Corrects pose-aware Jacobians.
`src/aliceVision/camera/Equidistant.hpp`	Adds circle comparison and rescaling.
`src/aliceVision/camera/Equidistant.cpp`	Refactors projection and updates image-circle state.
`src/aliceVision/camera/equidistant_test.cpp`	Removes superseded equidistant tests.
`src/aliceVision/camera/DistortionRadial.cpp`	Implements exact inverse parameter Jacobians.
`src/aliceVision/camera/DistortionFisheye1.cpp`	Implements exact inverse parameter Jacobian.
`src/aliceVision/camera/DistortionFisheye.cpp`	Implements exact inverse parameter Jacobians.
`src/aliceVision/camera/DistortionBrown.cpp`	Implements Brown inverse Jacobians.
`src/aliceVision/camera/distortion_test.cpp`	Extends inverse-Jacobian validation.
`src/aliceVision/camera/CMakeLists.txt`	Registers consolidated camera tests.
`src/aliceVision/camera/cameraUndistortImage_test.cpp`	Adds image-undistortion tests.
`src/aliceVision/camera/cameraCommon_test.cpp`	Adds common camera API tests.
`src/aliceVision/camera/camera_test_common.hpp`	Provides shared model fixtures and numerical helpers.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The fused models mirror a projection through the Distortion alone; they also took cameras with a Distortion and an Undistortion, where upstream applied only the Distortion. alicevision/AliceVision#2202 composes the two and compares in undistorted space with a weight, so the fused model must step aside for any Undistortion. No test camera has one: the 41 views and the engine bay give the 2026-10-06 cameras (361ddd5cf663cfe6, af98cde31a5f153c). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Odd-dimension resizing shifts undistortion geometry, and derivative checks can silently accept non-finite Jacobians.

2 open findings

2 resolved since last review

🧠 Review effort: Balanced

+    const Vec2 offset = _pUndistortion->getOffset().cwiseProduct(Vec2(factorW, factorH));
+
+    _pUndistortion->setSize(static_cast<int>(_w), static_cast<int>(_h));


setFocalLength and setInitialFocalLength converted millimeters to pixels with the image width, while getFocalLength, getInitialFocalLength and CameraInit use the largest side. On portrait images, saving and reloading an .sfm file changed the focal length by a factor w/h. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

rescale() only updated the size, scale and offset of the intrinsic. The Undistortion object works in pixels and kept the size, center, offset and diagonal of the original image, so a rescaled camera with a 3DE or radial undistortion projected tens of pixels away. The undistortion size, offset and diagonal are now rescaled. When the factors cancel the pixel aspect ratio, as when imageProcessing desqueezes an anamorphic image, the undistortion is marked desqueezed and keeps its pixel aspect ratio, which the anamorphic model also uses as its lens squeeze. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

operator== ignored the image circle radius and center, so two equidistant cameras with different circles were reported equal. rescale() left the circle at the original resolution, so a rescaled camera projected hundreds of pixels away. The circle is now compared and rescaled (exact for uniform rescales, as a circle cannot become an ellipse). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

assign() and copy assignment used the implicit operator=, which copied the shared_ptr to the Distortion and Undistortion objects instead of cloning them like the copy constructor. Both intrinsics then shared, and modified, the same distortion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- getDerivativeTransformProjectWrtPoint3 differentiated with respect to the point before the pose was applied, ignoring the pose, and the derivative of atan2(sx, sz) divided by sx^2 + sy^2 instead of sx^2 + sz^2. - getDerivativeTransformProjectWrtScale also ignored the pose. - getDerivativetoUnitSphereWrtPoint filled row 0 twice and left row 2 uninitialized. - The distortion Jacobians returned one column of zeros although the model has no distortion parameter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

getDerivativeRemoveDistoWrtDisto of the radial and fisheye models differentiated the distortion coefficient at a fixed undistorted radius, ignoring how the undistorted point moves with the parameters: a first-order approximation, off by up to 14% for strong distortions. It is used by the bundle adjustment back-projection residuals (intrinsicsLift). It is now computed from the forward model, with p = add(q, theta) fixed: dq/dtheta = -(dadd/dq)^-1 * dadd/dtheta at q = removeDistortion(p). DistortionBrown threw for both inverse Jacobians; they are now implemented the same way, so Brown cameras can be refined with back-projection residuals. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…rtion only addDistortion/removeDistortion applied either the Distortion or the Undistortion object, never both. They now compose them: the Distortion in the camera plane, then the Undistortion in pixels (and the reverse for removal). The analytic Jacobians only model the Distortion object, so they now use addDistortionOnly/removeDistortionOnly, and the matching projection is exposed as transformProjectDistortionOnly/backProjectUnitDistortionOnly. Equidistant shares its projection code through cameraPlanePoint. Add Undistortion::getDerivativeUndistortWrtPoint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

For cameras with an Undistortion, CostIntrinsicsProject and CostIntrinsicsLift used project/backProjectUnit, which include the Undistortion, while their Jacobians only model the Distortion object. The Undistortion is constant during the adjustment, so the observation is undistorted once and compared with transformProjectDistortionOnly (lifted with backProjectUnitDistortionOnly), consistent with the Jacobians. The projection residual is weighted by the inverse Jacobian of the undistortion to keep an error in raw pixels, except where the undistortion folds and its Jacobian is singular. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Replace the per-model tests (pinholeRadial, pinholeBrown, pinholeFisheye, pinholeFisheye1, equidistant) with tests run over every camera model, sharing their fixtures in camera_test_common.hpp: - intrinsicProjection: project/backProject round trips, with and without Distortion and Undistortion - intrinsicDerivatives: analytic Jacobians against numerical ones, including the DistortionOnly projections - intrinsicState: parameters, copies and comparisons of the intrinsics - cameraCommon: shared IntrinsicBase behaviour - cameraUndistortImage: image undistortion Extend the distortion and undistortion tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- costIntrinsics: analytic Jacobians of CostIntrinsicsProject and CostIntrinsicsLift against numerical ones, the projection residual expressed in raw pixels, and the fallback when the undistortion Jacobian is singular - bundleAdjustment_undistortion: bundle adjustment of cameras with an Undistortion converges, also when refining the Distortion Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Use setK(K) after setting the image size instead, which stores the principal point relative to the image center. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

servantftransperfect added this to the 3.4.0 milestone Oct 12, 2026

servantftransperfect requested review from CedricThebault, cbentejac and fabiencastan October 12, 2026 11:01

servantftransperfect added scope:ci bugfix labels Oct 12, 2026

servantftransperfect changed the title ~~Rework Camera module unit tests and bugs removals~~ Camera model fixes and unit test overhaul Oct 12, 2026

servantftransperfect force-pushed the dev/cameraTests branch 2 times, most recently from 3fdf384 to 23d92e8 Compare October 13, 2026 12:37

CedricThebault requested a balanced review from Copilot October 13, 2026 13:47

Copilot started reviewing on behalf of CedricThebault October 13, 2026 13:48

View session

Copilot AI reviewed Oct 13, 2026

View reviewed changes

Comment thread src/aliceVision/camera/Pinhole.hpp Outdated

Comment thread src/aliceVision/sfm/bundle/costfunctions/intrinsicsProject.hpp Outdated

servantftransperfect force-pushed the dev/cameraTests branch from 23d92e8 to 9d9f8eb Compare October 14, 2026 07:16

CedricThebault requested a balanced review from Copilot October 15, 2026 07:42

Copilot started reviewing on behalf of CedricThebault October 15, 2026 07:45

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

servantftransperfect force-pushed the dev/cameraTests branch from 9d9f8eb to c54567a Compare October 16, 2026 08:57

CedricThebault previously approved these changes Oct 16, 2026

View reviewed changes

servantftransperfect dismissed CedricThebault’s stale review via 9ddb2e1 October 16, 2026 10:23

servantftransperfect force-pushed the dev/cameraTests branch from c54567a to 9ddb2e1 Compare October 16, 2026 10:23

Fabien Servant and others added 9 commits October 16, 2026 13:09

Fabien Servant and others added 2 commits October 16, 2026 13:09

[camera] Remove the Pinhole(w, h, K) constructor

b34db54

Use setK(K) after setting the image size instead, which stores the principal point relative to the image center. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

servantftransperfect force-pushed the dev/cameraTests branch from 9ddb2e1 to b34db54 Compare October 16, 2026 11:09

CedricThebault approved these changes Oct 16, 2026

View reviewed changes

servantftransperfect merged commit 3afba20 into develop Oct 16, 2026
5 checks passed

servantftransperfect deleted the dev/cameraTests branch October 16, 2026 11:13

Navigation Menu

Repository navigation

Camera model fixes and unit test overhaul - #2202

Camera model fixes and unit test overhaul#2202
servantftransperfect merged 11 commits into
developfrom
dev/cameraTests

servantftransperfect commented Oct 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		const Vec2 offset = _pUndistortion->getOffset().cwiseProduct(Vec2(factorW, factorH));

		_pUndistortion->setSize(static_cast<int>(_w), static_cast<int>(_h));

Conversation

servantftransperfect commented Oct 12, 2026

[camera] [sfm] Camera model fixes and unit test overhaul

Description

Fixes

Camera models

Distortion and Undistortion (afca83629)

Bundle adjustment (fe5ca19d6)

Behavior changes

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Distortion and Undistortion (`afca83629`)

Bundle adjustment (`fe5ca19d6`)