CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
improved version of HoughCircles (HOUGH_GRADIENT_ALT method) #16561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* make use of param2. make it minCos2 (minimal value of squared cosine between the gradient at the pixel edge and the vector connecting it with circle center). with minCos2=0.85 we can detect some more eyes :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New algorithm is interesting, but linked issue doesn't ask for that.
The issue blames on regression(changes) in implementation of existed algorithm between OpenCV versions.
@@ -473,7 +473,8 @@ enum HoughModes { | |||
/** multi-scale variant of the classical Hough transform. The lines are encoded the same way as | |||
HOUGH_STANDARD. */ | |||
HOUGH_MULTI_SCALE = 2, | |||
HOUGH_GRADIENT = 3 //!< basically *21HT*, described in @cite Yuen90 | |||
HOUGH_GRADIENT = 3, //!< basically *21HT*, described in @cite Yuen90 | |||
HOUGH_GRADIENT_ALT = 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
modules/imgproc/src/hough.cpp
Outdated
static int circle_popcnt(uint64 mask) | ||
{ | ||
int count = 0; | ||
for(int b = 0; b < 64; b++, mask >>= 1) | ||
count += (mask & 1) != 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reuse existed popcnt implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not found simple inline function that takes uint64(_t) on input, just universal intrinsic that I do not need. I improved the implementation to use CV_POPCOUNT_U64 if possible. Someone (maybe me) in some subsequent patch can move it to some of core headers and rename cv::circle_popcnt() to cv::popcount() or cvPopCount64() or something like that.
modules/imgproc/src/hough.cpp
Outdated
if( maxRadius <= 0 ) maxRadius = std::min(img.cols, img.rows)*0.5; | ||
if( minRadius > maxRadius ) std::swap(minRadius, maxRadius); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid merging of if's conditions and if's body.
Let make code coverage reports more useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Mat Dx, Dy, edges; | ||
Scharr(img, Dx, CV_16S, 1, 0); | ||
Scharr(img, Dy, CV_16S, 0, 1); | ||
Canny(Dx, Dy, edges, cannyThreshold/2, cannyThreshold, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe such pre-processing should be optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not preprocessing. Canny edge detector is the essential part of HoughCircles that takes grayscale image on input. HOUGH_GRADIENT method does the same.
{ | ||
for( int x = 0; x < edges.cols; x++ ) | ||
{ | ||
if(!edgeData[y*estep + x] || mdata[y*mstep + x]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid manual address arithmetic.
There is still efficient .at(y, x)
method for release builds and there are extra checks in debug builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the inner loop where performance matters. Address arithmetic is faster here and will not be replaced with .at
circles.clear(); | ||
std::vector<Vec4f> nz; | ||
|
||
std::vector<Point> stack; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is std::stack
for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::stack is an adapter on top of std::vector (or list or deque). Why use this adapter instead of plain simple std::vector?
modules/imgproc/src/hough.cpp
Outdated
int sx = cvRound(vx * 1024 / mag); | ||
int sy = cvRound(vy * 1024 / mag); | ||
|
||
int x0 = cvRound((p.x * idp) * 1024); | ||
int y0 = cvRound((p.y * idp) * 1024); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is 1024 magic number?
Consider declaring this constant separately with descriptive name.
Same notes are about 64
, 7
, 3
below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
modules/imgproc/src/hough.cpp
Outdated
} | ||
while(!stack.empty()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't add line break before while
in do while
statements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
modules/imgproc/src/hough.cpp
Outdated
int prev = prev_idx[j]; | ||
prev_idx[j] = i; | ||
|
||
if( std::abs(rij - r_arc) < (r_arc + 80)*0.03 && prev+1 == i && !stop_marker ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
80
0.03
and below
4000
0.06
"magic numbers"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
for( int k = 1; k < 2; k++ ) | ||
{ | ||
int method = k == 0 ? HOUGH_GRADIENT : HOUGH_GRADIENT_ALT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to split/parametrize test itself instead of internal loops.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
* cleaned up the implementation; added comments, replaced built-in numeic constants with symbolic constants * rewrote circle_popcount() to use built-in popcount() if possible * modified some of HoughCircles tests to use method parameter instead of the built-in loop
@alalek, the original issue complains about HoughCircles quality on some particular images, but the problem is much deeper. For a long time I knew that cv::HoughCircles() does not work well. But I was not sure about the effects of the latest modifications, if they improved or degraded quality and if there is some good revision of this implementation. So, for confirmation I took several revisions of OpenCV and ran them on a dataset of ~100 images (mostly from here: https://github.com/AlanLuSun/Circle-detection, but also my little collection of circles images and photos from Internet). The results are really bad. The existing implementation is very advanced in terms of speed optimization efforts, but it's fundamentally wrong. The newly proposed algorithm, on which I spent the whole week, uses the similar first phase with 2D accumulator update, but the center selection and the radius estimation are completely different. It's not compatible with the existing implementation. It needs somewhat higher cannyThreshold (param1). It treats minRadius a bit differently. It treats param2 completely differently, because accumThreshold is very difficult to set to some "universally good" value, you need to vary it from image to image and too bad if you have circles of very different size in the same image - there is no good accumThreshold value in such case. It does not have "center only" mode, because any Hough-based algorithm for circle detection, be it HOUGH_GRADIENT or HOUGH_GRADIENT_ALT, produce a lot of fake centers. So I decided to make a separate algorithm without having to keep compatibility with the previous version. In OpenCV 5.0, perhaps, HOUGH_GRADIENT can be removed completely. |
If Also, it would be great to have some numbers to evaluate the performance of the new method on a ground truth dataset. |
@catree, thanks! More detailed test that evaluates performance over a real dataset is being developed. |
@vpisarev When will a test evaluating the performance of the Hough circles implementations be published? |
Hi, |
* improved version of HoughCircles (HOUGH_GRADIENT_ALT method) * trying to fix build problems on Windows * fixed typo * * fixed warnings on Windows * make use of param2. make it minCos2 (minimal value of squared cosine between the gradient at the pixel edge and the vector connecting it with circle center). with minCos2=0.85 we can detect some more eyes :) * * added description of HOUGH_GRADIENT_ALT * cleaned up the implementation; added comments, replaced built-in numeic constants with symbolic constants * rewrote circle_popcount() to use built-in popcount() if possible * modified some of HoughCircles tests to use method parameter instead of the built-in loop * fixed warnings on Windows
relates #16187 (on one image the eye is not detected with param2=0.9, need to lower it down to 0.85). Overall, the quality is much better than the existing HOUGH_GRADIENT method.
Usage:
this code detects circles on most of the images that I have (~100 images; the proper regression test will be added later). When it does not, try to narrow down the radius search range
[minRadius, maxRadius]
and decrease a bitminCos2
from 0.9 down to 0.85..0.75.Patch to opencv_extra has the same branch name.