Intelligent scissor for openCV.js by SimpleVlad · Pull Request #18344 · opencv/opencv

SimpleVlad · 2020-09-15T20:48:22Z

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 other license that is incompatible with OpenCV
The PR is proposed to proper branch
There is reference to 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

force_builders_only=linux,docs,custom
buildworker:Custom=linux-1,linux-2,linux-4
build_image:Docs=docs-js
build_image:Custom=javascript

SimpleVlad · 2020-09-15T20:51:03Z

@dkurt

python ./platforms/js/build_js.py build_js --emscripten_dir ~/emsdk/fastcomp/emscripten/ 
Args: Namespace(build_dir='build_js', build_doc=False, build_flags=None, build_perf=False, build_test=False, build_wasm=False, build_wasm_intrin_test=False, clean_build_dir=False, cmake_option=None, config='/home/vlad/opencv2/opencv/platforms/js/opencv_js.config.py', config_only=False, disable_wasm=False, emscripten_dir='/home/vlad/emsdk/fastcomp/emscripten/', enable_exception=False, opencv_dir='/home/vlad/opencv2/opencv', simd=False, skip_config=False, threads=False)
Check dir /home/vlad/opencv2/opencv/build_js (create: True, clean: False)
Check dir /home/vlad/opencv2/opencv (create: False, clean: False)
Check dir /home/vlad/emsdk/fastcomp/emscripten (create: False, clean: False)
=====
===== Config OpenCV.js build for default target
=====
Executing: ['cmake', '-DENABLE_PIC=FALSE', '-DCMAKE_BUILD_TYPE=Release', "-DCMAKE_TOOLCHAIN_FILE='/home/vlad/emsdk/fastcomp/emscripten/cmake/Modules/Platform/Emscripten.cmake'", "-DCPU_BASELINE=''", "-DCPU_DISPATCH=''", '-DCV_TRACE=OFF', '-DBUILD_SHARED_LIBS=OFF', '-DWITH_1394=OFF', '-DWITH_VTK=OFF', '-DWITH_EIGEN=OFF', '-DWITH_FFMPEG=OFF', '-DWITH_GSTREAMER=OFF', '-DWITH_GTK=OFF', '-DWITH_GTK_2_X=OFF', '-DWITH_IPP=OFF', '-DWITH_JASPER=OFF', '-DWITH_JPEG=OFF', '-DWITH_WEBP=OFF', '-DWITH_OPENEXR=OFF', '-DWITH_OPENGL=OFF', '-DWITH_OPENVX=OFF', '-DWITH_OPENNI=OFF', '-DWITH_OPENNI2=OFF', '-DWITH_PNG=OFF', '-DWITH_TBB=OFF', '-DWITH_TIFF=OFF', '-DWITH_V4L=OFF', '-DWITH_OPENCL=OFF', '-DWITH_OPENCL_SVM=OFF', '-DWITH_OPENCLAMDFFT=OFF', '-DWITH_OPENCLAMDBLAS=OFF', '-DWITH_GPHOTO2=OFF', '-DWITH_LAPACK=OFF', '-DWITH_ITT=OFF', '-DWITH_QUIRC=OFF', '-DBUILD_ZLIB=ON', '-DBUILD_opencv_apps=OFF', '-DBUILD_opencv_calib3d=ON', '-DBUILD_opencv_dnn=ON', '-DBUILD_opencv_features2d=ON', '-DBUILD_opencv_flann=OFF', '-DBUILD_opencv_ml=OFF', '-DBUILD_opencv_photo=OFF', '-DBUILD_opencv_imgcodecs=OFF', '-DBUILD_opencv_shape=OFF', '-DBUILD_opencv_videoio=OFF', '-DBUILD_opencv_videostab=OFF', '-DBUILD_opencv_highgui=OFF', '-DBUILD_opencv_superres=OFF', '-DBUILD_opencv_stitching=OFF', '-DBUILD_opencv_java=OFF', '-DBUILD_opencv_java_bindings_generator=OFF', '-DBUILD_opencv_js=ON', '-DBUILD_opencv_python2=OFF', '-DBUILD_opencv_python3=OFF', '-DBUILD_opencv_python_bindings_generator=OFF', '-DBUILD_EXAMPLES=OFF', '-DBUILD_PACKAGE=OFF', '-DBUILD_TESTS=OFF', '-DBUILD_PERF_TESTS=OFF', '-DBUILD_DOCS=OFF', '-DWITH_PTHREADS_PF=OFF', '-DCV_ENABLE_INTRINSICS=OFF', '-DBUILD_WASM_INTRIN_TESTS=OFF', "-DCMAKE_C_FLAGS='-s USE_PTHREADS=0 '", "-DCMAKE_CXX_FLAGS='-s USE_PTHREADS=0 '", '/home/vlad/opencv2/opencv']
Re-run cmake no build system arguments
CMake Error at /usr/local/share/cmake-3.15/Modules/CMakeDetermineSystem.cmake:99 (message):
  Could not find toolchain file:
  /home/vlad/emsdk/fastcomp/emscripten/cmake/Modules/Platform/Emscripten.cmake
