Remove dead code corresponding to SOLVEPNP_DLS and SOLVEPNP_UPNP flags #27358
Conversation
…s. These flags are internally mapped to SOLVEPNP_EPNP for more than 10 years and thus unavailable.
| //!< Exhaustive Linearization for Robust Camera Pose and Focal Length Estimation @cite penate2013exhaustive | ||
| SOLVEPNP_AP3P = 5, //!< An Efficient Algebraic Solution to the Perspective-Three-Point Problem @cite Ke17 | ||
| SOLVEPNP_IPPE = 6, //!< Infinitesimal Plane-Based Pose Estimation @cite Collins14 \n | ||
| SOLVEPNP_AP3P = 3, //!< An Efficient Algebraic Solution to the Perspective-Three-Point Problem @cite Ke17 |
There was a problem hiding this comment.
The different flags are set to some hardcoded values. Was it done because of Java/Python bindings?
Could this be an issue to change the default value? I mean someone using SOLVEPNP_DLS in OpenCV 5, the code will not build. But could there be some side effects where the default value for SOLVEPNP_AP3P be an issue after the PR?
| min_solver = P3PSolver::create(points, calib_points, K1); // --> | ||
| non_min_solver = DLSPnP::create(points, calib_points, K1); // --> |
There was a problem hiding this comment.
Be coherent. Either the DLS method has been silently mapped to EPnP due to an inherent flaw in the conception of the method or because of a bad implementation?
Because here for USAC, it looks like the DLS method is the default method in the general case.
If the DLS implementation in USAC is correct, it should be better to copy this implementation and reactive the SOLVEPNP_DLS codepath in my opinion.
| @@ -372,8 +372,8 @@ void UniversalRANSAC::initialize (int state, Ptr<MinimalSolver> &min_solver, Ptr | |||
| } else if (params->isPnP()) { | |||
| degeneracy = makePtr<Degeneracy>(); | |||
| if (min_sample_size == 3) { | |||
| min_solver = P3PSolver::create(points, calib_points, K1); | |||
| non_min_solver = DLSPnP::create(points, calib_points, K1); | |||
| min_solver = P3PSolver::create(points, calib_points, K1); // --> | |||
There was a problem hiding this comment.
Nitpicking.
Another P3P implementation (beside SOLVEPNP_P3P and SOLVEPNP_AP3P). Would have been great to reuse code from SOLVEPNP_AP3P (AP3P method is more stable than P3P from my experience) for instance.
Or made this implementation code available in the overall lib if it performs better.
Do not merge.
Cf #27330
SOLVEPNP_DLSandSOLVEPNP_UPNPflags are internally mapped to SOLVEPNP_EPNP for more than 10 years.Remove these functions in OpenCV 5 to clean-up the codebase and avoid potential misconceptions where someone thinks he uses DLS/UPnP methods while it is just internally mapped to EPnP.
Pros:
Cons:
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.