| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
[G-API]: morphologyEx() Standard Kernel Implementation #18652
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
- implemented (without separate 3x3 version) - tests added: check only different operations, not kernels/borders
|
@mpashchenkov please, take a look |
|
@anna-khakimova please, review |
| apply successively: erode -> erode -> dilate -> dilate | ||
| (and not erode -> dilate -> erode -> dilate). | ||
| */ | ||
| GAPI_EXPORTS GMat morphologyEx(const GMat &src, int op, const Mat &kernel, |
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.
const int op
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.
agree, done
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.
Looks good.
|
@TolyaTalamanov please, review |
|
Do you have a plan to implement performance test ? |
It is in plans, but after all the new kernels are merged, not in this PR |
| cv::MorphTypes::MORPH_TOPHAT, | ||
| cv::MorphTypes::MORPH_BLACKHAT))); | ||
|
|
||
| INSTANTIATE_TEST_CASE_P(MorphologyExHitMissTestCPU, MorphologyExTest, |
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.
Why it's tested separately ?
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.
"hit or miss" .- Only supported for CV_8UC1 binary images.
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.
'Cause only CV_8U is supported for this type of morphological operation, so I couldn't test it with others
|
@dmatveev please take a look |
| GAPI_EXPORTS GMat morphologyEx(const GMat &src, const int op, const Mat &kernel, | ||
| const Point &anchor = Point(-1,-1), | ||
| const int iterations = 1, | ||
| const int borderType = BORDER_CONSTANT, |
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.
const int op
int borderType
consider using enums instead
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, thanks! Sorry, forgot to change in this PR
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.
consider using enums instead
So does OpenCV now, right? We currently follow the OpenCV API.
- added operator<< overload for cv::MorphTypes for tests output
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!
[G-API]: morphologyEx() Standard Kernel Implementation * cv::gapi::morphologyEx() kernel - implemented (without separate 3x3 version) - tests added: check only different operations, not kernels/borders * Address comments: add `const` where needed * Replaced fundamental tyeps -> enums where needed - added operator<< overload for cv::MorphTypes for tests output
These changes:
Pull 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.