Call Stack (most recent call first):
  CMakeLists.txt:106 (enable_language)


CMake Error: CMake was unable to find a build program corresponding to "Unix Makefiles".  CMAKE_MAKE_PROGRAM is not set.  You probably need to select a different build tool.
CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage
CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
-- Configuring incomplete, errors occurred!
Traceback (most recent call last):
  File "./platforms/js/build_js.py", line 267, in <module>
    builder.config()
  File "./platforms/js/build_js.py", line 190, in config
    execute(cmd)
  File "./platforms/js/build_js.py", line 23, in execute
    raise Fail("Child returned: %s" % retcode)
__main__.Fail: Child returned: 1

dkurt · 2020-09-15T20:55:36Z

modules/imgproc/include/opencv2/imgproc.hpp


+CV_EXPORTS_W float local_cost(const Point& p, const Point& q, const Mat& gradient_magnitude, const Mat& Iy, const Mat& Ix, const Mat& zero_crossing);
+
+CV_EXPORTS_W void find_min_path(const Point& start, Mat img, Mat zero_crossing, Mat gradient_magnitude, Mat Ix, Mat Iy, Mat& hit_map_x, Mat& hit_map_y);


This should be a class which computes all the intermediate buffers inside.

Agreed.
imgproc module location is fine.
But we need to define "Algorithm" class for this task (see QRCodeDetector as reference).
API should be clear, simple and easy to use (instead of local functions with many parameters which are hard to use).
Also add documentation for that.

mshabunin · 2020-09-16T10:41:59Z

CMake was unable to find a build program corresponding to "Unix Makefiles"

You have to install make tool.

dkurt · 2020-09-16T11:40:14Z

@mshabunin, sorry, forgot to respond. Issue was related to incorrect build arguments. Right is

python ./platforms/js/build_js.py build_js --emscripten_dir ~/emsdk/upstream/emscripten/

alalek · 2020-09-17T12:14:13Z

Please learn this topic: https://github.com/opencv/opencv/wiki/Coding_Style_Guide

asmorkalov · 2020-11-26T07:54:34Z

@SimpleVlad Do you have a chance to finish the patch? The work looks like abandoned.

alalek · 2020-12-07T17:12:58Z

modules/imgproc/include/opencv2/imgproc.hpp

+    This class is used to  find the minimum path between points
+    of image.
+    */
+class CV_EXPORTS_W Intelligent_scissors


Broken indentation

alalek · 2020-12-07T17:14:04Z

modules/imgproc/include/opencv2/imgproc.hpp

