Merge pull request #25680 from mshabunin:fix-approx-2

mshabunin · web-flow · commit 29f91a08d53c · 2024-05-31T19:28:03.000+03:00
Reverted contour approximation behavior #25680 Related issue #25663 - revert new function behavior despite it returning different result than the old one (reverts PR #25672). Also added Coverity issue fix.
diff --git a/modules/imgproc/src/contours_approx.cpp b/modules/imgproc/src/contours_approx.cpp
@@ -248,10 +248,10 @@ static void pass_cleanup(vector<ApproxItem>& ares, size_t start_idx)
 
                     if (s1 > s2 || (s1 == s2 && ares[prev].k <= ares[i].k))
                         /* remove second */
-                        clear_until(ares, prev, get_next_idx(ares, i));
+                        ares[i].removed = true;
                     else
                         /* remove first */
-                        clear_until(ares, get_next_idx(ares, first), i);
+                        ares[prev].removed = true;
                 }
                 else
                 {
diff --git a/modules/imgproc/src/contours_link.cpp b/modules/imgproc/src/contours_link.cpp
@@ -252,7 +252,6 @@ void LinkRunner::establishLinks(int& prev_point,
             lower_run = rns[rns[lower_run].next].next;
             continue;
         }
-        rns[rns[lower_run].next] = rns[rns[lower_run].next];
         rns[lower_run].link = rns[lower_run].next;
 
         // First point of contour
@@ -269,7 +268,6 @@ void LinkRunner::establishLinks(int& prev_point,
             upper_run = rns[rns[upper_run].next].next;
             continue;
         }
-        rns[rns[upper_run].next] = rns[rns[upper_run].next];
         rns[rns[upper_run].next].link = upper_run;
         upper_run = rns[rns[upper_run].next].next;
     }
diff --git a/modules/imgproc/test/test_contours_new.cpp b/modules/imgproc/test/test_contours_new.cpp
@@ -531,6 +531,9 @@ TEST_P(Imgproc_FindContours_Modes2, approx)
         findContours(img, contours, hierarchy, mode, method);
 
 #if CHECK_OLD
+        // NOTE: old and new function results might not match when approximation mode is TC89.
+        // Currently this test passes, but might fail for other random data.
+        // See https://github.com/opencv/opencv/issues/25663 for details.
         vector<vector<Point>> contours_o;
         vector<Vec4i> hierarchy_o;
         findContours_legacy(img, contours_o, hierarchy_o, mode, method);
@@ -602,23 +605,4 @@ TEST(Imgproc_FindContours, link_runs)
 #endif
 }
 
-TEST(Imgproc_FindContours, regression_25663)
-{
-    uint8_t data[]={0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,0,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0,0,0,0,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1};
-    Mat1b img(32,32,data);
-    vector<vector<Point>> contours;
-    vector<Vec4i> hierarchy;
-    findContours(img, contours, RETR_EXTERNAL, CHAIN_APPROX_TC89_L1);
-#if CHECK_OLD
-    vector<vector<Point>> contours_o;
-    findContours_legacy(img, contours_o, RETR_EXTERNAL, CHAIN_APPROX_TC89_L1);
-    ASSERT_EQ(contours_o.size(), contours.size());
-    for (size_t i = 0; i < contours_o.size(); ++i)
-    {
-        SCOPED_TRACE(format("contour = %zu", i));
-        EXPECT_MAT_NEAR(Mat(contours_o[i]), Mat(contours[i]), 0);
-    }
-#endif
-}
-
 }}  // namespace opencv_test

Original file line number	Diff line number	Diff line change
`@@ -248,10 +248,10 @@ static void pass_cleanup(vector<ApproxItem>& ares, size_t start_idx)`
`248`	`248`
`249`	`249`	`if (s1 > s2 \|\| (s1 == s2 && ares[prev].k <= ares[i].k))`
`250`	`250`	`/* remove second */`
`251`		`- clear_until(ares, prev, get_next_idx(ares, i));`
	`251`	`+ ares[i].removed = true;`
`252`	`252`	`else`
`253`	`253`	`/* remove first */`
`254`		`- clear_until(ares, get_next_idx(ares, first), i);`
	`254`	`+ ares[prev].removed = true;`
`255`	`255`	`}`
`256`	`256`	`else`
`257`	`257`	`{`
Original file line number	Diff line number	Diff line change
`@@ -252,7 +252,6 @@ void LinkRunner::establishLinks(int& prev_point,`
`252`	`252`	`lower_run = rns[rns[lower_run].next].next;`
`253`	`253`	`continue;`
`254`	`254`	`}`
`255`		`- rns[rns[lower_run].next] = rns[rns[lower_run].next];`
`256`	`255`	`rns[lower_run].link = rns[lower_run].next;`
`257`	`256`
`258`	`257`	`// First point of contour`
`@@ -269,7 +268,6 @@ void LinkRunner::establishLinks(int& prev_point,`
`269`	`268`	`upper_run = rns[rns[upper_run].next].next;`
`270`	`269`	`continue;`
`271`	`270`	`}`
`272`		`- rns[rns[upper_run].next] = rns[rns[upper_run].next];`
`273`	`271`	`rns[rns[upper_run].next].link = upper_run;`
`274`	`272`	`upper_run = rns[rns[upper_run].next].next;`
`275`	`273`	`}`