Conversation
| ///////////////////////////////// Stereo Calibration ///////////////////////////////////// | ||
|
|
||
| class CV_StereoCalibrationCornerTest : public cvtest::BaseTest | ||
| { |
There was a problem hiding this comment.
please, avoid obsolete cvtest::BaseTest-based tests. Use pure Google Test tests.
There was a problem hiding this comment.
I follow the "consistency is better than brilliance" principle. Every other test in this file looks like this. So, I follow the common pattern. If the conversion should happen, it should, probably, happen all at once.
There was a problem hiding this comment.
These test are obsolete and hard to read. It's easy and better to support Google Test based tests
Every other test in this file looks like this
Let's create another file with proper tests 😄
There was a problem hiding this comment.
Ok, we'll need an arbiter =) @vpisarev what do you think?
| // perform resize-remap | ||
| Mat rsz_result; | ||
| resize(image, rsz_result, Size(), scale, scale, INTER_LINEAR); | ||
| remap(rsz_result, rsz_result, rszRmap[0], rszRmap[1], INTER_LINEAR); |
There was a problem hiding this comment.
this part looks a bit weird to me. You check that resize(remap(img)) == remap(resize(img)), which is generally not true, because of rounding operations and many other reasons; It can only be true if img is constant image. In your test the image is simple but non-constant, and this is strange that it passed the test, probably, the results should be visualized to see what's going on
There was a problem hiding this comment.
This is what happens - we work on an image with a constant rectangle area close the the center (moved in top-left direction). Prior to change (buggy behavior) - it would completely disappear from one of the images. After the change (correct behavior) - it stays roughly where it was. You are correct in that the borders of the rectangle do not match perfectly (because of rounding, etc.) - but below I check only the inner part (3.1 divisor instead of 3) to work around this.
I agree, that it's not the tidiest test ever. But still, it seems kind of ok to me as a "regression" test for a corner-case issue.
|
I see. Ok, let's merge it in! 👍 |
@vpisarev, @vicproon I've converted the reproducing sample from #6836 to a regression test