Skip to content

Increase decomposeProjectionMatrix precision for small scales#25182

Merged
asmorkalov merged 4 commits intoopencv:4.xfrom
MaximSmolskiy:increase-decomposeProjectionMatrix-precision-for-small-scales
Mar 27, 2024
Merged

Increase decomposeProjectionMatrix precision for small scales#25182
asmorkalov merged 4 commits intoopencv:4.xfrom
MaximSmolskiy:increase-decomposeProjectionMatrix-precision-for-small-scales

Conversation

@MaximSmolskiy
Copy link
Copy Markdown
Contributor

@MaximSmolskiy MaximSmolskiy commented Mar 7, 2024 •
edited
Loading

Pull Request Readiness Checklist

Fix #23733

It is checked before that |s| > DBL_EPSILON (if not, then s will be equal to 0, but c will be equal to 1, then z will be equal to 1 and there will be no any problems with small values), so sqrt(c^2 + s^2) >= |s| > DBL_EPSILON and thus small values are already taken into account before and there is no need to add DBL_EPSILON to c^2 + s^2 (and I think adding DBL_EPSILON^2 instead of DBL_EPSILON would be more correct).

I ran Python script from issue.

NumPy and SciPy results

*Numpy*
P (case 1): 
[[1. 0. 0. 0.]
 [0. 1. 0. 0.]
 [0. 0. 1. 0.]]
Numpy, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
Numpy scaled 1e-6, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
P (case 2): 
[[52. -7.  4. 12.]
 [-6. 49. 12.  8.]
 [ 4. 17.  1.  0.]]
Numpy, R.T @ R - I: 
[[-8.88178420e-16 -3.86302608e-16 -2.52050796e-17]
 [-3.86302608e-16 -5.55111512e-16  7.11675423e-18]
 [-2.52050796e-17  7.11675423e-18 -5.55111512e-16]]
Numpy scaled 1e-6, R.T @ R - I: 
[[ 2.22044605e-16 -2.00683644e-16 -1.90063998e-17]
 [-2.00683644e-16  0.00000000e+00  1.16926308e-17]
 [-1.90063998e-17  1.16926308e-17  2.22044605e-16]]

*Scipy*
P (case 1): 
[[1. 0. 0. 0.]
 [0. 1. 0. 0.]
 [0. 0. 1. 0.]]
Scipy, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
Scipy scaled 1e-6, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
P (case 2): 
[[52. -7.  4. 12.]
 [-6. 49. 12.  8.]
 [ 4. 17.  1.  0.]]
Scipy, R.T @ R - I: 
[[-1.11022302e-16 -8.74062812e-18 -1.26178867e-17]
 [-8.74062812e-18 -1.11022302e-16  2.07820373e-17]
 [-1.26178867e-17  2.07820373e-17 -1.11022302e-16]]
Scipy scaled 1e-6, R.T @ R - I: 
[[0.00000000e+00 4.04691435e-17 1.12452918e-16]
 [4.04691435e-17 4.44089210e-16 3.74164141e-16]
 [1.12452918e-16 3.74164141e-16 4.44089210e-16]]

*Numpy*
Numpy, P' - P:
 [[ 1.35525272e-20 -9.31736242e-21  8.47032947e-22  3.38813179e-21]
 [-3.38813179e-21  6.77626358e-21  1.69406589e-21  0.00000000e+00]
 [-1.69406589e-21  0.00000000e+00  0.00000000e+00  4.85524279e-22]]

*Scipy*
Scipy, P' - P:
 [[0.00000000e+00 8.47032947e-22 3.38813179e-21 3.38813179e-21]
 [3.38813179e-21 1.35525272e-20 1.52465931e-20 1.52465931e-20]
 [8.47032947e-22 3.38813179e-21 2.54109884e-21 3.39866995e-21]]

OpenCV results before

*OpenCV*
P (case 1): 
[[1. 0. 0. 0.]
 [0. 1. 0. 0.]
 [0. 0. 1. 0.]]
OpenCV, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
OpenCV scaled 1e-6, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
P (case 2): 
[[52. -7.  4. 12.]
 [-6. 49. 12.  8.]
 [ 4. 17.  1.  0.]]
OpenCV, R.T @ R - I: 
[[ 2.22044605e-16 -9.12253504e-17 -2.20527203e-19]
 [-9.12253504e-17  0.00000000e+00 -9.12405093e-18]
 [-2.20527203e-19 -9.12405093e-18  2.22044605e-16]]
OpenCV scaled 1e-6, R.T @ R - I: 
[[-1.28197013e-06  1.30450769e-07  7.67357467e-09]
 [ 1.30450769e-07 -1.52141637e-06 -9.92455574e-09]
 [ 7.67357467e-09 -9.92455574e-09 -1.35328272e-06]]

