diff --git a/src/App/Property.cpp b/src/App/Property.cpp index 201404b49734..0a88753a13b7 100644 --- a/src/App/Property.cpp +++ b/src/App/Property.cpp @@ -27,6 +27,8 @@ # include #endif +#include + /// Here the FreeCAD includes sorted by Base,App,Gui...... #include "Property.h" #include "ObjectIdentifier.h" @@ -48,11 +50,12 @@ TYPESYSTEM_SOURCE_ABSTRACT(App::Property , Base::Persistence) //************************************************************************** // Construction/Destruction +static std::atomic _PropID; + // Here is the implementation! Description should take place in the header file! Property::Property() - :father(0), myName(0) + :father(0), myName(0), _id(++_PropID) { - } Property::~Property() diff --git a/src/App/Property.h b/src/App/Property.h index be321d346c1d..2f78f0dbd351 100644 --- a/src/App/Property.h +++ b/src/App/Property.h @@ -237,6 +237,16 @@ class AppExport Property : public Base::Persistence /// Called before a child property changing value virtual void aboutToSetChildValue(Property &) {} + /** Return a unique ID for the property + * + * The ID of a property is generated from an monotonically increasing + * internal counter. The intention of the ID is to be used as a key for + * mapping, instead of using the raw pointer. Because, it is possible for + * the runtime memory allocator to reuse just deleted memory, which will + * cause hard to debug problem if use pointer as key. + */ + long getID() const {return _id;} + friend class PropertyContainer; friend struct PropertyData; friend class DynamicProperty; @@ -273,6 +283,7 @@ class AppExport Property : public Base::Persistence private: PropertyContainer *father; const char *myName; + long _id; }; diff --git a/src/App/Transactions.cpp b/src/App/Transactions.cpp index 57359c5beee9..0e57ff0e81b1 100644 --- a/src/App/Transactions.cpp +++ b/src/App/Transactions.cpp @@ -299,7 +299,7 @@ void TransactionObject::applyChn(Document & /*Doc*/, TransactionalObject *pcObj, // Property change order is not preserved, as it is recursive in nature for(auto &v : _PropChangeMap) { auto &data = v.second; - auto prop = const_cast(v.first); + auto prop = const_cast(data.propertyOrig); if(!data.property) { // here means we are undoing/redoing and property add operation @@ -311,9 +311,9 @@ void TransactionObject::applyChn(Document & /*Doc*/, TransactionalObject *pcObj, // been destroies. We must prepare for the case where user removed // a dynamic property but does not recordered as transaction. auto name = pcObj->getPropertyName(prop); - if(!name) { + if(!name || (data.name.size() && data.name != name) || data.propertyType != prop->getTypeId()) { // Here means the original property is not found, probably removed - if(v.second.name.empty()) { + if(data.name.empty()) { // not a dynamic property, nothing to do continue; } @@ -322,12 +322,12 @@ void TransactionObject::applyChn(Document & /*Doc*/, TransactionalObject *pcObj, // restored. But since restoring property is actually creating // a new property, the property key inside redo stack will not // match. So we search by name first. - prop = pcObj->getDynamicPropertyByName(v.second.name.c_str()); + prop = pcObj->getDynamicPropertyByName(data.name.c_str()); if(!prop) { // Still not found, re-create the property prop = pcObj->addDynamicProperty( - data.property->getTypeId().getName(), - v.second.name.c_str(), data.group.c_str(), data.doc.c_str(), + data.propertyType.getName(), + data.name.c_str(), data.group.c_str(), data.doc.c_str(), data.attr, data.readonly, data.hidden); if(!prop) continue; @@ -363,10 +363,11 @@ void TransactionObject::applyChn(Document & /*Doc*/, TransactionalObject *pcObj, void TransactionObject::setProperty(const Property* pcProp) { - auto &data = _PropChangeMap[pcProp]; + auto &data = _PropChangeMap[pcProp->getID()]; if(!data.property && data.name.empty()) { static_cast(data) = pcProp->getContainer()->getDynamicPropertyData(pcProp); + data.propertyOrig = pcProp; data.property = pcProp->Copy(); data.propertyType = pcProp->getTypeId(); data.property->setStatusValue(pcProp->getStatus()); @@ -379,12 +380,12 @@ void TransactionObject::addOrRemoveProperty(const Property* pcProp, bool add) if(!pcProp || !pcProp->getContainer()) return; - auto &data = _PropChangeMap[pcProp]; + auto &data = _PropChangeMap[pcProp->getID()]; if(data.name.size()) { if(!add && !data.property) { // this means add and remove the same property inside a single // transaction, so they cancel each other out. - _PropChangeMap.erase(pcProp); + _PropChangeMap.erase(pcProp->getID()); } return; } @@ -392,7 +393,7 @@ void TransactionObject::addOrRemoveProperty(const Property* pcProp, bool add) delete data.property; data.property = 0; } - + data.propertyOrig = pcProp; static_cast(data) = pcProp->getContainer()->getDynamicPropertyData(pcProp); if(add) diff --git a/src/App/Transactions.h b/src/App/Transactions.h index 94eceb7a44b4..53425abcfe1d 100644 --- a/src/App/Transactions.h +++ b/src/App/Transactions.h @@ -132,8 +132,9 @@ class AppExport TransactionObject : public Base::Persistence struct PropData : DynamicProperty::PropData { Base::Type propertyType; + const Property *propertyOrig = nullptr; }; - std::unordered_map _PropChangeMap; + std::unordered_map _PropChangeMap; std::string _NameInDocument; };