diff --git a/CMakeLists.txt b/CMakeLists.txt index 1770be5..15bab9f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,6 +20,7 @@ project(plugin_python) # order matters! add_subdirectory(src/pybind11-qt) +add_subdirectory(src/pybind11-utils) add_subdirectory(src/mobase) add_subdirectory(src/runner) add_subdirectory(src/proxy) diff --git a/src/mobase/CMakeLists.txt b/src/mobase/CMakeLists.txt index fb596e2..461ab3f 100644 --- a/src/mobase/CMakeLists.txt +++ b/src/mobase/CMakeLists.txt @@ -8,7 +8,7 @@ mo2_configure_library(mobase TRANSLATIONS OFF PRIVATE_DEPENDS uibase Qt::Core ) -target_link_libraries(mobase PRIVATE pybind11::qt) +target_link_libraries(mobase PRIVATE pybind11::qt pybind11::utils) target_include_directories(mobase PRIVATE ${PYTHON_ROOT}/Include) install(TARGETS mobase DESTINATION bin/plugins/plugin_python/libs) diff --git a/src/mobase/pybind11_all.h b/src/mobase/pybind11_all.h index 93e8e08..8d34288 100644 --- a/src/mobase/pybind11_all.h +++ b/src/mobase/pybind11_all.h @@ -12,4 +12,10 @@ #include "pybind11_utils/shared_cpp_owner.h" +#include +#include + +MO2_PYBIND11_SHARED_CPP_HOLDER(MOBase::IPluginRequirement) +MO2_PYBIND11_SHARED_CPP_HOLDER(MOBase::ISaveGame) + #endif diff --git a/src/pybind11-utils/CMakeLists.txt b/src/pybind11-utils/CMakeLists.txt new file mode 100644 index 0000000..32a13d7 --- /dev/null +++ b/src/pybind11-utils/CMakeLists.txt @@ -0,0 +1,8 @@ +cmake_minimum_required(VERSION 3.16) + +add_library(pybind11-utils INTERFACE) +target_link_libraries(pybind11-utils INTERFACE pybind11::pybind11) +target_include_directories(pybind11-utils + INTERFACE ${PYTHON_ROOT}/Include ${CMAKE_CURRENT_SOURCE_DIR}/include) + +add_library(pybind11::utils ALIAS pybind11-utils) diff --git a/src/mobase/pybind11_utils/functional.h b/src/pybind11-utils/include/pybind11_utils/functional.h similarity index 100% rename from src/mobase/pybind11_utils/functional.h rename to src/pybind11-utils/include/pybind11_utils/functional.h diff --git a/src/mobase/pybind11_utils/shared_cpp_owner.h b/src/pybind11-utils/include/pybind11_utils/shared_cpp_owner.h similarity index 94% rename from src/mobase/pybind11_utils/shared_cpp_owner.h rename to src/pybind11-utils/include/pybind11_utils/shared_cpp_owner.h index 23d2e0d..0fb0fe6 100644 --- a/src/mobase/pybind11_utils/shared_cpp_owner.h +++ b/src/pybind11-utils/include/pybind11_utils/shared_cpp_owner.h @@ -1,6 +1,8 @@ #ifndef PYTHON_PYBIND11_SHARED_CPP_OWNER_H #define PYTHON_PYBIND11_SHARED_CPP_OWNER_H +#include + // pybind11 has some issues when a Python classes extend a C++ wrapper since the Python // object is not kept alive alongside the returned object // @@ -11,11 +13,9 @@ // definition in mo2::python::detail below // // IMPORTANT: this only works for classes that are managed by shared_ptr on the C++ -// side, not Qt object +// side, not Qt object (see pybind11-qt holder for that) // -// TODO: WIP for Qt object - namespace mo2::python::detail { template @@ -89,10 +89,4 @@ namespace mo2::python::detail { }; \ } -#include -#include - -MO2_PYBIND11_SHARED_CPP_HOLDER(MOBase::IPluginRequirement) -MO2_PYBIND11_SHARED_CPP_HOLDER(MOBase::ISaveGame) - #endif diff --git a/tests/python/CMakeLists.txt b/tests/python/CMakeLists.txt index 5475f3a..7e9d9ae 100644 --- a/tests/python/CMakeLists.txt +++ b/tests/python/CMakeLists.txt @@ -44,8 +44,7 @@ foreach (test_file ${test_files}) endif() mo2_add_dependencies(${target} PRIVATE uibase Qt::Core Qt::Widgets) - # set_property(TARGET ${target} PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded") - target_link_libraries(${target} PRIVATE pybind11::qt GTest::gmock) + target_link_libraries(${target} PRIVATE pybind11::qt pybind11::utils GTest::gmock) target_include_directories(${target} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../mocks) diff --git a/tests/python/test_shared_cpp_owner.cpp b/tests/python/test_shared_cpp_owner.cpp new file mode 100644 index 0000000..f24cafd --- /dev/null +++ b/tests/python/test_shared_cpp_owner.cpp @@ -0,0 +1,67 @@ +#include "pybind11_utils/shared_cpp_owner.h" + +#include + +#include +#include + +namespace py = pybind11; +using namespace pybind11::literals; + +class Base; +static std::unordered_map bases; + +class Base { + std::string name_; + +public: + Base(std::string const& name) : name_{name} { bases[name] = this; } + virtual std::string fn() const = 0; + virtual ~Base() { bases.erase(name_); } +}; + +MO2_PYBIND11_SHARED_CPP_HOLDER(Base); + +class CppBase : public Base { +public: + using Base::Base; + std::string fn() const override { return "CppBase::fn()"; } +}; + +class PyBase : public Base { +public: + using Base::Base; + std::string fn() const override { PYBIND11_OVERRIDE_PURE(std::string, Base, fn, ); } +}; + +PYBIND11_MODULE(shared_cpp_owner, m) +{ + static std::shared_ptr base_ptr; + + py::class_>(m, "Base") + .def(py::init()) + .def("fn", &Base::fn); + + m.def("is_alive", [](std::string const& name) { + return bases.find(name) != bases.end(); + }); + + m.def("create", [](std::string const& name) -> std::shared_ptr { + return std::make_shared(name); + }); + m.def("create_and_store", [](std::string const& name) { + base_ptr = std::make_shared(name); + return base_ptr; + }); + m.def("store", [](std::shared_ptr ptr) { + base_ptr = ptr; + }); + m.def("clear", []() { + base_ptr.reset(); + }); + + m.def("call_fn", [](std::string const& name) { + auto it = bases.find(name); + return it != bases.end() ? it->second->fn() : ""; + }); +} diff --git a/tests/python/test_shared_cpp_owner.py b/tests/python/test_shared_cpp_owner.py new file mode 100644 index 0000000..4b38cdc --- /dev/null +++ b/tests/python/test_shared_cpp_owner.py @@ -0,0 +1,77 @@ +from re import A + +import pytest +from PyQt6.QtCore import QDateTime, Qt + +m = pytest.importorskip("mobase_tests.shared_cpp_owner") + + +class PyBase(m.Base): + def __init__(self, name: str, value: int): + super().__init__(name) + self.value = value + + def fn(self): + return f"PyBase.fn({self.value})" + + +def test_shared_cpp_owner_1(): + # create from C++, owned by Python + + # create from C++ + p = m.create("tmp") + assert m.is_alive("tmp") + + # should delete since it's not owner by C++ + del p + assert not m.is_alive("tmp") + + +def test_shared_cpp_owner_2(): + # create from C++, owned by C++ (and Python) + + # create from C++ + p = m.create_and_store("tmp") + assert m.is_alive("tmp") + + # should not delete since it's owned by both C++ and Python + del p + assert m.is_alive("tmp") + + # clear from C++ should free it + m.clear() + assert not m.is_alive("tmp") + + +def test_shared_cpp_owner_3(): + # create from Python, owned by Python + + p = PyBase("foo", 1) + assert m.is_alive("foo") + assert m.call_fn("foo") == "PyBase.fn(1)" + + del p + assert not m.is_alive("foo") + + +def test_shared_cpp_owner_3(): + # create from Python, owned by C++ + + p = PyBase("foo", 2) + assert m.is_alive("foo") + + # send to C++ + m.store(p) + assert m.is_alive("foo") + assert m.call_fn("foo") == "PyBase.fn(2)" + + # delete in Python, should still be alived + del p + assert m.is_alive("foo") + + # should still be able to call fn() + assert m.call_fn("foo") == "PyBase.fn(2)" + + # clear in C++, should kill Python + m.clear() + assert not m.is_alive("foo")