diff --git a/src/App/Property.cpp b/src/App/Property.cpp index 7e05364964..a9990267a5 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" @@ -50,11 +52,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 9a7732542f..8258724db3 100644 --- a/src/App/Property.h +++ b/src/App/Property.h @@ -252,6 +252,16 @@ public: /// Compare if this property has the same content as the given one virtual bool isSame(const Property &other) const; + /** Return a unique ID for the property + * + * The ID of a property is generated from a 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. + */ + int64_t getID() const {return _id;} + friend class PropertyContainer; friend struct PropertyData; friend class DynamicProperty; @@ -288,6 +298,7 @@ private: private: PropertyContainer *father; const char *myName; + int64_t _id; public: boost::signals2::signal signalChanged; diff --git a/src/App/Transactions.cpp b/src/App/Transactions.cpp index c64a231fee..5b547dafc6 100644 --- a/src/App/Transactions.cpp +++ b/src/App/Transactions.cpp @@ -429,7 +429,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 @@ -441,9 +441,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; } @@ -452,12 +452,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; @@ -493,10 +493,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()); @@ -509,12 +510,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; } @@ -522,7 +523,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 01bcd1d2c2..cd35022d57 100644 --- a/src/App/Transactions.h +++ b/src/App/Transactions.h @@ -137,8 +137,9 @@ protected: struct PropData : DynamicProperty::PropData { Base::Type propertyType; + const Property *propertyOrig = nullptr; }; - std::unordered_map _PropChangeMap; + std::unordered_map _PropChangeMap; std::string _NameInDocument; };