G-API: Wrap render functionality to python by TolyaTalamanov · Pull Request #20284 · opencv/opencv

TolyaTalamanov · 2021-06-20T14:54:08Z

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

Build configuration

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

Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

build_image:Custom=ubuntu-openvino-2021.3.0:20.04
build_image:Custom Win=openvino-2021.2.0
build_image:Custom Mac=openvino-2021.2.0

test_modules:Custom=gapi,python2,python3,java
test_modules:Custom Win=gapi,python2,python3,java
test_modules:Custom Mac=gapi,python2,python3,java

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

modules/gapi/include/opencv2/gapi/render/render_types.hpp

TolyaTalamanov · 2021-06-22T06:51:15Z

modules/gapi/misc/python/package/gapi/__init__.py

+
+
+# FIXME: On the c++ side every class is placed in cv2 module.
+cv.gapi.wip.draw.Rect = cv.gapi_wip_draw_Rect


@alalek How ugly is it ? Is there another option ?

Not sure why you need that. There are many other classes from nested namespaces which are located properly.

AFAIK, there is some bug with Python ctors (this is why "create" method is used).
It is better to fix that problem instead of replacing classes with functions.

Not sure why you need that. There are many other classes from nested namespaces which are located properly.
Could you provide any examples ? Looks like all classes is placed in cv2 module.

looks like it is not supported properly for now.

BTW, cv.gapi_wip_draw_Rect is a function, not a type.

From generated file:

CVPY_TYPE(gapi_wip_draw_Rect, gapi_wip_draw_Rect, cv::gapi::wip::draw::Rect, Rect, NoBase, pyopencv_cv_gapi_wip_draw_gapi_wip_draw_Rect_Rect)

TolyaTalamanov · 2021-06-22T06:52:19Z

modules/gapi/misc/python/pyopencv_gapi.hpp

