Skip to content

Commit 6945bd0

Browse files
committed
Address CodeRabbit feedback on #2081
- Rename SolvePnPMethod.IPPE_SQUARE to IppeSquare: once an acronym is glued to another word to form a compound member name, this repo's enum-naming convention PascalCases the whole compound rather than leaving part of it in caps (see .github/copilot-instructions.md). - Use a positive-Z tvec in the new synthetic poses so the projected points sit in front of the camera instead of behind it. - Strengthen SolvePnPTestByArrayMethods/IppeSquare to reproject with the recovered pose and assert it matches the input points, instead of only asserting the call doesn't throw. Confirmed this actually matters: with the old AP3P/IPPE values, SolvePnP silently dispatches to a different native solver that still completes without throwing, so a throws-only assertion doesn't catch it - the reprojection check does.
1 parent e44ae11 commit 6945bd0

2 files changed

Lines changed: 21 additions & 4 deletions

File tree

src/OpenCvSharp/Modules/geometry/Enum/SolvePnPMethod.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public enum SolvePnPMethod
5050
/// - point 2: [squareLength / 2, -squareLength / 2, 0]
5151
/// - point 3: [-squareLength / 2, -squareLength / 2, 0]
5252
/// </summary>
53-
IPPE_SQUARE = 5,
53+
IppeSquare = 5,
5454

5555
/// <summary>
5656
/// Method is based on the paper "A Consistently Fast and Globally Optimal Solution to the

test/OpenCvSharp.Tests/calib3d/Calib3dTest.cs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ public void SolvePnPTestByArray(bool useExtrinsicGuess)
489489
public void SolvePnPTestByArrayMethods(SolvePnPMethod method)
490490
{
491491
var rvec = new double[] { 3, 0, 0 };
492-
var tvec = new double[] { 0, 0, -10 };
492+
var tvec = new double[] { 0, 0, 10 };
493493
var cameraMatrix = new double[,]
494494
{
495495
{ 1, 0, 0 },
@@ -509,13 +509,23 @@ public void SolvePnPTestByArrayMethods(SolvePnPMethod method)
509509
Cv2.ProjectPoints(objPts, rvec, tvec, cameraMatrix, dist, out var imgPts, out _);
510510

511511
Cv2.SolvePnP(objPts, imgPts, cameraMatrix, dist, ref rvec, ref tvec, flags: method);
512+
513+
// Verify the recovered pose actually reprojects onto imgPts (rather than just "didn't throw"):
514+
// a stale/misdirected enum value can silently dispatch to a different native solver that
515+
// still runs to completion without throwing, but produces a wrong pose.
516+
Cv2.ProjectPoints(objPts, rvec, tvec, cameraMatrix, dist, out var reprojected, out _);
517+
for (var i = 0; i < imgPts.Length; i++)
518+
{
519+
Assert.Equal(imgPts[i].X, reprojected[i].X, 3);
520+
Assert.Equal(imgPts[i].Y, reprojected[i].Y, 3);
521+
}
512522
}
513523

514524
[Fact]
515525
public void SolvePnPTestByArrayIppeSquare()
516526
{
517527
var rvec = new double[] { 3, 0, 0 };
518-
var tvec = new double[] { 0, 0, -10 };
528+
var tvec = new double[] { 0, 0, 10 };
519529
var cameraMatrix = new double[,]
520530
{
521531
{ 1, 0, 0 },
@@ -534,7 +544,14 @@ public void SolvePnPTestByArrayIppeSquare()
534544

535545
Cv2.ProjectPoints(objPts, rvec, tvec, cameraMatrix, dist, out var imgPts, out _);
536546

537-
Cv2.SolvePnP(objPts, imgPts, cameraMatrix, dist, ref rvec, ref tvec, flags: SolvePnPMethod.IPPE_SQUARE);
547+
Cv2.SolvePnP(objPts, imgPts, cameraMatrix, dist, ref rvec, ref tvec, flags: SolvePnPMethod.IppeSquare);
548+
549+
Cv2.ProjectPoints(objPts, rvec, tvec, cameraMatrix, dist, out var reprojected, out _);
550+
for (var i = 0; i < imgPts.Length; i++)
551+
{
552+
Assert.Equal(imgPts[i].X, reprojected[i].X, 3);
553+
Assert.Equal(imgPts[i].Y, reprojected[i].Y, 3);
554+
}
538555
}
539556

540557
[Fact]

0 commit comments

Comments
 (0)