Repository navigation
Conversation
Commit 8321647 (2025-03-21) rewrote distort_brown_affin with scx applied to both axes and to the shear term: x' = scx*(x_d - sin(she)*y_d), y' = scx*cos(she)*y_d Until then liboptv, like the Tcl/Tk 3DPTV trafo.c it was ported from, used x' = scx*x_d - sin(she)*y_d, y' = cos(she)*y_d scx is the x-only scale factor of the El-Hakim (1986) / Gruen & Beyer (1986) affine pair; on both axes it duplicates cc. orientation.c's Jacobian (dy/dscx = 0) still assumes the original model. The same commit left correct_brown_affine_exact (used by dist_to_flat) inverting the original model, so flat_to_dist -> dist_to_flat missed by 0.04 mm at scx = 1.02. Every round-trip test used scx = 1, where the two models coincide. Restored the original affine step and made correct_brown_affin and correct_brown_affine_exact exact inverses of it. Decentering stays evaluated at one undistorted point (Brown 1971), as in 8321647. New tests in check_trafo.c: affine_matches_3dptv, scx_scales_x_only, affine_round_trip (scx = 1.02, she = 0.01 with radial and decentering). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The forward and inverse formulas are algebraically consistent and adequately covered by targeted tests.
0 open findings
What changed in this PR
Restores the x-only Brown affine scale model and aligns inverse transformations with it.
Changes:
- Applies
scxonly to the x-coordinate. - Corrects both inverse affine calculations.
- Adds regression and round-trip tests.
| File | Description |
|---|---|
liboptv/src/trafo.c |
Restores and exactly inverts the original affine model. |
liboptv/tests/check_trafo.c |
Adds affine-model and inverse regression tests. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Local run of the full C test suite (all 17 |
Problem
Commit 8321647 (2025-03-21) rewrote
distort_brown_affinso thatscxscales both axes and the shear term:Until then liboptv, like the Tcl/Tk 3DPTV
trafo.cit was ported from, used:scxis the x-only scale factor of the El-Hakim (1986) / Grün & Beyer (1986) affine pair. It compensated the pixel-clock vs. frame-grabber mismatch, which stretches only the scan-line direction. Applied to both axes it duplicatescc.orientation.c's Jacobian (∂y/∂scx = 0) still assumes the original model, so the fit and the projection disagree.The same commit left
correct_brown_affine_exact(used bydist_to_flat) inverting the original model.flat_to_dist→dist_to_flattherefore misses by 0.04 mm (≈ 8 px at 5 µm pixels) atscx = 1.02. Every round-trip test usedscx = 1, where the two models coincide, so nothing caught it.Change
distort_brown_affin.correct_brown_affinandcorrect_brown_affine_exactexact inverses of it, in both the initial guess and the iteration target.Tests
check_trafo.cgains three tests:affine_matches_3dptv: forward equals the Tcl/Tk formula.scx_scales_x_only: changingscxleaves y unchanged.affine_round_trip:scx = 1.02,she = 0.01, radial + decentering, through bothcorrect_brown_affinanddist_to_flat.Verified locally with a standalone harness compiled against
trafo.cover 5 (scx,she) combinations × 3 points:scx ≠ 1dist_to_flatround tripThe
checksuite itself was not run locally (nocheck/cmakeon this machine), so CI is the first full run. Note thattests/testing_fodder/track/cal/cam{1,2}.tif.addparhavescx≈ 1.0016–1.0020. Their tracking expectations predate 2025, so they should agree with the restored model, but CI needs to confirm that.The same fix is in openptv2 (
fix/brown-affine-scx-x-only), where the formula was also inlined in the tracking kernels.🤖 Generated with Claude Code