Skip to content

Commit

Permalink
Sketcher: Internal Transaction Support and ensure valid constraint ge…
Browse files Browse the repository at this point in the history
…ometry indices

===================================================================================

On changing the geometry property (for example from Python), the constraints geometry indices was not rebuild in order to avoid
redundant and unnecessary rebuilds. However, this might cause crashes, as the status of the sketch (or its properties) may be invalid.

It also refactors into OnChanged common functionality.

This commit does NOT solve that the user may be inserting invalid geometry indices to the First/Second/Third of Constraints (invalid input).
Only makes sure that geometry indices (geometry types) of PropertyConstraintList match the geometry.

Solution:

1. Force the rebuild of the constraint geometry indices upon assignment of new Geometry.
2. Force the rebuild of the constraint geometry indices upon assigment of constraints, if they result in invalid geometry indices.
3. Introduce the concept of internal transaction to avoid those rebuilds, checks and updates in case of an ongoing internal transaction,
thereby preventing them as it was done before introducing 1 and 2 (in the case of SketchObject internal transactions).
  • Loading branch information
abdullahtahiriyo committed Jul 1, 2020
1 parent 1fe4328 commit 3941b69
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 153 deletions.
14 changes: 8 additions & 6 deletions src/Mod/Sketcher/App/PropertyConstraintList.cpp
Expand Up @@ -200,7 +200,7 @@ void PropertyConstraintList::applyValues(std::vector<Constraint*>&& lValue)
std::map<App::ObjectIdentifier, App::ObjectIdentifier> renamed;
std::set<App::ObjectIdentifier> removed;
boost::unordered_map<boost::uuids::uuid, std::size_t> newValueMap;

/* Check for renames */
for (unsigned int i = 0; i < lValue.size(); i++) {
boost::unordered_map<boost::uuids::uuid, std::size_t>::const_iterator j = valueMap.find(lValue[i]->tag);
Expand All @@ -221,7 +221,7 @@ void PropertyConstraintList::applyValues(std::vector<Constraint*>&& lValue)
}

/* Collect info about removed elements */
for(auto &v : valueMap)
for(auto &v : valueMap)
removed.insert(makePath(v.second,_lValueList[v.second]));

/* Update value map with new tags from new array */
Expand All @@ -234,7 +234,7 @@ void PropertyConstraintList::applyValues(std::vector<Constraint*>&& lValue)
/* Signal renames */
if (renamed.size() > 0 && !restoreFromTransaction)
signalConstraintsRenamed(renamed);

_lValueList = std::move(lValue);

/* Clean-up; remove old values */
Expand All @@ -258,7 +258,7 @@ bool PropertyConstraintList::getPyPathValue(const App::ObjectIdentifier &path, P

const Constraint *cstr = 0;

if (c1.isArray())
if (c1.isArray())
cstr = _lValueList[c1.getIndex(_lValueList.size())];
else if (c1.isSimple()) {
ObjectIdentifier::Component c1 = path.getPropertyComponent(1);
Expand Down Expand Up @@ -384,11 +384,11 @@ void PropertyConstraintList::applyValidGeometryKeys(const std::vector<unsigned i
validGeometryKeys = keys;
}

void PropertyConstraintList::checkGeometry(const std::vector<Part::Geometry *> &GeoList)
bool PropertyConstraintList::checkGeometry(const std::vector<Part::Geometry *> &GeoList)
{
if (!scanGeometry(GeoList)) {
invalidGeometry = true;
return;
return invalidGeometry;
}

//if we made it here, geometry is OK
Expand All @@ -397,6 +397,8 @@ void PropertyConstraintList::checkGeometry(const std::vector<Part::Geometry *> &
invalidGeometry = false;
touch();
}

return invalidGeometry;
}

/*!
Expand Down
4 changes: 2 additions & 2 deletions src/Mod/Sketcher/App/PropertyConstraintList.h
Expand Up @@ -62,7 +62,7 @@ class SketcherExport PropertyConstraintList : public App::PropertyLists

virtual void setSize(int newSize) override;
virtual int getSize(void) const override;

const char* getEditorName(void) const override {
return "SketcherGui::PropertyConstraintListItem";
}
Expand Down Expand Up @@ -121,7 +121,7 @@ class SketcherExport PropertyConstraintList : public App::PropertyLists
virtual unsigned int getMemSize(void) const override;

void acceptGeometry(const std::vector<Part::Geometry *> &GeoList);
void checkGeometry(const std::vector<Part::Geometry *> &GeoList);
bool checkGeometry(const std::vector<Part::Geometry *> &GeoList);
bool scanGeometry(const std::vector<Part::Geometry *> &GeoList) const;

/// Return status of geometry for better error reporting
Expand Down

0 comments on commit 3941b69

Please sign in to comment.