Skip to content

Commit

Permalink
Spreadsheet: docDeps property was not properly maintained, resulting …
Browse files Browse the repository at this point in the history
…in spurious links to other objects.
  • Loading branch information
eivindkv authored and wwmayer committed Feb 17, 2015
1 parent 2621c22 commit 43e777c
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 10 deletions.
2 changes: 2 additions & 0 deletions src/Mod/Spreadsheet/App/Cell.cpp
Expand Up @@ -161,6 +161,8 @@ void Cell::setExpression(Expression *expr)

/* Update dependencies */
owner->addDependencies(address);

owner->rebuildDocDepList();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/Mod/Spreadsheet/App/Expression.h
Expand Up @@ -506,10 +506,10 @@ class SpreadsheetExport VariableExpression : public UnitExpression {

void renameDocument(const std::string &oldName, const std::string &newName);

protected:

const App::Property *getProperty() const;

protected:

Path var; /**< Variable name */
};

Expand Down
56 changes: 51 additions & 5 deletions src/Mod/Spreadsheet/App/PropertySheet.cpp
Expand Up @@ -77,6 +77,34 @@ class RelabelDocumentObjectExpressionVisitor : public ExpressionVisitor {
std::string newName;
};

class BuildDocDepsExpressionVisitor : public ExpressionVisitor {
public:

BuildDocDepsExpressionVisitor(std::set<DocumentObject*> & _docDeps)
: docDeps(_docDeps)
{

}

void visit(Expression * node) {
VariableExpression *expr = freecad_dynamic_cast<VariableExpression>(node);

if (expr) {
const App::Property * prop = expr->getProperty();

if (prop) {
App::DocumentObject * docObj = freecad_dynamic_cast<DocumentObject>(prop->getContainer());

if (docObj)
docDeps.insert(docObj);
}
}
}

private:
std::set<App::DocumentObject*> & docDeps;
};

class RelabelDocumentExpressionVisitor : public ExpressionVisitor {
public:

Expand Down Expand Up @@ -436,6 +464,8 @@ void PropertySheet::clear(CellAddress address)

// Erase from internal struct
data.erase(i);

rebuildDocDepList();
}

void PropertySheet::moveCell(CellAddress currPos, CellAddress newPos)
Expand All @@ -461,6 +491,8 @@ void PropertySheet::moveCell(CellAddress currPos, CellAddress newPos)
data[newPos] = cell;
addDependencies(newPos);
setDirty(newPos);

rebuildDocDepList();
}
}

Expand Down Expand Up @@ -813,11 +845,6 @@ void PropertySheet::addDependencies(CellAddress key)

documentObjectName[docObject] = docObject->Label.getValue();
documentName[docObject->getDocument()] = docObject->getDocument()->Label.getValue();

if (docObject != owner) {
docDeps.resize(docDeps.size() + 1);
docDeps[docDeps.size() - 1] = docObject;
}
}

// Observe document to trach changes to the property
Expand Down Expand Up @@ -1046,8 +1073,27 @@ const std::set<std::string> &PropertySheet::getDeps(CellAddress pos) const

void PropertySheet::recomputeDependencies(CellAddress key)
{
Signaller signaller(*this);

removeDependencies(key);
addDependencies(key);
rebuildDocDepList();
}

void PropertySheet::rebuildDocDepList()
{
Signaller signaller(*this);

docDeps.clear();
BuildDocDepsExpressionVisitor v(docDeps);

std::map<CellAddress, Cell* >::iterator i = data.begin();

/* Resolve all cells */
while (i != data.end()) {
i->second->visit(v);
++i;
}
}

PyObject *PropertySheet::getPyObject()
Expand Down
6 changes: 4 additions & 2 deletions src/Mod/Spreadsheet/App/PropertySheet.h
Expand Up @@ -123,7 +123,7 @@ class PropertySheet : public App::Property {

const std::set<std::string> &getDeps(CellAddress pos) const;

const std::vector<App::DocumentObject*> & getDocDeps() const { return docDeps; }
const std::set<App::DocumentObject*> & getDocDeps() const { return docDeps; }

class Signaller {
public:
Expand Down Expand Up @@ -184,6 +184,8 @@ class PropertySheet : public App::Property {

void recomputeDependants(const App::DocumentObject * docObj);

void rebuildDocDepList();

/* Cell dependencies, i.e when a change occurs to property given in key,
the set of addresses needs to be recomputed.
*/
Expand All @@ -201,7 +203,7 @@ class PropertySheet : public App::Property {
std::map<CellAddress, std::set< std::string > > cellToDocumentObjectMap;

/* Other document objects the sheet depends on */
std::vector<App::DocumentObject*> docDeps;
std::set<App::DocumentObject*> docDeps;

/* Name of document objects, used for renaming */
std::map<const App::DocumentObject*, std::string> documentObjectName;
Expand Down
4 changes: 3 additions & 1 deletion src/Mod/Spreadsheet/App/Sheet.cpp
Expand Up @@ -796,7 +796,9 @@ App::DocumentObjectExecReturn *Sheet::execute(void)
currRow.purgeTouched();
currColumn.purgeTouched();

docDeps.setValues(cells.getDocDeps());
std::set<App::DocumentObject*> ds(cells.getDocDeps());
std::vector<App::DocumentObject*> dv(ds.begin(), ds.end());
docDeps.setValues(dv);

purgeTouched();

Expand Down

0 comments on commit 43e777c

Please sign in to comment.