*OpenCV*
OpenCV, P' - P:
 [[-6.75449076e-11  1.73936564e-11 -4.94463312e-12 -1.61106020e-11]
 [ 1.41759913e-11 -7.54512016e-11 -1.67717374e-11 -9.74644390e-12]
 [-2.90254385e-12 -2.53521998e-11 -1.49130587e-12  4.00724440e-13]]

OpenCV results after

*OpenCV*
P (case 1): 
[[1. 0. 0. 0.]
 [0. 1. 0. 0.]
 [0. 0. 1. 0.]]
OpenCV, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
OpenCV scaled 1e-6, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
P (case 2): 
[[52. -7.  4. 12.]
 [-6. 49. 12.  8.]
 [ 4. 17.  1.  0.]]
OpenCV, R.T @ R - I: 
[[ 2.22044605e-16 -9.12253504e-17 -2.20527203e-19]
 [-9.12253504e-17  0.00000000e+00 -9.12405093e-18]
 [-2.20527203e-19 -9.12405093e-18  2.22044605e-16]]
OpenCV scaled 1e-6, R.T @ R - I: 
[[ 0.00000000e+00  4.36198333e-17 -2.66855078e-17]
 [ 4.36198333e-17  2.22044605e-16  3.17216400e-17]
 [-2.66855078e-17  3.17216400e-17 -2.22044605e-16]]

*OpenCV*
OpenCV, P' - P:
 [[ 6.77626358e-21  0.00000000e+00 -8.47032947e-22  1.69406589e-21]
 [-1.69406589e-21  6.77626358e-21  0.00000000e+00  3.38813179e-21]
 [ 8.47032947e-22  3.38813179e-21  2.11758237e-22  1.45657284e-21]]

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@asmorkalov asmorkalov added this to the 4.10.0 milestone Mar 8, 2024
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Mar 26, 2024

cvRQDecomp3x3 itself is exposed and old OpenCV 1.x C API. The patch affects it's behaviour. Could you point the place where |s| > DBL_EPSILON check is done?

@asmorkalov asmorkalov self-assigned this Mar 26, 2024
@MaximSmolskiy
Copy link
Copy Markdown
Contributor Author

MaximSmolskiy commented Mar 26, 2024 •
edited
Loading

cvRQDecomp3x3 itself is exposed and old OpenCV 1.x C API. The patch affects it's behaviour. Could you point the place where |s| > DBL_EPSILON check is done?

There are two cases:

  1. std::abs(mat...) <= DBL_EPSILON => s = 0, c = 1 => c^2 + s^2 = 1 - and there are no any numerical issues with 1./std::sqrt(c * c + s * s)
  2. std::abs(mat...) > DBL_EPSILON => |s| = std::abs(mat...) > DBL_EPSILON

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Mar 27, 2024

Ah, yes! My fault.

@asmorkalov asmorkalov merged commit e3ff6ce into opencv:4.x Mar 27, 2024
@MaximSmolskiy MaximSmolskiy deleted the increase-decomposeProjectionMatrix-precision-for-small-scales branch March 27, 2024 09:45
@asmorkalov asmorkalov mentioned this pull request Apr 1, 2024
Merged
klatism pushed a commit to klatism/opencv that referenced this pull request May 17, 2024
@MaximSmolskiy @klatism
Merge pull request opencv#25182 from MaximSmolskiy:increase-decompose…
15d609f 
…ProjectionMatrix-precision-for-small-scales

Increase decomposeProjectionMatrix precision for small scales opencv#25182 

### Pull Request Readiness Checklist

Fix opencv#23733

It is checked before that `|s| > DBL_EPSILON` (if not, then `s` will be equal to `0`, but `c` will be equal to `1`, then `z` will be equal to `1` and there will be no any problems with small values), so `sqrt(c^2 + s^2) >= |s| > DBL_EPSILON` and thus small values are already taken into account before and there is no need to add `DBL_EPSILON` to `c^2 + s^2` (and I think adding `DBL_EPSILON^2` instead of `DBL_EPSILON` would be more correct).

I ran `Python` script from issue.