+    of image.
+    */
+class CV_EXPORTS_W Intelligent_scissors
+{


Intelligent_scissors

Please learn and follow OpenCV naming conventions and other rules: https://github.com/opencv/opencv/wiki/Coding_Style_Guide

alalek · 2020-12-07T17:14:30Z

modules/imgproc/include/opencv2/imgproc.hpp

+    */
+    CV_WRAP Intelligent_scissors();
+
+    CV_WRAP Intelligent_scissors(const int input_border);


input_border

type? size?
Please add documentation for this parameter

alalek · 2020-12-07T17:15:38Z

modules/imgproc/include/opencv2/imgproc.hpp

+protected:
+    int border;


Implementation details are not allowed in public headers.
Use PImpl design pattern.

alalek · 2020-12-07T17:16:31Z

modules/imgproc/include/opencv2/imgproc.hpp

+
+    CV_WRAP void setBorder(const int input_border);
+
+    CV_WRAP void Intelligent_scissors_one_point(Mat img, OutputArray total_hit_map_x, OutputArray total_hit_map_y, const Point start_point);


Mat

InputArray must be used: https://github.com/opencv/opencv/wiki/Coding_Style_Guide

alalek · 2020-12-07T17:33:46Z

modules/imgproc/src/intelligent_scissors.cpp

+
+        threshold(img_canny, zero_crossing, 254, 1, THRESH_BINARY_INV);
+        Sobel(grayscale, Ix, CV_32FC1, 1, 0, 1);
+        Sobel(grayscale, Iy, CV_32FC1, 0, 1, 1);


Sobel() is used in Canny() implementation too.
Are parameters of these calls the same?

alalek · 2020-12-07T17:37:29Z

modules/imgproc/src/intelligent_scissors.cpp

+
+        // Compute gradients magnitude.
+        magnitude(Iy, Ix, gradient_magnitude);
+        minMaxLoc(gradient_magnitude, 0, &max_val);


max_val

define this variable right before its usage. No need to put it somewhere in the beginning.

alalek · 2020-12-07T17:38:15Z

modules/imgproc/src/intelligent_scissors.cpp

+        // Compute gradients magnitude.
+        magnitude(Iy, Ix, gradient_magnitude);
+        minMaxLoc(gradient_magnitude, 0, &max_val);
+        gradient_magnitude.convertTo(gradient_magnitude, CV_32F, -1/max_val, 1.0);


-1/max_val

Check for near-zero value is necessary before division.

alalek · 2020-12-07T21:23:43Z

modules/imgproc/src/intelligent_scissors.cpp

+                            continue;
+                        if (expand.at<uchar>(ty, tx) == 0)
+                        {
+                            Point q = Point(tx, ty);


-Point q = Point(tx, ty); +Point q(tx, ty);

alalek · 2020-12-07T21:24:38Z

samples/cpp/intelligent_scissors.cpp

-}
+// static float local_cost(const Point& p, const Point& q, const Mat& gradient_magnitude, const Mat& Iy, const Mat& Ix, const Mat& zero_crossing)
+// {
+//     float fG = gradient_magnitude.at<float>(q.y, q.x);


Remove dead code from repository.

platforms/js/opencv_js.config.py

dkurt · 2020-12-08T16:10:23Z

platforms/js/opencv_js.config.py

-                'fillPoly', 'fillConvexPoly'],
+                'undistort','warpAffine','warpPerspective','warpPolar','watershed', 'Intelligent_scissors',\
+                'fillPoly', 'fillConvexPoly'], \
+                'Intelligent_scissors':['Intelligent_scissors', 'Intelligent_scissors_one_point', 'setBorder'], \


Define create method. Not sure if bindings support C++ constructors

dkurt · 2020-12-08T16:48:30Z

platforms/js/opencv_js.config.py

-                'fillPoly', 'fillConvexPoly'],
+                'undistort','warpAffine','warpPerspective','warpPolar','watershed',\
+                'fillPoly', 'fillConvexPoly'], \
+                'Intelligent_scissors': ['Intelligent_scissors', 'Intelligent_scissors_one_point', 'setBorder'], \


Please align with 'CLAHE' (less whitespaces) and remove extra \ at the end

dkurt · 2020-12-08T16:48:38Z

platforms/js/opencv_js.config.py

-                'undistort','warpAffine','warpPerspective','warpPolar','watershed', \
-                'fillPoly', 'fillConvexPoly'],
+                'undistort','warpAffine','warpPerspective','warpPolar','watershed',\
+                'fillPoly', 'fillConvexPoly'], \


dkurt · 2020-12-08T16:48:50Z

platforms/js/opencv_js.config.py

                'pointPolygonTest', 'putText','pyrDown','pyrUp','rectangle','remap', 'resize','sepFilter2D','threshold', \
-                'undistort','warpAffine','warpPerspective','warpPolar','watershed', \
-                'fillPoly', 'fillConvexPoly'],
+                'undistort','warpAffine','warpPerspective','warpPolar','watershed',\


revert this line

dkurt · 2020-12-08T16:58:33Z

@alalek, Do you know the source of this problem? https://pullrequest.opencv.org/buildbot/builders/precommit_docs/builds/28227

/build/precommit_docs/build/js/modules/js_bindings_generator/gen/bindings.cpp:367:39: error: no member named 'CamShift' in namespace 'cv'
        RotatedRect rotatedRect = cv::CamShift(arg1, arg2, arg3);
                                  ~~~~^
/build/precommit_docs/build/js/modules/js_bindings_generator/gen/bindings.cpp:376:21: error: no member named 'meanShift' in namespace 'cv'
        int n = cv::meanShift(arg1, arg2, arg3);
                ~~~~^
2 errors generated.
shared:ERROR: compiler frontend failed to generate LLVM bitcode, halting

