[G-API] Introduce GOpaque and GArray for python#19319
Conversation
9ac459b to
5eccf38
Compare
|
@dmatveev Could you take a look ? |
| { | ||
|
|
||
| #define HANDLE_CASE(T, K) case gapi::ArgType::CV_##T: \ | ||
| m_arg = cv::GArray<K>(); \ |
There was a problem hiding this comment.
Can some of this code become template to support both array and opaque? All those duplicates of HANDLE_CASE with the same types looks way too massive
There was a problem hiding this comment.
E.g. move some switches to template functions which return desired type and then use those functions in constructors/etc
There was a problem hiding this comment.
What should return this template function ?
There was a problem hiding this comment.
Can try to utilize something like getFromStream (from serialization.cpp), non-blocking though
smirnov-alexey
left a comment
There was a problem hiding this comment.
Except for code duplication looks good
| HANDLE_CASE(bool); | ||
| HANDLE_CASE(int); | ||
| HANDLE_CASE(double); | ||
| HANDLE_CASE(float); | ||
| HANDLE_CASE(std::string); | ||
| HANDLE_CASE(cv::Point); | ||
| HANDLE_CASE(cv::Point2f); | ||
| HANDLE_CASE(cv::Size); | ||
| HANDLE_CASE(cv::Rect); | ||
| HANDLE_CASE(cv::Scalar); | ||
| HANDLE_CASE(cv::Mat); | ||
| HANDLE_CASE(cv::GMat); |
There was a problem hiding this comment.
Please declare this type list once, and then reuse in your functions.
There was a problem hiding this comment.
Giving you some idea.
#include <iostream>
#include <tuple>
#include <vector>
#include <string>
#include <type_traits>
#define ID(X) X
#define ID_(X) ID(X),
#define NOTHING(X)
#define SPACE
#define COMMA ,
#define TYPE_LIST_G(G, X, A1, L1, A2, L2) \
G(A1(int) X A2("int")) \
G(A1(double) X A2("double")) \
G(L1(char) X L2("char"))
#define TYPE_LIST(A1, L1, A2, L2) \
TYPE_LIST_G(ID, SPACE, A1, L1, A2, L2)
#define TYPE_LIST2_G(G, A1, A2) \
TYPE_LIST_G(G, COMMA, A1, A1, A2, A2)
int main() {
// Example 1: generate a tuple from our type list
// A1 is identity plus ','
// L1 is identity (no comma)
// A2 is nothing
// L2 is nothing
using T = std::tuple< TYPE_LIST(ID_, ID, NOTHING, NOTHING) >;
static_assert(std::is_same<int, std::tuple_element<0, T>::type>::value, "First type is int");
static_assert(std::is_same<double, std::tuple_element<1, T>::type>::value, "Second type is double");
// Example 2: generate a string list from our type list
// A1 is nothing
// L1 is nothing
// A2 is identity plus ','
// L2 is identity (no comma)
std::vector<std::string> names = {
TYPE_LIST(NOTHING, NOTHING, ID_, ID)
};
for (const auto &s : names) std::cout << s << std::endl;
// Example 3: generate code around elements
// Functors for all and terminating elements are the same
#define HANDLE_CASE(T, S) std::cout << "sizeof(" << S << ") is " << sizeof(T) << std::endl;
TYPE_LIST2_G(HANDLE_CASE, ID, ID)
return 0;
}int
double
char
sizeof(int) is 4
sizeof(double) is 8
sizeof(char) is 1
There was a problem hiding this comment.
Many thanks! Fixed
| CV_RECT, | ||
| CV_SCALAR, | ||
| CV_MAT, | ||
| CV_GMAT, |
There was a problem hiding this comment.
We already have enum for Opaque types. Is this one different from that? Can you reuse that one?
There was a problem hiding this comment.
- It is placed in
cv::detail-cv.detail_CV_MATfrom python looks ugly CV_GMATisn't in thisenumyou mentioned
There was a problem hiding this comment.
BTW, It make sense to update bindings generators to support CV_WRAP_AS for enums (perhaps with full name, to escape `detail namespace)
There was a problem hiding this comment.
Will file a ticket, thanks!
|
@dmatveev Could you check it ? |
bb1a3e9 to
8c1520e
Compare
8c1520e to
afc381c
Compare
| // Copyright (C) 2021 Intel Corporation | ||
|
|
||
| #ifndef OPENCV_GAPI_PYTHON_BRIDGE_HPP | ||
| #define OPENCV_GAPI_PYTHON_BRIDGE_HPP |
There was a problem hiding this comment.
modules/gapi/include
This location should be used for C++ public API.
Python-specific files should go into modules/gapi/misc/python
.cpp
No need to store this content in gapi C++ binary file.
There was a problem hiding this comment.
Agree, but didn't know how to do that. Thank for you commit.
| CV_RECT, | ||
| CV_SCALAR, | ||
| CV_MAT, | ||
| CV_GMAT, |
There was a problem hiding this comment.
BTW, It make sense to update bindings generators to support CV_WRAP_AS for enums (perhaps with full name, to escape `detail namespace)
|
Please take a look on Windows compilation errors (they were here before my commit too) |
c41ece6 to
158824e
Compare
…que-garray-for-python [G-API] Introduce GOpaque and GArray for python * Introduce GOpaque and GArray for python * Fix ctor * Avoid code duplication by using macros * gapi: move Python-specific files to misc/python * Fix windows build Co-authored-by: Alexander Alekhin <alexander.a.alekhin@gmail.com>
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.
Overview
This PR is a part of #18900. To make review process easier decided to split this on several PR's.
Templates classes that GOpaque/GArray are can't be wrapped into python by using macros. (Parser ignores templates objects)
gapi/include/python_bridge.hppandgapi/src/gapi/python_bridge.cpp. They must be in public api to allowpythonmodule work with entities from there.GOpaqueT/GArrayTwrapper, thus all types collected in a single class. This makes it easier to handle them.ArgTypeis implemented asenumnotenum classto usecv.gapi.CV_BOOLfrom python instead ofcv.gapi.ArgType_CV_BOOL.ArgTypecontainsCV_GMATvalue to work withGArray<cv::GMat>Build configuration