`NumPy` and `SciPy` results
```
*Numpy*
P (case 1): 
[[1. 0. 0. 0.]
 [0. 1. 0. 0.]
 [0. 0. 1. 0.]]
Numpy, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
Numpy scaled 1e-6, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
P (case 2): 
[[52. -7.  4. 12.]
 [-6. 49. 12.  8.]
 [ 4. 17.  1.  0.]]
Numpy, R.T @ R - I: 
[[-8.88178420e-16 -3.86302608e-16 -2.52050796e-17]
 [-3.86302608e-16 -5.55111512e-16  7.11675423e-18]
 [-2.52050796e-17  7.11675423e-18 -5.55111512e-16]]
Numpy scaled 1e-6, R.T @ R - I: 
[[ 2.22044605e-16 -2.00683644e-16 -1.90063998e-17]
 [-2.00683644e-16  0.00000000e+00  1.16926308e-17]
 [-1.90063998e-17  1.16926308e-17  2.22044605e-16]]

*Scipy*
P (case 1): 
[[1. 0. 0. 0.]
 [0. 1. 0. 0.]
 [0. 0. 1. 0.]]
Scipy, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
Scipy scaled 1e-6, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
P (case 2): 
[[52. -7.  4. 12.]
 [-6. 49. 12.  8.]
 [ 4. 17.  1.  0.]]
Scipy, R.T @ R - I: 
[[-1.11022302e-16 -8.74062812e-18 -1.26178867e-17]
 [-8.74062812e-18 -1.11022302e-16  2.07820373e-17]
 [-1.26178867e-17  2.07820373e-17 -1.11022302e-16]]
Scipy scaled 1e-6, R.T @ R - I: 
[[0.00000000e+00 4.04691435e-17 1.12452918e-16]
 [4.04691435e-17 4.44089210e-16 3.74164141e-16]
 [1.12452918e-16 3.74164141e-16 4.44089210e-16]]

*Numpy*
Numpy, P' - P:
 [[ 1.35525272e-20 -9.31736242e-21  8.47032947e-22  3.38813179e-21]
 [-3.38813179e-21  6.77626358e-21  1.69406589e-21  0.00000000e+00]
 [-1.69406589e-21  0.00000000e+00  0.00000000e+00  4.85524279e-22]]

*Scipy*
Scipy, P' - P:
 [[0.00000000e+00 8.47032947e-22 3.38813179e-21 3.38813179e-21]
 [3.38813179e-21 1.35525272e-20 1.52465931e-20 1.52465931e-20]
 [8.47032947e-22 3.38813179e-21 2.54109884e-21 3.39866995e-21]]
```

`OpenCV` results before
```
*OpenCV*
P (case 1): 
[[1. 0. 0. 0.]
 [0. 1. 0. 0.]
 [0. 0. 1. 0.]]
OpenCV, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
OpenCV scaled 1e-6, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
P (case 2): 
[[52. -7.  4. 12.]
 [-6. 49. 12.  8.]
 [ 4. 17.  1.  0.]]
OpenCV, R.T @ R - I: 
[[ 2.22044605e-16 -9.12253504e-17 -2.20527203e-19]
 [-9.12253504e-17  0.00000000e+00 -9.12405093e-18]
 [-2.20527203e-19 -9.12405093e-18  2.22044605e-16]]
OpenCV scaled 1e-6, R.T @ R - I: 
[[-1.28197013e-06  1.30450769e-07  7.67357467e-09]
 [ 1.30450769e-07 -1.52141637e-06 -9.92455574e-09]
 [ 7.67357467e-09 -9.92455574e-09 -1.35328272e-06]]

*OpenCV*
OpenCV, P' - P:
 [[-6.75449076e-11  1.73936564e-11 -4.94463312e-12 -1.61106020e-11]
 [ 1.41759913e-11 -7.54512016e-11 -1.67717374e-11 -9.74644390e-12]
 [-2.90254385e-12 -2.53521998e-11 -1.49130587e-12  4.00724440e-13]]
```

`OpenCV` results after
```
*OpenCV*
P (case 1): 
[[1. 0. 0. 0.]
 [0. 1. 0. 0.]
 [0. 0. 1. 0.]]
OpenCV, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
OpenCV scaled 1e-6, R.T @ R - I: 
[[0. 0. 0.]
 [0. 0. 0.]
 [0. 0. 0.]]
P (case 2): 
[[52. -7.  4. 12.]
 [-6. 49. 12.  8.]
 [ 4. 17.  1.  0.]]
OpenCV, R.T @ R - I: 
[[ 2.22044605e-16 -9.12253504e-17 -2.20527203e-19]
 [-9.12253504e-17  0.00000000e+00 -9.12405093e-18]
 [-2.20527203e-19 -9.12405093e-18  2.22044605e-16]]
OpenCV scaled 1e-6, R.T @ R - I: 
[[ 0.00000000e+00  4.36198333e-17 -2.66855078e-17]
 [ 4.36198333e-17  2.22044605e-16  3.17216400e-17]
 [-2.66855078e-17  3.17216400e-17 -2.22044605e-16]]

*OpenCV*
OpenCV, P' - P:
 [[ 6.77626358e-21  0.00000000e+00 -8.47032947e-22  1.69406589e-21]
 [-1.69406589e-21  6.77626358e-21  0.00000000e+00  3.38813179e-21]
 [ 8.47032947e-22  3.38813179e-21  2.11758237e-22  1.45657284e-21]]
```

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [ ] The feature is well documented and sample code can be built with the project CMake
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov asmorkalov approved these changes

@vpisarev vpisarev Awaiting requested review from vpisarev

Assignees

@asmorkalov asmorkalov

Labels

category: calib3d

Projects

None yet

Milestone

4.10.0

Development

Successfully merging this pull request may close these issues.

Results from decomposeProjectionMatrix() when using different scales for P are not consistent

2 participants

@MaximSmolskiy @asmorkalov