Skip to content

Commit 24c22d3

Browse files
damienmarchaldamienmarchal
committed
[SofaPython3] Remove -at-best- the use of this infamous string conversion while creating an object.
1 parent b5c9cfb commit 24c22d3

File tree

3 files changed

+110
-120
lines changed

3 files changed

+110
-120
lines changed

‎Plugin/src/SofaPython3/DataHelper.cpp

+28-4
Original file line numberDiff line numberDiff line change
@@ -45,25 +45,48 @@ std::string toSofaParsableString(const py::handle& p)
4545
if(py::isinstance<py::list>(p) || py::isinstance<py::tuple>(p))
4646
{
4747
std::stringstream tmp;
48-
for(auto pa : p){
48+
for(auto pa : p)
49+
{
4950
tmp << toSofaParsableString(pa) << " ";
5051
}
5152
return tmp.str();
5253
}
5354
//TODO(dmarchal) This conversion to string is so bad.
5455
if(py::isinstance<py::str>(p))
56+
{
5557
return py::str(p);
58+
}
59+
5660
return py::repr(p);
5761
}
5862

59-
/// RVO optimized function. Don't care about copy on the return code.
6063
void fillBaseObjectdescription(sofa::core::objectmodel::BaseObjectDescription& desc,
61-
const py::dict& dict)
64+
const py::dict& dict)
65+
{
66+
for(auto kv : dict)
67+
{
68+
desc.setAttribute(py::str(kv.first), toSofaParsableString(kv.second));
69+
}
70+
71+
return;
72+
}
73+
74+
void processKwargsForObjectCreation(const py::dict dict,
75+
py::list& parametersToLink,
76+
py::list& parametersToCopy,
77+
sofa::core::objectmodel::BaseObjectDescription& parametersAsString)
6278
{
79+
auto t = py::detail::get_type_handle(typeid(BaseData), false);
6380
for(auto kv : dict)
6481
{
65-
desc.setAttribute(py::str(kv.first), toSofaParsableString(kv.second));
82+
if (py::isinstance(kv.second, t))
83+
parametersToLink.append(kv.first);
84+
else if (py::isinstance<py::str>(kv.second))
85+
parametersAsString.setAttribute(py::str(kv.first), py::cast<std::string>(kv.second));
86+
else
87+
parametersToCopy.append(kv.first);
6688
}
89+
return;
6790
}
6891

6992
PythonTrampoline::~PythonTrampoline(){}
@@ -76,6 +99,7 @@ void PythonTrampoline::setInstance(py::object s)
7699

77100
pyobject = std::shared_ptr<PyObject>( s.ptr(), [](PyObject* ob)
78101
{
102+
SOFA_UNUSED(ob);
79103
// runSofa Sofa/tests/pyfiles/ScriptController.py => CRASH
80104
// Py_DECREF(ob);
81105
});

‎Plugin/src/SofaPython3/DataHelper.h

+63-114
Original file line numberDiff line numberDiff line change
@@ -42,119 +42,63 @@ along with sofaqtquick. If not, see <http://www.gnu.org/licenses/>.
4242

