@ivashmak Could you take a look too?

@ivashmak Could you take a look too?

The idea to not normalize h33 to 1 is valid. In my USAC implementation, h33 is not constrained to 1. However, I am not sure about these changes:

Before, ww was either 1/scale or 0. But new scaleFor function returns ww either 1/scale or 1. If scale = 0 then points are at infinity, and perhaps ww should be 1 in this case, otherwise newly transformed points become 0.

@ivashmak, thank you for the prompt review! As I understand, both ww from computeError perform H*src projection to estimate reprojection error. Both assume normalized H with h33=1. That's why they do + 1.0.

Moreover I don't understand how ww may be zero in the following equation end then used in residual calculation:

For complete picture, these are homography matrices for 0, RANSAC and LMEDS methods with only the following patch:

- _H0.convertTo(_model, _H0.type(), 1./_H0.at<double>(2,2) );
+ _H0.convertTo(_model, _H0.type(), scaleFor(_H0.at<double>(2,2)));

1. method 0 (max reproj error 0.00473197):

   H: [368.0887100147034, 0.0159247465476191, 368.0446295465918;
    92.15359859037321, 92.1888090966786, -0.09174469371200097;
    -183.5667621743224, 0.00123721357596798, 0]
       
   reprojection: [-0.000326887168340034, -1.004731974700604;
    -1.002815316369015, -1.004482078393191;
    -0.0001533803367575149, -0.0003079740891392978;
    -1.002628296372845, -5.808115023404001e-05;
    -0.0002401331678432291, -0.5025165895582193]
   

2. method RANSAC (max reproj error 0.00473197):

   H: [368.0887100147034, 0.0159247465476191, 368.0446295465918;
    92.15359859037321, 92.1888090966786, -0.09174469371200097;
    -183.5667621743224, 0.00123721357596798, 0]
   reprojection: [-0.000326887168340034, -1.004731974700604;
    -1.002815316369015, -1.004482078393191;
    -0.0001533803367575149, -0.0003079740891392978;
    -1.002628296372845, -5.808115023404001e-05;
    -0.0002401331678432291, -0.5025165895582193]
   

3. method LMEDS (max reproj error 2.81711):

   H: [-0.3549734087389866, -3.406047867269289e-17, -0.3549734087389865;
    0.1612566478152516, 0.161256647815252, -5.975639861076076e-16;
    0.6774867043694947, 1.281788930749374e-15, 1]
   reprojection: [3.442410094922453e-16, -1;
    -0.9999999999999852, 1.8171124241458;
    0, -9.061724223317423e-16;
    -0.9999999999999998, -3.678776244851225e-17;
    1.721205047461219e-16, -0.4999999999999987]
   

🤔 Or maybe this is something expected and we should not get exact matrix. I'll review it more carefully.

About ww I meant that before the update it was (line: https://github.com/opencv/opencv/blob/4.x/modules/calib3d/src/fundam.cpp#L247)

double ww = h[6]*Mx + h[7]*My + 1.;
ww = fabs(ww) > DBL_EPSILON ? 1./ww : 0;

So, ww is either 1/scale or 0. But then you changed it to

float ww = scaleFor(Hf[6]*M[i].x + Hf[7]*M[i].y + Hf[8]); 

Where, ww is either 1/scale or 1, because of scaleFor function.

I saw the latest commit, and you removed the scaleFor function, and just put error as infinity if ww is close to 0. Basically, if ww is 0, then we need to compare vanishing points, the error does not have to be necessarily infinity. I think if ww is 0, perhaps we should compare just x and y coordinates?

@ivashmak, I see your concern. Reverted zero value condition back to the origin one.

@ivashmak @vpisarev could you take a look again?

@ivashmak @vpisarev could you take a look again?

I approve.

@asmorkalov, this PR has not been ported to 5.x: https://github.com/opencv/opencv/blob/5.x/modules/3d/src/fundam.cpp

Thanks for the reminder. I have not merged 4.x->5.x for a month aprox. Will do right after release.