alalek · 2020-12-08T17:23:44Z

This happens when JS binding generator lost headers of used modules.
It should be addressed by this patch #19000 (and it should be merged to the master branch). I will take a look on this.

alalek · 2020-12-08T19:54:07Z

Need to cherry-pick commit from PR #19059 (fix is targeted to 3.4 branch first)

Need to squash commits and rebase first, because base commit is from 2020-04-24

Rebase master branch

alalek · 2020-12-16T02:12:30Z

modules/imgproc/src/intelligent_scissors.cpp

+
+        cvtColor(src, grayscale, COLOR_BGR2GRAY);
+        Canny(grayscale, img_canny, EDGE_THRESHOLD_LOW, EDGE_THRESHOLD_HIGH);
+        threshold(img_canny, zero_crossing, 254, 1, THRESH_BINARY_INV);


zero_crossing computation does not follow the article. Canny() is not used there.

See Section 3.1

alalek · 2020-12-16T02:21:19Z

modules/imgproc/src/intelligent_scissors.cpp

+        Ix.convertTo(Ix, CV_32F, 1.0 / 255);
+        Iy.convertTo(Iy, CV_32F, 1.0 / 255);


The gradient direction is the unit vector defined by Ix and Iy.

This code doesn't provide the normalized vector: Ix(pt)^2 + Iy(pt)^2 = 1

alalek · 2020-12-17T12:40:49Z

modules/imgproc/src/intelligent_scissors.cpp

+                            float cost = cost_map.at<float>(p) + lcost(p, q, gradient_magnitude, Iy, Ix, zero_crossing);
+                            if (processed.at<uchar>(q) == 1 && cost < cost_map.at<float>(q))
+                            {
+                                removed.at<uchar>(q) = 1;


In article "updated" point is removed from "sorted" queue to be re-inserted again with new cost.
This code:

removes point completely from processing queue L (see if (removed.at<uchar>(p) == 0) condition above)

does not update "better" cost value

dkurt reviewed Sep 15, 2020

View reviewed changes

alalek reviewed Dec 7, 2020

View reviewed changes

dkurt reviewed Dec 8, 2020

View reviewed changes

platforms/js/opencv_js.config.py Outdated Show resolved Hide resolved

dkurt reviewed Dec 8, 2020

View reviewed changes

Merge pull request #1 from opencv/master

513b088

Rebase master branch

alvoron force-pushed the re_c branch from 39201b1 to 683be92 Compare December 11, 2020 16:20

intelligent scissors - squashed commit

da20384

alvoron force-pushed the re_c branch from 683be92 to da20384 Compare December 11, 2020 16:30

alvoron added 4 commits December 11, 2020 20:10

rename the class and fix indentation

1bdb3db

refactoring

35e028c

apply PImpl pattern

338e86e

fix namespace bug

da4e5ac

alalek reviewed Dec 16, 2020

View reviewed changes

alalek reviewed Dec 18, 2020

View reviewed changes

alalek mentioned this pull request Dec 22, 2020

imgproc: add IntelligentScissors #19194

Merged

4 tasks


		CV_EXPORTS_W float local_cost(const Point& p, const Point& q, const Mat& gradient_magnitude, const Mat& Iy, const Mat& Ix, const Mat& zero_crossing);

		CV_EXPORTS_W void find_min_path(const Point& start, Mat img, Mat zero_crossing, Mat gradient_magnitude, Mat Ix, Mat Iy, Mat& hit_map_x, Mat& hit_map_y);


		CV_WRAP void setBorder(const int input_border);

		CV_WRAP void Intelligent_scissors_one_point(Mat img, OutputArray total_hit_map_x, OutputArray total_hit_map_y, const Point start_point);

		Ix.convertTo(Ix, CV_32F, 1.0 / 255);
		Iy.convertTo(Iy, CV_32F, 1.0 / 255);

Uh oh!

Conversation

SimpleVlad commented Sep 15, 2020 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

SimpleVlad commented Sep 15, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshabunin commented Sep 16, 2020

Uh oh!

dkurt commented Sep 16, 2020

Uh oh!

alalek commented Sep 17, 2020

Uh oh!

asmorkalov commented Nov 26, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkurt commented Dec 8, 2020

Uh oh!

alalek commented Dec 8, 2020

Uh oh!

alalek commented Dec 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

SimpleVlad commented Sep 15, 2020 •

edited by alalek

Loading

alalek commented Dec 8, 2020 •

edited

Loading