From b5619342b83433dfad5f19d26f0dd284b3cdd724 Mon Sep 17 00:00:00 2001 From: Damien Marchal Date: Tue, 14 Sep 2021 22:48:41 +0200 Subject: [PATCH 1/5] [Binding/Sofa.Core] Implement a custom getClassName() for trampoline classes. Trampoline classes are used to implement sofa components in python. The default getClassName() returns the name of the c++ trampoline class name instead of the name of the python overrides. This is why you can see stuff like that: ``` Controller_Trampoline(myObject) ForceField_Trampoline(myFF) ``` This PR retrieve the real class name from python to returns it in a custom getClassName. Eg: ``` class RoboticsController(Sofa.Core.Controller): pass ``` will appears with its real name in the GUI: ``` RoboticController(myObject) ``` --- .../Sofa/Core/Binding_Controller.cpp | 150 +++++++++--------- .../Sofa/Core/Binding_Controller.h | 2 + .../Sofa/Core/Binding_DataEngine.cpp | 112 +++++++------ .../Sofa/Core/Binding_DataEngine.h | 1 + .../Sofa/Core/Binding_ForceField.cpp | 9 ++ .../Sofa/Core/Binding_ForceField.h | 1 + 6 files changed, 152 insertions(+), 123 deletions(-) diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.cpp index d9b69ace..002286df 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.cpp @@ -35,88 +35,96 @@ namespace py { using namespace pybind11; } namespace sofapython3 { - using sofa::core::objectmodel::Event; - using sofa::core::objectmodel::BaseObject; +using sofa::core::objectmodel::Event; +using sofa::core::objectmodel::BaseObject; - void Controller_Trampoline::init() - { - PythonEnvironment::gil acquire {"Controller_init"}; - PYBIND11_OVERLOAD(void, Controller, init, ); - } +std::string Controller_Trampoline::getClassName() const +{ + PythonEnvironment::gil acquire {"getClassName"}; - void Controller_Trampoline::reinit() - { - PythonEnvironment::gil acquire {"Controller_reinit"}; - PYBIND11_OVERLOAD(void, Controller, reinit, ); - } + // Get the actual class name from python. + return py::str(py::cast(this).get_type().attr("__name__")); +} + +void Controller_Trampoline::init() +{ + PythonEnvironment::gil acquire {"Controller_init"}; + PYBIND11_OVERLOAD(void, Controller, init, ); +} + +void Controller_Trampoline::reinit() +{ + PythonEnvironment::gil acquire {"Controller_reinit"}; + PYBIND11_OVERLOAD(void, Controller, reinit, ); +} - /// If a method named "methodName" exists in the python controller, - /// methodName is called, with the Event's dict as argument - void Controller_Trampoline::callScriptMethod( - const py::object& self, Event* event, const std::string & methodName) +/// If a method named "methodName" exists in the python controller, +/// methodName is called, with the Event's dict as argument +void Controller_Trampoline::callScriptMethod( + const py::object& self, Event* event, const std::string & methodName) +{ + if( py::hasattr(self, methodName.c_str()) ) { - if( py::hasattr(self, methodName.c_str()) ) - { - py::object fct = self.attr(methodName.c_str()); - fct(PythonFactory::toPython(event)); - } + py::object fct = self.attr(methodName.c_str()); + fct(PythonFactory::toPython(event)); } +} + +void Controller_Trampoline::handleEvent(Event* event) +{ + PythonEnvironment::gil acquire {"Controller_handleEvent"}; - void Controller_Trampoline::handleEvent(Event* event) + py::object self = py::cast(this); + std::string name = std::string("on")+event->getClassName(); + /// Is there a method with this name in the class ? + if( py::hasattr(self, name.c_str()) ) { - PythonEnvironment::gil acquire {"Controller_handleEvent"}; - - py::object self = py::cast(this); - std::string name = std::string("on")+event->getClassName(); - /// Is there a method with this name in the class ? - if( py::hasattr(self, name.c_str()) ) - { - py::object fct = self.attr(name.c_str()); - if (PyCallable_Check(fct.ptr())) { - callScriptMethod(self, event, name); - return; - } + py::object fct = self.attr(name.c_str()); + if (PyCallable_Check(fct.ptr())) { + callScriptMethod(self, event, name); + return; } - - /// Is the fallback method available. - callScriptMethod(self, event, "onEvent"); } - void moduleAddController(py::module &m) { - py::class_> f(m, "Controller", - py::dynamic_attr(), - sofapython3::doc::controller::Controller); - - f.def(py::init([](py::args& /*args*/, py::kwargs& kwargs) - { - auto c = sofa::core::sptr (new Controller_Trampoline()); - c->f_listening.setValue(true); - - for(auto kv : kwargs) - { - std::string key = py::cast(kv.first); - py::object value = py::reinterpret_borrow(kv.second); - - if( key == "name") - c->setName(py::cast(kv.second)); - try { - BindingBase::SetAttr(*c, key, value); - } catch (py::attribute_error& /*e*/) { - /// kwargs are used to set datafields to their initial values, - /// but they can also be used as simple python attributes, unrelated to SOFA. - /// thus we catch & ignore the py::attribute_error thrown by SetAttr - } - } - return c; - })); - - f.def("init", &Controller::init); - f.def("reinit", &Controller::reinit); - } + /// Is the fallback method available. + callScriptMethod(self, event, "onEvent"); +} + +void moduleAddController(py::module &m) { + py::class_> f(m, "Controller", + py::dynamic_attr(), + sofapython3::doc::controller::Controller); + + f.def(py::init([](py::args& /*args*/, py::kwargs& kwargs) + { + auto c = sofa::core::sptr (new Controller_Trampoline()); + c->f_listening.setValue(true); + + for(auto kv : kwargs) + { + std::string key = py::cast(kv.first); + py::object value = py::reinterpret_borrow(kv.second); + + if( key == "name") + c->setName(py::cast(kv.second)); + try { + BindingBase::SetAttr(*c, key, value); + } catch (py::attribute_error& /*e*/) { + /// kwargs are used to set datafields to their initial values, + /// but they can also be used as simple python attributes, unrelated to SOFA. + /// thus we catch & ignore the py::attribute_error thrown by SetAttr + } + } + return c; + })); + + f.def("init", &Controller::init); + f.def("reinit", &Controller::reinit); +} } diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.h b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.h index 7e04b6c3..0524e6f3 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.h +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.h @@ -45,6 +45,8 @@ class Controller_Trampoline : public Controller void reinit() override; void handleEvent(sofa::core::objectmodel::Event* event) override; + std::string getClassName() const override; + private: void callScriptMethod(const pybind11::object& self, sofa::core::objectmodel::Event* event, const std::string& methodName); diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.cpp index 8ecf8bee..d44fc8c0 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.cpp @@ -34,67 +34,75 @@ namespace py { using namespace pybind11; } namespace sofapython3 { - using sofa::core::objectmodel::Event; - using sofa::core::DataEngine; - using sofa::core::objectmodel::BaseData; - using sofa::core::objectmodel::BaseObject; - using sofa::core::objectmodel::DDGNode; +using sofa::core::objectmodel::Event; +using sofa::core::DataEngine; +using sofa::core::objectmodel::BaseData; +using sofa::core::objectmodel::BaseObject; +using sofa::core::objectmodel::DDGNode; - void DataEngine_Trampoline::init() - { - PythonEnvironment::gil acquire; - PYBIND11_OVERLOAD(void, DataEngine, init, ); - } +std::string DataEngine_Trampoline::getClassName() const +{ + PythonEnvironment::gil acquire {"getClassName"}; + + // Get the actual class name from python. + return py::str(py::cast(this).get_type().attr("__name__")); +} + +void DataEngine_Trampoline::init() +{ + PythonEnvironment::gil acquire; + PYBIND11_OVERLOAD(void, DataEngine, init, ); +} - void DataEngine_Trampoline::doUpdate() - { - PythonEnvironment::gil acquire; - py::object self = py::cast(this); - if (py::hasattr(self, "update")) { - py::object fct = self.attr("update"); - try { - fct(); - return; - } catch (std::exception& /*e*/) { - throw py::type_error(this->getName() + ": The DataEngine requires an update method with no parameter and no return type"); - } +void DataEngine_Trampoline::doUpdate() +{ + PythonEnvironment::gil acquire; + py::object self = py::cast(this); + if (py::hasattr(self, "update")) { + py::object fct = self.attr("update"); + try { + fct(); + return; + } catch (std::exception& /*e*/) { + throw py::type_error(this->getName() + ": The DataEngine requires an update method with no parameter and no return type"); } - throw py::type_error(this->getName() + " has no update() method."); } + throw py::type_error(this->getName() + " has no update() method."); +} - void moduleAddDataEngine(pybind11::module &m) - { - py::class_> f(m, "DataEngine", - py::dynamic_attr(), - sofapython3::doc::dataengine::DataEngine); +void moduleAddDataEngine(pybind11::module &m) +{ + py::class_> f(m, "DataEngine", + py::dynamic_attr(), + sofapython3::doc::dataengine::DataEngine); - f.def(py::init([](py::args& /*args*/, py::kwargs& kwargs) - { - auto c = new DataEngine_Trampoline(); + f.def(py::init([](py::args& /*args*/, py::kwargs& kwargs) + { + auto c = new DataEngine_Trampoline(); - for(auto kv : kwargs) - { - std::string key = py::cast(kv.first); - py::object value = py::reinterpret_borrow(kv.second); + for(auto kv : kwargs) + { + std::string key = py::cast(kv.first); + py::object value = py::reinterpret_borrow(kv.second); - if( key == "name") - c->setName(py::cast(kv.second)); - try { - BindingBase::SetAttr(*c, key, value); - } catch (py::attribute_error& /*e*/) { - /// kwargs are used to set datafields to their initial values, - /// but they can also be used as simple python attributes, unrelated to SOFA. - /// thus we catch & ignore the py::attribute_error thrown by SetAttr - } + if( key == "name") + c->setName(py::cast(kv.second)); + try { + BindingBase::SetAttr(*c, key, value); + } catch (py::attribute_error& /*e*/) { + /// kwargs are used to set datafields to their initial values, + /// but they can also be used as simple python attributes, unrelated to SOFA. + /// thus we catch & ignore the py::attribute_error thrown by SetAttr } - return c; - })); + } + return c; + })); - f.def("addInput", &DataEngine::addInput, sofapython3::doc::dataengine::addInput); - f.def("addOutput", &DataEngine::addOutput, sofapython3::doc::dataengine::addOutput); - } + f.def("addInput", &DataEngine::addInput, sofapython3::doc::dataengine::addInput); + f.def("addOutput", &DataEngine::addOutput, sofapython3::doc::dataengine::addOutput); +} } /// namespace sofapython3 diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.h b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.h index b1360fc7..f2afb0ad 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.h +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.h @@ -32,6 +32,7 @@ class DataEngine_Trampoline : public sofa::core::DataEngine void init() override; void doUpdate() override; + std::string getClassName() const override; }; void moduleAddDataEngine(pybind11::module &m); diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_ForceField.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_ForceField.cpp index b2dd89b6..33015d73 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_ForceField.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_ForceField.cpp @@ -58,6 +58,15 @@ namespace sofapython3 template ForceField_Trampoline::~ForceField_Trampoline() = default; + template + std::string ForceField_Trampoline::getClassName() const + { + PythonEnvironment::gil acquire {"getClassName"}; + + // Get the actual class name from python. + return py::str(py::cast(this).get_type().attr("__name__")); + } + template void ForceField_Trampoline::init() { diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_ForceField.h b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_ForceField.h index 3c3cfa74..6f6861bf 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_ForceField.h +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_ForceField.h @@ -42,6 +42,7 @@ class ForceField_Trampoline : public sofa::core::behavior::ForceField ~ForceField_Trampoline() override; void init() override; + std::string getClassName() const override; void addForce(const sofa::core::MechanicalParams* mparams, DataVecDeriv& f, const DataVecCoord& x, const DataVecDeriv& v) override; void addDForce(const sofa::core::MechanicalParams* mparams, DataVecDeriv& df, const DataVecDeriv& dx ) override; From d732ddb2407b21784fc8c9dd54371e4e8ad5b6fc Mon Sep 17 00:00:00 2001 From: Damien Marchal Date: Tue, 14 Sep 2021 23:09:57 +0200 Subject: [PATCH 2/5] [SofaPython3/PythonEnvironment] Add new method for executePython This function needs to be used to properly box python code execution with a properly installed gil & try/catch pair. --- Plugin/src/SofaPython3/PythonEnvironment.cpp | 26 +++++++++++++++++--- Plugin/src/SofaPython3/PythonEnvironment.h | 6 ++++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/Plugin/src/SofaPython3/PythonEnvironment.cpp b/Plugin/src/SofaPython3/PythonEnvironment.cpp index 4dc4c5d9..f9299b17 100644 --- a/Plugin/src/SofaPython3/PythonEnvironment.cpp +++ b/Plugin/src/SofaPython3/PythonEnvironment.cpp @@ -253,18 +253,38 @@ void PythonEnvironment::Init() } } -void PythonEnvironment::executePython(std::function cb) +// Single implementation for the three different versions +template +void executePython_(const T& emitter, std::function cb) { sofapython3::PythonEnvironment::gil acquire; try{ cb(); - }catch(std::exception& e) + }catch(py::error_already_set& e) { - msg_error("SofaPython3") << e.what() ; + std::stringstream tmp; + tmp << "Unable to execute code." << msgendl + << "Python exception:" << msgendl + << " " << e.what(); + msg_error(emitter) << tmp.str(); } } +void PythonEnvironment::executePython(const std::string& emitter, std::function cb) +{ + return executePython_(emitter, cb); +} + +void PythonEnvironment::executePython(std::function cb) +{ + return executePython_("SofaPython3::executePython", cb); +} + +void PythonEnvironment::executePython(const sofa::core::objectmodel::Base* b, std::function cb) +{ + return executePython_(b, cb); +} void PythonEnvironment::Release() { diff --git a/Plugin/src/SofaPython3/PythonEnvironment.h b/Plugin/src/SofaPython3/PythonEnvironment.h index 2236e072..0275c8a5 100644 --- a/Plugin/src/SofaPython3/PythonEnvironment.h +++ b/Plugin/src/SofaPython3/PythonEnvironment.h @@ -98,7 +98,11 @@ class SOFAPYTHON3_API PythonEnvironment /// excluding a module from automatic reload static void excludeModuleFromReload( const std::string& moduleName ); - static void executePython(std::function); + /// execute a function 'f' after acquiring the GIL and having installed + /// an handler to catch python exception. + static void executePython(std::function f); + static void executePython(const sofa::core::objectmodel::Base* emitter, std::function f); + static void executePython(const std::string& emitter, std::function cb); /// to be able to react when a scene is loaded struct SceneLoaderListerner : public SceneLoader::Listener From dabb39eb1e3fa22d8516b78ffe373e199b76fb4a Mon Sep 17 00:00:00 2001 From: Damien Marchal Date: Tue, 14 Sep 2021 23:11:02 +0200 Subject: [PATCH 3/5] [SofaPython3] Change the error message in SceneLoaderPY3 To unify it with how python message are printed in PythonEnvironment. --- Plugin/src/SofaPython3/SceneLoaderPY3.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Plugin/src/SofaPython3/SceneLoaderPY3.cpp b/Plugin/src/SofaPython3/SceneLoaderPY3.cpp index f49408fd..b20e8048 100644 --- a/Plugin/src/SofaPython3/SceneLoaderPY3.cpp +++ b/Plugin/src/SofaPython3/SceneLoaderPY3.cpp @@ -106,9 +106,11 @@ void SceneLoaderPY3::loadSceneWithArguments(const char *filename, py::object createScene = module.attr("createScene"); createScene( PythonFactory::toPython(root_out.get()) ); - }catch(std::exception& e) + }catch(py::error_already_set& e) { - msg_error() << e.what(); + msg_error() << "Unable to completely load the scene from file '"<< filename << "'." << msgendl + << "Python exception: " << msgendl + << " " << e.what(); } } From 3e86cc83dc70a487880dfe2febd6b8c67e0cf445 Mon Sep 17 00:00:00 2001 From: Damien Marchal Date: Tue, 14 Sep 2021 23:21:27 +0200 Subject: [PATCH 4/5] [Binding/Sofa.Core] Use executePython to protect calls to DataEngine_Trampoline --- .../Sofa/Core/Binding_DataEngine.cpp | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.cpp index d44fc8c0..8c27e9f6 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_DataEngine.cpp @@ -50,24 +50,26 @@ std::string DataEngine_Trampoline::getClassName() const void DataEngine_Trampoline::init() { - PythonEnvironment::gil acquire; - PYBIND11_OVERLOAD(void, DataEngine, init, ); + PythonEnvironment::executePython(this, [this](){ + PYBIND11_OVERLOAD(void, DataEngine, init, ); + }); } void DataEngine_Trampoline::doUpdate() { - PythonEnvironment::gil acquire; - py::object self = py::cast(this); - if (py::hasattr(self, "update")) { - py::object fct = self.attr("update"); - try { - fct(); - return; - } catch (std::exception& /*e*/) { - throw py::type_error(this->getName() + ": The DataEngine requires an update method with no parameter and no return type"); + PythonEnvironment::executePython(this, [this](){ + py::object self = py::cast(this); + if (py::hasattr(self, "update")) { + py::object fct = self.attr("update"); + try { + fct(); + return; + } catch (std::exception& /*e*/) { + throw py::type_error(this->getName() + ": The DataEngine requires an update method with no parameter and no return type"); + } } - } - throw py::type_error(this->getName() + " has no update() method."); + throw py::type_error(this->getName() + " has no update() method."); + }); } void moduleAddDataEngine(pybind11::module &m) From daa6ed0b86d475b1f7deaaf166ed946719ef33f2 Mon Sep 17 00:00:00 2001 From: Damien Marchal Date: Tue, 14 Sep 2021 23:21:38 +0200 Subject: [PATCH 5/5] [Binding/Sofa.Core] Use executePython to protect calls to Controller_Trampolin --- .../Sofa/Core/Binding_Controller.cpp | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.cpp index 002286df..30d5f91f 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Controller.cpp @@ -48,17 +48,18 @@ std::string Controller_Trampoline::getClassName() const void Controller_Trampoline::init() { - PythonEnvironment::gil acquire {"Controller_init"}; - PYBIND11_OVERLOAD(void, Controller, init, ); + PythonEnvironment::executePython(this, [this](){ + PYBIND11_OVERLOAD(void, Controller, init, ); + }); } void Controller_Trampoline::reinit() { - PythonEnvironment::gil acquire {"Controller_reinit"}; - PYBIND11_OVERLOAD(void, Controller, reinit, ); + PythonEnvironment::executePython(this, [this](){ + PYBIND11_OVERLOAD(void, Controller, reinit, ); + }); } - /// If a method named "methodName" exists in the python controller, /// methodName is called, with the Event's dict as argument void Controller_Trampoline::callScriptMethod( @@ -73,22 +74,22 @@ void Controller_Trampoline::callScriptMethod( void Controller_Trampoline::handleEvent(Event* event) { - PythonEnvironment::gil acquire {"Controller_handleEvent"}; - - py::object self = py::cast(this); - std::string name = std::string("on")+event->getClassName(); - /// Is there a method with this name in the class ? - if( py::hasattr(self, name.c_str()) ) - { - py::object fct = self.attr(name.c_str()); - if (PyCallable_Check(fct.ptr())) { - callScriptMethod(self, event, name); - return; + PythonEnvironment::executePython(this, [this,event](){ + py::object self = py::cast(this); + std::string name = std::string("on")+event->getClassName(); + /// Is there a method with this name in the class ? + if( py::hasattr(self, name.c_str()) ) + { + py::object fct = self.attr(name.c_str()); + if (PyCallable_Check(fct.ptr())) { + callScriptMethod(self, event, name); + return; + } } - } - /// Is the fallback method available. - callScriptMethod(self, event, "onEvent"); + /// Is the fallback method available. + callScriptMethod(self, event, "onEvent"); + }); } void moduleAddController(py::module &m) {