Skip to content

Python binding for RotatedRect#23702

Merged
asmorkalov merged 5 commits intoopencv:4.xfrom
dkurt:py_rotated_rect
Jun 22, 2023
Merged

Python binding for RotatedRect#23702
asmorkalov merged 5 commits intoopencv:4.xfrom
dkurt:py_rotated_rect

Conversation

@dkurt
Copy link
Copy Markdown
Member

@dkurt dkurt commented May 29, 2023

Pull Request Readiness Checklist

related: #23546 (comment)

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 another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the 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

{
return true;
}
// This is a workaround for compatibility with an initialization from tuple.
Copy link
Copy Markdown
Contributor

@VadimLevin VadimLevin May 29, 2023

Choose a reason for hiding this comment

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

After changing CV_EXPORTS to CV_EXPORTS_W_SIMPLE code generator produces struct pyopencv_RotatedRect_t as well as specialization of struct PyOpenCV_Converter for cv::RotatedRect (see: build/modules/python_bindings_generator/pyopencv_generated_types_content.h)

Introducing one more struct pyopencv_RotatedRect_t in separate compilation unit leads to ODR violation.

Templated version of pyopencv_to uses PyOpenCV_Converter<T>::to method if there is no specialization of pyopencv_to (cv::RotatedRect has it), so you can call PyOpenCV_Converter<cv::RotatedRect>::to and check against successful conversion.

template<>
bool pyopencv_to(PyObject* obj, RotatedRect& dst, const ArgInfo& info)
{
    if (!obj || obj == Py_None)
    {
        return true;
    }
    // Check if user passed `cv::RotatedRect` instance
    if (PyOpenCV_Converter<T>::to(obj, dst, info))
    {
        return true;
    }
    PyErr_Clear();
    // Fallback to [[center_x, center_y], [width, height], angle] parsing 
    if (!PySequence_Check(obj))
    {
        failmsg("Can't parse '%s' as RotatedRect."
                "Input argument is neither cv::RotatedRect object instance nor provides sequence protocol",
                info.name);
        return false;
    }

Copy link
Copy Markdown
Member Author

@dkurt dkurt May 29, 2023

Choose a reason for hiding this comment

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

Templated version of pyopencv_to uses PyOpenCV_Converter<T>::to method if there is no specialization of pyopencv_to (cv::RotatedRect has it), so you can call PyOpenCV_Converter<cv::RotatedRect>::to and check against successful conversion.

This way there is a compile time problem:

error: ‘to’ is not a member of ‘PyOpenCV_Converter<cv::RotatedRect>’
  743 |     return PyOpenCV_Converter<cv::RotatedRect>::to(obj, dst, info);

However, the struct can be defined locally (see e406cbde36466c37bd741db758f6e7ca0696efe8).

Another problem is with PYTHON3_LIMITED_API=ON (Working on it):

rror: invalid use of incomplete type ‘struct _typeobject’
  739 |     if (std::string(Py_TYPE(obj)->tp_name) == "cv2.RotatedRect")

@dkurt dkurt force-pushed the py_rotated_rect branch from 4ce1326 to 24a6503 Compare May 30, 2023 10:35
@asmorkalov
Copy link
Copy Markdown
Contributor

@dkurt @VadimLevin Any ideas?

Copy link
Copy Markdown
Contributor

@VadimLevin VadimLevin left a comment

Choose a reason for hiding this comment

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

I don't like the solution with opaque struct, but there is no flexible solution to "fallback" to another parser in current bindings.

@asmorkalov asmorkalov merged commit 22b747e into opencv:4.x Jun 22, 2023
@asmorkalov asmorkalov mentioned this pull request Jul 27, 2023
thewoz pushed a commit to thewoz/opencv that referenced this pull request Jan 4, 2024
Python binding for RotatedRect opencv#23702

### Pull Request Readiness Checklist

related: opencv#23546 (comment)

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
thewoz pushed a commit to thewoz/opencv that referenced this pull request May 29, 2024
Python binding for RotatedRect opencv#23702

### Pull Request Readiness Checklist

related: opencv#23546 (comment)

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants