diff --git a/src/Mod/Part/TestPartApp.py b/src/Mod/Part/TestPartApp.py index c6669e35c5ed8..0b4d08ee310e5 100644 --- a/src/Mod/Part/TestPartApp.py +++ b/src/Mod/Part/TestPartApp.py @@ -110,6 +110,7 @@ def testIssue2671(self): Box = self.Doc.addObject("Part::Box","Box") Mirroring = self.Doc.addObject("Part::Mirroring", 'Mirroring') Spreadsheet = self.Doc.addObject('Spreadsheet::Sheet', 'Spreadsheet') + Mirroring.Source = Box Mirroring.Base = (8, 5, 25) Mirroring.Normal = (0.5, 0.2, 0.9) Spreadsheet.set('A1', '=Mirroring.Base.x') diff --git a/src/Mod/Spreadsheet/App/Cell.cpp b/src/Mod/Spreadsheet/App/Cell.cpp index 7166cd231726f..3a835e5c64de8 100644 --- a/src/Mod/Spreadsheet/App/Cell.cpp +++ b/src/Mod/Spreadsheet/App/Cell.cpp @@ -166,8 +166,6 @@ void Cell::setExpression(App::Expression *expr) /* Update dependencies */ owner->addDependencies(address); - - owner->rebuildDocDepList(); } /** @@ -185,7 +183,7 @@ const App::Expression *Cell::getExpression() const * */ -bool Cell::getStringContent(std::string & s) const +bool Cell::getStringContent(std::string & s, bool persistent) const { if (expression) { auto sexpr = freecad_dynamic_cast(expression); @@ -206,7 +204,7 @@ bool Cell::getStringContent(std::string & s) const else if (freecad_dynamic_cast(expression)) s = expression->toString(); else - s = "=" + expression->toString(); + s = "=" + expression->toString(persistent); return true; } @@ -216,6 +214,12 @@ bool Cell::getStringContent(std::string & s) const } } +void Cell::afterRestore() { + auto expr = dynamic_cast(expression); + if(expr) + setContent(expr->getText().c_str()); +} + void Cell::setContent(const char * value) { PropertySheet::AtomicPropertyChange signaller(*owner); @@ -223,6 +227,11 @@ void Cell::setContent(const char * value) setUsed(PARSE_EXCEPTION_SET, false); if (value != 0) { + if(owner->sheet()->isRestoring()) { + expression = new StringExpression(owner->sheet(),value); + setUsed(EXPRESSION_SET, true); + return; + } if (*value == '=') { try { expr = App::ExpressionParser::parse(owner->sheet(), value + 1); diff --git a/src/Mod/Spreadsheet/App/Cell.h b/src/Mod/Spreadsheet/App/Cell.h index 0f23c8ce734c3..10f739e179460 100644 --- a/src/Mod/Spreadsheet/App/Cell.h +++ b/src/Mod/Spreadsheet/App/Cell.h @@ -61,7 +61,7 @@ class SpreadsheetExport Cell { const App::Expression * getExpression() const; - bool getStringContent(std::string & s) const; + bool getStringContent(std::string & s, bool persistent=false) const; void setContent(const char * value); @@ -107,6 +107,8 @@ class SpreadsheetExport Cell { void restore(Base::XMLReader &reader); + void afterRestore(); + void save(Base::Writer &writer) const; bool isUsed() const; @@ -190,6 +192,8 @@ class SpreadsheetExport Cell { int colSpan; std::string exceptionStr; App::CellAddress anchor; + + friend class PropertySheet; }; } diff --git a/src/Mod/Spreadsheet/App/PropertySheet.cpp b/src/Mod/Spreadsheet/App/PropertySheet.cpp index 7f7a23dda3a03..290605bb4def2 100644 --- a/src/Mod/Spreadsheet/App/PropertySheet.cpp +++ b/src/Mod/Spreadsheet/App/PropertySheet.cpp @@ -46,7 +46,7 @@ using namespace App; using namespace Base; using namespace Spreadsheet; -TYPESYSTEM_SOURCE(Spreadsheet::PropertySheet , App::Property); +TYPESYSTEM_SOURCE(Spreadsheet::PropertySheet , App::PropertyLinkBase); void PropertySheet::clear() { @@ -66,7 +66,15 @@ void PropertySheet::clear() propertyNameToCellMap.clear(); documentObjectToCellMap.clear(); + + if(getContainer()==owner && owner && owner->getNameInDocument()) { + for(auto obj : docDeps) { + if(obj && obj->getNameInDocument()) + obj->_removeBackLink(owner); + } + } docDeps.clear(); + aliasProp.clear(); revAliasProp.clear(); } @@ -167,16 +175,13 @@ Cell * PropertySheet::createCell(CellAddress address) } PropertySheet::PropertySheet(Sheet *_owner) - : Property() - , AtomicPropertyChangeInterface() - , owner(_owner) + : owner(_owner) + , updateCount(0) { } PropertySheet::PropertySheet(const PropertySheet &other) - : Property() - , AtomicPropertyChangeInterface() - , dirty(other.dirty) + : dirty(other.dirty) , mergedCells(other.mergedCells) , owner(other.owner) , propertyNameToCellMap(other.propertyNameToCellMap) @@ -184,10 +189,10 @@ PropertySheet::PropertySheet(const PropertySheet &other) , documentObjectToCellMap(other.documentObjectToCellMap) , cellToDocumentObjectMap(other.cellToDocumentObjectMap) , docDeps(other.docDeps) - , documentObjectName(other.documentObjectName) , documentName(other.documentName) , aliasProp(other.aliasProp) , revAliasProp(other.revAliasProp) + , updateCount(other.updateCount) { std::map::const_iterator i = other.data.begin(); @@ -504,8 +509,6 @@ void PropertySheet::clear(CellAddress address) // Erase from internal struct data.erase(i); - - rebuildDocDepList(); } void PropertySheet::moveCell(CellAddress currPos, CellAddress newPos, std::map & renames) @@ -549,8 +552,6 @@ void PropertySheet::moveCell(CellAddress currPos, CellAddress newPos, std::mapLabel.getValue(); std::string docObjName = docName + "#" + docObj->getNameInDocument(); - documentObjectName[docObj] = docObj->Label.getValue(); documentName[doc] = docName; owner->observeDocument(doc); documentObjectToCellMap[docObjName].insert(key); cellToDocumentObjectMap[key].insert(docObjName); + ++updateCount; for(auto &props : dep.second) { std::string propName = docObjName + "." + props.first; @@ -1006,6 +1007,7 @@ void PropertySheet::removeDependencies(CellAddress key) } cellToDocumentObjectMap.erase(i2); + ++updateCount; } } @@ -1078,6 +1080,9 @@ void PropertySheet::invalidateDependants(const App::DocumentObject *docObj) void PropertySheet::renamedDocumentObject(const App::DocumentObject * docObj) { +#if 1 + (void)docObj; +#else if (documentObjectName.find(docObj) == documentObjectName.end()) return; @@ -1092,6 +1097,7 @@ void PropertySheet::renamedDocumentObject(const App::DocumentObject * docObj) } ++i; } +#endif } void PropertySheet::renamedDocument(const App::Document * doc) @@ -1158,18 +1164,52 @@ void PropertySheet::recomputeDependencies(CellAddress key) removeDependencies(key); addDependencies(key); - rebuildDocDepList(); } -void PropertySheet::rebuildDocDepList() +void PropertySheet::hasSetValue() { - AtomicPropertyChange signaller(*this); - - docDeps.clear(); - for(auto &v : data) { - if(v.second->getExpression()) - v.second->getExpression()->getDepObjects(docDeps); + if(!updateCount || + !owner || !owner->getNameInDocument() || owner->isRestoring() || + this!=&owner->cells) + { + PropertyLinkBase::hasSetValue(); + return; + } + updateCount = 0; + + std::set deps; + std::vector labels; + unregisterElementReference(); + UpdateElementReferenceExpressionVisitor v(*this); + for(auto &d : data) { + auto expr = d.second->expression; + if(expr) { + expr->getDepObjects(deps,&labels); + expr->visit(v); + } } + registerLabelReferences(labels); + + deps.erase(owner); + +#ifndef USE_OLD_DAG + for(auto obj : deps) { + if(obj && obj->getNameInDocument()) { + auto it = docDeps.find(obj); + if(it == docDeps.end()) + obj->_addBackLink(owner); + else + docDeps.erase(it); + } + } + for(auto obj : docDeps) { + if(obj && obj->getNameInDocument()) + obj->_removeBackLink(owner); + } +#endif + docDeps.swap(deps); + + PropertyLinkBase::hasSetValue(); } PyObject *PropertySheet::getPyObject() @@ -1181,16 +1221,110 @@ PyObject *PropertySheet::getPyObject() return Py::new_reference_to(PythonObject); } -void PropertySheet::resolveAll() +void PropertySheet::afterRestore() { - std::map::iterator i = data.begin(); - - /* Resolve all cells */ AtomicPropertyChange signaller(*this); - while (i != data.end()) { - recomputeDependencies(i->first); - setDirty(i->first); - ++i; + for(auto &d : data) + d.second->afterRestore(); +} + +void PropertySheet::breakLink(App::DocumentObject *obj, bool) { + invalidateDependants(obj); + deletedDocumentObject(obj); +} + +bool PropertySheet::adjustLink(const std::set &inList) { + std::unique_ptr signaler; + + for(auto &d : data) { + auto expr = d.second->expression; + if(!expr) + continue; + try { + bool need_adjust = false; + for(auto docObj : expr->getDepObjects()) { + if (docObj && docObj != owner && inList.count(docObj)) { + need_adjust = true; + break; + } + } + if(!need_adjust) + continue; + if(!signaler) + signaler.reset(new AtomicPropertyChange(*this)); + + removeDependencies(d.first); + expr->adjustLinks(inList); + addDependencies(d.first); + + }catch(Base::Exception &e) { + addDependencies(d.first); + std::ostringstream ss; + ss << "Failed to adjust link for " << owner->getFullName() << " in expression " + << expr->toString() << ": " << e.what(); + throw Base::RuntimeError(ss.str()); + } } - touch(); + return !!signaler; +} + +void PropertySheet::updateElementReference(DocumentObject *feature,bool reverse) { + unregisterElementReference(); + UpdateElementReferenceExpressionVisitor visitor(*this,feature,reverse); + for(auto &d : data) { + auto expr = d.second->expression; + if(!expr) + continue; + expr->visit(visitor); + } +} + +bool PropertySheet::referenceChanged() const { + return false; +} + +void PropertySheet::getLinks(std::vector &objs, + bool, std::vector * /*subs*/, bool /*newStyle*/) const +{ + objs.insert(objs.end(),docDeps.begin(),docDeps.end()); +} + +Property *PropertySheet::CopyOnImportExternal( + const std::map &nameMap) const +{ + std::map > changed; + for(auto &d : data) { + auto expr = d.second->expression; + if(expr) + expr = expr->importSubNames(nameMap); + if(!expr) + continue; + changed[d.first].reset(expr); + } + if(changed.empty()) + return 0; + std::unique_ptr copy(new PropertySheet(*this)); + for(auto &change : changed) + copy->data[change.first]->setExpression(change.second.release()); + return copy.release(); +} + +Property *PropertySheet::CopyOnLabelChange(App::DocumentObject *obj, + const std::string &ref, const char *newLabel) const +{ + std::map > changed; + for(auto &d : data) { + auto expr = d.second->expression; + if(expr) + expr = expr->updateLabelReference(obj,ref,newLabel); + if(!expr) + continue; + changed[d.first].reset(expr); + } + if(changed.empty()) + return 0; + std::unique_ptr copy(new PropertySheet(*this)); + for(auto &change : changed) + copy->data[change.first]->setExpression(change.second.release()); + return copy.release(); } diff --git a/src/Mod/Spreadsheet/App/PropertySheet.h b/src/Mod/Spreadsheet/App/PropertySheet.h index 89e9ce1a223ad..aad0e111ca233 100644 --- a/src/Mod/Spreadsheet/App/PropertySheet.h +++ b/src/Mod/Spreadsheet/App/PropertySheet.h @@ -26,7 +26,7 @@ #include #include #include -#include +#include #include #include "Cell.h" @@ -37,7 +37,7 @@ class Sheet; class PropertySheet; class SheetObserver; -class PropertySheet : public App::Property, private App::AtomicPropertyChangeInterface { +class PropertySheet : public App::PropertyLinkBase, private App::AtomicPropertyChangeInterface { TYPESYSTEM_HEADER(); public: @@ -45,6 +45,18 @@ class PropertySheet : public App::Property, private App::AtomicPropertyChangeInt ~PropertySheet(); + virtual void updateElementReference(App::DocumentObject *feature,bool reverse=false) override; + virtual bool referenceChanged() const override; + virtual void getLinks(std::vector &objs, + bool all=false, std::vector *subs=0, bool newStyle=true) const override; + virtual void breakLink(App::DocumentObject *obj, bool clear) override; + virtual bool adjustLink(const std::set &inList) override; + virtual Property *CopyOnImportExternal(const std::map &nameMap) const override; + virtual Property *CopyOnLabelChange(App::DocumentObject *obj, + const std::string &ref, const char *newLabel) const override; + + virtual void afterRestore() override; + virtual Property *Copy(void) const; virtual void Paste(const Property &from); @@ -133,8 +145,6 @@ class PropertySheet : public App::Property, private App::AtomicPropertyChangeInt PyObject *getPyObject(void); - void resolveAll(); - void invalidateDependants(const App::DocumentObject *docObj); void renamedDocumentObject(const App::DocumentObject *docObj); @@ -147,6 +157,9 @@ class PropertySheet : public App::Property, private App::AtomicPropertyChangeInt void documentSet(); +protected: + virtual void hasSetValue() override; + private: PropertySheet(const PropertySheet & other); @@ -193,8 +206,6 @@ class PropertySheet : public App::Property, private App::AtomicPropertyChangeInt void recomputeDependants(const App::DocumentObject *obj, const App::Property * prop); - void rebuildDocDepList(); - /*! Cell dependencies, i.e when a change occurs to property given in key, the set of addresses needs to be recomputed. */ @@ -214,9 +225,6 @@ class PropertySheet : public App::Property, private App::AtomicPropertyChangeInt /*! Other document objects the sheet depends on */ std::set docDeps; - /*! Name of document objects, used for renaming */ - std::map documentObjectName; - /*! Name of documents, used for renaming */ std::map documentName; @@ -228,6 +236,8 @@ class PropertySheet : public App::Property, private App::AtomicPropertyChangeInt /*! The associated python object */ Py::Object PythonObject; + + int updateCount; }; } diff --git a/src/Mod/Spreadsheet/App/Sheet.cpp b/src/Mod/Spreadsheet/App/Sheet.cpp index c86e77a70b217..6c9c7c1df2ec7 100644 --- a/src/Mod/Spreadsheet/App/Sheet.cpp +++ b/src/Mod/Spreadsheet/App/Sheet.cpp @@ -79,14 +79,10 @@ Sheet::Sheet() , props(this) , cells(this) { - ADD_PROPERTY_TYPE(docDeps, (0), "Spreadsheet", (PropertyType)(Prop_Transient|Prop_ReadOnly|Prop_Hidden), "Dependencies"); ADD_PROPERTY_TYPE(cells, (), "Spreadsheet", (PropertyType)(Prop_ReadOnly|Prop_Hidden), "Cell contents"); ADD_PROPERTY_TYPE(columnWidths, (), "Spreadsheet", (PropertyType)(Prop_ReadOnly|Prop_Hidden|Prop_Output), "Column widths"); ADD_PROPERTY_TYPE(rowHeights, (), "Spreadsheet", (PropertyType)(Prop_ReadOnly|Prop_Hidden|Prop_Output), "Row heights"); - docDeps.setSize(0); - docDeps.allowExternalLink(true); - onRenamedDocumentConnection = GetApplication().signalRenameDocument.connect(boost::bind(&Spreadsheet::Sheet::onRenamedDocument, this, _1)); onRelabledDocumentConnection = GetApplication().signalRelabelDocument.connect(boost::bind(&Spreadsheet::Sheet::onRelabledDocument, this, _1)); @@ -122,7 +118,6 @@ void Sheet::clearAll() columnWidths.clear(); rowHeights.clear(); removedAliases.clear(); - docDeps.setValues(std::vector()); for (ObserverMap::iterator i = observers.begin(); i != observers.end(); ++i) delete i->second; @@ -859,14 +854,6 @@ DocumentObjectExecReturn *Sheet::execute(void) rowHeights.clearDirty(); columnWidths.clearDirty(); - std::set ds(cells.getDocDeps()); - - // Make sure we don't reference ourselves - ds.erase(this); - - std::vector dv(ds.begin(), ds.end()); - docDeps.setValues(dv); - purgeTouched(); if (cellErrors.size() == 0) @@ -884,10 +871,8 @@ short Sheet::mustExecute(void) const { if (cellErrors.size() > 0 || cells.isTouched() || columnWidths.isTouched() || rowHeights.isTouched()) return 1; - else if (cells.getDocDeps().size() == 0) - return 0; else - return -1; + return 0; } @@ -915,15 +900,6 @@ void Sheet::clear(CellAddress address, bool /*all*/) cells.clear(address); - // Update dependencies - std::set ds(cells.getDocDeps()); - - // Make sure we don't reference ourselves - ds.erase(this); - - std::vector dv(ds.begin(), ds.end()); - docDeps.setValues(dv); - propAddress.erase(prop); this->removeDynamicProperty(addr.c_str()); } @@ -1229,19 +1205,6 @@ void Sheet::setSpans(CellAddress address, int rows, int columns) cells.setSpans(address, rows, columns); } -/** - * @brief Called when a document object is renamed. - * @param docObj Renamed document object. - */ - -void Sheet::renamedDocumentObject(const DocumentObject * docObj) -{ - if(docObj->getOldLabel() == docObj->Label.getStrValue()) - return; - cells.renamedDocumentObject(docObj); - cells.touch(); -} - /** * @brief Called when alias \a alias at \a address is removed. * @param address Address of alias. @@ -1297,7 +1260,6 @@ void Sheet::providesTo(CellAddress address, std::set & result) cons void Sheet::onDocumentRestored() { - cells.resolveAll(); execute(); } @@ -1328,6 +1290,11 @@ void Sheet::onRenamedDocument(const Document & /*document*/) void Sheet::observeDocument(Document * document) { + // observer is no longer required as PropertySheet is now derived from + // PropertyLinkBase and will handle all the link related behavior +#if 1 + (void)document; +#else ObserverMap::const_iterator it = observers.find(document->getName()); if (it != observers.end()) { @@ -1340,6 +1307,7 @@ void Sheet::observeDocument(Document * document) observers[document->getName()] = observer; } +#endif } void Sheet::renameObjectIdentifiers(const std::map &paths) diff --git a/src/Mod/Spreadsheet/App/Sheet.h b/src/Mod/Spreadsheet/App/Sheet.h index 82f34fde91410..9398af06a0db8 100644 --- a/src/Mod/Spreadsheet/App/Sheet.h +++ b/src/Mod/Spreadsheet/App/Sheet.h @@ -260,8 +260,6 @@ class SpreadsheetExport Sheet : public App::DocumentObject App::Property *setQuantityProperty(App::CellAddress key, double value, const Base::Unit &unit); - void renamedDocumentObject(const App::DocumentObject * docObj); - void aliasRemoved(App::CellAddress address, const std::string &alias); void removeAliases(); @@ -291,9 +289,6 @@ class SpreadsheetExport Sheet : public App::DocumentObject /* Row heights */ PropertyRowHeights rowHeights; - /* Dependencies to other documents */ - App::PropertyLinkList docDeps; - /* Document observers to track changes to external properties */ typedef std::map ObserverMap; ObserverMap observers; diff --git a/src/Mod/Spreadsheet/TestSpreadsheet.py b/src/Mod/Spreadsheet/TestSpreadsheet.py index c6db7f1c57a1b..49277b8852776 100644 --- a/src/Mod/Spreadsheet/TestSpreadsheet.py +++ b/src/Mod/Spreadsheet/TestSpreadsheet.py @@ -492,10 +492,17 @@ def testPrecedence(self): sheet.set('A52', '=+(-1 + -1)') self.doc.addObject("Part::Cylinder", "Cylinder") - self.doc.addObject("Part::Thickness", "Pipe") + # We cannot use Thickness, as this feature requires a source shape, + # otherwise it will cause recomputation failure. The new logic of + # App::Document will not continue recompute any dependent objects + + # self.doc.addObject("Part::Thickness", "Pipe") + self.doc.addObject("Part::Box", "Box") + self.doc.Box.Length = 1 + sheet.set('B1', '101') sheet.set('A53', '=-(-(B1-1)/2)') - sheet.set('A54', '=-(Cylinder.Radius + Pipe.Value - 1"/2)') + sheet.set('A54', '=-(Cylinder.Radius + Box.Length - 1"/2)') self.doc.recompute() self.assertEqual(sheet.getContents("A1"), "=1 < 2 ? 3 : 4")