+template <>
+PyObject* pyopencv_from(const cv::gapi::wip::draw::Prim& prim)
+{
+    switch (prim.index()) {


I believe it's possible to implement pyopencv_from for generic variant by using visitor, but it's not merged yet.

TolyaTalamanov · 2021-06-22T06:56:02Z

modules/gapi/misc/python/pyopencv_gapi.hpp

+bool pyopencv_to(PyObject* obj, cv::gapi::wip::draw::Prim& value, const ArgInfo& info)
+{
+#define TRY_EXTRACT(Prim)                                                                                  \
+    if (PyObject_TypeCheck(obj, reinterpret_cast<PyTypeObject*>(pyopencv_gapi_wip_draw_##Prim##_TypePtr))) \


@alalek Is there any way to implemente it via more high level functions ? pyopencv_to/pyopencv_to_safe aren't suitable there, are they ? Because they set an error in case mismatch.

Not sure that do you mean here.
"dynamic types" are handled though switches/virtual functions.

If you don't want to follow pyopencv_to/pyopencv_from conventions, then please change names of these functions.

What do u mean by "dynamic types". Is the cv::variant<Types...> is a dynamic type ?
It seems to me since cv::variant<> has the limited number of possible types it can be wrapped like this:

cv::gapi::wip::draw::Rect rect; if (pyopencv_to(obj, rect, info)) { value = rect; return true; } ... if(pyopencv_to(obj, text, info) { value = text; return false; } // set error return false;

But it doesn't work because in case fail first pyopencv_to setup an error.

Is it possible to implement pyopencv_to for variant<> following all conventions somehow else it's done above ?

Variant implementation always has a tag/kind field which says which underlying object is stored.
There is NO sequence of calls.
There is switch(tag) with ONE call.

So you need to determine what is the "tag" value in that case. Perhaps switching over PyObject_TypeCheck is enough.

But in this case variant<> is empty, isn't it ?
It seems I can't just go through switch to determinate the type, cuz there is no stored object so far

I've implemented pyopencv_from like you proposed above

alalek · 2021-06-23T00:26:12Z

modules/gapi/misc/python/package/gapi/__init__.py

+
+
+# FIXME: On the c++ side every class is placed in cv2 module.
+cv.gapi.wip.draw.Rect = cv.gapi_wip_draw_Rect


Not sure why you need that. There are many other classes from nested namespaces which are located properly.

AFAIK, there is some bug with Python ctors (this is why "create" method is used).
It is better to fix that problem instead of replacing classes with functions.

alalek · 2021-06-23T00:33:23Z

modules/gapi/misc/python/pyopencv_gapi.hpp

+bool pyopencv_to(PyObject* obj, cv::gapi::wip::draw::Prim& value, const ArgInfo& info)
+{
+#define TRY_EXTRACT(Prim)                                                                                  \
+    if (PyObject_TypeCheck(obj, reinterpret_cast<PyTypeObject*>(pyopencv_gapi_wip_draw_##Prim##_TypePtr))) \


Not sure that do you mean here.
"dynamic types" are handled though switches/virtual functions.

If you don't want to follow pyopencv_to/pyopencv_from conventions, then please change names of these functions.

alalek · 2021-06-23T00:33:51Z

modules/gapi/misc/python/pyopencv_gapi.hpp

+    TRY_EXTRACT(Image)
+    TRY_EXTRACT(Poly)
+
+    return false;


Python error should be raised with proper message.

TolyaTalamanov · 2021-06-24T08:00:42Z

@alalek @mpashchenkov Could you have a look, please ?

TolyaTalamanov · 2021-06-28T07:08:32Z

@alalek I answered the comments, could check it one more time, please ?

TolyaTalamanov · 2021-06-29T09:25:45Z

@alalek CI is green, could you check it one more time, please ?

alalek

Looks good to me

alalek · 2021-06-29T09:47:33Z

modules/gapi/include/opencv2/gapi/render/render_types.hpp

-         int lt_ = 8,
-         bool bottom_left_origin_ = false) :
+    GAPI_WRAP Text(const std::string& text_,
+                   const cv::Point& org_,


It makes sense to put modifiers on a separate line:

GAPI_WRAP Text(const std::string& text_,

(to avoid touching indentation on changes)

alalek · 2021-06-29T09:53:45Z

modules/gapi/include/opencv2/gapi/render/render_types.hpp

+    GAPI_WRAP Text() = default;

    /*@{*/
-    std::string text;               //!< The text string to be drawn
-    cv::Point   org;                //!< The bottom-left corner of the text string in the image
-    int         ff;                 //!< The font type, see #HersheyFonts
-    double      fs;                 //!< The font scale factor that is multiplied by the font-specific base size
-    cv::Scalar  color;              //!< The text color
-    int         thick;              //!< The thickness of the lines used to draw a text
-    int         lt;                 //!< The line type. See #LineTypes
-    bool        bottom_left_origin; //!< When true, the image data origin is at the bottom-left corner. Otherwise, it is at the top-left corner
+    GAPI_PROP std::string text;               //!< The text string to be drawn
+    GAPI_PROP cv::Point   org;                //!< The bottom-left corner of the text string in the image
+    GAPI_PROP int         ff;                 //!< The font type, see #HersheyFonts


If you wrap default ctor, then it makes sense to use RW properties (like CV_PROP_RW) to allow updating of them.

alalek · 2021-06-29T09:55:05Z

modules/gapi/misc/python/package/gapi/__init__.py

+
+
+# FIXME: On the c++ side every class is placed in cv2 module.
+cv.gapi.wip.draw.Rect = cv.gapi_wip_draw_Rect


looks like it is not supported properly for now.

BTW, cv.gapi_wip_draw_Rect is a function, not a type.

…p-render

alalek · 2021-06-30T09:10:58Z

please rebase PR on new master code

…p-render

TolyaTalamanov · 2021-06-30T11:21:51Z

@alalek Rebased, CI is green

alalek

Looks good to me 👍

modules/gapi/include/opencv2/gapi/render/render_types.hpp

mpashchenkov · 2021-07-01T07:18:11Z

modules/gapi/include/opencv2/gapi/render/render_types.hpp

-         int thick_ = 1,
-         int lt_ = 8,
-         int shift_ = 0) :
+                   const cv::Scalar& color_,


TolyaTalamanov · 2021-07-01T09:25:22Z

@alalek Could you merge it, please ?

G-API: Wrap render functionality to python * Wrap render Rect prim * Add all primitives and tests * Cover mosaic and image * Handle error in pyopencv_to(Prim) * Move Mosaic and Rect ctors wrappers to shadow file * Use GAPI_PROP_RW * Fix indent

Wrap render Rect prim

b9de0da

TolyaTalamanov changed the title ~~[G-API] Wrap render functionality to python~~ G-API: Wrap render functionality to python Jun 21, 2021

Add all primitives and tests

1685290

TolyaTalamanov force-pushed the at/wrap-render branch from c90fe6b to 1685290 Compare June 21, 2021 11:53

Cover mosaic and image

bc3a51b

TolyaTalamanov commented Jun 22, 2021

View reviewed changes

alalek reviewed Jun 23, 2021

View reviewed changes

TolyaTalamanov requested a review from mpashchenkov June 24, 2021 08:00

Handle error in pyopencv_to(Prim)

6e17f69

TolyaTalamanov force-pushed the at/wrap-render branch from 5fb7744 to 6e17f69 Compare June 28, 2021 07:06

Move Mosaic and Rect ctors wrappers to shadow file

4aefa4e

alalek reviewed Jun 29, 2021

View reviewed changes

TolyaTalamanov added 2 commits June 30, 2021 11:09

Use GAPI_PROP_RW

9cef443

Merge branch 'master' of https://github.com/opencv/opencv into at/wra…

5d56d6c

…p-render

Merge branch 'master' of https://github.com/opencv/opencv into at/wra…

02b00f7

…p-render

alalek reviewed Jun 30, 2021

View reviewed changes

mpashchenkov approved these changes Jul 1, 2021

View reviewed changes

Fix indent

d88d87c

alalek assigned mpashchenkov Jul 1, 2021

alalek merged commit 9fe4949 into opencv:master Jul 1, 2021

alalek mentioned this pull request Oct 15, 2021

(5.x) Merge 4.x #20886

Merged



		# FIXME: On the c++ side every class is placed in cv2 module.
		cv.gapi.wip.draw.Rect = cv.gapi_wip_draw_Rect

Uh oh!

Conversation

TolyaTalamanov commented Jun 20, 2021

Pull Request Readiness Checklist

Build configuration

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov commented Jun 24, 2021

Uh oh!

TolyaTalamanov commented Jun 28, 2021

Uh oh!

TolyaTalamanov commented Jun 29, 2021

Uh oh!

alalek left a comment

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!

alalek commented Jun 30, 2021

Uh oh!

TolyaTalamanov commented Jun 30, 2021

Uh oh!

alalek left a comment

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!

TolyaTalamanov commented Jul 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants