From d8267dd4877d9ffab962bc135f6f1137a2a846ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20=C5=81ukawski?= Date: Mon, 17 May 2021 14:53:31 +0200 Subject: [PATCH 1/4] Dismiss configurationcache bindings if `OPT_PYTHON` is `OFF` --- plugins/configurationcache/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/configurationcache/CMakeLists.txt b/plugins/configurationcache/CMakeLists.txt index b1d1c84c2b..0e4b7c40d0 100644 --- a/plugins/configurationcache/CMakeLists.txt +++ b/plugins/configurationcache/CMakeLists.txt @@ -8,7 +8,7 @@ set_target_properties(configurationcache PROPERTIES COMPILE_FLAGS "${PLUGIN_COMP install(TARGETS configurationcache DESTINATION ${OPENRAVE_PLUGINS_INSTALL_DIR} COMPONENT ${PLUGINS_BASE}) # python bindings -if( HAVE_ALL_PYTHON_HEADERS ) +if( OPT_PYTHON AND HAVE_ALL_PYTHON_HEADERS ) # include include_directories(${PYTHON_INCLUDE_PATH} ${PYTHON_INCLUDE_DIRS} ${OPENRAVE_CORE_INCLUDE_LOCAL_DIRS} ${OPENRAVEPY_INCLUDE_LOCAL_DIRS} From 24e6b63abe20d961ae8ab5e272995653bf0ae5af Mon Sep 17 00:00:00 2001 From: Hamdi Sahloul Date: Fri, 30 Jul 2021 13:35:17 +0900 Subject: [PATCH 2/4] Introduce PyCapsule --- CMakeLists.txt | 4 +- .../include/openravepy/openravepy_int.h | 58 ++++++++++++++++++- python/bindings/openravepy_global.cpp | 6 +- python/bindings/openravepy_int.cpp | 6 +- 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index eb48ed8dc5..5e9e2870d8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,8 +4,8 @@ set( CMAKE_ALLOW_LOOSE_LOOP_CONSTRUCTS TRUE ) # Define here the needed parameters set (OPENRAVE_VERSION_MAJOR 0) -set (OPENRAVE_VERSION_MINOR 83) -set (OPENRAVE_VERSION_PATCH 1) +set (OPENRAVE_VERSION_MINOR 84) +set (OPENRAVE_VERSION_PATCH 0) set (OPENRAVE_VERSION ${OPENRAVE_VERSION_MAJOR}.${OPENRAVE_VERSION_MINOR}.${OPENRAVE_VERSION_PATCH}) set (OPENRAVE_SOVERSION ${OPENRAVE_VERSION_MAJOR}.${OPENRAVE_VERSION_MINOR}) message(STATUS "Compiling OpenRAVE Version ${OPENRAVE_VERSION}, soversion=${OPENRAVE_SOVERSION}") diff --git a/python/bindings/include/openravepy/openravepy_int.h b/python/bindings/include/openravepy/openravepy_int.h index 9d8989e59f..09fc05e4ce 100644 --- a/python/bindings/include/openravepy/openravepy_int.h +++ b/python/bindings/include/openravepy/openravepy_int.h @@ -203,6 +203,8 @@ class OPENRAVEPY_API PythonThreadSaver PyThreadState *_save; }; +typedef OPENRAVE_SHARED_PTR PythonThreadSaverPtr; + /// \brief release and restore the python GIL... no thread state saved..? class OPENRAVEPY_API PythonGILSaver { @@ -215,6 +217,7 @@ class OPENRAVEPY_API PythonGILSaver } }; +// TODO: Remove AutoPyArrayObjectDereferencer in favor of PyCapsule class OPENRAVEPY_API AutoPyArrayObjectDereferencer { public: @@ -228,7 +231,60 @@ class OPENRAVEPY_API AutoPyArrayObjectDereferencer PyArrayObject* _pyarrobj; }; -typedef OPENRAVE_SHARED_PTR PythonThreadSaverPtr; +template +class OPENRAVEPY_API PyCapsule +{ +public: + // \brief disables copy constructor to avoid calling Py_DECREF more than once + PyCapsule(const PyCapsule&) = delete; + + // \brief the only entrypoint that allows a nullptr. Expects a `reset` call to follow + PyCapsule() + : _ptr(nullptr) + { + } + + // \param ptr a not-null pointer + PyCapsule(T* ptr) + : _ptr(_PreventNullPtr(ptr)) + { + } + + // \param ptr a not-null pointer + void reset(T* ptr) + { + _ptr = _PreventNullPtr(ptr); + } + + T* get() const + { + return _ptr; + } + + operator T*() const + { + return _ptr; + } + + ~PyCapsule() { + if (_ptr != nullptr) { + Py_DECREF(_ptr); + } + } + +private: + T* _ptr; + + static T* _PreventNullPtr(T* ptr) + { + if (ptr == nullptr) { + throw OPENRAVE_EXCEPTION_FORMAT0(_("Invalid Numpy-array pointer. Failed to get contiguous array?"), ORE_InvalidArguments); + } + return ptr; + } +}; + +typedef PyCapsule PyArrayCapsule; inline RaveVector ExtractFloat3(const py::object& o) { diff --git a/python/bindings/openravepy_global.cpp b/python/bindings/openravepy_global.cpp index 87de87cf6f..0dbeccc35d 100644 --- a/python/bindings/openravepy_global.cpp +++ b/python/bindings/openravepy_global.cpp @@ -350,8 +350,7 @@ class PyTriMesh if (!PyArray_ISFLOAT(pPyVertices)) { throw openrave_exception(_("vertices must be in float"), ORE_InvalidArguments); } - PyArrayObject* pPyVerticesContiguous = PyArray_GETCONTIGUOUS(reinterpret_cast(pPyVertices)); - AutoPyArrayObjectDereferencer pydecref(pPyVerticesContiguous); + PyArrayCapsule pPyVerticesContiguous(PyArray_GETCONTIGUOUS(reinterpret_cast(pPyVertices))); const size_t typeSize = PyArray_ITEMSIZE(pPyVerticesContiguous); const size_t n = PyArray_DIM(pPyVerticesContiguous, 0); @@ -393,8 +392,7 @@ class PyTriMesh if (PyArray_NDIM(pPyIndices) != 2 || PyArray_DIM(pPyIndices, 1) != 3 || !PyArray_ISINTEGER(pPyIndices)) { throw openrave_exception(_("indices must be a Nx3 int array"), ORE_InvalidArguments); } - PyArrayObject* pPyIndiciesContiguous = PyArray_GETCONTIGUOUS(reinterpret_cast(pPyIndices)); - AutoPyArrayObjectDereferencer pydecref(pPyIndiciesContiguous); + PyArrayCapsule pPyIndiciesContiguous(PyArray_GETCONTIGUOUS(reinterpret_cast(pPyIndices))); const size_t typeSize = PyArray_ITEMSIZE(pPyIndiciesContiguous); const bool signedInt = PyArray_ISSIGNED(pPyIndiciesContiguous); diff --git a/python/bindings/openravepy_int.cpp b/python/bindings/openravepy_int.cpp index ddd1905bdb..4a0d9855b8 100644 --- a/python/bindings/openravepy_int.cpp +++ b/python/bindings/openravepy_int.cpp @@ -300,8 +300,7 @@ void toRapidJSONValue(const object &obj, rapidjson::Value &value, rapidjson::Doc #endif // USE_PYBIND11_PYTHON_BINDINGS } else if (PyArray_Check(obj.ptr()) ) { - PyArrayObject* pyarrayvalues = PyArray_GETCONTIGUOUS(reinterpret_cast(obj.ptr())); - AutoPyArrayObjectDereferencer pydecref(pyarrayvalues); + PyArrayCapsule pyarrayvalues(PyArray_GETCONTIGUOUS(reinterpret_cast(obj.ptr()))); int ndims = PyArray_NDIM(pyarrayvalues); npy_intp* dims = PyArray_DIMS(pyarrayvalues); @@ -1586,8 +1585,7 @@ object PyEnvironmentBase::CheckCollisionRays(py::numeric::array rays, PyKinBodyP CollisionReport report; CollisionReportPtr preport(&report,null_deleter()); - PyArrayObject *pPyRays = PyArray_GETCONTIGUOUS(reinterpret_cast(rays.ptr())); - AutoPyArrayObjectDereferencer pyderef(pPyRays); + PyArrayCapsule pPyRays(PyArray_GETCONTIGUOUS(reinterpret_cast(rays.ptr()))); if( !PyArray_ISFLOAT(pPyRays) ) { throw OpenRAVEException(_("rays has to be a float array\n")); From f8eadc7051e5c22ffc80858ae3fa4670536bae08 Mon Sep 17 00:00:00 2001 From: Hamdi Sahloul Date: Fri, 30 Jul 2021 13:54:54 +0900 Subject: [PATCH 3/4] Remove AutoPyArrayObjectDereferencer & use a typdef for backwards compatibility --- .../bindings/include/openravepy/openravepy_int.h | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/python/bindings/include/openravepy/openravepy_int.h b/python/bindings/include/openravepy/openravepy_int.h index 09fc05e4ce..cc31f1bfca 100644 --- a/python/bindings/include/openravepy/openravepy_int.h +++ b/python/bindings/include/openravepy/openravepy_int.h @@ -217,20 +217,6 @@ class OPENRAVEPY_API PythonGILSaver } }; -// TODO: Remove AutoPyArrayObjectDereferencer in favor of PyCapsule -class OPENRAVEPY_API AutoPyArrayObjectDereferencer -{ -public: - AutoPyArrayObjectDereferencer(PyArrayObject* pyarrobj) : _pyarrobj(pyarrobj) { - } - ~AutoPyArrayObjectDereferencer() { - Py_DECREF(_pyarrobj); - } - -private: - PyArrayObject* _pyarrobj; -}; - template class OPENRAVEPY_API PyCapsule { @@ -286,6 +272,8 @@ class OPENRAVEPY_API PyCapsule typedef PyCapsule PyArrayCapsule; +typedef PyArrayCapsule AutoPyArrayObjectDereferencer; // Backwards compatibility + inline RaveVector ExtractFloat3(const py::object& o) { return RaveVector(py::extract(o[0]), py::extract(o[1]), py::extract(o[2])); From 670b3d2485502fe550dac3124184ef2e86c0b842 Mon Sep 17 00:00:00 2001 From: Hamdi Sahloul Date: Tue, 17 Aug 2021 14:52:51 +0900 Subject: [PATCH 4/4] Bump minor version only --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f959b2f7d2..5e9e2870d8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,7 +5,7 @@ set( CMAKE_ALLOW_LOOSE_LOOP_CONSTRUCTS TRUE ) # Define here the needed parameters set (OPENRAVE_VERSION_MAJOR 0) set (OPENRAVE_VERSION_MINOR 84) -set (OPENRAVE_VERSION_PATCH 1) +set (OPENRAVE_VERSION_PATCH 0) set (OPENRAVE_VERSION ${OPENRAVE_VERSION_MAJOR}.${OPENRAVE_VERSION_MINOR}.${OPENRAVE_VERSION_PATCH}) set (OPENRAVE_SOVERSION ${OPENRAVE_VERSION_MAJOR}.${OPENRAVE_VERSION_MINOR}) message(STATUS "Compiling OpenRAVE Version ${OPENRAVE_VERSION}, soversion=${OPENRAVE_SOVERSION}")