4343
////////////////////////// FORWARD DECLARATION ///////////////////////////
4444
namespace sofa {
45-
namespace defaulttype {
46-
class AbstractTypeInfo;
47-
}
48-
namespace core {
49-
namespace objectmodel {
50-
class BaseData;
51-
52-
53-
class SOFAPYTHON3_API PrefabLink
54-
{
55-
public:
56-
PrefabLink() {}
57-
PrefabLink(const Base::SPtr& targetBase) { m_targetBase = targetBase; }
58-
PrefabLink(BaseLink* targetLink) { m_targetBase = targetLink->getLinkedBase(); }
59-
PrefabLink(const std::string& targetPath) { m_targetPath = targetPath; }
60-
61-
const Base::SPtr& getTargetBase() const { return m_targetBase; }
62-
void setTargetBase(const Base::SPtr& targetBase) { m_targetBase = targetBase; }
63-
64-
const std::string& getTargetPath() const { return m_targetPath; }
65-
void setTargetPath(const std::string& targetPath) { m_targetPath = targetPath; }
66-
67-
friend std::ostream& operator << ( std::ostream& out, const PrefabLink& l)
68-
{
69-
if (l.getTargetBase())
70-
{
71-
auto bn = l.getTargetBase()->toBaseNode();
72-
auto bo = l.getTargetBase()->toBaseObject();
73-
out << "@" + (bn ? bn->getPathName() : bo->getPathName());
74-
}
75-
out << l.getTargetPath();
76-
return out;
77-
}
78-
79-
friend std::istream& operator >> ( std::istream& in, PrefabLink& l)
80-
{
81-
std::string s;
82-
in >> s;
83-
l.setTargetPath(s);
84-
return in;
85-
}
86-
87-
private:
88-
Base::SPtr m_targetBase { nullptr };
89-
std::string m_targetPath {""};
90-
};
91-
92-
class SOFAPYTHON3_API DataLink : public Data<PrefabLink>
93-
{
94-
typedef Data<PrefabLink> Inherit;
95-
96-
DataLink( const std::string& helpMsg="", bool isDisplayed=true, bool isReadOnly=false )
97-
: Inherit(helpMsg, isDisplayed, isReadOnly)
98-
{
99-
}
100-
101-
DataLink( const std::string& value, const std::string& helpMsg="", bool isDisplayed=true, bool isReadOnly=false )
102-
: Inherit(value, helpMsg, isDisplayed, isReadOnly)
103-
{
104-
}
105-
106-
explicit DataLink(const BaseData::BaseInitData& init)
107-
: Inherit(init)
108-
{
109-
}
110-
111-
const PrefabLink& getValue() const
112-
{
113-
updateIfDirty();
114-
if (m_value.getValue().getTargetBase()) return m_value.getValue();
115-
116-
auto self = const_cast<DataLink*>(this);
117-
118-
Base* dst = nullptr;
119-
this->getOwner()->findLinkDest(dst, self->m_value.getValue().getTargetPath(), nullptr);
120-
if (dst) {
121-
auto edit = self->m_value.beginEdit();
122-
edit->setTargetBase(dst);
123-
edit->setTargetPath("");
124-
self->m_value.endEdit();
125-
}
126-
return m_value.getValue();
127-
}
128-
129-
std::string getValueString() const
130-
{
131-
const auto& ptr = getValue();
132-
if (ptr.getTargetBase())
133-
{
134-
auto bn = ptr.getTargetBase()->toBaseNode();
135-
auto bo = ptr.getTargetBase()->toBaseObject();
136-
return "@" + (bn ? bn->getPathName() : bo->getPathName());
137-
}
138-
return ptr.getTargetPath();
139-
}
140-
141-
142-
bool read(const std::string& value)
143-
{
144-
Base* dst;
145-
auto data = m_value.beginEdit();
146-
if (this->getOwner()->findLinkDest(dst, value, nullptr) && dst != nullptr)
147-
data->setTargetBase(dst);
148-
else {
149-
data->setTargetBase(nullptr);
150-
data->setTargetPath(value);
151-
}
152-
return true;
153-
}
154-
};
45+
namespace defaulttype {
46+
class AbstractTypeInfo;
47+
}
48+
namespace core {
49+
namespace objectmodel {
50+
class BaseData;
51+
52+
53+
class SOFAPYTHON3_API PrefabLink
54+
{
55+
public:
56+
PrefabLink() {}
57+
PrefabLink(const Base::SPtr& targetBase) { m_targetBase = targetBase; }
58+
PrefabLink(BaseLink* targetLink) { m_targetBase = targetLink->getLinkedBase(); }
59+
PrefabLink(const std::string& targetPath) { m_targetPath = targetPath; }
15560

61+
const Base::SPtr& getTargetBase() const { return m_targetBase; }
62+
void setTargetBase(const Base::SPtr& targetBase) { m_targetBase = targetBase; }
63+
64+
const std::string& getTargetPath() const { return m_targetPath; }
65+
void setTargetPath(const std::string& targetPath) { m_targetPath = targetPath; }
66+
67+
friend std::ostream& operator << ( std::ostream& out, const PrefabLink& l)
68+
{
69+
if (l.getTargetBase())
70+
{
71+
auto bn = l.getTargetBase()->toBaseNode();
72+
auto bo = l.getTargetBase()->toBaseObject();
73+
out << "@" + (bn ? bn->getPathName() : bo->getPathName());
15674
}
75+
out << l.getTargetPath();
76+
return out;
77+
}
78+
79+
friend std::istream& operator >> ( std::istream& in, PrefabLink& l)
80+
{
81+
std::string s;
82+
in >> s;
83+
l.setTargetPath(s);
84+
return in;
15785
}
86+
87+
private:
88+
Base::SPtr m_targetBase { nullptr };
89+
std::string m_targetPath {""};
90+
};
91+
}
92+
}
93+
namespace defaulttype
94+
{
95+
template <>
96+
struct DataTypeName<core::objectmodel::PrefabLink>
97+
{
98+
static const char* name() { return "PrefabLink"; }
99+
};
100+
101+
}
158102
}
159103

160104
/////////////////////////////// DECLARATION //////////////////////////////
@@ -192,7 +136,7 @@ template <typename T> class py_shared_ptr : public sofa::core::sptr<T>
192136

193137
SOFAPYTHON3_API void setItem2D(py::array a, py::slice slice, py::object o);
194138
SOFAPYTHON3_API void setItem2D(py::array a, const py::slice& slice,
195-
const py::slice& slice1, py::object o);
139+
const py::slice& slice1, py::object o);
196140
SOFAPYTHON3_API void setItem1D(py::array a, py::slice slice, py::object o);
197141
SOFAPYTHON3_API void setItem(py::array a, py::slice slice, py::object value);
198142

@@ -216,11 +160,16 @@ void SOFAPYTHON3_API copyFromListScalar(BaseData& d, const AbstractTypeInfo& nfo
216160

217161
std::string SOFAPYTHON3_API toSofaParsableString(const py::handle& p);
218162

219-
//py::object SOFAPYTHON3_API dataToPython(BaseData* d);
220-
221163
/// RVO optimized function. Don't care about copy on the return code.
222164
void SOFAPYTHON3_API fillBaseObjectdescription(sofa::core::objectmodel::BaseObjectDescription& desc,
223-
const py::dict& dict);
165+
const py::dict& dict);
166+
167+
/// Split the content of the dictionnary 'dict' in three set.
168+
/// On containing the data to parent, one containing the data to copy and on containing the data to parse in the BaseObjectDescription
169+
void SOFAPYTHON3_API processKwargsForObjectCreation(const py::dict dict,
170+
py::list& parametersToLink,
171+
py::list& parametersToCopy,
172+
sofa::core::objectmodel::BaseObjectDescription& parametersAsString);
224173

225174
template<typename T>
226175
void copyScalar(BaseData* a, const AbstractTypeInfo& nfo, py::array_t<T, py::array::c_style> src)

‎bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp

+19-2
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,9 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs
166166
/// Prepare the description to hold the different python attributes as data field's
167167
/// arguments then create the object.
168168
BaseObjectDescription desc {type.c_str(), type.c_str()};
169-
fillBaseObjectdescription(desc, kwargs);
169+
py::list parametersToCopy;
170+
py::list parametersToLink;
171+
processKwargsForObjectCreation(kwargs, parametersToLink, parametersToCopy, desc);
170172
auto object = ObjectFactory::getInstance()->createObject(self, &desc);
171173

172174
/// After calling createObject the returned value can be either a nullptr
@@ -195,7 +197,13 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs
195197
{
196198
BaseData* d = object->findData(py::cast<std::string>(a.first));
197199
if(d)
200+
{
201+
if (parametersToLink.contains(a.first))
202+
d->setParent(a.second.cast<BaseData*>());
203+
else if(parametersToCopy.contains(a.first))
204+
PythonFactory::fromPython(d, py::cast<py::object>(a.second));
198205
d->setPersistent(true);
206+
}
199207
}
200208
return PythonFactory::toPython(object.get());
201209
}
@@ -258,15 +266,24 @@ py::object addChildKwargs(Node* self, const std::string& name, const py::kwargs&
258266
if (sofapython3::isProtectedKeyword(name))
259267
throw py::value_error("addChild: Cannot call addChild with name " + name + ": Protected keyword");
260268
BaseObjectDescription desc (name.c_str());
261-
fillBaseObjectdescription(desc,kwargs);
269+
py::list parametersToCopy;
270+
py::list parametersToLink;
271+
processKwargsForObjectCreation(kwargs, parametersToLink, parametersToCopy, desc);
272+
262273
auto node=simpleapi::createChild(self, desc);
263274
checkParamUsage(desc);
264275

265276
for(auto a : kwargs)
266277
{
267278
BaseData* d = node->findData(py::cast<std::string>(a.first));
268279
if(d)
280+
{
281+
if (parametersToLink.contains(a.first))
282+
d->setParent(a.second.cast<BaseData*>());
283+
else if(parametersToCopy.contains(a.first))
284+
PythonFactory::fromPython(d, py::cast<py::object>(a.second));
269285
d->setPersistent(true);
286+
}
270287
}
271288

272289
return py::cast(node);

0 commit comments

Comments
 (0)