From efe33757ee0fb27f641f90518fbfd11347fd94be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Wed, 9 Aug 2017 19:20:44 +0200 Subject: [PATCH] Make sure all relevant links are found for object in GeoFeatureGroup. issue0003150 --- src/App/GeoFeatureGroupExtension.cpp | 126 ++++++++++++++------------- src/App/GeoFeatureGroupExtension.h | 22 +++-- src/Mod/Test/Document.py | 22 ++++- 3 files changed, 100 insertions(+), 70 deletions(-) diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 0262cec270..6622f4a183 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -96,8 +96,12 @@ DocumentObject* GeoFeatureGroupExtension::getGroupOfObject(const DocumentObject* //is no reason to distinguish between GeoFeatuerGroups, there is only between group/geofeaturegroup auto list = obj->getInList(); for (auto obj : list) { - if(obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) + if(obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) { + + //TODO: There may be links to geofeaturegroups to objects not inside that group. We need to + //check that! return obj; + } } return nullptr; @@ -132,8 +136,7 @@ std::vector GeoFeatureGroupExtension::addObjects(std::vector links; - getCSRelevantLinks(object, links); + std::vector links = getCSRelevantLinks(object); links.push_back(object); for( auto obj : links) { @@ -160,8 +163,7 @@ std::vector GeoFeatureGroupExtension::removeObjects(std::vector for(auto object : objects) { //cross CoordinateSystem links are not allowed, so we need to remove the whole link group - std::vector< DocumentObject* > links; - getCSRelevantLinks(object, links); + std::vector< DocumentObject* > links = getCSRelevantLinks(object); links.push_back(object); //remove all links out of group @@ -214,6 +216,9 @@ void GeoFeatureGroupExtension::extensionOnChanged(const Property* p) { std::vector< DocumentObject* > GeoFeatureGroupExtension::getScopedObjectsFromLinks(const DocumentObject* obj, LinkScope scope) { + if(!obj) + std::vector< DocumentObject* >(); + //we get all linked objects. We can't use outList() as this includes the links from expressions std::vector< App::DocumentObject* > result; std::vector list; @@ -233,6 +238,9 @@ std::vector< DocumentObject* > GeoFeatureGroupExtension::getScopedObjectsFromLin std::vector< DocumentObject* > GeoFeatureGroupExtension::getScopedObjectsFromLink(App::Property* prop, LinkScope scope) { + if(!prop) + return std::vector< DocumentObject* >(); + std::vector< App::DocumentObject* > result; if(prop->getTypeId().isDerivedFrom(App::PropertyLink::getClassTypeId()) && @@ -268,106 +276,98 @@ std::vector< DocumentObject* > GeoFeatureGroupExtension::getScopedObjectsFromLin -void GeoFeatureGroupExtension::getCSOutList(const App::DocumentObject* obj, std::vector< DocumentObject* >& vec) { +void GeoFeatureGroupExtension::getCSOutList(const App::DocumentObject* obj, + std::vector< DocumentObject* >& vec) { if(!obj) return; - - //if the object is a GeoFeatureGroup then all dependencies belong to that CS, we don't want them - if(obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) - return; - + //we get all relevant linked objects. We can't use outList() as this includes the links from expressions, //also we only want links with scope Local - auto result = getScopedObjectsFromLinks(obj); + auto result = getScopedObjectsFromLinks(obj, LinkScope::Local); //we remove all links to origin features and origins, they belong to a CS too and can't be moved result.erase(std::remove_if(result.begin(), result.end(), [](App::DocumentObject* obj)->bool { return (obj->isDerivedFrom(App::OriginFeature::getClassTypeId()) || obj->isDerivedFrom(App::Origin::getClassTypeId())); }), result.end()); - - //collect all dependencies of those objects and store them in the result vector - for(App::DocumentObject *obj : result) { - - //prevent infinite recursion - if(std::find(vec.begin(), vec.end(), obj) != vec.end()) - throw Base::Exception("Graph is not DAG"); - - vec.push_back(obj); - std::vector< DocumentObject* > links; - getCSOutList(obj, links); - vec.insert(vec.end(), links.begin(), links.end()); - } - + + vec.insert(vec.end(), result.begin(), result.end()); + //post process the vector std::sort(vec.begin(), vec.end()); vec.erase(std::unique(vec.begin(), vec.end()), vec.end()); } -void GeoFeatureGroupExtension::getCSInList(const DocumentObject* obj, std::vector< DocumentObject* >& vec) { +void GeoFeatureGroupExtension::getCSInList(const DocumentObject* obj, + std::vector< DocumentObject* >& vec) { if(!obj) return; - - //we get all objects that link to it - std::vector< App::DocumentObject* > result; - + //search the inlist for objects that have non-expression links to us for(App::DocumentObject* parent : obj->getInList()) { - + //not interested in other groups (and here we mean all groups, normal ones and geofeaturegroup) if(parent->hasExtension(App::GroupExtension::getExtensionClassTypeId())) continue; - //prevent infinite recursion - if(std::find(vec.begin(), vec.end(), parent) != vec.end()) - throw Base::Exception("Graph is not DAG"); - //check if the link is real Local scope one or if it is a expression one (could also be both, so it is not //enough to check the expressions) - auto res = getScopedObjectsFromLinks(parent); + auto res = getScopedObjectsFromLinks(parent, LinkScope::Local); if(std::find(res.begin(), res.end(), obj) != res.end()) - result.push_back(parent); + vec.push_back(parent); } //clear all duplicates - std::sort(result.begin(), result.end()); - result.erase(std::unique(result.begin(), result.end()), result.end()); - - //collect all links to those objects - for(App::DocumentObject *obj : result) { - vec.push_back(obj); - getCSInList(obj, vec); - } - std::sort(vec.begin(), vec.end()); vec.erase(std::unique(vec.begin(), vec.end()), vec.end()); - } -void GeoFeatureGroupExtension::getCSRelevantLinks(const DocumentObject* obj, std::vector< DocumentObject* >& vec ) { +std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSRelevantLinks(const DocumentObject* obj) { + if(!obj) + return std::vector< DocumentObject* >(); + //get all out links - getCSOutList(obj, vec); + std::vector vec; - //we need to get the outlist of all inlist objects. This is needed to handle things - //like Booleans: the boolean is our parent, than there is a second object under it which relates - //to obj and needs to be handled. - std::vector< DocumentObject* > in; - getCSInList(obj, in); - for(auto o : in) { - vec.push_back(o); - getCSOutList(o, vec); - } + recursiveCSRelevantLinks(obj, vec); - //post process + //post process the list after we added many things std::sort(vec.begin(), vec.end()); vec.erase(std::unique(vec.begin(), vec.end()), vec.end()); + vec.erase(std::remove(vec.begin(), vec.end(), obj), vec.end()); + + return vec; } +void GeoFeatureGroupExtension::recursiveCSRelevantLinks(const DocumentObject* obj, + std::vector< DocumentObject* >& vec) { + + if(!obj) + return; + + std::vector< DocumentObject* > links; + getCSOutList(obj, links); + getCSInList(obj, links); + + //go on traversing the graph in all directions! + for(auto o : links) { + if(!o || o == obj || std::find(vec.begin(), vec.end(), o) != vec.end()) + continue; + + vec.push_back(o); + recursiveCSRelevantLinks(o, vec); + } +} + + bool GeoFeatureGroupExtension::areLinksValid(const DocumentObject* obj) { + if(!obj) + return true; + //no cross CS link for local links. //Base::Console().Message("Check object links: %s\n", obj->getNameInDocument()); std::vector list; @@ -384,6 +384,9 @@ bool GeoFeatureGroupExtension::areLinksValid(const DocumentObject* obj) { bool GeoFeatureGroupExtension::isLinkValid(App::Property* prop) { + if(!prop) + return true; + //get the object that holds the property if(!prop->getContainer()->isDerivedFrom(App::DocumentObject::getClassTypeId())) return true; //this link comes not from a document object, scopes are meaningless @@ -412,6 +415,9 @@ bool GeoFeatureGroupExtension::isLinkValid(App::Property* prop) { void GeoFeatureGroupExtension::getInvalidLinkObjects(const DocumentObject* obj, std::vector< DocumentObject* >& vec) { + if(!obj) + return; + //no cross CS link for local links. auto result = getScopedObjectsFromLinks(obj, LinkScope::Local); auto group = obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId()) ? obj : getGroupOfObject(obj); diff --git a/src/App/GeoFeatureGroupExtension.h b/src/App/GeoFeatureGroupExtension.h index ba0a9a3bc4..7ef646a3e5 100644 --- a/src/App/GeoFeatureGroupExtension.h +++ b/src/App/GeoFeatureGroupExtension.h @@ -99,18 +99,10 @@ public: virtual std::vector< DocumentObject* > addObjects(std::vector< DocumentObject* > obj) override; virtual std::vector< DocumentObject* > removeObjects(std::vector< DocumentObject* > obj) override; - /// Collects GeoFeatureGroup relevant objects that are linked from the given one. That means all linked objects - /// including their links (recursively) except GeoFeatureGroups, where the recursion stops. Expressions - /// links are ignored. An exception is thrown when there are dependency loops. - static void getCSOutList(const App::DocumentObject* obj, std::vector& vec); - /// Collects GeoFeatureGroup relevant objects that link to the given one. That means all objects - /// including their parents (recursively) except GeoFeatureGroups, where the recursion stops. Expression - /// links are ignored. An exception is thrown when there are dependency loops. - static void getCSInList(const App::DocumentObject* obj, std::vector& vec); /// Collects all links that are relevant for the coordinate system, meaning all recursive links to /// obj and from obj excluding expressions and stopping the recursion at other geofeaturegroups. /// The result is the combination of CSOutList and CSInList. - static void getCSRelevantLinks(const App::DocumentObject* obj, std::vector& vec); + static std::vector getCSRelevantLinks(const App::DocumentObject* obj); /// Checks if the links of the given object comply with all GeoFeatureGroup requrirements, that means /// if normal links are only withing the parent GeoFeatureGroup. static bool areLinksValid(const App::DocumentObject* obj); @@ -126,6 +118,18 @@ private: static std::vector getScopedObjectsFromLinks(const App::DocumentObject*, LinkScope scope = LinkScope::Local); static std::vector getScopedObjectsFromLink(App::Property*, LinkScope scope = LinkScope::Local); + /// Collects GeoFeatureGroup relevant objects that are linked from the given one. That means all linked objects + /// except GeoFeatureGroups. Expressions links are ignored. Only local scope links are considered. There is no + /// recursion. An exception is thrown when there are dependency loops. + static void getCSOutList(const App::DocumentObject* obj, std::vector& vec); + /// Collects GeoFeatureGroup relevant objects that link to the given one. That means all objects + /// except GeoFeatureGroups. Expression links are ignored. Only local scope links are relevant, and + /// there is no recursion. An exception is thrown when there are dependency loops. + static void getCSInList(const App::DocumentObject* obj, std::vector& vec); + + static void recursiveCSRelevantLinks(const App::DocumentObject* obj, + std::vector& vec); + }; typedef ExtensionPythonT> GeoFeatureGroupExtensionPython; diff --git a/src/Mod/Test/Document.py b/src/Mod/Test/Document.py index 353a55fe46..b3a8a6b263 100644 --- a/src/Mod/Test/Document.py +++ b/src/Mod/Test/Document.py @@ -793,7 +793,7 @@ class UndoRedoCases(unittest.TestCase): self.failUnless(len(self.Cylinder.InList) == 1) self.failUnless(self.Cylinder.InList[0] == self.Doc.Fuse) - def testUndoIssue0003150(self): + def testUndoIssue0003150Part1(self): self.Doc.UndoMode = 1 @@ -1015,6 +1015,26 @@ class DocumentGroupCases(unittest.TestCase): self.fail("Exception is expected") self.Doc.recompute() + + def testIssue0003150Part2(self): + self.box = self.Doc.addObject("Part::Box") + self.cyl = self.Doc.addObject("Part::Cylinder") + self.sph = self.Doc.addObject("Part::Sphere") + + self.fus1 = self.Doc.addObject("Part::MultiFuse") + self.fus2 = self.Doc.addObject("Part::MultiFuse") + + self.fus1.Shapes = [self.box, self.cyl]; + self.fus2.Shapes = [self.sph, self.cyl]; + + self.prt = self.Doc.addObject("App::Part") + self.prt.addObject(self.fus1) + self.failUnless(len(self.prt.Group)==5) + self.failUnless(self.fus2.getParentGeoFeatureGroup() == self.prt) + self.failUnless(self.prt.hasObject(self.sph)) + + self.prt.removeObject(self.fus1) + self.failUnless(len(self.prt.Group)==0) def tearDown(self): # closing doc