| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
[G-API]: findContours() and boundingRect() Standard Kernels Implementation #18510
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
|
@anna-khakimova please take a look |
|
@AsyaPronina please take a look |
|
Warnings & failure with custom build |
Seems like this failure (most of warnings and streaming test fails) is in master as all the custom Linux builds are red because of this Working on my own warnings still, thanks |
- API and documentation provided: - as OpenCV provides two overloads whether to calculate hierarchy or not, but they differ by only the output in sight of G-API, two different G-API functions and kernels implemented - G-API Imgproc documentation divided into more parts according to imgproc module parts - some typos connected with division into parts corrected - `GArray<GArray<U>>` overload for `get_out` function provided to coonvert correctly into `vector<vector<U>>` - OCV backend supported - accuracy tests provided
- API and documentation provided:
- GOpaque<Rect> used as an output
- as OpenCV provides two possibilities whether to take a gray-scale image or a set of 2D points (`Point2i` or `Point2f` supported), three different overloads of a single G-API function and three kernels implemented
- for a gray-scale image the overload via `GMat`
- for a set of `Point2i` - the one via GArray<`Point2i`>
- set of `Point2f` -> GArray<`Point2f`>
- OCV backend supported
- accuracy tests provided
- comparison function for Rects provided
- some typos in `gapi_tests_common` corrected
OrestChura
force-pushed
the
oc/boundingRect
branch
from
ffb4ff6 to
ede8c65
Compare
- split tests - Fix Windows warnings
| // Draw random ellipses on given mat of given size and type | ||
| void initMatForFindingContours(cv::Mat& mat, const cv::Size& sz, int type) | ||
| { | ||
| cv::RNG rng(time(nullptr)); |
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.
Never do that.
Tests must be reproducible (they are here to catch regressions in the code, not an issues from algorithms).
Reuse cv::theRNG() (probably by reference).
/cc @dmatveev
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.
Oh, I'm really sorry! I've got it now much more clearly, I think.
So, even if it is not what caused issues, I should get rid of this time() I've added to tests when will fix this...
Thank you!
- Fix unnecessary precision losses
- Hierarchical -> H - added cv::GMatDesc::isVectorPoins() - added support of giving a set of points to boundingRect()
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.
Ok.
| @@ -109,6 +109,9 @@ struct GAPI_EXPORTS GMatDesc | |||
|
|
|||
| bool isND() const { return !dims.empty(); } | |||
|
|
|||
| // Checks if the passed mat is a set of n-dimetional points of the given depth | |||
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.
typo
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.
corrected
| The function retrieves contours from the binary image using the algorithm @cite Suzuki85 . | ||
| The contours are a useful tool for shape analysis and object detection and recognition. | ||
| See squares.cpp in the OpenCV sample directory. | ||
| @note Since opencv 3.2 source image is not modified by this function. |
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 note doesn't make sense for G-API module.
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 see, can agree with you. Input Mat actually never changes in G-API..
But on the one hand, I thought it could be important somehow to preserve the notes from OpenCV docs.
On the other hand - since it is a new operation for G-API, there is no need to mention the changes of API of OpenCV 3.2, right?
So, I should just remove this note, am I correct about the logic of this decision?
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.
removed
| @return GArray of detected contours. Each contour is stored as a GArray of points. | ||
| */ | ||
| GAPI_EXPORTS GArray<GArray<Point>> | ||
| findContours(const GMat &src, const int mode, const int method, const Point &offset = Point()); |
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.
int mode
int method
Consider using enums instead.
offset
Not really needed.
At least in form of constant (used to process ROIs, so it should be some dynamic parameter).
/cc @dmatveev
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.
enums used instead of ints
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.
offset is GOpaque now to be dynamic. It is required argument now though. Will return default value back later, when GOpaque supports ConstVal initialization
| @@ -765,6 +791,42 @@ class AbsToleranceScalar : public WrappableScalar<AbsToleranceScalar> | |||
| double _tol; | |||
| }; | |||
|
|
|||
| class AbsToleranceRect : public WrappableRect<AbsToleranceRect> | |||
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.
Intersection over union?
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.
replaced this version of comparison by IoU. Thanks for advice!
| AbsToleranceRect(double tol) : _tol(tol) {} | ||
| bool operator() (const cv::Rect& in1, const cv::Rect& in2) const | ||
| { | ||
| double abs_err = ( static_cast<double>(std::abs(in1.x - in2.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.
"Intersection over union" (IoU)?
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.
replaced. Thanks for advice!
- IoU comparison function added for Rects - isPointsVector moved from a GMatDesc method to a separate function in imgproc.hpp - enums instead of int - typos corrected
| @@ -271,6 +271,11 @@ template<> struct get_out<cv::GArray<cv::GMat> >: public get_out<cv::GArray<cv:: | |||
| { | |||
| }; | |||
|
|
|||
| //FIXME(dm): GArray<vector<U>>/GArray<GArray<U>> conversion should be done more gracefully in the system | |||
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.
Put space after //
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
| / std::max(1, std::abs(in2.height))) / 4.0; | ||
| if (abs_err > _tol) | ||
| { | ||
| std::cout << "AbsToleranceRect error: abs_err=" << abs_err << " tolerance=" << _tol |
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.
Debug prints ?
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.
Leaved as is for now, will discuss the possibility -> std::cerr later
- findContours: Point offset -> GOpaque<Point> - removed "straight" comparison for Rects, IoU available only - changed vectors initialization -> fix Debug test run - Some typos
|
@dmatveev Please take a look |
| GAPI_EXPORTS GArray<GArray<Point>> | ||
| findContours(const GMat &src, const RetrievalModes mode, const ContourApproximationModes method, | ||
| // FIXME oc: make default value offset = Point() | ||
| const GOpaque<Point> &offset); |
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.
add overload without offset?
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.
Yes! Thank you! It will be better when there is ConstVal for GOpaque appeared, but for a temporary workaround for functionality it's great
| } | ||
|
|
||
| // Checks if the passed mat is a set of n-dimentional points of the given depth | ||
| bool isPointsVector(const int chan, const cv::Size &size, const int depth, |
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.
isPointsVector
Why "points"?
Refer to OpenCV functionality. It is called checkVector. Also you may want to use checkArray() naming here.
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.
@alalek This checks if it's vector of points represented via GMatDesc. Although, I agree that this naming is debatable.
Don't want to refer fully to checkVector() method as this is a cut version of it, not fully corresponding to all the functionality of the original Mat::checkVector().
Do you insist to rename to checkArray()?
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 name works well, given its intent, I believe
| @} | ||
| */ | ||
|
|
||
| namespace { | ||
| void checkMetaForFindingContours(const int depth, const int chan, const int mode) |
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.
"validateSomething"?
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.
@alalek like validateFindingContoursMeta? Changed
- overload without offset added (as a temporary workaround) - checkMetaForFindingContours -> validateFindingContoursMeta - added ostream overload for enums used in tests
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.
LGTM, đź‘Ť thanks!
| } | ||
|
|
||
| // Checks if the passed mat is a set of n-dimentional points of the given depth | ||
| bool isPointsVector(const int chan, const cv::Size &size, const int depth, |
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 name works well, given its intent, I believe
[G-API]: findContours() and boundingRect() Standard Kernels Implementation
* Add findContours() standard kernel
- API and documentation provided:
- as OpenCV provides two overloads whether to calculate hierarchy or not, but they differ by only the output in sight of G-API, two different G-API functions and kernels implemented
- G-API Imgproc documentation divided into more parts according to imgproc module parts
- some typos connected with division into parts corrected
- `GArray<GArray<U>>` overload for `get_out` function provided to coonvert correctly into `vector<vector<U>>`
- OCV backend supported
- accuracy tests provided
* Add boundingRect() standard kernel
- API and documentation provided:
- GOpaque<Rect> used as an output
- as OpenCV provides two possibilities whether to take a gray-scale image or a set of 2D points (`Point2i` or `Point2f` supported), three different overloads of a single G-API function and three kernels implemented
- for a gray-scale image the overload via `GMat`
- for a set of `Point2i` - the one via GArray<`Point2i`>
- set of `Point2f` -> GArray<`Point2f`>
- OCV backend supported
- accuracy tests provided
- comparison function for Rects provided
- some typos in `gapi_tests_common` corrected
* Fix precommit windows warnings
* - Addressing comments:
- split tests
- Fix Windows warnings
* Static_cast for warnings
* - Remove randomness
- Fix unnecessary precision losses
* - Forgot reference for RNG
* addressing comments
* equalizeHist -> no group
* `const` addedin new functions
* Address suggestions:
- Hierarchical -> H
- added cv::GMatDesc::isVectorPoins()
- added support of giving a set of points to boundingRect()
* Addressing comments
- IoU comparison function added for Rects
- isPointsVector moved from a GMatDesc method to a separate function in imgproc.hpp
- enums instead of int
- typos corrected
* Addressing comments
- findContours: Point offset -> GOpaque<Point>
- removed "straight" comparison for Rects, IoU available only
- changed vectors initialization -> fix Debug test run
- Some typos
* added comment for later upgrades
* Fix not to corrupt docs by FIXME
* Addressing commens
- overload without offset added (as a temporary workaround)
- checkMetaForFindingContours -> validateFindingContoursMeta
- added ostream overload for enums used in tests
These changes:
Add findContours() standard kernel
divide G-API Imgproc documentation into more parts according to imgproc module parts
provide
GArray<GArray<U>>overload forget_outfunction to convert correctly intovector<vector<U>>Add boundingRect() standard kernel
Point2iorPoint2fsupported), three different overloads of a single G-API function and three kernels implementedGMatPoint2i- the one via GArray<Point2i>Point2f-> GArray<Point2f>comparison function for Rects provided
some typos in
gapi_tests_commoncorrectedPull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.