Splitting this out of #1072 rather than expanding that PR's scope. It is pre-existing, but #1072 makes it reachable: AP3P could not run inside solve_pnp_ransac at all before that fix, so nobody could hit this.
Behaviour
RansacParams::default().refine == true. With an AP3P kernel, the non-minimal refit over all inliers uses EPnP (a minimal solver cannot consume more than its exact correspondence count — this is the standard minimal-kernel / non-minimal-refit split and is correct in principle).
But in the small-object / long-range regime EPnP is ill-conditioned, so the refit can return a worse pose than the AP3P consensus it replaced.
Measured on a 20 cm object at 6 m:
refine |
reproj rmse |
dR |
dt |
false |
5.8e-5 |
~0 |
~0 |
true |
0.81 px |
0.028 |
0.29 m |
LM refinement does not rescue it — this is consistent with the existing epnp_long_range_regression_documented test, which pins >10 px with LM enabled, and with the epnp.rs module docs, which name "prefer P3P-RANSAC when N >= 4" as the workaround for exactly this regime.
So a caller who selects AP3P specifically to escape EPnP's long-range behaviour silently gets an EPnP pose back, and the good AP3P pose is discarded.
Why it was not fixed in #1072
Every candidate fix is a behaviour change beyond that PR's six review findings:
- Score the refit against
best_pose and keep the better one. Most conservative, but it means refine no longer guarantees the refined pose is returned — callers relying on that would see a silent semantic shift.
- Do not refit with EPnP when the kernel is AP3P. Loses the noise-averaging benefit of consuming all inliers, which is the whole point of the refit in the well-conditioned case.
- Refit with
refine_pose_lm seeded from best_pose over the inliers (it accepts n >= 3), instead of routing through EPnP's control-point construction at all. Probably the right answer, but it is a new refit path that needs its own validation across both regimes.
Option 3 looks best but deserves measurement rather than assertion, in both the long-range and the ordinary regime.
Note
crates/kornia-3d/src/ransac/estimators/ap3p.rs has the same shape (AP3P sampling, EPnP refit), so whatever is decided here likely applies there too.
Splitting this out of #1072 rather than expanding that PR's scope. It is pre-existing, but #1072 makes it reachable: AP3P could not run inside
solve_pnp_ransacat all before that fix, so nobody could hit this.Behaviour
RansacParams::default().refine == true. With an AP3P kernel, the non-minimal refit over all inliers uses EPnP (a minimal solver cannot consume more than its exact correspondence count — this is the standard minimal-kernel / non-minimal-refit split and is correct in principle).But in the small-object / long-range regime EPnP is ill-conditioned, so the refit can return a worse pose than the AP3P consensus it replaced.
Measured on a 20 cm object at 6 m:
refinefalsetrueLM refinement does not rescue it — this is consistent with the existing
epnp_long_range_regression_documentedtest, which pins >10 px with LM enabled, and with theepnp.rsmodule docs, which name "prefer P3P-RANSAC when N >= 4" as the workaround for exactly this regime.So a caller who selects AP3P specifically to escape EPnP's long-range behaviour silently gets an EPnP pose back, and the good AP3P pose is discarded.
Why it was not fixed in #1072
Every candidate fix is a behaviour change beyond that PR's six review findings:
best_poseand keep the better one. Most conservative, but it meansrefineno longer guarantees the refined pose is returned — callers relying on that would see a silent semantic shift.refine_pose_lmseeded frombest_poseover the inliers (it accepts n >= 3), instead of routing through EPnP's control-point construction at all. Probably the right answer, but it is a new refit path that needs its own validation across both regimes.Option 3 looks best but deserves measurement rather than assertion, in both the long-range and the ordinary regime.
Note
crates/kornia-3d/src/ransac/estimators/ap3p.rshas the same shape (AP3P sampling, EPnP refit), so whatever is decided here likely applies there too.