Skip to content

[G-API]: morphologyEx() Standard Kernel Implementation#18652

Merged
alalek merged 3 commits intoopencv:masterfrom
OrestChura:oc/morphologyEx
Nov 10, 2020
Merged

[G-API]: morphologyEx() Standard Kernel Implementation#18652
alalek merged 3 commits intoopencv:masterfrom
OrestChura:oc/morphologyEx

Conversation

@OrestChura
Copy link
Copy Markdown
Contributor

@OrestChura OrestChura commented Oct 23, 2020 •
edited
Loading

These changes:

  • Add morphologyEx() standard kernel
    • provide API and documentation (without separate 3x3 version)
    • support OCV backend
    • provide accuracy tests: check only different operations, not kernels/borders

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 accuracy 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=Custom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

build_image:Custom=centos:7
buildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

Xbuild_image:Custom=ubuntu-openvino-2020.3.0:16.04
Xbuild_image:Custom Win=openvino-2020.3.0
Xbuild_image:Custom Mac=openvino-2020.3.0

test_modules:Custom=gapi
test_modules:Custom Win=gapi
test_modules:Custom Mac=gapi

buildworker:Custom=linux-1
# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

@OrestChura
cv::gapi::morphologyEx() kernel
245a9f8 
 - implemented (without separate 3x3 version)
 - tests added: check only different operations, not kernels/borders
@OrestChura
Copy link
Copy Markdown
Contributor Author

OrestChura commented Oct 25, 2020

@mpashchenkov please, take a look

@OrestChura
Copy link
Copy Markdown
Contributor Author

OrestChura commented Oct 26, 2020

@anna-khakimova please, review

mpashchenkov
mpashchenkov reviewed Oct 26, 2020
View reviewed changes
modules/gapi/include/opencv2/gapi/imgproc.hpp Outdated
apply successively: erode -> erode -> dilate -> dilate
(and not erode -> dilate -> erode -> dilate).
*/
GAPI_EXPORTS GMat morphologyEx(const GMat &src, int op, const Mat &kernel,
Copy link
Copy Markdown
Contributor

@mpashchenkov mpashchenkov Oct 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const int op

Copy link
Copy Markdown
Contributor Author

@OrestChura OrestChura Oct 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, done

mpashchenkov
mpashchenkov approved these changes Oct 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@mpashchenkov mpashchenkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@OrestChura
Copy link
Copy Markdown
Contributor Author

OrestChura commented Nov 2, 2020

@TolyaTalamanov please, review

@TolyaTalamanov
Copy link
Copy Markdown
Contributor

TolyaTalamanov commented Nov 2, 2020

Do you have a plan to implement performance test ?

@OrestChura
Copy link
Copy Markdown
Contributor Author

OrestChura commented Nov 2, 2020 •
edited
Loading

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

TolyaTalamanov
TolyaTalamanov reviewed Nov 2, 2020
View reviewed changes
modules/gapi/test/cpu/gapi_imgproc_tests_cpu.cpp
cv::MorphTypes::MORPH_TOPHAT,
cv::MorphTypes::MORPH_BLACKHAT)));

INSTANTIATE_TEST_CASE_P(MorphologyExHitMissTestCPU, MorphologyExTest,
Copy link
Copy Markdown
Contributor

@TolyaTalamanov TolyaTalamanov Nov 2, 2020

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 ?

Copy link
Copy Markdown
Contributor

@mpashchenkov mpashchenkov Nov 2, 2020

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.

Copy link
Copy Markdown
Contributor Author

@OrestChura OrestChura Nov 2, 2020

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

@OrestChura
Copy link
Copy Markdown
Contributor Author

OrestChura commented Nov 6, 2020

@dmatveev please take a look

alalek
alalek reviewed Nov 6, 2020
View reviewed changes
modules/gapi/include/opencv2/gapi/imgproc.hpp Outdated
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,
Copy link
Copy Markdown
Member

@alalek alalek Nov 6, 2020

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

Copy link
Copy Markdown
Contributor Author

@OrestChura OrestChura Nov 9, 2020

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

Copy link
Copy Markdown
Contributor

@dmatveev dmatveev Nov 10, 2020

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.

@OrestChura
Replaced fundamental tyeps -> enums where needed
7df3631 
 - added operator<< overload for cv::MorphTypes for tests output
dmatveev
dmatveev approved these changes Nov 10, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@alalek alalek merged commit 5f1ca33 into opencv:master Nov 10, 2020
@alalek alalek mentioned this pull request Nov 27, 2020
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
@OrestChura
Merge pull request opencv#18652 from OrestChura:oc/morphologyEx
45c50c3 
[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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek left review comments

@mpashchenkov mpashchenkov mpashchenkov approved these changes

@dmatveev dmatveev dmatveev approved these changes

@TolyaTalamanov TolyaTalamanov TolyaTalamanov approved these changes

Assignees

@dmatveev dmatveev

Labels

category: g-api / gapi feature test

Projects

None yet

Milestone

4.5.1

Development

Successfully merging this pull request may close these issues.

5 participants

@OrestChura @TolyaTalamanov @dmatveev @alalek @mpashchenkov