imgproc: add contour values check to IntelligentScissorsMB tests by cpoerschke · Pull Request #23688 · opencv/opencv

cpoerschke · 2023-05-25T21:53:34Z

Preparation for the #21959 changes as per @asmorkalov's #21959 (comment) suggestion.

Pull Request Readiness Checklist

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

cpoerschke · 2023-05-26T18:41:34Z

5 failing, 14 successful, 1 skipped, and 1 errored checks

The failed tests seemed to be only for some tests on some platforms with small differences between the actual and the reference contour. Therefore the latest commit adds epsilon logic for the contour values check where there is epsilon logic already for the contour size check.

…pute_full_local_cost flag;

modules/imgproc/test/test_intelligent_scissors.cpp

asmorkalov · 2023-05-27T06:58:35Z

modules/imgproc/test/test_intelligent_scissors.cpp

+    else if (pts.size() == reference_pts.size())
+    {
+        for (size_t idx = 0; idx < reference_pts.size(); ++idx)
+        {
+            EXPECT_GE(pts[idx].x, reference_pts[idx].x - PTS_EPS);
+            EXPECT_GE(pts[idx].y, reference_pts[idx].y - PTS_EPS);
+            EXPECT_LE(pts[idx].x, reference_pts[idx].x + PTS_EPS);
+            EXPECT_LE(pts[idx].y, reference_pts[idx].y + PTS_EPS);
+        }


Else branch looks strange. It checks nothing, if size is different. In case if +-1 result is expected, then you can use cv::norm(..., NORM_INF) could be used. In case if amount of contour pixels is unstable, please do not call getContours, but do all steps in test body.

Thanks for the cv::norm(..., NORM_INF) suggestion!

I'm not sure if I understood the "... but do all steps in test body." suggestion correctly, did you mean something this?

double getContour(segmentation::IntelligentScissorsMB& tool, const Point& target_point, std::vector<Point>& pts, const bool backward = false) { ... return cv::norm(pts, reference_pts, cv::NORM_INF); } ... EXPECT_EQ(getContour(tool, target_point, pts), 0); ... EXPECT_LE(getContour(tool, target_point, pts), PTS_EPS); ...

... I'm not sure if I understood the "... but do all steps in test body." suggestion correctly ...

Never mind, I see now e.g. from the https://pullrequest.opencv.org/buildbot/builders/precommit_windows64/builds/101746/steps/test_imgproc/logs/tests%20summary CI failure that the cv::norm call should only be made if the vector sizes match and therefore the call is better placed in the test body alongside the PTS_SIZE_EPS check.

modules/imgproc/test/test_intelligent_scissors.cpp

cpoerschke · 2023-05-28T16:07:23Z

19 successful, 1 failing, and 1 skipped checks

The 1 failing seems to be unrelated to the changes in this PR.

asmorkalov · 2023-05-29T09:15:52Z

modules/imgproc/test/test_intelligent_scissors.cpp

-    tool.getContour(target_point, pts2, true/*backward*/);
+    getAndCheckContour(tool, target_point, pts2, true/*backward*/);

    EXPECT_EQ(pts.size(), pts2.size());


getAndCheckContour checks contour size too. The following size checks are not required here and bellow.

asmorkalov · 2023-06-06T10:33:12Z

@cpoerschke Thanks a lot for the tests. I reworked them a bit to check contour area, but not contour pixels. It allows to compare cases, when contour does not match exactly. Please take a look on the solution and let me know, if you agree on it.

asmorkalov · 2023-06-06T11:40:27Z

Hm.. Github does not show my last commit by some reason. Could you cherry-pick it from my branch: https://github.com/asmorkalov/opencv/commits/4.x-pr-21959-prep

cpoerschke · 2023-06-06T21:38:43Z

@cpoerschke Thanks a lot for the tests. I reworked them a bit to check contour area, but not contour pixels. It allows to compare cases, when contour does not match exactly. Please take a look on the solution and let me know, if you agree on it.

Thanks @asmorkalov for the reworked test to check the contour area and not just the contour pixels! LGTM.

Hm.. Github does not show my last commit by some reason. Could you cherry-pick it from my branch: https://github.com/asmorkalov/opencv/commits/4.x-pr-21959-prep

Hmm, very strange, the commit was there on my branch (as expected since allow-edits-by-maintainers is checked for this PR) but github did not show it. Anyhow, I've pushed a commit with three small style tweaks and that resynced things then apparently.

asmorkalov

👍

@asmorkalov

imgproc: add contour values check to IntelligentScissorsMB tests Preparation for the opencv#21959 changes as per @asmorkalov's opencv#21959 (comment) suggestion. ### Pull Request Readiness Checklist 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 - [ ] There is a reference to the original bug report and related work - [X] 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

imgproc: add contour values check to IntelligentScissorsMB tests Preparation for the opencv#21959 changes as per @asmorkalov's opencv#21959 (comment) suggestion. ### Pull Request Readiness Checklist 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 - [ ] There is a reference to the original bug report and related work - [X] 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

imgproc: add contour values check to IntelligentScissorsMB tests Preparation for the opencv#21959 changes as per @asmorkalov's opencv#21959 (comment) suggestion. ### Pull Request Readiness Checklist 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 - [ ] There is a reference to the original bug report and related work - [X] 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

imgproc: add contour values check to IntelligentScissorsMB tests

05de67f

This was referenced May 25, 2023

imgproc: add contour values check to IntelligentScissorsMB tests opencv/opencv_extra#1065

Merged

imgproc: optimise local cost computation in IntelligentScissorsMB::buildMap #21959

Merged

asmorkalov self-requested a review May 26, 2023 15:57

asmorkalov added test category: imgproc labels May 26, 2023

asmorkalov self-assigned this May 26, 2023

asmorkalov added this to the 4.8.0 milestone May 26, 2023

add PTS_EPS similar to PTS_SIZE_EPS

f6be107

cpoerschke added a commit to cpoerschke/opencv that referenced this pull request May 26, 2023

undo test changes in favour of the opencv#23688 tests; remove the com…

a01344e

…pute_full_local_cost flag;

asmorkalov reviewed May 27, 2023

View reviewed changes

check[Exact]ContourValues rename; GENERATE_TEST_DATA use; cv::norm use;

261dbc4

cpoerschke marked this pull request as draft May 27, 2023 20:10

cpoerschke and others added 2 commits May 27, 2023 21:41

split getContour into getContours + getAndCheckContour

87ec882

Update test_intelligent_scissors.cpp

6479a74

cpoerschke marked this pull request as ready for review May 27, 2023 22:28

asmorkalov reviewed May 29, 2023

View reviewed changes

cpoerschke marked this pull request as draft May 29, 2023 10:07

remove now redundant pts.size() checks

5251afa

cpoerschke marked this pull request as ready for review May 29, 2023 20:57

cpoerschke requested a review from asmorkalov June 4, 2023 07:31

Check contour area instead of contour coordinates.

446c4e5

asmorkalov approved these changes Jun 6, 2023

View reviewed changes

asmorkalov self-requested a review June 6, 2023 13:05

three minor style tweaks

e3c15e0

asmorkalov approved these changes Jun 7, 2023

View reviewed changes

asmorkalov merged commit d3e7968 into opencv:4.x Jun 7, 2023

asmorkalov mentioned this pull request Jul 12, 2023

(5.x) Merge 4.x #23979

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

imgproc: add contour values check to IntelligentScissorsMB tests#23688

imgproc: add contour values check to IntelligentScissorsMB tests#23688
asmorkalov merged 8 commits intoopencv:4.xfrom
cpoerschke:4.x-pr-21959-prep

cpoerschke commented May 25, 2023

Uh oh!

cpoerschke commented May 26, 2023

Uh oh!

Uh oh!

asmorkalov May 27, 2023

Uh oh!

cpoerschke May 27, 2023

Uh oh!

cpoerschke May 27, 2023

Uh oh!

Uh oh!

cpoerschke commented May 28, 2023

Uh oh!

asmorkalov May 29, 2023

Uh oh!

asmorkalov commented Jun 6, 2023

Uh oh!

asmorkalov commented Jun 6, 2023

Uh oh!

cpoerschke commented Jun 6, 2023

Uh oh!

asmorkalov left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

cpoerschke commented May 25, 2023

Pull Request Readiness Checklist

Uh oh!

cpoerschke commented May 26, 2023

Uh oh!

Uh oh!

asmorkalov May 27, 2023

Choose a reason for hiding this comment

Uh oh!

cpoerschke May 27, 2023

Choose a reason for hiding this comment

Uh oh!

cpoerschke May 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cpoerschke commented May 28, 2023

Uh oh!

asmorkalov May 29, 2023

Choose a reason for hiding this comment

Uh oh!

asmorkalov commented Jun 6, 2023

Uh oh!

asmorkalov commented Jun 6, 2023

Uh oh!

cpoerschke commented Jun 6, 2023

Uh oh!

asmorkalov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants