From a91784ea792745dba466c6d51079a3841f05f586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Thu, 19 Jan 2017 06:00:36 +0100 Subject: [PATCH 01/29] Fix drag&drop of geofeature groups. fixes #0002835 fixes #0002796 --- src/App/ExtensionContainer.cpp | 4 ++-- src/App/ExtensionContainer.h | 6 +++--- src/App/GeoFeatureGroupExtension.cpp | 8 ++++---- src/App/GeoFeatureGroupExtension.h | 2 +- src/Gui/ViewProviderGeoFeatureGroupExtension.cpp | 9 +++++++-- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/App/ExtensionContainer.cpp b/src/App/ExtensionContainer.cpp index b3688d51ba79..be52ac6922ff 100644 --- a/src/App/ExtensionContainer.cpp +++ b/src/App/ExtensionContainer.cpp @@ -94,7 +94,7 @@ bool ExtensionContainer::hasExtension(const std::string& name) const { } -Extension* ExtensionContainer::getExtension(Base::Type t) { +Extension* ExtensionContainer::getExtension(Base::Type t) const { auto result = _extensions.find(t); if(result == _extensions.end()) { @@ -115,7 +115,7 @@ bool ExtensionContainer::hasExtensions() const { return !_extensions.empty(); } -Extension* ExtensionContainer::getExtension(const std::string& name) { +Extension* ExtensionContainer::getExtension(const std::string& name) const { //and for types derived from it, as they can be cast to the extension for(auto entry : _extensions) { diff --git a/src/App/ExtensionContainer.h b/src/App/ExtensionContainer.h index 1108a75dfb9a..511d70e6edf8 100644 --- a/src/App/ExtensionContainer.h +++ b/src/App/ExtensionContainer.h @@ -127,12 +127,12 @@ class AppExport ExtensionContainer : public App::PropertyContainer bool hasExtension(Base::Type) const; //returns first of type (or derived from) and throws otherwise bool hasExtension(const std::string& name) const; //this version does not check derived classes bool hasExtensions() const; - App::Extension* getExtension(Base::Type); //returns first of type (or derived from) and throws otherwise - App::Extension* getExtension(const std::string& name); //this version does not check derived classes + App::Extension* getExtension(Base::Type) const; + App::Extension* getExtension(const std::string& name) const; //this version does not check derived classes //returns first of type (or derived from) and throws otherwise template - ExtensionT* getExtensionByType() { + ExtensionT* getExtensionByType() const { return dynamic_cast(getExtension(ExtensionT::getExtensionClassTypeId())); }; diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 71dfe2d9cc97..d10391559f8d 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -92,7 +92,7 @@ std::vector GeoFeatureGroupExtension::getGeoSubObjects () std::set nextSearchSet; for ( auto obj: curSearchSet) { if ( isNonGeoGroup (obj) ) { - const App::DocumentObjectGroup *grp = static_cast (obj); + auto *grp = obj->getExtensionByType(); // Check if we havent already processed the element may happen in case of nontree structure // Note: if the condition is false this generally indicates malformed structure if ( processedGroups.find (grp) == processedGroups.end() ) { @@ -127,7 +127,7 @@ bool GeoFeatureGroupExtension::geoHasObject (const DocumentObject* obj) const { std::set nextSearchSet; for ( auto obj: curSearchSet) { if ( isNonGeoGroup (obj) ) { - const App::DocumentObjectGroup *grp = static_cast (obj); + auto *grp = obj->getExtensionByType(); if ( processedGroups.find (grp) == processedGroups.end() ) { processedGroups.insert ( grp ); const auto & objs = grp->Group.getValues(); @@ -148,11 +148,11 @@ DocumentObject* GeoFeatureGroupExtension::getGroupOfObject(const DocumentObject* GeoFeatureGroupExtension* grp = (*it)->getExtensionByType(); if ( indirect ) { if (grp->geoHasObject(obj)) { - return dynamic_cast(grp); + return grp->getExtendedObject(); } } else { if (grp->hasObject(obj)) { - return dynamic_cast(grp); + return grp->getExtendedObject(); } } } diff --git a/src/App/GeoFeatureGroupExtension.h b/src/App/GeoFeatureGroupExtension.h index 901b34bcfd26..c6fe4a9f9fc5 100644 --- a/src/App/GeoFeatureGroupExtension.h +++ b/src/App/GeoFeatureGroupExtension.h @@ -73,7 +73,7 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension /// Returns true if the given DocumentObject is DocumentObjectGroup but not GeoFeatureGroup static bool isNonGeoGroup(const DocumentObject* obj) { - return obj->hasExtension(GroupExtension::getExtensionClassTypeId()) & + return obj->hasExtension(GroupExtension::getExtensionClassTypeId()) && !obj->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()); } }; diff --git a/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp b/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp index d8593d4ad7ac..d9221c3da18b 100644 --- a/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp +++ b/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp @@ -99,7 +99,12 @@ std::vector< App::DocumentObject* > ViewProviderGeoFeatureGroupExtension::getLin if(!obj) return std::vector< App::DocumentObject* >(); - //we get all linked objects, and that recursively + //if the object is a geofeaturegroup than all dependencies belong to that CS, we are not allowed + //to grap them + if(obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) + return std::vector< App::DocumentObject* >(); + + //we get all linked objects std::vector< App::DocumentObject* > result; std::vector list; obj->getPropertyList(list); @@ -123,7 +128,7 @@ std::vector< App::DocumentObject* > ViewProviderGeoFeatureGroupExtension::getLin //collect all dependencies of those objects std::vector< App::DocumentObject* > links; - for(App::DocumentObject *obj : result) { + for(App::DocumentObject *obj : result) { auto vec = getLinkedObjects(obj); links.insert(links.end(), vec.begin(), vec.end()); } From d2420b6d07718c56e1251dcd8a6662b55003b1b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sun, 29 Jan 2017 16:09:08 +0100 Subject: [PATCH 02/29] Introduce global placement calculations --- src/App/GeoFeature.cpp | 11 +++++++++++ src/App/GeoFeature.h | 14 ++++++++++++++ src/App/GeoFeatureGroupExtension.cpp | 19 +++++++++++++++++++ src/App/GeoFeatureGroupExtension.h | 15 +++++++++++++++ 4 files changed, 59 insertions(+) diff --git a/src/App/GeoFeature.cpp b/src/App/GeoFeature.cpp index 8142ec1ec1b1..8ce3c9471880 100644 --- a/src/App/GeoFeature.cpp +++ b/src/App/GeoFeature.cpp @@ -27,6 +27,7 @@ #endif #include "GeoFeature.h" +#include "GeoFeatureGroupExtension.h" #include using namespace App; @@ -55,6 +56,16 @@ void GeoFeature::transformPlacement(const Base::Placement &transform) this->Placement.setValue(plm); } +Base::Placement GeoFeature::globalPlacement() +{ + auto* group = GeoFeatureGroupExtension::getGroupOfObject(this); + if(group) { + auto ext = group->getExtensionByType(); + return ext->globalGroupPlacement() * Placement.getValue(); + } + return Placement.getValue(); +} + const PropertyComplexGeoData* GeoFeature::getPropertyOfGeometry() const { return nullptr; diff --git a/src/App/GeoFeature.h b/src/App/GeoFeature.h index 292d63ca9d89..b030c606c0e6 100644 --- a/src/App/GeoFeature.h +++ b/src/App/GeoFeature.h @@ -66,6 +66,20 @@ class AppExport GeoFeature : public App::DocumentObject * @return the Python binding object */ virtual PyObject* getPyObject(void); + + /** + * @brief Calculates the placement in the global reference coordinate system + * + * In FreeCAD the GeoFeature placement describes the local placement of the object in its parent + * coordinate system. This is however not always the same as the global reference system. If the + * object is in a GeoFeatureGroup, hence in annother local coordinate system, the Placement + * property does only give the local transformation. This function can be used to calculate the + * placement of the object in the global reference coordinate system taking all stacked local + * system into account. + * + * @return Base::Placement The transformation from the global reference coordinate system + */ + Base::Placement globalPlacement(); }; } //namespace App diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index d10391559f8d..3b3b9a7e54ac 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -160,6 +160,25 @@ DocumentObject* GeoFeatureGroupExtension::getGroupOfObject(const DocumentObject* return 0; } +Base::Placement GeoFeatureGroupExtension::globalGroupPlacement() { + + return recursiveGroupPlacement(this); +} + + +Base::Placement GeoFeatureGroupExtension::recursiveGroupPlacement(GeoFeatureGroupExtension* group) { + + + auto inList = group->getExtendedObject()->getInList(); + for(auto* link : inList) { + if(link->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) + return recursiveGroupPlacement(link->getExtensionByType()) * group->placement().getValue(); + } + + return group->placement().getValue(); +} + + // Python feature --------------------------------------------------------- namespace App { diff --git a/src/App/GeoFeatureGroupExtension.h b/src/App/GeoFeatureGroupExtension.h index c6fe4a9f9fc5..ecb51cf66ab2 100644 --- a/src/App/GeoFeatureGroupExtension.h +++ b/src/App/GeoFeatureGroupExtension.h @@ -70,12 +70,27 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension * default is true */ static DocumentObject* getGroupOfObject(const DocumentObject* obj, bool indirect=true); + + /** + * @brief Calculates the global placement of this group + * + * The returned placement describes the transformation from the global reference coordinate + * system to the local coordinate system of this geo feature group. If this group has a no parent + * GeoFeatureGroup the returned placement is the one of this group. For multiple stacked + * GeoFeatureGroups the returned Placement is the combination of all parent placements including + * ths one of this group. + * @return Base::Placement The transformation from global reference system to the groups local system + */ + Base::Placement globalGroupPlacement(); /// Returns true if the given DocumentObject is DocumentObjectGroup but not GeoFeatureGroup static bool isNonGeoGroup(const DocumentObject* obj) { return obj->hasExtension(GroupExtension::getExtensionClassTypeId()) && !obj->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()); } + +private: + Base::Placement recursiveGroupPlacement(GeoFeatureGroupExtension* group); }; typedef ExtensionPythonT> GeoFeatureGroupExtensionPython; From b035add4cba2f8b4fe9c5678b2e2fa33ffa0c8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sun, 29 Jan 2017 16:28:54 +0100 Subject: [PATCH 03/29] Sketcher: Use correct global placements --- src/Mod/Sketcher/Gui/ViewProviderSketch.cpp | 29 ++++----------------- src/Mod/Sketcher/Gui/ViewProviderSketch.h | 5 ---- 2 files changed, 5 insertions(+), 29 deletions(-) diff --git a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp index e166919ec76d..368b3b866f38 100644 --- a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp +++ b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp @@ -531,7 +531,7 @@ void ViewProviderSketch::getCoordsOnSketchPlane(double &u, double &v,const SbVec Base::Vector3d R0(0,0,0),RN(0,0,1),RX(1,0,0),RY(0,1,0); // move to position of Sketch - Base::Placement Plz = getPlacement(); + Base::Placement Plz = getSketchObject()->globalPlacement(); R0 = Plz.getPosition() ; Base::Rotation tmp(Plz.getRotation()); tmp.multVec(RN,RN); @@ -1930,7 +1930,7 @@ void ViewProviderSketch::doBoxSelection(const SbVec2s &startPos, const SbVec2s & Sketcher::SketchObject *sketchObject = getSketchObject(); App::Document *doc = sketchObject->getDocument(); - Base::Placement Plm = getPlacement(); + Base::Placement Plm = getSketchObject()->globalPlacement(); int intGeoCount = sketchObject->getHighestCurveIndex() + 1; int extGeoCount = sketchObject->getExternalGeometryCount(); @@ -4803,7 +4803,7 @@ void ViewProviderSketch::rebuildConstraintsVisual(void) Base::Vector3d RN(0,0,1); // move to position of Sketch - Base::Placement Plz = getPlacement(); + Base::Placement Plz = getSketchObject()->globalPlacement(); Base::Rotation tmp(Plz.getRotation()); tmp.multVec(RN,RN); Plz.setRotation(tmp); @@ -5016,13 +5016,6 @@ void ViewProviderSketch::setupContextMenu(QMenu *menu, QObject *receiver, const bool ViewProviderSketch::setEdit(int ModNum) { Q_UNUSED(ModNum); - //find the Part and body object the feature belongs to for placement calculations - //TODO: this needs to be replaced with GRAPH methods to get the real stacked placement - parentBody = Part::BodyBase::findBodyOf(getSketchObject()); - if(parentBody) - parentPart = App::Part::getPartOfObject(parentBody); - else - parentPart = App::Part::getPartOfObject(getSketchObject()); // always change to sketcher WB, remember where we come from oldWb = Gui::Command::assureWorkbench("SketcherWorkbench"); @@ -5539,7 +5532,7 @@ void ViewProviderSketch::setEditViewer(Gui::View3DInventorViewer* viewer, int Mo } } - Base::Placement plm = getPlacement(); + Base::Placement plm = getSketchObject()->globalPlacement(); Base::Rotation tmp(plm.getRotation()); SbRotation rot((float)tmp[0],(float)tmp[1],(float)tmp[2],(float)tmp[3]); @@ -5825,21 +5818,9 @@ bool ViewProviderSketch::onDelete(const std::vector &subList) return PartGui::ViewProviderPart::onDelete(subList); } -Base::Placement ViewProviderSketch::getPlacement() { - //TODO: use GRAPH placement handling, as this function right now only works with one parent - //part object - Base::Placement Plz = getSketchObject()->Placement.getValue(); - if(parentBody) - Plz = parentBody->Placement.getValue()*Plz; - if(parentPart) - Plz = parentPart->Placement.getValue()*Plz; - - return Plz; } void ViewProviderSketch::showRestoreInformationLayer() { visibleInformationChanged = true ; - draw(false,false); -} - + draw(false,false); \ No newline at end of file diff --git a/src/Mod/Sketcher/Gui/ViewProviderSketch.h b/src/Mod/Sketcher/Gui/ViewProviderSketch.h index 4a0985a222bb..e9dd81d2293a 100644 --- a/src/Mod/Sketcher/Gui/ViewProviderSketch.h +++ b/src/Mod/Sketcher/Gui/ViewProviderSketch.h @@ -357,9 +357,6 @@ class SketcherGuiExport ViewProviderSketch : public PartGui::ViewProvider2DObjec void addSelectPoint(int SelectPoint); void removeSelectPoint(int SelectPoint); void clearSelectPoints(void); - - // handle stacked placements of App::Parts - Base::Placement getPlacement(); // modes while sketching SketchMode Mode; @@ -405,8 +402,6 @@ class SketcherGuiExport ViewProviderSketch : public PartGui::ViewProvider2DObjec std::string oldWb; Gui::Rubberband* rubberband; - App::Part* parentPart = nullptr; - Part::BodyBase* parentBody = nullptr; // information layer variables bool visibleInformationChanged; From 4beb26ebe345a8d53ac67810ff3a191cea9b9a7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sun, 29 Jan 2017 16:40:52 +0100 Subject: [PATCH 04/29] Consistently name origins --- src/App/OriginGroupExtension.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/App/OriginGroupExtension.cpp b/src/App/OriginGroupExtension.cpp index b2bf065f002f..212a390f1eb0 100644 --- a/src/App/OriginGroupExtension.cpp +++ b/src/App/OriginGroupExtension.cpp @@ -110,9 +110,7 @@ App::DocumentObjectExecReturn *OriginGroupExtension::extensionExecute() { void OriginGroupExtension::onExtendedSetupObject () { App::Document *doc = getExtendedObject()->getDocument (); - std::string objName = std::string ( getExtendedObject()->getNameInDocument()).append ( "Origin" ); - - App::DocumentObject *originObj = doc->addObject ( "App::Origin", objName.c_str () ); + App::DocumentObject *originObj = doc->addObject ( "App::Origin", "Origin" ); assert ( originObj && originObj->isDerivedFrom ( App::Origin::getClassTypeId () ) ); Origin.setValue (originObj); From d0eaa8d0b2461d81be4f9906c3ad6d60c9833d84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sun, 29 Jan 2017 22:20:10 +0100 Subject: [PATCH 05/29] DependencyGraph: Show GeoFeatureGroups as subgraph. fixes #0002142 --- src/App/Document.cpp | 80 +++++++++++++++++++++++++++++++--------- src/Gui/GraphvizView.cpp | 1 + 2 files changed, 64 insertions(+), 17 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index b69d6d3e07f8..b21916cfa1c1 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -106,6 +106,7 @@ recompute path. Also enables more complicated dependencies beyond trees. #include "Application.h" #include "Transactions.h" +#include "GeoFeatureGroupExtension.h" using Base::Console; using Base::streq; @@ -290,9 +291,9 @@ void Document::exportGraphviz(std::ostream& out) const void setGraphAttributes(const DocumentObject * obj) { assert(GraphList[obj] != 0); - get_property(*GraphList[obj], graph_name) = getClusterName(obj); - get_property(*GraphList[obj], graph_graph_attribute)["bgcolor"] = "#e0e0e0"; - get_property(*GraphList[obj], graph_graph_attribute)["style"] = "rounded,filled"; + get_property(*GraphList[obj], graph_name) = getClusterName(obj); + get_property(*GraphList[obj], graph_graph_attribute)["bgcolor"] = "#e0e0e0"; + get_property(*GraphList[obj], graph_graph_attribute)["style"] = "rounded,filled"; } /** @@ -310,19 +311,27 @@ void Document::exportGraphviz(std::ostream& out) const } /** - * @brief addSubgraphIfNeeded Add a subgraph to the main graph if it is needed, i.e there are defined at least one expression in hte + * @brief addExpressionSubgraphIfNeeded Add a subgraph to the main graph if it is needed, i.e there are defined at least one expression in hte * document object, or other objects are referencing properties in it. * @param obj DocumentObject to assess. */ - void addSubgraphIfNeeded(DocumentObject * obj) { - boost::unordered_map expressions = obj->ExpressionEngine.getExpressions(); + void addExpressionSubgraphIfNeeded(DocumentObject * obj) { + + //coordinate systems already have a subgraph + if(obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) + return; + + boost::unordered_map expressions = obj->ExpressionEngine.getExpressions(); if (expressions.size() > 0) { + + auto group = GeoFeatureGroupExtension::getGroupOfObject(obj); + auto graph = group ? GraphList[group] : &DepList; // If documentObject has an expression, create a subgraph for it if (!GraphList[obj]) { - GraphList[obj] = &DepList.create_subgraph(); + GraphList[obj] = &graph->create_subgraph(); setGraphAttributes(obj); } @@ -339,7 +348,7 @@ void Document::exportGraphviz(std::ostream& out) const // Doesn't exist already? if (!GraphList[o]) { - GraphList[o] = &DepList.create_subgraph(); + GraphList[o] = &graph->create_subgraph(); setGraphAttributes(o); } ++j; @@ -356,8 +365,22 @@ void Document::exportGraphviz(std::ostream& out) const */ void add(DocumentObject * docObj, const std::string & name, const std::string & label) { - Graph * sgraph = GraphList[docObj] ? GraphList[docObj] : &DepList; - + + //don't add objects twice + if(std::find(objects.begin(), objects.end(), docObj) != objects.end()) + return; + + //find the correct graph to add the vertex too. Check first expressions graphs, afterwards + //the parent CS graphs + Graph * sgraph = GraphList[docObj]; + if(!sgraph) { + auto group = GeoFeatureGroupExtension::getGroupOfObject(docObj); + if(group) + sgraph = GraphList[group]; + } + if(!sgraph) + sgraph = &DepList; + // Keep a list of all added document objects. objects.insert(docObj); @@ -370,14 +393,15 @@ void Document::exportGraphviz(std::ostream& out) const get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["label"] = name; else get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["label"] = name + "\n(" + label + ")"; - - // If node is in main graph, style it with rounded corners. If not, remove the border as the subgraph will contain it. - if (sgraph == &DepList) { + + // If node is in main graph, style it with rounded corners. If not, make it invisible. + if (!GraphList[docObj]) { get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["style"] = "filled"; get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["shape"] = "Mrecord"; } - else + else { get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["color"] = "none"; + } // Add expressions and its dependencies boost::unordered_map expressions = docObj->ExpressionEngine.getExpressions(); @@ -421,13 +445,34 @@ void Document::exportGraphviz(std::ostream& out) const } ++i; } - } + void recursiveCSSubgraphs(DocumentObject* cs, DocumentObject* parent) { + + auto graph = parent ? GraphList[parent] : &DepList; + auto& sub = graph->create_subgraph(); + GraphList[cs] = ⊂ + get_property(sub, graph_name) = getClusterName(cs); + std::string name(cs->getNameInDocument()); + std::string label(cs->Label.getValue()); + + for(auto obj : cs->getOutList()) { + if(obj->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId())) + recursiveCSSubgraphs(obj, cs); + } + } + void addSubgraphs() { + + //first build up the coordinate system subgraphs + for (auto objectIt : d->objectArray) { + if (objectIt->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()) && objectIt->getInList().empty()) + recursiveCSSubgraphs(objectIt, nullptr); + } + // Internal document objects for (std::map::const_iterator It = d->objectMap.begin(); It != d->objectMap.end();++It) - addSubgraphIfNeeded(It->second); + addExpressionSubgraphIfNeeded(It->second); // Add external document objects for (std::map::const_iterator It = d->objectMap.begin(); It != d->objectMap.end();++It) { @@ -437,7 +482,7 @@ void Document::exportGraphviz(std::ostream& out) const std::map::const_iterator item = GlobalVertexList.find(getId(*It2)); if (item == GlobalVertexList.end()) - addSubgraphIfNeeded(*It2); + addExpressionSubgraphIfNeeded(*It2); } } } @@ -509,6 +554,7 @@ void Document::exportGraphviz(std::ostream& out) const // Add edges between document objects for (std::map::const_iterator It = d->objectMap.begin(); It != d->objectMap.end();++It) { + std::map dups; std::vector OutList = It->second->getOutList(); const DocumentObject * docObj = It->second; diff --git a/src/Gui/GraphvizView.cpp b/src/Gui/GraphvizView.cpp index 1c95654a17f0..3ca85146b227 100644 --- a/src/Gui/GraphvizView.cpp +++ b/src/Gui/GraphvizView.cpp @@ -230,6 +230,7 @@ void GraphvizView::updateSvgItem(const App::Document &doc) graphCode = stream.str(); // Update worker thread, and start it + std::cout<setData(QByteArray(graphCode.c_str(), graphCode.size())); thread->startThread(); } From 9ac2595003d417268f941b05222aeab5d1c53df7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sun, 29 Jan 2017 22:38:34 +0100 Subject: [PATCH 06/29] DependencyGraph: Show origins correctly --- src/App/Document.cpp | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index b21916cfa1c1..5517bd89ed02 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -107,6 +107,8 @@ recompute path. Also enables more complicated dependencies beyond trees. #include "Application.h" #include "Transactions.h" #include "GeoFeatureGroupExtension.h" +#include "Origin.h" +#include "OriginGroupExtension.h" using Base::Console; using Base::streq; @@ -371,13 +373,17 @@ void Document::exportGraphviz(std::ostream& out) const return; //find the correct graph to add the vertex too. Check first expressions graphs, afterwards - //the parent CS graphs + //the parent CS and origin graphs Graph * sgraph = GraphList[docObj]; if(!sgraph) { auto group = GeoFeatureGroupExtension::getGroupOfObject(docObj); if(group) sgraph = GraphList[group]; } + if(!sgraph) { + if(docObj->isDerivedFrom(OriginFeature::getClassTypeId())) + sgraph = GraphList[static_cast(docObj)->getOrigin()]; + } if(!sgraph) sgraph = &DepList; @@ -453,13 +459,19 @@ void Document::exportGraphviz(std::ostream& out) const auto& sub = graph->create_subgraph(); GraphList[cs] = ⊂ get_property(sub, graph_name) = getClusterName(cs); - std::string name(cs->getNameInDocument()); - std::string label(cs->Label.getValue()); for(auto obj : cs->getOutList()) { if(obj->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId())) recursiveCSSubgraphs(obj, cs); } + + //setup the origin if available + if(cs->hasExtension(App::OriginGroupExtension::getExtensionClassTypeId())) { + auto origin = cs->getExtensionByType()->Origin.getValue(); + auto& osub = sub.create_subgraph(); + GraphList[origin] = &osub; + get_property(osub, graph_name) = getClusterName(origin); + } } void addSubgraphs() { @@ -469,7 +481,7 @@ void Document::exportGraphviz(std::ostream& out) const if (objectIt->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()) && objectIt->getInList().empty()) recursiveCSSubgraphs(objectIt, nullptr); } - + // Internal document objects for (std::map::const_iterator It = d->objectMap.begin(); It != d->objectMap.end();++It) addExpressionSubgraphIfNeeded(It->second); @@ -554,7 +566,15 @@ void Document::exportGraphviz(std::ostream& out) const // Add edges between document objects for (std::map::const_iterator It = d->objectMap.begin(); It != d->objectMap.end();++It) { - + + //coordinate systems are represented by subgraphs + if(It->second->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId())) + continue; + + //as well as origins + if(It->second->isDerivedFrom(Origin::getClassTypeId())) + continue; + std::map dups; std::vector OutList = It->second->getOutList(); const DocumentObject * docObj = It->second; From d0474058ea3d2e4f79b5c8fe02f42d63581c0ccd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Mon, 30 Jan 2017 06:55:11 +0100 Subject: [PATCH 07/29] DependencyGraph: Add unflatten step --- src/Gui/GraphvizView.cpp | 64 +++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/src/Gui/GraphvizView.cpp b/src/Gui/GraphvizView.cpp index 3ca85146b227..91336b4dd09f 100644 --- a/src/Gui/GraphvizView.cpp +++ b/src/Gui/GraphvizView.cpp @@ -68,14 +68,16 @@ class GraphvizWorker : public QThread { : QThread(parent) { #if QT_VERSION < 0x050000 - proc.moveToThread(this); + dotProc.moveToThread(this); + unflattenProc.moveToThread(this); #endif } virtual ~GraphvizWorker() { #if QT_VERSION >= 0x050000 - proc.moveToThread(this); + dotProc.moveToThread(this); + unflattenProc.moveToThread(this); #endif } @@ -97,20 +99,31 @@ class GraphvizWorker : public QThread { } void run() { - // Write data to process - proc.write(str); - proc.closeWriteChannel(); - if (!proc.waitForFinished()) { + // Write data to unflatten process + unflattenProc.write(str); + unflattenProc.closeWriteChannel(); + QByteArray preprocessed = str; + //no error handling: unflatten is optional + unflattenProc.waitForFinished(); + preprocessed = unflattenProc.readAll(); + + dotProc.write(preprocessed); + dotProc.closeWriteChannel(); + if (!dotProc.waitForFinished()) { Q_EMIT error(); quit(); } // Emit result; it will get queued for processing in the main thread - Q_EMIT svgFileRead(proc.readAll()); + Q_EMIT svgFileRead(dotProc.readAll()); } - QProcess * process() { - return &proc; + QProcess * dotProcess() { + return &dotProc; + } + + QProcess * unflattenProcess() { + return &unflattenProc; } Q_SIGNALS: @@ -118,8 +131,8 @@ class GraphvizWorker : public QThread { void error(); private: - QProcess proc; - QByteArray str; + QProcess dotProc, unflattenProc; + QByteArray str, flatStr; }; } @@ -176,9 +189,12 @@ void GraphvizView::updateSvgItem(const App::Document &doc) return; ParameterGrp::handle hGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/Paths"); - QProcess * proc = thread->process(); - QStringList args; + QProcess * dotProc = thread->dotProcess(); + QProcess * flatProc = thread->unflattenProcess(); + QStringList args, flatArgs; args << QLatin1String("-Tsvg"); + flatArgs << QLatin1String("-c3 -l3"); + #ifdef FC_OS_LINUX QString path = QString::fromUtf8(hGrp->GetASCII("Graphviz", "/usr/bin").c_str()); #else @@ -186,14 +202,19 @@ void GraphvizView::updateSvgItem(const App::Document &doc) #endif bool pathChanged = false; #ifdef FC_OS_WIN32 - QString exe = QString::fromLatin1("\"%1/dot\"").arg(path); + QString dot = QString::fromLatin1("\"%1/dot\"").arg(path); + QString unflatten = QString::fromLatin1("\"%1/unflatten\"").arg(path); #else - QString exe = QString::fromLatin1("%1/dot").arg(path); + QString dot = QString::fromLatin1("%1/dot").arg(path); + QString unflatten = QString::fromLatin1("%1/unflatten").arg(path); #endif - proc->setEnvironment(QProcess::systemEnvironment()); + dotProc->setEnvironment(QProcess::systemEnvironment()); + flatProc->setEnvironment(QProcess::systemEnvironment()); do { - proc->start(exe, args); - if (!proc->waitForStarted()) { + flatProc->start(unflatten, flatArgs); + flatProc->waitForStarted(); + dotProc->start(dot, args); + if (!dotProc->waitForStarted()) { int ret = QMessageBox::warning(Gui::getMainWindow(), qApp->translate("Std_ExportGraphviz","Graphviz not found"), qApp->translate("Std_ExportGraphviz","Graphviz couldn't be found on your system.\n" @@ -211,9 +232,11 @@ void GraphvizView::updateSvgItem(const App::Document &doc) } pathChanged = true; #ifdef FC_OS_WIN32 - exe = QString::fromLatin1("\"%1/dot\"").arg(path); + dot = QString::fromLatin1("\"%1/dot\"").arg(path); + unflatten = QString::fromLatin1("\"%1/unflatten\"").arg(path); #else - exe = QString::fromLatin1("%1/dot").arg(path); + dot = QString::fromLatin1("%1/dot").arg(path); + unflatten = QString::fromLatin1("%1/unflatten").arg(path); #endif } else { @@ -230,7 +253,6 @@ void GraphvizView::updateSvgItem(const App::Document &doc) graphCode = stream.str(); // Update worker thread, and start it - std::cout<setData(QByteArray(graphCode.c_str(), graphCode.size())); thread->startThread(); } From c04d57103d6cc2ac896147efc633d1cf70eeb010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Mon, 30 Jan 2017 07:10:51 +0100 Subject: [PATCH 08/29] DependencyGraph: Subgraphs are identified by label --- src/App/Document.cpp | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 5517bd89ed02..98d138f1bd2d 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -285,6 +285,15 @@ void Document::exportGraphviz(std::ostream& out) const std::string getClusterName(const DocumentObject * docObj) const { return std::string("cluster") + docObj->getNameInDocument(); } + + void setGraphLabel(Graph& g, const DocumentObject* obj) const { + std::string name(obj->getNameInDocument()); + std::string label(obj->Label.getValue()); + if (name == label) + get_property(g, graph_graph_attribute)["label"] = name; + else + get_property(g, graph_graph_attribute)["label"] = name + "\n(" + label + ")"; + } /** * @brief setGraphAttributes Set graph attributes on a subgraph for a DocumentObject node. @@ -295,7 +304,8 @@ void Document::exportGraphviz(std::ostream& out) const assert(GraphList[obj] != 0); get_property(*GraphList[obj], graph_name) = getClusterName(obj); get_property(*GraphList[obj], graph_graph_attribute)["bgcolor"] = "#e0e0e0"; - get_property(*GraphList[obj], graph_graph_attribute)["style"] = "rounded,filled"; + get_property(*GraphList[obj], graph_graph_attribute)["style"] = "rounded,filled"; + setGraphLabel(*GraphList[obj], obj); } /** @@ -393,20 +403,22 @@ void Document::exportGraphviz(std::ostream& out) const // Add vertex to graph. Track global and local index LocalVertexList[getId(docObj)] = add_vertex(*sgraph); GlobalVertexList[getId(docObj)] = vertex_no++; - - // Set node label - if (name == label) - get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["label"] = name; - else - get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["label"] = name + "\n(" + label + ")"; - + // If node is in main graph, style it with rounded corners. If not, make it invisible. if (!GraphList[docObj]) { get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["style"] = "filled"; get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["shape"] = "Mrecord"; + // Set node label + if (name == label) + get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["label"] = name; + else + get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["label"] = name + "\n(" + label + ")"; } else { - get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["color"] = "none"; + get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["style"] = "invis"; + get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["fixedsize"] = "true"; + get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["width"] = "0"; + get(vertex_attribute, *sgraph)[LocalVertexList[getId(docObj)]]["height"] = "0"; } // Add expressions and its dependencies @@ -459,6 +471,7 @@ void Document::exportGraphviz(std::ostream& out) const auto& sub = graph->create_subgraph(); GraphList[cs] = ⊂ get_property(sub, graph_name) = getClusterName(cs); + setGraphLabel(sub, cs); for(auto obj : cs->getOutList()) { if(obj->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId())) @@ -471,6 +484,7 @@ void Document::exportGraphviz(std::ostream& out) const auto& osub = sub.create_subgraph(); GraphList[origin] = &osub; get_property(osub, graph_name) = getClusterName(origin); + setGraphLabel(osub, origin); } } From 43868ddb984f922c9d1f59feac052f042ed2bf37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Mon, 30 Jan 2017 22:52:42 +0100 Subject: [PATCH 09/29] DependencyGraph: Add colors to coordinate systems --- src/App/Document.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 98d138f1bd2d..355f129aa275 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -77,7 +77,6 @@ recompute path. Also enables more complicated dependencies beyond trees. #include #include - #include "Document.h" #include "Application.h" #include "DocumentObject.h" @@ -242,7 +241,7 @@ void Document::exportGraphviz(std::ostream& out) const class GraphCreator { public: - GraphCreator(struct DocumentP* _d) : d(_d), vertex_no(0) { + GraphCreator(struct DocumentP* _d) : d(_d), vertex_no(0), seed(std::random_device()()), distribution(0,255) { build(); } @@ -471,6 +470,11 @@ void Document::exportGraphviz(std::ostream& out) const auto& sub = graph->create_subgraph(); GraphList[cs] = ⊂ get_property(sub, graph_name) = getClusterName(cs); + + std::stringstream stream; + stream << "#" << std::hex << distribution(seed) << distribution(seed) << distribution(seed) << 80; + get_property(sub, graph_graph_attribute)["bgcolor"] = stream.str(); + get_property(sub, graph_graph_attribute)["style"] = "rounded,filled"; setGraphLabel(sub, cs); for(auto obj : cs->getOutList()) { @@ -484,6 +488,7 @@ void Document::exportGraphviz(std::ostream& out) const auto& osub = sub.create_subgraph(); GraphList[origin] = &osub; get_property(osub, graph_name) = getClusterName(origin); + get_property(osub, graph_graph_attribute)["bgcolor"] = "none"; setGraphLabel(osub, origin); } } @@ -730,6 +735,9 @@ void Document::exportGraphviz(std::ostream& out) const std::map GlobalVertexList; std::set objects; std::map GraphList; + //random color generation + std::mt19937 seed; + std::uniform_int_distribution distribution; }; GraphCreator g(d); From 5c8585fac7d968fb01cf0f6f073cded9418fd38f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Tue, 31 Jan 2017 06:23:07 +0100 Subject: [PATCH 10/29] DependencyGraph: Fix colors and unflatten --- src/App/Document.cpp | 9 +++++++-- src/Gui/GraphvizView.cpp | 33 +++++++++++++++++++++++---------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 355f129aa275..039fc3813789 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -471,9 +471,14 @@ void Document::exportGraphviz(std::ostream& out) const GraphList[cs] = ⊂ get_property(sub, graph_name) = getClusterName(cs); + //build random color string std::stringstream stream; - stream << "#" << std::hex << distribution(seed) << distribution(seed) << distribution(seed) << 80; - get_property(sub, graph_graph_attribute)["bgcolor"] = stream.str(); + stream << "#" << std::setfill('0') << std::setw(2)<< std::hex << distribution(seed) + << std::setfill('0') << std::setw(2)<< std::hex << distribution(seed) + << std::setfill('0') << std::setw(2)<< std::hex << distribution(seed) << 80; + std::string result(stream.str()); + + get_property(sub, graph_graph_attribute)["bgcolor"] = result; get_property(sub, graph_graph_attribute)["style"] = "rounded,filled"; setGraphLabel(sub, cs); diff --git a/src/Gui/GraphvizView.cpp b/src/Gui/GraphvizView.cpp index 91336b4dd09f..670210ced999 100644 --- a/src/Gui/GraphvizView.cpp +++ b/src/Gui/GraphvizView.cpp @@ -193,7 +193,7 @@ void GraphvizView::updateSvgItem(const App::Document &doc) QProcess * flatProc = thread->unflattenProcess(); QStringList args, flatArgs; args << QLatin1String("-Tsvg"); - flatArgs << QLatin1String("-c3 -l3"); + flatArgs << QLatin1String("-c2 -l3"); #ifdef FC_OS_LINUX QString path = QString::fromUtf8(hGrp->GetASCII("Graphviz", "/usr/bin").c_str()); @@ -299,9 +299,10 @@ void GraphvizView::disconnectSignals() QByteArray GraphvizView::exportGraph(const QString& format) { ParameterGrp::handle hGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/Paths"); - QProcess proc; - QStringList args; + QProcess dotProc, flatProc; + QStringList args, flatArgs; args << QString::fromLatin1("-T%1").arg(format); + flatArgs << QLatin1String("-c2 -l3"); #ifdef FC_OS_LINUX QString path = QString::fromUtf8(hGrp->GetASCII("Graphviz", "/usr/bin").c_str()); @@ -311,21 +312,33 @@ QByteArray GraphvizView::exportGraph(const QString& format) #ifdef FC_OS_WIN32 QString exe = QString::fromLatin1("\"%1/dot\"").arg(path); + QString unflatten = QString::fromLatin1("\"%1/unflatten\"").arg(path); #else QString exe = QString::fromLatin1("%1/dot").arg(path); + QString unflatten = QString::fromLatin1("%1/unflatten").arg(path); #endif - proc.setEnvironment(QProcess::systemEnvironment()); - proc.start(exe, args); - if (!proc.waitForStarted()) { + flatProc.setEnvironment(QProcess::systemEnvironment()); + flatProc.start(unflatten, flatArgs); + if (!flatProc.waitForStarted()) { + return QByteArray(); + } + flatProc.write(graphCode.c_str(), graphCode.size()); + flatProc.closeWriteChannel(); + if (!flatProc.waitForFinished()) + return QByteArray(); + + dotProc.setEnvironment(QProcess::systemEnvironment()); + dotProc.start(exe, args); + if (!dotProc.waitForStarted()) { return QByteArray(); } - proc.write(graphCode.c_str(), graphCode.size()); - proc.closeWriteChannel(); - if (!proc.waitForFinished()) + dotProc.write(flatProc.readAll()); + dotProc.closeWriteChannel(); + if (!dotProc.waitForFinished()) return QByteArray(); - return proc.readAll(); + return dotProc.readAll(); } bool GraphvizView::onMsg(const char* pMsg,const char**) From afd3495e523bd16b0aacfcd57055e2a21ccc98a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Tue, 31 Jan 2017 07:22:14 +0100 Subject: [PATCH 11/29] DependencyGraph: grouping & unflatten is optional Property group "User parameter:BaseApp/Preferences/DependencyGraph" has two booleans to enable subgraphing and unflatten (by default on): "Unflatten", "GeoFeatureSubgraphs" --- src/App/Document.cpp | 93 ++++++++++++++++++++++++++-------------- src/Gui/GraphvizView.cpp | 54 ++++++++++++++--------- 2 files changed, 94 insertions(+), 53 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 039fc3813789..af2622a671db 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -325,20 +325,22 @@ void Document::exportGraphviz(std::ostream& out) const * @brief addExpressionSubgraphIfNeeded Add a subgraph to the main graph if it is needed, i.e there are defined at least one expression in hte * document object, or other objects are referencing properties in it. * @param obj DocumentObject to assess. + * @param CSSubgraphs Boolean if the GeoFeatureGroups are created as subgraphs */ - void addExpressionSubgraphIfNeeded(DocumentObject * obj) { - - //coordinate systems already have a subgraph - if(obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) - return; + void addExpressionSubgraphIfNeeded(DocumentObject * obj, bool CSsubgraphs) { boost::unordered_map expressions = obj->ExpressionEngine.getExpressions(); if (expressions.size() > 0) { - auto group = GeoFeatureGroupExtension::getGroupOfObject(obj); - auto graph = group ? GraphList[group] : &DepList; + Graph* graph; + if(CSsubgraphs) { + auto group = GeoFeatureGroupExtension::getGroupOfObject(obj); + graph = group ? GraphList[group] : &DepList; + } + else + graph = &DepList; // If documentObject has an expression, create a subgraph for it if (!GraphList[obj]) { @@ -359,7 +361,15 @@ void Document::exportGraphviz(std::ostream& out) const // Doesn't exist already? if (!GraphList[o]) { - GraphList[o] = &graph->create_subgraph(); + + if(CSsubgraphs) { + auto group = GeoFeatureGroupExtension::getGroupOfObject(o); + auto graph2 = group ? GraphList[group] : &DepList; + GraphList[o] = &graph2->create_subgraph(); + } + else + GraphList[o] = &graph->create_subgraph(); + setGraphAttributes(o); } ++j; @@ -375,7 +385,7 @@ void Document::exportGraphviz(std::ostream& out) const * @param name Name of node. */ - void add(DocumentObject * docObj, const std::string & name, const std::string & label) { + void add(DocumentObject * docObj, const std::string & name, const std::string & label, bool CSSubgraphs) { //don't add objects twice if(std::find(objects.begin(), objects.end(), docObj) != objects.end()) @@ -384,15 +394,17 @@ void Document::exportGraphviz(std::ostream& out) const //find the correct graph to add the vertex too. Check first expressions graphs, afterwards //the parent CS and origin graphs Graph * sgraph = GraphList[docObj]; - if(!sgraph) { - auto group = GeoFeatureGroupExtension::getGroupOfObject(docObj); - if(group) - sgraph = GraphList[group]; + if(CSSubgraphs) { + if(!sgraph) { + auto group = GeoFeatureGroupExtension::getGroupOfObject(docObj); + if(group) + sgraph = GraphList[group]; + } + if(!sgraph) { + if(docObj->isDerivedFrom(OriginFeature::getClassTypeId())) + sgraph = GraphList[static_cast(docObj)->getOrigin()]; + } } - if(!sgraph) { - if(docObj->isDerivedFrom(OriginFeature::getClassTypeId())) - sgraph = GraphList[static_cast(docObj)->getOrigin()]; - } if(!sgraph) sgraph = &DepList; @@ -500,15 +512,20 @@ void Document::exportGraphviz(std::ostream& out) const void addSubgraphs() { - //first build up the coordinate system subgraphs - for (auto objectIt : d->objectArray) { - if (objectIt->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()) && objectIt->getInList().empty()) - recursiveCSSubgraphs(objectIt, nullptr); + ParameterGrp::handle depGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/DependencyGraph"); + bool CSSubgraphs = depGrp->GetBool("GeoFeatureSubgraphs", true); + + if(CSSubgraphs) { + //first build up the coordinate system subgraphs + for (auto objectIt : d->objectArray) { + if (objectIt->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()) && objectIt->getInList().empty()) + recursiveCSSubgraphs(objectIt, nullptr); + } } // Internal document objects for (std::map::const_iterator It = d->objectMap.begin(); It != d->objectMap.end();++It) - addExpressionSubgraphIfNeeded(It->second); + addExpressionSubgraphIfNeeded(It->second, CSSubgraphs); // Add external document objects for (std::map::const_iterator It = d->objectMap.begin(); It != d->objectMap.end();++It) { @@ -518,7 +535,7 @@ void Document::exportGraphviz(std::ostream& out) const std::map::const_iterator item = GlobalVertexList.find(getId(*It2)); if (item == GlobalVertexList.end()) - addExpressionSubgraphIfNeeded(*It2); + addExpressionSubgraphIfNeeded(*It2, CSSubgraphs); } } } @@ -526,9 +543,13 @@ void Document::exportGraphviz(std::ostream& out) const // Filling up the adjacency List void buildAdjacencyList() { + + ParameterGrp::handle depGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/DependencyGraph"); + bool CSSubgraphs = depGrp->GetBool("GeoFeatureSubgraphs", true); + // Add internal document objects for (std::map::const_iterator It = d->objectMap.begin(); It != d->objectMap.end();++It) - add(It->second, It->second->getNameInDocument(), It->second->Label.getValue()); + add(It->second, It->second->getNameInDocument(), It->second->Label.getValue(), CSSubgraphs); // Add external document objects for (std::map::const_iterator It = d->objectMap.begin(); It != d->objectMap.end();++It) { @@ -540,7 +561,8 @@ void Document::exportGraphviz(std::ostream& out) const if (item == GlobalVertexList.end()) add(*It2, std::string((*It2)->getDocument()->getName()) + "#" + (*It2)->getNameInDocument(), - std::string((*It2)->getDocument()->getName()) + "#" + (*It2)->Label.getValue()); + std::string((*It2)->getDocument()->getName()) + "#" + (*It2)->Label.getValue(), + CSSubgraphs); } } } @@ -588,16 +610,21 @@ void Document::exportGraphviz(std::ostream& out) const ++j; } + ParameterGrp::handle depGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/DependencyGraph"); + bool omitGeoFeatureGroups = depGrp->GetBool("GeoFeatureSubgraphs", true); + // Add edges between document objects for (std::map::const_iterator It = d->objectMap.begin(); It != d->objectMap.end();++It) { - - //coordinate systems are represented by subgraphs - if(It->second->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId())) - continue; - - //as well as origins - if(It->second->isDerivedFrom(Origin::getClassTypeId())) - continue; + + if(omitGeoFeatureGroups) { + //coordinate systems are represented by subgraphs + if(It->second->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId())) + continue; + + //as well as origins + if(It->second->isDerivedFrom(Origin::getClassTypeId())) + continue; + } std::map dups; std::vector OutList = It->second->getOutList(); diff --git a/src/Gui/GraphvizView.cpp b/src/Gui/GraphvizView.cpp index 670210ced999..051575dcc1f1 100644 --- a/src/Gui/GraphvizView.cpp +++ b/src/Gui/GraphvizView.cpp @@ -99,13 +99,20 @@ class GraphvizWorker : public QThread { } void run() { - // Write data to unflatten process - unflattenProc.write(str); - unflattenProc.closeWriteChannel(); QByteArray preprocessed = str; - //no error handling: unflatten is optional - unflattenProc.waitForFinished(); - preprocessed = unflattenProc.readAll(); + + ParameterGrp::handle hGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/DependencyGraph"); + if(hGrp->GetBool("Unflatten", true)) { + // Write data to unflatten process + unflattenProc.write(str); + unflattenProc.closeWriteChannel(); + //no error handling: unflatten is optional + unflattenProc.waitForFinished(); + preprocessed = unflattenProc.readAll(); + } else { + unflattenProc.closeWriteChannel(); + unflattenProc.waitForFinished(); + } dotProc.write(preprocessed); dotProc.closeWriteChannel(); @@ -193,7 +200,7 @@ void GraphvizView::updateSvgItem(const App::Document &doc) QProcess * flatProc = thread->unflattenProcess(); QStringList args, flatArgs; args << QLatin1String("-Tsvg"); - flatArgs << QLatin1String("-c2 -l3"); + flatArgs << QLatin1String("-c2 -l2"); #ifdef FC_OS_LINUX QString path = QString::fromUtf8(hGrp->GetASCII("Graphviz", "/usr/bin").c_str()); @@ -302,7 +309,7 @@ QByteArray GraphvizView::exportGraph(const QString& format) QProcess dotProc, flatProc; QStringList args, flatArgs; args << QString::fromLatin1("-T%1").arg(format); - flatArgs << QLatin1String("-c2 -l3"); + flatArgs << QLatin1String("-c2 -l2"); #ifdef FC_OS_LINUX QString path = QString::fromUtf8(hGrp->GetASCII("Graphviz", "/usr/bin").c_str()); @@ -317,23 +324,30 @@ QByteArray GraphvizView::exportGraph(const QString& format) QString exe = QString::fromLatin1("%1/dot").arg(path); QString unflatten = QString::fromLatin1("%1/unflatten").arg(path); #endif - flatProc.setEnvironment(QProcess::systemEnvironment()); - flatProc.start(unflatten, flatArgs); - if (!flatProc.waitForStarted()) { - return QByteArray(); - } - flatProc.write(graphCode.c_str(), graphCode.size()); - flatProc.closeWriteChannel(); - if (!flatProc.waitForFinished()) - return QByteArray(); - + dotProc.setEnvironment(QProcess::systemEnvironment()); dotProc.start(exe, args); if (!dotProc.waitForStarted()) { return QByteArray(); } - - dotProc.write(flatProc.readAll()); + + ParameterGrp::handle depGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/DependencyGraph"); + if(depGrp->GetBool("Unflatten", true)) { + flatProc.setEnvironment(QProcess::systemEnvironment()); + flatProc.start(unflatten, flatArgs); + if (!flatProc.waitForStarted()) { + return QByteArray(); + } + flatProc.write(graphCode.c_str(), graphCode.size()); + flatProc.closeWriteChannel(); + if (!flatProc.waitForFinished()) + return QByteArray(); + + dotProc.write(flatProc.readAll()); + } + else + dotProc.write(graphCode.c_str(), graphCode.size()); + dotProc.closeWriteChannel(); if (!dotProc.waitForFinished()) return QByteArray(); From c4e8836abbf732829e2fc7e7e037b7cd1b8a7eab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Wed, 8 Feb 2017 07:08:45 +0100 Subject: [PATCH 12/29] Unify and fix group handling in geofeaturegroups --- src/App/Document.cpp | 10 +- src/App/Document.h | 3 +- src/App/ExtensionContainer.cpp | 8 +- src/App/ExtensionContainer.h | 4 +- src/App/GeoFeatureGroupExtension.cpp | 106 +++++------------- src/App/GeoFeatureGroupExtension.h | 25 +++-- src/App/GroupExtension.cpp | 18 +-- src/App/GroupExtension.h | 4 +- src/App/OriginGroupExtension.cpp | 26 ++--- src/App/OriginGroupExtension.h | 2 +- src/App/Part.cpp | 25 ++--- src/App/Part.h | 2 +- .../ViewProviderGeoFeatureGroupExtension.cpp | 28 ++++- .../ViewProviderGeoFeatureGroupExtension.h | 1 + src/Gui/ViewProviderOriginGroupExtension.cpp | 2 +- src/Mod/Sketcher/App/SketchObject.cpp | 4 +- 16 files changed, 121 insertions(+), 147 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index af2622a671db..7275adc431f9 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -301,8 +301,10 @@ void Document::exportGraphviz(std::ostream& out) const void setGraphAttributes(const DocumentObject * obj) { assert(GraphList[obj] != 0); - get_property(*GraphList[obj], graph_name) = getClusterName(obj); - get_property(*GraphList[obj], graph_graph_attribute)["bgcolor"] = "#e0e0e0"; + get_property(*GraphList[obj], graph_name) = getClusterName(obj); + + get_property(*GraphList[obj], graph_graph_attribute)["bgcolor"] = "#e0e0e0"; + get_property(*GraphList[obj], graph_graph_attribute)["style"] = "rounded,filled"; setGraphLabel(*GraphList[obj], obj); } @@ -2886,11 +2888,11 @@ std::vector Document::getObjectsOfType(const Base::Type& typeId return Objects; } -std::vector< DocumentObject* > Document::getObjectsWithExtension(const Base::Type& typeId) const { +std::vector< DocumentObject* > Document::getObjectsWithExtension(const Base::Type& typeId, bool derived) const { std::vector Objects; for (std::vector::const_iterator it = d->objectArray.begin(); it != d->objectArray.end(); ++it) { - if ((*it)->hasExtension(typeId)) + if ((*it)->hasExtension(typeId, derived)) Objects.push_back(*it); } return Objects; diff --git a/src/App/Document.h b/src/App/Document.h index c507d71799ae..996d37abf2e9 100644 --- a/src/App/Document.h +++ b/src/App/Document.h @@ -230,7 +230,8 @@ class AppExport Document : public App::PropertyContainer /// Returns a list of all Objects std::vector getObjects() const; std::vector getObjectsOfType(const Base::Type& typeId) const; - std::vector getObjectsWithExtension(const Base::Type& typeId) const; + /// Returns all object with given extensions. If derived=true also all objects with extenions derived from the given one + std::vector getObjectsWithExtension(const Base::Type& typeId, bool derived = true) const; std::vector findObjects(const Base::Type& typeId, const char* objname) const; /// Returns an array with the correct types already. template inline std::vector getObjectsOfType() const; diff --git a/src/App/ExtensionContainer.cpp b/src/App/ExtensionContainer.cpp index be52ac6922ff..64ff37606c9c 100644 --- a/src/App/ExtensionContainer.cpp +++ b/src/App/ExtensionContainer.cpp @@ -68,11 +68,11 @@ void ExtensionContainer::registerExtension(Base::Type extension, Extension* ext) _extensions[extension] = ext; } -bool ExtensionContainer::hasExtension(Base::Type t) const { +bool ExtensionContainer::hasExtension(Base::Type t, bool derived) const { //check for the exact type bool found = _extensions.find(t) != _extensions.end(); - if(!found) { + if(!found && derived) { //and for types derived from it, as they can be cast to the extension for(auto entry : _extensions) { if(entry.first.isDerivedFrom(t)) @@ -94,10 +94,10 @@ bool ExtensionContainer::hasExtension(const std::string& name) const { } -Extension* ExtensionContainer::getExtension(Base::Type t) const { +Extension* ExtensionContainer::getExtension(Base::Type t, bool derived) const { auto result = _extensions.find(t); - if(result == _extensions.end()) { + if((result == _extensions.end()) && derived) { //we need to check for derived types for(auto entry : _extensions) { if(entry.first.isDerivedFrom(t)) diff --git a/src/App/ExtensionContainer.h b/src/App/ExtensionContainer.h index 511d70e6edf8..43e169f87b27 100644 --- a/src/App/ExtensionContainer.h +++ b/src/App/ExtensionContainer.h @@ -124,10 +124,10 @@ class AppExport ExtensionContainer : public App::PropertyContainer virtual ~ExtensionContainer(); void registerExtension(Base::Type extension, App::Extension* ext); - bool hasExtension(Base::Type) const; //returns first of type (or derived from) and throws otherwise + bool hasExtension(Base::Type, bool derived=true) const; //returns first of type (or derived from if set to true) and throws otherwise bool hasExtension(const std::string& name) const; //this version does not check derived classes bool hasExtensions() const; - App::Extension* getExtension(Base::Type) const; + App::Extension* getExtension(Base::Type, bool derived = true) const; App::Extension* getExtension(const std::string& name) const; //this version does not check derived classes //returns first of type (or derived from) and throws otherwise diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 3b3b9a7e54ac..85843bdcfb51 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -77,87 +77,18 @@ void GeoFeatureGroupExtension::transformPlacement(const Base::Placement &transfo this->placement().setValue(plm); } -std::vector GeoFeatureGroupExtension::getGeoSubObjects () const { - const auto & objs = Group.getValues(); - - std::set processedGroups; - std::set rvSet; - std::set curSearchSet (objs.begin(), objs.end()); - - processedGroups.insert ( this ); - - while ( !curSearchSet.empty() ) { - rvSet.insert ( curSearchSet.begin (), curSearchSet.end () ); - - std::set nextSearchSet; - for ( auto obj: curSearchSet) { - if ( isNonGeoGroup (obj) ) { - auto *grp = obj->getExtensionByType(); - // Check if we havent already processed the element may happen in case of nontree structure - // Note: if the condition is false this generally indicates malformed structure - if ( processedGroups.find (grp) == processedGroups.end() ) { - processedGroups.insert ( grp ); - const auto & objs = grp->Group.getValues(); - nextSearchSet.insert (objs.begin(), objs.end()); - } - } - } - nextSearchSet.swap (curSearchSet); - } - - return std::vector ( rvSet.begin(), rvSet.end() ); -} - -bool GeoFeatureGroupExtension::geoHasObject (const DocumentObject* obj) const { - const auto & objs = Group.getValues(); - - if (!obj) { - return false; - } - - std::set processedGroups; - std::set curSearchSet (objs.begin(), objs.end()); - - processedGroups.insert ( this ); - - while ( !curSearchSet.empty() ) { - if ( curSearchSet.find (obj) != curSearchSet.end() ) { - return true; - } - std::set nextSearchSet; - for ( auto obj: curSearchSet) { - if ( isNonGeoGroup (obj) ) { - auto *grp = obj->getExtensionByType(); - if ( processedGroups.find (grp) == processedGroups.end() ) { - processedGroups.insert ( grp ); - const auto & objs = grp->Group.getValues(); - nextSearchSet.insert (objs.begin(), objs.end()); - } - } - } - nextSearchSet.swap (curSearchSet); - } - return false; -} - -DocumentObject* GeoFeatureGroupExtension::getGroupOfObject(const DocumentObject* obj, bool indirect) +DocumentObject* GeoFeatureGroupExtension::getGroupOfObject(const DocumentObject* obj) { - const Document* doc = obj->getDocument(); - std::vector grps = doc->getObjectsWithExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()); - for (std::vector::const_iterator it = grps.begin(); it != grps.end(); ++it) { - GeoFeatureGroupExtension* grp = (*it)->getExtensionByType(); - if ( indirect ) { - if (grp->geoHasObject(obj)) { - return grp->getExtendedObject(); - } - } else { - if (grp->hasObject(obj)) { - return grp->getExtendedObject(); - } - } + //compared to GroupExtension we do return here all geofeaturegroups including all extensions erived from it + //like origingroup. That is needed as we use this function to get all local coordinate systems. Also there + //is no reason to distuinguish between geofeatuergroups, there is only between group/geofeaturegroup + auto list = obj->getInList(); + for (auto obj : list) { + if(obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) + return obj; } - return 0; + return nullptr; } Base::Placement GeoFeatureGroupExtension::globalGroupPlacement() { @@ -178,6 +109,25 @@ Base::Placement GeoFeatureGroupExtension::recursiveGroupPlacement(GeoFeatureGrou return group->placement().getValue(); } +void GeoFeatureGroupExtension::addObject(App::DocumentObject* obj) { + + if(!allowObject(obj)) + return; + + //only one geofeaturegroup per object. This is the reason why we need to override addObject, + //we need to check here for GeoFeatureGroups only. It is allowed to be at the same time in a + //GeoFeatureGroup and a Group + auto *group = App::GeoFeatureGroupExtension::getGroupOfObject(obj); + if(group && group != getExtendedObject()) + group->getExtensionByType()->removeObject(obj); + + if (!hasObject(obj)) { + std::vector grp = Group.getValues(); + grp.push_back(obj); + Group.setValues(grp); + } +} + // Python feature --------------------------------------------------------- diff --git a/src/App/GeoFeatureGroupExtension.h b/src/App/GeoFeatureGroupExtension.h index ecb51cf66ab2..77d1b729ee9f 100644 --- a/src/App/GeoFeatureGroupExtension.h +++ b/src/App/GeoFeatureGroupExtension.h @@ -34,7 +34,19 @@ namespace App { /** - * The base class for placeable group of DocumentObjects + * @brief The base class for placeable group of DocumentObjects. It represents a local coordnate system + * + * This class is the FreeCAD way of representing local coordinate systems. It groups its childs beneath + * it and transforms them all with the GeoFeatureGroup placement. A few important properties: + * - Every child that belongs to the CS must be in the Group proeprty. Even if a sketch is part of a pad, + * it must be in the Group property of the same GeoFeatureGroup as pad. This also holds for normal + * GroupExtensions. They can be added to a GeoFeatureGroup, but all objects that the group holds must + * also be added to the GeoFeatureGroup + * - Objects can be only in a single GeoFeatureGroup. It is not allowed to have a document object in + * multiple GeoFeatureGroups + * - PropertyLinks between different GeoFeatureGroups are forbidden. There are special link proeprties + * that allow such cross-CS links. + * - Expressions can cross GeoFeatureGroup borders */ class AppExport GeoFeatureGroupExtension : public App::GroupExtension { @@ -52,16 +64,11 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension * @param transform (input). */ virtual void transformPlacement(const Base::Placement &transform); + /// Constructor GeoFeatureGroupExtension(void); virtual ~GeoFeatureGroupExtension(); - /// Returns all geometrically controlled objects: all objects of this group and it's non-geo subgroups - std::vector getGeoSubObjects () const; - - /// Returns true if either the group or one of it's non-geo subgroups has the object - bool geoHasObject (const DocumentObject* obj) const; - /** Returns the geo feature group which contains this object. * In case this object is not part of any geoFeatureGroup 0 is returned. * Unlike DocumentObjectGroup::getGroupOfObject serches only for GeoFeatureGroups @@ -69,7 +76,7 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension * @param indirect if true return if the group that so-called geoHas the object, @see geoHasObject() * default is true */ - static DocumentObject* getGroupOfObject(const DocumentObject* obj, bool indirect=true); + static DocumentObject* getGroupOfObject(const DocumentObject* obj); /** * @brief Calculates the global placement of this group @@ -89,6 +96,8 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension !obj->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()); } + virtual void addObject(DocumentObject* obj); + private: Base::Placement recursiveGroupPlacement(GeoFeatureGroupExtension* group); }; diff --git a/src/App/GroupExtension.cpp b/src/App/GroupExtension.cpp index 3e2b9541d900..0af4e9b638ec 100644 --- a/src/App/GroupExtension.cpp +++ b/src/App/GroupExtension.cpp @@ -63,7 +63,8 @@ void GroupExtension::addObject(DocumentObject* obj) if(!allowObject(obj)) return; - //only one group per object + //only one group per object. Note that it is allowed to be in a group and geofeaturegroup. However, + //getGroupOfObject() returns only normal groups, no GeoFeatureGroups. Hence this works. auto *group = App::GroupExtension::getGroupOfObject(obj); if(group && group != getExtendedObject()) group->getExtensionByType()->removeObject(obj); @@ -207,15 +208,16 @@ int GroupExtension::countObjectsOfType(const Base::Type& typeId) const DocumentObject* GroupExtension::getGroupOfObject(const DocumentObject* obj) { - const Document* doc = obj->getDocument(); - std::vector grps = doc->getObjectsWithExtension(GroupExtension::getExtensionClassTypeId()); - for (std::vector::const_iterator it = grps.begin(); it != grps.end(); ++it) { - GroupExtension* grp = (*it)->getExtensionByType(); - if (grp->hasObject(obj)) - return *it; + //note that we return here only Groups, but nothing derived from it, e.g. no GeoFeatureGroups. + //That is important as there are clear differences between groups/geofeature groups (e.g. a object + //can be in only one group, and only one geofeaturegroup, however, it can be in both at the same time) + auto list = obj->getInList(); + for (auto obj : list) { + if(obj->hasExtension(App::GroupExtension::getExtensionClassTypeId(), false)) + return obj; } - return 0; + return nullptr; } PyObject* GroupExtension::getExtensionPyObject(void) { diff --git a/src/App/GroupExtension.h b/src/App/GroupExtension.h index 8fd70d42731f..97b2aaf6ebbf 100644 --- a/src/App/GroupExtension.h +++ b/src/App/GroupExtension.h @@ -91,7 +91,9 @@ class AppExport GroupExtension : public DocumentObjectExtension */ int countObjectsOfType(const Base::Type& typeId) const; /** Returns the object group of the document which the given object \a obj is part of. - * In case this object is not part of a group 0 is returned. + * In case this object is not part of a group 0 is returned. + * @note This only returns objects that are normal groups, not any special derived type + * like geofeaturegroups or origingroups. To retrieve those please youse their appropriate functions */ static DocumentObject* getGroupOfObject(const DocumentObject* obj); //@} diff --git a/src/App/OriginGroupExtension.cpp b/src/App/OriginGroupExtension.cpp index 212a390f1eb0..f6f869a1569d 100644 --- a/src/App/OriginGroupExtension.cpp +++ b/src/App/OriginGroupExtension.cpp @@ -65,27 +65,15 @@ App::Origin *OriginGroupExtension::getOrigin () const { } } -App::DocumentObject *OriginGroupExtension::getGroupOfObject (const DocumentObject* obj, bool indirect) { - const Document* doc = obj->getDocument(); - std::vector grps = doc->getObjectsWithExtension ( OriginGroupExtension::getExtensionClassTypeId() ); - for (auto grpObj: grps) { - OriginGroupExtension* grp = dynamic_cast (grpObj->getExtension( - OriginGroupExtension::getExtensionClassTypeId())); - - if(!grp) throw Base::TypeError("Wrong type in origin group extenion"); - - if ( indirect ) { - if ( grp->geoHasObject (obj) ) { - return grp->getExtendedObject(); - } - } else { - if ( grp->hasObject (obj) ) { - return grp->getExtendedObject(); - } - } +App::DocumentObject *OriginGroupExtension::getGroupOfObject (const DocumentObject* obj) { + + auto list = obj->getInList(); + for (auto obj : list) { + if(obj->hasExtension(App::OriginGroupExtension::getExtensionClassTypeId())) + return obj; } - return 0; + return nullptr; } short OriginGroupExtension::extensionMustExecute() { diff --git a/src/App/OriginGroupExtension.h b/src/App/OriginGroupExtension.h index 6b393c9c09fb..2e98bae3a034 100644 --- a/src/App/OriginGroupExtension.h +++ b/src/App/OriginGroupExtension.h @@ -55,7 +55,7 @@ class AppExport OriginGroupExtension : public App::GeoFeatureGroupExtension * @param indirect if true return if the group that so-called geoHas the object, @see geoHasObject() * default is true */ - static DocumentObject* getGroupOfObject (const DocumentObject* obj, bool indirect=true); + static DocumentObject* getGroupOfObject (const DocumentObject* obj); /// Returns true on changing OriginFeature set virtual short extensionMustExecute () override; diff --git a/src/App/Part.cpp b/src/App/Part.cpp index 360004cf01cb..4c2c06bc8535 100644 --- a/src/App/Part.cpp +++ b/src/App/Part.cpp @@ -67,24 +67,17 @@ Part::~Part(void) { } -App::Part *Part::getPartOfObject (const DocumentObject* obj, bool indirect) { - const Document* doc = obj->getDocument(); - std::vector grps = doc->getObjectsOfType ( Part::getClassTypeId() ); - - for (auto partObj: grps) { - Part* part = static_cast (partObj); - if ( indirect ) { - if ( part->geoHasObject (obj) ) { - return part; - } - } else { - if ( part->hasObject (obj) ) { - return part; - } - } +App::Part *Part::getPartOfObject (const DocumentObject* obj) { + + //as a Part is a geofeaturegroup it must directly link to all objects it contains, even + //if they are in additional groups etc. + auto list = obj->getInList(); + for (auto obj : list) { + if(obj->isDerivedFrom(App::Part::getClassTypeId())) + return static_cast(obj); } - return 0; + return nullptr; } diff --git a/src/App/Part.h b/src/App/Part.h index 416e665a77c8..a3726251a10e 100644 --- a/src/App/Part.h +++ b/src/App/Part.h @@ -91,7 +91,7 @@ class AppExport Part : public App::GeoFeature, public App::OriginGroupExtension * @param indirect if true return if the part that so-called geoHas the object, @see geoHasObject() * default is true */ - static App::Part* getPartOfObject (const DocumentObject* obj, bool indirect=true); + static App::Part* getPartOfObject (const DocumentObject* obj); virtual PyObject *getPyObject(void); }; diff --git a/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp b/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp index d9221c3da18b..2f9dce8e83d8 100644 --- a/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp +++ b/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp @@ -33,6 +33,7 @@ #include "Application.h" #include "Document.h" #include +#include #include using namespace Gui; @@ -55,8 +56,33 @@ ViewProviderGeoFeatureGroupExtension::~ViewProviderGeoFeatureGroupExtension() std::vector ViewProviderGeoFeatureGroupExtension::extensionClaimChildren3D(void) const { + + //all object in the group must be claimed in 3D, as we are a coordinate system for all of them auto* ext = getExtendedViewProvider()->getObject()->getExtensionByType(); - return ext ? ext->getGeoSubObjects() : std::vector(); + if(ext) { + auto objs = ext->Group.getValues(); + return objs; + } + return std::vector(); +} + +std::vector ViewProviderGeoFeatureGroupExtension::extensionClaimChildren(void) const { + + //we must be carefull which objects to claim, as there might be stacked relations inside the coordinate system, + //like pad/sketch + auto* ext = getExtendedViewProvider()->getObject()->getExtensionByType(); + if(ext) { + //filter out all objects with more than one inlink, as they are most likely hold by annother + //object in the tree + std::vector claim; + auto objs = ext->Group.getValues(); + for(auto obj : objs) { + if(obj->getInList().size()<=1) + claim.push_back(obj); + } + return claim; + } + return std::vector(); } void ViewProviderGeoFeatureGroupExtension::extensionAttach(App::DocumentObject* pcObject) diff --git a/src/Gui/ViewProviderGeoFeatureGroupExtension.h b/src/Gui/ViewProviderGeoFeatureGroupExtension.h index c1ec419748b7..d409ae27ee98 100644 --- a/src/Gui/ViewProviderGeoFeatureGroupExtension.h +++ b/src/Gui/ViewProviderGeoFeatureGroupExtension.h @@ -42,6 +42,7 @@ class GuiExport ViewProviderGeoFeatureGroupExtension : public ViewProviderGroupE virtual ~ViewProviderGeoFeatureGroupExtension(); virtual std::vector extensionClaimChildren3D(void)const override; + virtual std::vector< App::DocumentObject* > extensionClaimChildren(void) const override; virtual SoGroup* extensionGetChildRoot(void) const override {return pcGroupChildren;}; virtual void extensionAttach(App::DocumentObject* pcObject) override; virtual void extensionSetDisplayMode(const char* ModeName) override; diff --git a/src/Gui/ViewProviderOriginGroupExtension.cpp b/src/Gui/ViewProviderOriginGroupExtension.cpp index 5026463bc1d9..2ca1742a6a6d 100644 --- a/src/Gui/ViewProviderOriginGroupExtension.cpp +++ b/src/Gui/ViewProviderOriginGroupExtension.cpp @@ -173,7 +173,7 @@ void ViewProviderOriginGroupExtension::updateOriginSize () { // calculate the bounding box for out content SbBox3f bbox(0,0,0, 0,0,0); - for(App::DocumentObject* obj : group->getGeoSubObjects()) { + for(App::DocumentObject* obj : group->Group.getValues()) { ViewProvider *vp = Gui::Application::Instance->getViewProvider(obj); if (!vp) { continue; diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index ee1780ebc846..6d660de879ed 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -1990,8 +1990,8 @@ bool SketchObject::isExternalAllowed(App::Document *pDoc, App::DocumentObject *p //App::DocumentObject *support = this->Support.getValue(); Part::BodyBase* body_this = Part::BodyBase::findBodyOf(this); Part::BodyBase* body_obj = Part::BodyBase::findBodyOf(pObj); - App::Part* part_this = App::Part::getPartOfObject(this, true); - App::Part* part_obj = App::Part::getPartOfObject(pObj, true); + App::Part* part_this = App::Part::getPartOfObject(this); + App::Part* part_obj = App::Part::getPartOfObject(pObj); if (part_this == part_obj){ //either in the same part, or in the root of document if (body_this == NULL) { return true; From 264134f96ac2d56f0bfcdf4163a107d581310b5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Thu, 9 Feb 2017 17:22:41 +0100 Subject: [PATCH 13/29] GeoFeatureGroup: add object adds relevant links --- src/App/GeoFeatureGroupExtension.cpp | 147 ++++++++++++++++++++++++--- src/App/GeoFeatureGroupExtension.h | 13 +++ src/Mod/Test/Document.py | 34 ++++++- 3 files changed, 180 insertions(+), 14 deletions(-) diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 85843bdcfb51..c8867119cb7d 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -30,6 +30,8 @@ #include #include "GeoFeatureGroupExtension.h" +#include "OriginFeature.h" +#include "Origin.h" //#include "GeoFeatureGroupPy.h" //#include "FeaturePythonPyImp.h" @@ -109,25 +111,144 @@ Base::Placement GeoFeatureGroupExtension::recursiveGroupPlacement(GeoFeatureGrou return group->placement().getValue(); } -void GeoFeatureGroupExtension::addObject(App::DocumentObject* obj) { +void GeoFeatureGroupExtension::addObject(App::DocumentObject* object) { - if(!allowObject(obj)) + if(!allowObject(object)) return; - //only one geofeaturegroup per object. This is the reason why we need to override addObject, - //we need to check here for GeoFeatureGroups only. It is allowed to be at the same time in a - //GeoFeatureGroup and a Group - auto *group = App::GeoFeatureGroupExtension::getGroupOfObject(obj); - if(group && group != getExtendedObject()) - group->getExtensionByType()->removeObject(obj); - - if (!hasObject(obj)) { - std::vector grp = Group.getValues(); - grp.push_back(obj); - Group.setValues(grp); + //cross CoordinateSystem links are not allowed, so we need to move the whole link group + auto links = getCSRelevantLinks(object); + links.push_back(object); + + std::vector grp = Group.getValues(); + for( auto obj : links) { + //only one geofeaturegroup per object. + auto *group = App::GeoFeatureGroupExtension::getGroupOfObject(obj); + if(group && group != getExtendedObject()) + group->getExtensionByType()->removeObject(obj); + + if (!hasObject(obj)) + grp.push_back(obj); + } + + Group.setValues(grp); +} +std::vector< DocumentObject* > GeoFeatureGroupExtension::getObjectsFromLinks(DocumentObject* obj) { + + //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; + obj->getPropertyList(list); + for(App::Property* prop : list) { + if(prop->getTypeId().isDerivedFrom(App::PropertyLink::getClassTypeId())) + result.push_back(static_cast(prop)->getValue()); + else if(prop->getTypeId().isDerivedFrom(App::PropertyLinkList::getClassTypeId())) { + auto vec = static_cast(prop)->getValues(); + result.insert(result.end(), vec.begin(), vec.end()); + } + else if(prop->getTypeId().isDerivedFrom(App::PropertyLinkSub::getClassTypeId())) + result.push_back(static_cast(prop)->getValue()); + else if(prop->getTypeId().isDerivedFrom(App::PropertyLinkSubList::getClassTypeId())) { + auto vec = static_cast(prop)->getValues(); + result.insert(result.end(), vec.begin(), vec.end()); + } + } + + //clear all null objects and douplicates + result.erase(std::remove(result.begin(), result.end(), nullptr), result.end()); + std::sort(result.begin(), result.end()); + result.erase(std::unique(result.begin(), result.end()), result.end()); + + return result; +} + + +std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSOutList(App::DocumentObject* obj) { + + if(!obj) + return std::vector< App::DocumentObject* >(); + + //if the object is a geofeaturegroup than all dependencies belong to that CS, we don't want them + if(obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) + return std::vector< App::DocumentObject* >(); + + //we get all linked objects. We can't use outList() as this includes the links from expressions + auto result = getObjectsFromLinks(obj); + + //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 + std::vector< App::DocumentObject* > links; + for(App::DocumentObject *obj : result) { + auto vec = getCSOutList(obj); + links.insert(links.end(), vec.begin(), vec.end()); + } + + if (!links.empty()) { + result.insert(result.end(), links.begin(), links.end()); + std::sort(result.begin(), result.end()); + result.erase(std::unique(result.begin(), result.end()), result.end()); } + + return result; } +std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSInList(DocumentObject* obj) { + + if(!obj) + return std::vector< App::DocumentObject* >(); + + //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 + if(parent->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) + continue; + + //check if the link is real or if it is a expression one (could also be both, so it is not + //enough to check the expressions) + auto res = getObjectsFromLinks(parent); + if(std::find(res.begin(), res.end(), obj) != res.end()) + result.push_back(parent); + } + + //clear all douplicates + std::sort(result.begin(), result.end()); + result.erase(std::unique(result.begin(), result.end()), result.end()); + + //collect all links to those objects + std::vector< App::DocumentObject* > links; + for(App::DocumentObject *obj : result) { + auto vec = getCSInList(obj); + links.insert(links.end(), vec.begin(), vec.end()); + } + + if (!links.empty()) { + result.insert(result.end(), links.begin(), links.end()); + std::sort(result.begin(), result.end()); + result.erase(std::unique(result.begin(), result.end()), result.end()); + } + + return result; +} + +std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSRelevantLinks(DocumentObject* obj) { + + auto vec1 = getCSInList(obj); + auto vec2 = getCSOutList(obj); + + vec1.insert(vec1.end(), vec2.begin(), vec2.end()); + return vec1; +} + + // Python feature --------------------------------------------------------- diff --git a/src/App/GeoFeatureGroupExtension.h b/src/App/GeoFeatureGroupExtension.h index 77d1b729ee9f..abab0fa0470c 100644 --- a/src/App/GeoFeatureGroupExtension.h +++ b/src/App/GeoFeatureGroupExtension.h @@ -98,8 +98,21 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension virtual void addObject(DocumentObject* obj); + /// returns GeoFeatureGroup relevant objects that are linked from the given one. That meas all linked objects + /// including their linkes (recursively) except GeoFeatureGroups, where the recursion stops. Expressions + /// links are ignored. + static std::vector getCSOutList(App::DocumentObject* obj); + ///returns GeoFeatureGroup relevant objects that link to the given one. That meas all objects + /// including their parents (recursively) except GeoFeatureGroups, where the recursion stops. Expression + /// links are ignored + static std::vector getCSInList(App::DocumentObject* obj); + /// Returns 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 std::vector getCSRelevantLinks(App::DocumentObject* obj); private: Base::Placement recursiveGroupPlacement(GeoFeatureGroupExtension* group); + static std::vector getObjectsFromLinks(App::DocumentObject*); }; typedef ExtensionPythonT> GeoFeatureGroupExtensionPython; diff --git a/src/Mod/Test/Document.py b/src/Mod/Test/Document.py index 3ae2c8def687..f8fa39d39b55 100644 --- a/src/Mod/Test/Document.py +++ b/src/Mod/Test/Document.py @@ -836,7 +836,39 @@ def testGroup(self): self.Doc.removeObject("Group") self.Doc.removeObject("Label_2") self.Doc.removeObject("Label_3") - + + def testGroupAndGeoFeatureGroup(self): + + # an object can only be in one group at once, that must be enforced + obj1 = self.Doc.addObject("App::FeatureTest","obj1") + grp1 = self.Doc.addObject("App::DocumentObjectGroup","Group1") + grp2 = self.Doc.addObject("App::DocumentObjectGroup","Group2") + grp1.addObject(obj1) + self.failUnless(grp1.hasObject(obj1)) + grp2.addObject(obj1) + self.failUnless(grp1.hasObject(obj1)==False) + self.failUnless(grp2.hasObject(obj1)) + + # an object is allowed to be in a group and a geofeaturegroup + prt1 = self.Doc.addObject("App::Part","Part1") + prt2 = self.Doc.addObject("App::Part","Part2") + + prt1.addObject(grp2) + self.failUnless(grp2.hasObject(obj1)) + self.failUnless(prt1.hasObject(grp2)) + self.failUnless(prt1.hasObject(obj1)) + + #it is not allowed to be in 2 geofeaturegroups + prt2.addObject(grp2) + self.failUnless(grp2.hasObject(obj1)) + self.failUnless(prt1.hasObject(grp2)==False) + self.failUnless(prt1.hasObject(obj1)==False) + self.failUnless(prt2.hasObject(grp2)) + self.failUnless(prt2.hasObject(obj1)) + + #adding the object to a geofeaturegroup, but not its group, should handle it automatically when used + #addObject + def tearDown(self): # closing doc From cd4619cfe719f2d2fc67cb07c35c24c657d17d45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Thu, 9 Feb 2017 19:52:44 +0100 Subject: [PATCH 14/29] Groups: Handle add and remove object correctly --- src/App/GeoFeatureGroupExtension.cpp | 16 ++++ src/App/GeoFeatureGroupExtension.h | 1 + src/App/GroupExtension.cpp | 17 +++- .../ViewProviderGeoFeatureGroupExtension.cpp | 96 ------------------- .../ViewProviderGeoFeatureGroupExtension.h | 7 +- src/Gui/ViewProviderGroupExtension.cpp | 19 ---- src/Gui/ViewProviderOrigin.h | 2 + src/Gui/ViewProviderOriginGroupExtension.cpp | 58 ----------- src/Gui/ViewProviderOriginGroupExtension.h | 3 - 9 files changed, 32 insertions(+), 187 deletions(-) diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index c8867119cb7d..210bf801a60a 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -133,6 +133,22 @@ void GeoFeatureGroupExtension::addObject(App::DocumentObject* object) { Group.setValues(grp); } + + +void GeoFeatureGroupExtension::removeObject(App::DocumentObject* object) { + + //cross CoordinateSystem links are not allowed, so we need to remove the whole link group + auto links = getCSRelevantLinks(object); + links.push_back(object); + + //remove all links out of group + std::vector grp = Group.getValues(); + for(auto link : links) + grp.erase(std::remove(grp.begin(), grp.end(), link), grp.end()); + + Group.setValues(grp); +} + std::vector< DocumentObject* > GeoFeatureGroupExtension::getObjectsFromLinks(DocumentObject* obj) { //we get all linked objects. We can't use outList() as this includes the links from expressions diff --git a/src/App/GeoFeatureGroupExtension.h b/src/App/GeoFeatureGroupExtension.h index abab0fa0470c..20771add4baa 100644 --- a/src/App/GeoFeatureGroupExtension.h +++ b/src/App/GeoFeatureGroupExtension.h @@ -97,6 +97,7 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension } virtual void addObject(DocumentObject* obj); + virtual void removeObject(DocumentObject* obj); /// returns GeoFeatureGroup relevant objects that are linked from the given one. That meas all linked objects /// including their linkes (recursively) except GeoFeatureGroups, where the recursion stops. Expressions diff --git a/src/App/GroupExtension.cpp b/src/App/GroupExtension.cpp index 0af4e9b638ec..9051ccecf995 100644 --- a/src/App/GroupExtension.cpp +++ b/src/App/GroupExtension.cpp @@ -31,6 +31,7 @@ #include "GroupExtensionPy.h" #include "Document.h" #include "FeaturePythonPyImp.h" +#include "GeoFeatureGroupExtension.h" using namespace App; @@ -63,17 +64,23 @@ void GroupExtension::addObject(DocumentObject* obj) if(!allowObject(obj)) return; + if (hasObject(obj)) + return; + //only one group per object. Note that it is allowed to be in a group and geofeaturegroup. However, //getGroupOfObject() returns only normal groups, no GeoFeatureGroups. Hence this works. auto *group = App::GroupExtension::getGroupOfObject(obj); if(group && group != getExtendedObject()) group->getExtensionByType()->removeObject(obj); - if (!hasObject(obj)) { - std::vector grp = Group.getValues(); - grp.push_back(obj); - Group.setValues(grp); - } + //if we are on a geofeaturegroup we need to ensure the object is too + auto geogrp = GeoFeatureGroupExtension::getGroupOfObject(getExtendedObject()); + if( geogrp && (geogrp != GeoFeatureGroupExtension::getGroupOfObject(obj)) ) + geogrp->getExtensionByType()->addObject(obj); + + std::vector grp = Group.getValues(); + grp.push_back(obj); + Group.setValues(grp); } void GroupExtension::addObjects(const std::vector& objs) diff --git a/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp b/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp index 2f9dce8e83d8..323ac54264b2 100644 --- a/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp +++ b/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp @@ -120,102 +120,6 @@ void ViewProviderGeoFeatureGroupExtension::extensionUpdateData(const App::Proper } } -std::vector< App::DocumentObject* > ViewProviderGeoFeatureGroupExtension::getLinkedObjects(App::DocumentObject* obj) { - - if(!obj) - return std::vector< App::DocumentObject* >(); - - //if the object is a geofeaturegroup than all dependencies belong to that CS, we are not allowed - //to grap them - if(obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) - return std::vector< App::DocumentObject* >(); - - //we get all linked objects - std::vector< App::DocumentObject* > result; - std::vector list; - obj->getPropertyList(list); - for(App::Property* prop : list) { - if(prop->getTypeId().isDerivedFrom(App::PropertyLink::getClassTypeId())) - result.push_back(static_cast(prop)->getValue()); - else if(prop->getTypeId().isDerivedFrom(App::PropertyLinkList::getClassTypeId())) { - auto vec = static_cast(prop)->getValues(); - result.insert(result.end(), vec.begin(), vec.end()); - } - else if(prop->getTypeId().isDerivedFrom(App::PropertyLinkSub::getClassTypeId())) - result.push_back(static_cast(prop)->getValue()); - else if(prop->getTypeId().isDerivedFrom(App::PropertyLinkSubList::getClassTypeId())) { - auto vec = static_cast(prop)->getValues(); - result.insert(result.end(), vec.begin(), vec.end()); - } - } - - //clear all null objects - result.erase(std::remove(result.begin(), result.end(), nullptr), result.end()); - - //collect all dependencies of those objects - std::vector< App::DocumentObject* > links; - for(App::DocumentObject *obj : result) { - auto vec = getLinkedObjects(obj); - links.insert(links.end(), vec.begin(), vec.end()); - } - - if (!links.empty()) { - result.insert(result.end(), links.begin(), links.end()); - std::sort(result.begin(), result.end()); - result.erase(std::unique(result.begin(), result.end()), result.end()); - } - - return result; -} - -void ViewProviderGeoFeatureGroupExtension::extensionDropObject(App::DocumentObject* obj) { - - // Open command - App::DocumentObject* grp = static_cast(getExtendedViewProvider()->getObject()); - App::Document* doc = grp->getDocument(); - Gui::Document* gui = Gui::Application::Instance->getDocument(doc); - gui->openCommand("Move object"); - - //links between different CS are not allowed, hence we need to ensure if all dependencies are in - //the same geofeaturegroup - auto vec = getLinkedObjects(obj); - - //remove all objects already in the correct group - vec.erase(std::remove_if(vec.begin(), vec.end(), [this](App::DocumentObject* o){ - return App::GroupExtension::getGroupOfObject(o) == this->getExtendedViewProvider()->getObject(); - }), vec.end()); - - vec.push_back(obj); - - for(App::DocumentObject* o : vec) { - // build Python command for execution - QString cmd; - cmd = QString::fromLatin1("App.getDocument(\"%1\").getObject(\"%2\").addObject(" - "App.getDocument(\"%1\").getObject(\"%3\"))") - .arg(QString::fromLatin1(doc->getName())) - .arg(QString::fromLatin1(grp->getNameInDocument())) - .arg(QString::fromLatin1(o->getNameInDocument())); - - Gui::Command::doCommand(Gui::Command::App, cmd.toUtf8()); - } - gui->commitCommand(); -} - - -void ViewProviderGeoFeatureGroupExtension::extensionDragObject(App::DocumentObject* obj) { - //links between different coordinate systems are not allowed, hence draging one object also needs - //to drag out all dependend objects - auto vec = getLinkedObjects(obj); - - //add this object - vec.push_back(obj); - - for(App::DocumentObject* obj : vec) - ViewProviderGroupExtension::extensionDragObject(obj); -} - - - namespace Gui { EXTENSION_PROPERTY_SOURCE_TEMPLATE(Gui::ViewProviderGeoFeatureGroupExtensionPython, Gui::ViewProviderGeoFeatureGroupExtension) diff --git a/src/Gui/ViewProviderGeoFeatureGroupExtension.h b/src/Gui/ViewProviderGeoFeatureGroupExtension.h index d409ae27ee98..a0bf3b7cf107 100644 --- a/src/Gui/ViewProviderGeoFeatureGroupExtension.h +++ b/src/Gui/ViewProviderGeoFeatureGroupExtension.h @@ -58,14 +58,9 @@ class GuiExport ViewProviderGeoFeatureGroupExtension : public ViewProviderGroupE } virtual void extensionUpdateData(const App::Property*) override; - - virtual void extensionDropObject(App::DocumentObject*) override; - virtual void extensionDragObject(App::DocumentObject*) override; - + protected: SoGroup *pcGroupChildren; - - std::vector getLinkedObjects(App::DocumentObject* obj); }; typedef ViewProviderExtensionPythonT ViewProviderGeoFeatureGroupExtensionPython; diff --git a/src/Gui/ViewProviderGroupExtension.cpp b/src/Gui/ViewProviderGroupExtension.cpp index 78aecec395d5..d8a5beeb6ce0 100644 --- a/src/Gui/ViewProviderGroupExtension.cpp +++ b/src/Gui/ViewProviderGroupExtension.cpp @@ -86,13 +86,6 @@ bool ViewProviderGroupExtension::extensionCanDropObject(App::DocumentObject* obj if (group->hasObject(obj)) return false; - //group into group? - if (obj->hasExtension(App::GroupExtension::getExtensionClassTypeId())) { - if (group->isChildOf(obj->getExtensionByType())) - return false; - } - - //We need to find the correct App extension to ask if this is a supported type, there should only be one if (group->allowObject(obj)) return true; @@ -107,18 +100,6 @@ void ViewProviderGroupExtension::extensionDropObject(App::DocumentObject* obj) { Gui::Document* gui = Gui::Application::Instance->getDocument(doc); gui->openCommand("Move object"); - const App::DocumentObject* par = App::GroupExtension::getGroupOfObject(obj); - if (par) { - // allow an object to be in one group only - QString cmd; - cmd = QString::fromLatin1("App.getDocument(\"%1\").getObject(\"%2\").removeObject(" - "App.getDocument(\"%1\").getObject(\"%3\"))") - .arg(QString::fromLatin1(doc->getName())) - .arg(QString::fromLatin1(par->getNameInDocument())) - .arg(QString::fromLatin1(obj->getNameInDocument())); - Gui::Command::doCommand(Gui::Command::App, cmd.toUtf8()); - } - // build Python command for execution QString cmd; cmd = QString::fromLatin1("App.getDocument(\"%1\").getObject(\"%2\").addObject(" diff --git a/src/Gui/ViewProviderOrigin.h b/src/Gui/ViewProviderOrigin.h index e0658f803993..bd0c4f90a569 100644 --- a/src/Gui/ViewProviderOrigin.h +++ b/src/Gui/ViewProviderOrigin.h @@ -69,6 +69,8 @@ class GuiExport ViewProviderOrigin : public ViewProviderDocumentObject /// Reset the visibility void resetTemporaryVisibility (); ///@} + + virtual bool canDragObjects() const {return false;}; /// Returns default size. Use this if it is not possible to determine appropriate size by other means static double defaultSize() {return 10.;} diff --git a/src/Gui/ViewProviderOriginGroupExtension.cpp b/src/Gui/ViewProviderOriginGroupExtension.cpp index 2ca1742a6a6d..04e7cb20736b 100644 --- a/src/Gui/ViewProviderOriginGroupExtension.cpp +++ b/src/Gui/ViewProviderOriginGroupExtension.cpp @@ -199,64 +199,6 @@ void ViewProviderOriginGroupExtension::updateOriginSize () { vpOrigin->Size.setValue ( size * 1.3 ); } -void ViewProviderOriginGroupExtension::extensionDragObject(App::DocumentObject* obj) { - - //links between different coordinate systems are not allowed, hence draging one object also needs - //to drag out all dependend objects - auto vec = getLinkedObjects(obj); - - //remove all origin objects - vec.erase(std::remove_if(vec.begin(), vec.end(), [this](App::DocumentObject* o) { - return o->isDerivedFrom(App::OriginFeature::getClassTypeId());}), vec.end()); - - //add the original object - vec.push_back(obj); - - for(App::DocumentObject* obj : vec) - ViewProviderGroupExtension::extensionDragObject(obj); -} - -void ViewProviderOriginGroupExtension::extensionDropObject(App::DocumentObject* obj) { - - // Open command - App::DocumentObject* grp = static_cast(getExtendedViewProvider()->getObject()); - App::Document* doc = grp->getDocument(); - Gui::Document* gui = Gui::Application::Instance->getDocument(doc); - gui->openCommand("Move object"); - - //links between different CS are not allowed, hence we need to enure if all dependencies are in - //the same geofeaturegroup - auto vec = getLinkedObjects(obj); - - //remove all origin objects - vec.erase(std::remove_if(vec.begin(), vec.end(), [](App::DocumentObject* o) { - return o->isDerivedFrom(App::OriginFeature::getClassTypeId());}), vec.end()); - - //remove all objects already in the correct group - vec.erase(std::remove_if(vec.begin(), vec.end(), [this](App::DocumentObject* o){ - return App::GroupExtension::getGroupOfObject(o) == this->getExtendedViewProvider()->getObject(); - }), vec.end()); - - //add the original object - vec.push_back(obj); - - for(App::DocumentObject* o : vec) { - - // build Python command for execution - QString cmd; - cmd = QString::fromLatin1("App.getDocument(\"%1\").getObject(\"%2\").addObject(" - "App.getDocument(\"%1\").getObject(\"%3\"))") - .arg(QString::fromLatin1(doc->getName())) - .arg(QString::fromLatin1(grp->getNameInDocument())) - .arg(QString::fromLatin1(o->getNameInDocument())); - - Gui::Command::doCommand(Gui::Command::App, cmd.toUtf8()); - } - gui->commitCommand(); - -} - - namespace Gui { EXTENSION_PROPERTY_SOURCE_TEMPLATE(Gui::ViewProviderOriginGroupExtensionPython, Gui::ViewProviderOriginGroupExtension) diff --git a/src/Gui/ViewProviderOriginGroupExtension.h b/src/Gui/ViewProviderOriginGroupExtension.h index 7a34a5187d8c..a326dca013ff 100644 --- a/src/Gui/ViewProviderOriginGroupExtension.h +++ b/src/Gui/ViewProviderOriginGroupExtension.h @@ -45,9 +45,6 @@ class GuiExport ViewProviderOriginGroupExtension : public ViewProviderGeoFeature virtual void extensionAttach(App::DocumentObject *pcObject) override; virtual void extensionUpdateData(const App::Property* prop) override; - - virtual void extensionDragObject(App::DocumentObject*) override; - virtual void extensionDropObject(App::DocumentObject*) override; void updateOriginSize(); From b723a54a3ca100a1e9e25cac71a9eebaeb76acbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Thu, 9 Feb 2017 21:34:02 +0100 Subject: [PATCH 15/29] GeoFeatureGroup: Handle drag into document --- src/App/GeoFeatureGroupExtension.cpp | 11 ++++++++--- src/App/GeoFeatureGroupExtension.h | 4 ++-- src/App/GroupExtension.cpp | 14 ++++++++++---- src/App/GroupExtension.h | 8 ++++---- src/App/GroupExtensionPyImp.cpp | 16 ++++++++++++---- src/App/OriginGroupExtension.cpp | 4 ++-- src/App/OriginGroupExtension.h | 2 +- src/Gui/Tree.cpp | 26 +++++++++++++++++++++----- src/Mod/PartDesign/App/Body.cpp | 11 +++++++---- src/Mod/PartDesign/App/Body.h | 4 ++-- 10 files changed, 69 insertions(+), 31 deletions(-) diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 210bf801a60a..5b52d822cb61 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -111,15 +111,16 @@ Base::Placement GeoFeatureGroupExtension::recursiveGroupPlacement(GeoFeatureGrou return group->placement().getValue(); } -void GeoFeatureGroupExtension::addObject(App::DocumentObject* object) { +std::vector GeoFeatureGroupExtension::addObject(App::DocumentObject* object) { if(!allowObject(object)) - return; + return std::vector(); //cross CoordinateSystem links are not allowed, so we need to move the whole link group auto links = getCSRelevantLinks(object); links.push_back(object); + auto ret = links; std::vector grp = Group.getValues(); for( auto obj : links) { //only one geofeaturegroup per object. @@ -129,13 +130,16 @@ void GeoFeatureGroupExtension::addObject(App::DocumentObject* object) { if (!hasObject(obj)) grp.push_back(obj); + else + ret.erase(std::remove(ret.begin(), ret.end(), obj), ret.end()); } Group.setValues(grp); + return ret; } -void GeoFeatureGroupExtension::removeObject(App::DocumentObject* object) { +std::vector GeoFeatureGroupExtension::removeObject(App::DocumentObject* object) { //cross CoordinateSystem links are not allowed, so we need to remove the whole link group auto links = getCSRelevantLinks(object); @@ -147,6 +151,7 @@ void GeoFeatureGroupExtension::removeObject(App::DocumentObject* object) { grp.erase(std::remove(grp.begin(), grp.end(), link), grp.end()); Group.setValues(grp); + return links; } std::vector< DocumentObject* > GeoFeatureGroupExtension::getObjectsFromLinks(DocumentObject* obj) { diff --git a/src/App/GeoFeatureGroupExtension.h b/src/App/GeoFeatureGroupExtension.h index 20771add4baa..a9b5d0ad4d97 100644 --- a/src/App/GeoFeatureGroupExtension.h +++ b/src/App/GeoFeatureGroupExtension.h @@ -96,8 +96,8 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension !obj->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()); } - virtual void addObject(DocumentObject* obj); - virtual void removeObject(DocumentObject* obj); + virtual std::vector addObject(DocumentObject* obj) override; + virtual std::vector removeObject(DocumentObject* obj) override; /// returns GeoFeatureGroup relevant objects that are linked from the given one. That meas all linked objects /// including their linkes (recursively) except GeoFeatureGroups, where the recursion stops. Expressions diff --git a/src/App/GroupExtension.cpp b/src/App/GroupExtension.cpp index 9051ccecf995..1645f829752e 100644 --- a/src/App/GroupExtension.cpp +++ b/src/App/GroupExtension.cpp @@ -59,13 +59,13 @@ DocumentObject* GroupExtension::addObject(const char* sType, const char* pObject return obj; } -void GroupExtension::addObject(DocumentObject* obj) +std::vector GroupExtension::addObject(DocumentObject* obj) { if(!allowObject(obj)) - return; + return std::vector(); if (hasObject(obj)) - return; + return std::vector(); //only one group per object. Note that it is allowed to be in a group and geofeaturegroup. However, //getGroupOfObject() returns only normal groups, no GeoFeatureGroups. Hence this works. @@ -81,6 +81,9 @@ void GroupExtension::addObject(DocumentObject* obj) std::vector grp = Group.getValues(); grp.push_back(obj); Group.setValues(grp); + + std::vector vec = {obj}; + return vec; } void GroupExtension::addObjects(const std::vector& objs) @@ -106,7 +109,7 @@ void GroupExtension::addObjects(const std::vector& objs) Group.setValues(grp); } -void GroupExtension::removeObject(DocumentObject* obj) +std::vector GroupExtension::removeObject(DocumentObject* obj) { const std::vector & grp = Group.getValues(); std::vector newGrp; @@ -115,6 +118,9 @@ void GroupExtension::removeObject(DocumentObject* obj) if (grp.size() != newGrp.size()) { Group.setValues (newGrp); } + + std::vector vec = {obj}; + return vec; } void GroupExtension::removeObjectsFromDocument() diff --git a/src/App/GroupExtension.h b/src/App/GroupExtension.h index 97b2aaf6ebbf..02b24f05c092 100644 --- a/src/App/GroupExtension.h +++ b/src/App/GroupExtension.h @@ -50,9 +50,9 @@ class AppExport GroupExtension : public DocumentObjectExtension * append it to this group as well. */ virtual DocumentObject *addObject(const char* sType, const char* pObjectName); - /* Adds the object \a obj to this group. + /* Adds the object \a obj to this group. Returns all objects that have been added. */ - virtual void addObject(DocumentObject* obj); + virtual std::vector addObject(DocumentObject* obj); /* Adds an array of object \a objs to this group. */ virtual void addObjects(const std::vector& objs); @@ -60,9 +60,9 @@ class AppExport GroupExtension : public DocumentObjectExtension */ virtual bool allowObject(DocumentObject* ) {return true;} - /** Removes an object from this group. + /** Removes an object from this group. Returns all objects that have been removed. */ - virtual void removeObject(DocumentObject* obj); + virtual std::vector removeObject(DocumentObject* obj); /** Removes all children objects from this group and the document. */ virtual void removeObjectsFromDocument(); diff --git a/src/App/GroupExtensionPyImp.cpp b/src/App/GroupExtensionPyImp.cpp index 398dcfc30aa0..4d3fd7aa3eb6 100644 --- a/src/App/GroupExtensionPyImp.cpp +++ b/src/App/GroupExtensionPyImp.cpp @@ -86,8 +86,12 @@ PyObject* GroupExtensionPy::addObject(PyObject *args) GroupExtension* grp = getGroupExtensionPtr(); - grp->addObject(docObj->getDocumentObjectPtr()); - Py_Return; + auto vec = grp->addObject(docObj->getDocumentObjectPtr()); + Py::List list; + for (App::DocumentObject* obj : vec) + list.append(Py::asObject(obj->getPyObject())); + + return Py::new_reference_to(list); } PyObject* GroupExtensionPy::removeObject(PyObject *args) @@ -108,8 +112,12 @@ PyObject* GroupExtensionPy::removeObject(PyObject *args) GroupExtension* grp = getGroupExtensionPtr(); - grp->removeObject(docObj->getDocumentObjectPtr()); - Py_Return; + auto vec = grp->removeObject(docObj->getDocumentObjectPtr()); + Py::List list; + for (App::DocumentObject* obj : vec) + list.append(Py::asObject(obj->getPyObject())); + + return Py::new_reference_to(list); } PyObject* GroupExtensionPy::removeObjectsFromDocument(PyObject *args) diff --git a/src/App/OriginGroupExtension.cpp b/src/App/OriginGroupExtension.cpp index f6f869a1569d..a7b9ca550228 100644 --- a/src/App/OriginGroupExtension.cpp +++ b/src/App/OriginGroupExtension.cpp @@ -174,9 +174,9 @@ void OriginGroupExtension::relinkToOrigin(App::DocumentObject* obj) } } -void OriginGroupExtension::addObject(DocumentObject* obj) { +std::vector< DocumentObject* > OriginGroupExtension::addObject(DocumentObject* obj) { relinkToOrigin(obj); - App::GeoFeatureGroupExtension::addObject(obj); + return App::GeoFeatureGroupExtension::addObject(obj); } diff --git a/src/App/OriginGroupExtension.h b/src/App/OriginGroupExtension.h index 2e98bae3a034..c802308e2921 100644 --- a/src/App/OriginGroupExtension.h +++ b/src/App/OriginGroupExtension.h @@ -66,7 +66,7 @@ class AppExport OriginGroupExtension : public App::GeoFeatureGroupExtension //changes all links of obj to a origin to point to this groupes origin void relinkToOrigin(App::DocumentObject* obj); - virtual void addObject(DocumentObject* obj) override; + virtual std::vector addObject(DocumentObject* obj) override; protected: /// Checks integrity of the Origin diff --git a/src/Gui/Tree.cpp b/src/Gui/Tree.cpp index 53f463a44444..2b413f85fa3b 100644 --- a/src/Gui/Tree.cpp +++ b/src/Gui/Tree.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include "Tree.h" #include "Command.h" @@ -643,11 +644,26 @@ void TreeWidget::dropEvent(QDropEvent *event) vpp->dragObject(obj); } - std::list baseViews = gui->getMDIViews(); - for (MDIView* view : baseViews) { - View3DInventor *activeView = dynamic_cast(view); - if (activeView && !activeView->getViewer()->hasViewProvider(vpc)) { - activeView->getViewer()->addViewProvider(vpc); + //make sure it is not part of a geofeaturegroup anymore. When this has happen we need to handle + //all removed objects + std::vector vps = {vpc}; + auto grp = App::GeoFeatureGroupExtension::getGroupOfObject(obj); + if(grp) { + auto removed = grp->getExtensionByType()->removeObject(obj); + for(auto o : removed) { + auto remvp = gui->getViewProvider(o); + if(remvp) + vps.push_back(remvp); + } + } + + for(auto vp : vps) { + std::list baseViews = gui->getMDIViews(); + for (MDIView* view : baseViews) { + View3DInventor *activeView = dynamic_cast(view); + if (activeView && !activeView->getViewer()->hasViewProvider(vp)) { + activeView->getViewer()->addViewProvider(vp); + } } } } diff --git a/src/Mod/PartDesign/App/Body.cpp b/src/Mod/PartDesign/App/Body.cpp index b80d1aed2f5f..7ec008b9aa46 100644 --- a/src/Mod/PartDesign/App/Body.cpp +++ b/src/Mod/PartDesign/App/Body.cpp @@ -265,16 +265,19 @@ Body* Body::findBodyOf(const App::DocumentObject* feature) } -void Body::addObject(App::DocumentObject *feature) +std::vector Body::addObject(App::DocumentObject *feature) { if(!isAllowed(feature)) throw Base::Exception("Body: object is not allowed"); - + + //TODO: features should not add all links + /* //only one group per object auto *group = App::GroupExtension::getGroupOfObject(feature); if(group && group != getExtendedObject()) group->getExtensionByType()->removeObject(feature); - + */ + insertObject (feature, getNextSolidFeature (), /*after = */ false); // Move the Tip if we added a solid if (isSolidFeature(feature)) { @@ -345,7 +348,7 @@ void Body::insertObject(App::DocumentObject* feature, App::DocumentObject* targe } -void Body::removeObject(App::DocumentObject* feature) +std::vector Body::removeObject(App::DocumentObject* feature) { App::DocumentObject* nextSolidFeature = getNextSolidFeature(feature); App::DocumentObject* prevSolidFeature = getPrevSolidFeature(feature); diff --git a/src/Mod/PartDesign/App/Body.h b/src/Mod/PartDesign/App/Body.h index 43626df3f188..04efa9a88bdc 100644 --- a/src/Mod/PartDesign/App/Body.h +++ b/src/Mod/PartDesign/App/Body.h @@ -68,7 +68,7 @@ class PartDesignExport Body : public Part::BodyBase * Add the feature into the body at the current insert point. * The insertion poin is the before next solid after the Tip feature */ - virtual void addObject(App::DocumentObject*) override; + virtual std::vector addObject(App::DocumentObject*) override; /** * Insert the feature into the body after the given feature. @@ -84,7 +84,7 @@ class PartDesignExport Body : public Part::BodyBase void insertObject(App::DocumentObject* feature, App::DocumentObject* target, bool after=false); /// Remove the feature from the body - virtual void removeObject(DocumentObject* obj) override; + virtual std::vector removeObject(DocumentObject* obj) override; /** * Checks if the given document object lays after the current insert point From 56806e91ed5660af57a7923cfad82a9c0da3df7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Thu, 9 Feb 2017 22:06:05 +0100 Subject: [PATCH 16/29] Fix collecting of cs relevant links --- src/App/GeoFeatureGroupExtension.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 5b52d822cb61..8c9654492c4d 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -262,11 +262,23 @@ std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSInList(DocumentObj std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSRelevantLinks(DocumentObject* obj) { - auto vec1 = getCSInList(obj); - auto vec2 = getCSOutList(obj); + //we need to get the outlist of all inlist objects and ourself. 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. + auto in = getCSInList(obj); + in.push_back(obj); //there may be nothing in inlist + std::vector result; + for(auto o : in) { + + auto out = getCSOutList(o); + result.insert(result.end(), out.begin(), out.end()); + } + + //there will be many douplicates + std::sort(result.begin(), result.end()); + result.erase(std::unique(result.begin(), result.end()), result.end()); - vec1.insert(vec1.end(), vec2.begin(), vec2.end()); - return vec1; + return result; } From 6854d87c9c7f86847c3d5b4a7ca2faa531dac754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Thu, 9 Feb 2017 22:17:36 +0100 Subject: [PATCH 17/29] Drag&Drop: Tree is responsible for undo/redo The user expects to undo the drag and drop action with a single click (as it happens with a single drag&drop action). Hence the command must be handled by the tree, not the viewproviders. --- src/Gui/ViewProviderGroupExtension.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Gui/ViewProviderGroupExtension.cpp b/src/Gui/ViewProviderGroupExtension.cpp index d8a5beeb6ce0..63ff4387dc1e 100644 --- a/src/Gui/ViewProviderGroupExtension.cpp +++ b/src/Gui/ViewProviderGroupExtension.cpp @@ -94,11 +94,9 @@ bool ViewProviderGroupExtension::extensionCanDropObject(App::DocumentObject* obj void ViewProviderGroupExtension::extensionDropObject(App::DocumentObject* obj) { - // Open command App::DocumentObject* grp = static_cast(getExtendedViewProvider()->getObject()); App::Document* doc = grp->getDocument(); Gui::Document* gui = Gui::Application::Instance->getDocument(doc); - gui->openCommand("Move object"); // build Python command for execution QString cmd; @@ -109,8 +107,6 @@ void ViewProviderGroupExtension::extensionDropObject(App::DocumentObject* obj) { .arg(QString::fromLatin1(obj->getNameInDocument())); Gui::Command::doCommand(Gui::Command::App, cmd.toUtf8()); - - gui->commitCommand(); } std::vector< App::DocumentObject* > ViewProviderGroupExtension::extensionClaimChildren(void) const { From 9ddc42ec6ec22651a7fa2b7dcd5ed98f94d1ec1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Thu, 9 Feb 2017 22:19:23 +0100 Subject: [PATCH 18/29] Parent groups are irelevant, not only parent geofeaturegroups --- src/App/GeoFeatureGroupExtension.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 8c9654492c4d..10425f6b13b5 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -229,8 +229,8 @@ std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSInList(DocumentObj //search the inlist for objects that have non-expression links to us for(App::DocumentObject* parent : obj->getInList()) { - //not interested in other groups - if(parent->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) + //not interested in other groups (and here we mean all groups, normal ones and geofeaturegroup) + if(parent->hasExtension(App::GroupExtension::getExtensionClassTypeId())) continue; //check if the link is real or if it is a expression one (could also be both, so it is not From 2e650cfc6faf9b7598fd6bf5757e6156c297a97b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Thu, 9 Feb 2017 22:37:41 +0100 Subject: [PATCH 19/29] Group: ensure single group only --- src/App/GeoFeatureGroupExtension.cpp | 2 -- src/App/GeoFeatureGroupExtension.h | 1 + src/App/GroupExtension.cpp | 24 ++++++++++++++++++++++++ src/App/GroupExtension.h | 2 ++ src/Mod/Test/Document.py | 3 +++ 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 10425f6b13b5..0b7382f90569 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -281,8 +281,6 @@ std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSRelevantLinks(Docu return result; } - - // Python feature --------------------------------------------------------- namespace App { diff --git a/src/App/GeoFeatureGroupExtension.h b/src/App/GeoFeatureGroupExtension.h index a9b5d0ad4d97..d4ec727b67b2 100644 --- a/src/App/GeoFeatureGroupExtension.h +++ b/src/App/GeoFeatureGroupExtension.h @@ -111,6 +111,7 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension /// obj and from obj excluding expressions and stopping the recursion at other geofeaturegroups. /// The result is the combination of CSOutList and CSInList. static std::vector getCSRelevantLinks(App::DocumentObject* obj); + private: Base::Placement recursiveGroupPlacement(GeoFeatureGroupExtension* group); static std::vector getObjectsFromLinks(App::DocumentObject*); diff --git a/src/App/GroupExtension.cpp b/src/App/GroupExtension.cpp index 1645f829752e..bd092dd8c17b 100644 --- a/src/App/GroupExtension.cpp +++ b/src/App/GroupExtension.cpp @@ -243,6 +243,30 @@ PyObject* GroupExtension::getExtensionPyObject(void) { return Py::new_reference_to(ExtensionPythonObject); } +void GroupExtension::extensionOnChanged(const Property* p) { + + //we need to remove all object that have other parent geofeature groups + if(strcmp(p->getName(), "Group")==0) { + + bool error = false; + auto corrected = Group.getValues(); + for(auto obj : Group.getValues()) { + + auto grp = GroupExtension::getGroupOfObject(obj); + if(grp && (grp != getExtendedObject())) { + error = true; + corrected.erase(std::remove(corrected.begin(), corrected.end(), obj), corrected.end()); + } + } + if(error) { + Group.setValues(corrected); + throw Base::Exception("Object can only be in a single Group"); + } + } + + App::Extension::extensionOnChanged(p); +} + namespace App { EXTENSION_PROPERTY_SOURCE_TEMPLATE(App::GroupExtensionPython, App::GroupExtension) diff --git a/src/App/GroupExtension.h b/src/App/GroupExtension.h index 02b24f05c092..0dd0e9141320 100644 --- a/src/App/GroupExtension.h +++ b/src/App/GroupExtension.h @@ -100,6 +100,8 @@ class AppExport GroupExtension : public DocumentObjectExtension virtual PyObject* getExtensionPyObject(void); + virtual void extensionOnChanged(const Property* p) override; + /// Properties PropertyLinkList Group; diff --git a/src/Mod/Test/Document.py b/src/Mod/Test/Document.py index f8fa39d39b55..5f07cebb5d74 100644 --- a/src/Mod/Test/Document.py +++ b/src/Mod/Test/Document.py @@ -869,6 +869,9 @@ def testGroupAndGeoFeatureGroup(self): #adding the object to a geofeaturegroup, but not its group, should handle it automatically when used #addObject + #to test: try add obj to second group, once by addObject, once by .Group = [] + + def tearDown(self): # closing doc From 50ae054408c7426225392283b04aa1581eb800b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Fri, 10 Feb 2017 06:55:19 +0100 Subject: [PATCH 20/29] Links ensure uncyclic graph and correct groups --- src/App/GeoFeatureGroupExtension.cpp | 3 +- src/App/PropertyContainerPyImp.cpp | 1 + src/App/PropertyLinks.cpp | 97 ++++++++++++++++++++++++++++ src/Mod/Test/Document.py | 14 ++-- 4 files changed, 106 insertions(+), 9 deletions(-) diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 0b7382f90569..12333c56f9f5 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -274,9 +274,10 @@ std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSRelevantLinks(Docu result.insert(result.end(), out.begin(), out.end()); } - //there will be many douplicates + //there will be many douplicates and also the passed obj in std::sort(result.begin(), result.end()); result.erase(std::unique(result.begin(), result.end()), result.end()); + result.erase(std::remove(result.begin(), result.end(), obj), result.end()); return result; } diff --git a/src/App/PropertyContainerPyImp.cpp b/src/App/PropertyContainerPyImp.cpp index 6ff2a4e6c848..f50347672f70 100644 --- a/src/App/PropertyContainerPyImp.cpp +++ b/src/App/PropertyContainerPyImp.cpp @@ -1,3 +1,4 @@ + /*************************************************************************** * Copyright (c) Jürgen Riegel (juergen.riegel@web.de) 2007 * * * diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index cb92d0621cec..62838f2d0ecb 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -39,11 +39,79 @@ #include "Document.h" #include "PropertyLinks.h" +#include "GeoFeatureGroupExtension.h" using namespace App; using namespace Base; using namespace std; + +void ensureDAG(PropertyContainer* container, App::DocumentObject* object) { + + if(!container || !object) + return; + + if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) + throw Base::Exception("Only DocumentObjects are allowed to use PropertyLinks"); + + auto cont = static_cast(container); + + //on restore we don't need to do anything + if(cont->isRestoring() || object->isRestoring()) + return; + + //recursively traverse the object links and see if we reach the container, which would be a + //cyclic graph --> not allowed + for(auto obj : object->getOutList()) { + if(obj == cont) + throw Base::Exception("This link would create a cyclic dependency, hence it is rejected"); + + ensureDAG(container, obj); + } +}; + +//helper functions to ensure correct geofeaturegroups. Each object is only allowed to be in a +//single group, and links are not allowed to cross GeoFeatureGroup borders +void ensureCorrectGroups(PropertyContainer* container, App::DocumentObject* object) { + + //on object creation the container may be null, and linked objects may be null anyhow + if(!container || !object) + return; + + if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) + throw Base::Exception("Only DocumentObjects are allowed to use PropertyLinks"); + + auto cont = static_cast(container); + + //on restore we don't need to do anything + if(cont->isRestoring() || object->isRestoring()) + return; + + auto objs = GeoFeatureGroupExtension::getCSRelevantLinks(object); + objs.push_back(object); + + for(auto obj : objs) { + App::DocumentObject* grp = GeoFeatureGroupExtension::getGroupOfObject(obj); + + //if the container is a GeoFeatureGroup there are two allowed possibilites: + //1. the objet is in no group, hence in a single one after adding + //2. the object is already in the container group + if(cont->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) { + + if(!grp || (grp == cont)) + return; + } + //if the container is a normal object there is only a single allowed possibility: + //1. The object has the exact same GeoFeatureGrou as the container (null included) + else if(grp == GeoFeatureGroupExtension::getGroupOfObject(cont)) { + return; + } + + //if we arrive here the container and the object don't have compatible GeoFeatureGroups + throw Base::Exception("Links are only allowed within a GeoFeatureGroup"); + } +}; + //************************************************************************** //************************************************************************** // PropertyLink @@ -72,6 +140,8 @@ PropertyLink::~PropertyLink() void PropertyLink::setValue(App::DocumentObject * lValue) { + ensureDAG(getContainer(), lValue); + ensureCorrectGroups(getContainer(), lValue); aboutToSetValue(); #ifndef USE_OLD_DAG // maintain the back link in the DocumentObject class @@ -205,6 +275,9 @@ int PropertyLinkList::getSize(void) const void PropertyLinkList::setValue(DocumentObject* lValue) { + ensureDAG(getContainer(), lValue); + ensureCorrectGroups(getContainer(), lValue); + #ifndef USE_OLD_DAG //maintain the back link in the DocumentObject class for(auto *obj : _lValueList) @@ -228,6 +301,11 @@ void PropertyLinkList::setValue(DocumentObject* lValue) void PropertyLinkList::setValues(const std::vector& lValue) { + for(auto obj : lValue) { + ensureDAG(getContainer(), obj); + ensureCorrectGroups(getContainer(), obj); + } + aboutToSetValue(); #ifndef USE_OLD_DAG //maintain the back link in the DocumentObject class @@ -380,6 +458,9 @@ PropertyLinkSub::~PropertyLinkSub() void PropertyLinkSub::setValue(App::DocumentObject * lValue, const std::vector &SubList) { + ensureDAG(getContainer(), lValue); + ensureCorrectGroups(getContainer(), lValue); + aboutToSetValue(); #ifndef USE_OLD_DAG if (_pcLinkSub) @@ -576,6 +657,9 @@ int PropertyLinkSubList::getSize(void) const void PropertyLinkSubList::setValue(DocumentObject* lValue,const char* SubName) { + ensureDAG(getContainer(), lValue); + ensureCorrectGroups(getContainer(), lValue); + #ifndef USE_OLD_DAG //maintain backlinks for(auto *obj : _lValueList) @@ -605,6 +689,11 @@ void PropertyLinkSubList::setValues(const std::vector& lValue,c if (lValue.size() != lSubNames.size()) throw Base::ValueError("PropertyLinkSubList::setValues: size of subelements list != size of objects list"); + for(auto obj : lValue) { + ensureDAG(getContainer(), obj); + ensureCorrectGroups(getContainer(), obj); + } + #ifndef USE_OLD_DAG //maintain backlinks. _lValueList can contain items multiple times, but we trust the document //object to ensure that this works @@ -631,6 +720,11 @@ void PropertyLinkSubList::setValues(const std::vector& lValue,c if (lValue.size() != lSubNames.size()) throw Base::ValueError("PropertyLinkSubList::setValues: size of subelements list != size of objects list"); + for(auto obj : lValue) { + ensureDAG(getContainer(), obj); + ensureCorrectGroups(getContainer(), obj); + } + #ifndef USE_OLD_DAG //maintain backlinks. _lValueList can contain items multiple times, but we trust the document //object to ensure that this works @@ -651,6 +745,9 @@ void PropertyLinkSubList::setValues(const std::vector& lValue,c void PropertyLinkSubList::setValue(DocumentObject* lValue, const std::vector &SubList) { + ensureDAG(getContainer(), lValue); + ensureCorrectGroups(getContainer(), lValue); + #ifndef USE_OLD_DAG //maintain backlinks. _lValueList can contain items multiple times, but we trust the document //object to ensure that this works diff --git a/src/Mod/Test/Document.py b/src/Mod/Test/Document.py index 5f07cebb5d74..c1a9d051e910 100644 --- a/src/Mod/Test/Document.py +++ b/src/Mod/Test/Document.py @@ -328,10 +328,6 @@ def testRecompute(self): L1.touch() self.failUnless(self.Doc.recompute()==1) self.failUnless((7, 5, 4, 1, 2, 1)==(L1.ExecCount,L2.ExecCount,L3.ExecCount,L4.ExecCount,L5.ExecCount,L6.ExecCount)) - L6.Link = L1 # create a circular dependency - self.failUnless(self.Doc.recompute()==-1) - L6.Link = None # resolve the circular dependency - self.failUnless(self.Doc.recompute()==3) self.Doc.removeObject(L1.Name) self.Doc.removeObject(L2.Name) @@ -427,6 +423,7 @@ def setUp(self): self.Doc = FreeCAD.newDocument("SaveRestoreTests") L1 = self.Doc.addObject("App::FeatureTest","Label_1") L2 = self.Doc.addObject("App::FeatureTest","Label_2") + L3 = self.Doc.addObject("App::FeatureTest","Label_3") self.TempPath = tempfile.gettempdir() FreeCAD.Console.PrintLog( ' Using temp path: ' + self.TempPath + '\n') @@ -437,9 +434,9 @@ def testSaveAndRestore(self): self.Doc.Label_1.TypeTransient = 4712 # setup Linking self.Doc.Label_1.Link = self.Doc.Label_2 - self.Doc.Label_2.Link = self.Doc.Label_1 + self.Doc.Label_2.Link = self.Doc.Label_3 self.Doc.Label_1.LinkSub = (self.Doc.Label_2,["Sub1","Sub2"]) - self.Doc.Label_2.LinkSub = (self.Doc.Label_1,["Sub3","Sub4"]) + self.Doc.Label_2.LinkSub = (self.Doc.Label_3,["Sub3","Sub4"]) # save the document self.Doc.saveAs(SaveName) FreeCAD.closeDocument("SaveRestoreTests") @@ -448,9 +445,9 @@ def testSaveAndRestore(self): self.failUnless(self.Doc.Label_2.Integer == 4711) # test Linkage self.failUnless(self.Doc.Label_1.Link == self.Doc.Label_2) - self.failUnless(self.Doc.Label_2.Link == self.Doc.Label_1) + self.failUnless(self.Doc.Label_2.Link == self.Doc.Label_3) self.failUnless(self.Doc.Label_1.LinkSub == (self.Doc.Label_2,["Sub1","Sub2"])) - self.failUnless(self.Doc.Label_2.LinkSub == (self.Doc.Label_1,["Sub3","Sub4"])) + self.failUnless(self.Doc.Label_2.LinkSub == (self.Doc.Label_3,["Sub3","Sub4"])) # do NOT save transient properties self.failUnless(self.Doc.Label_1.TypeTransient == 4711) self.failUnless(self.Doc == FreeCAD.getDocument(self.Doc.Name)) @@ -871,6 +868,7 @@ def testGroupAndGeoFeatureGroup(self): #to test: try add obj to second group, once by addObject, once by .Group = [] + #test set links over geofeaturegroup borders def tearDown(self): From 7d17b90754465c4a3594e1744ce61da56fdedefa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sat, 11 Feb 2017 12:02:58 +0100 Subject: [PATCH 21/29] Group tests and fixes --- src/App/DocumentObject.cpp | 10 ++++++---- src/App/GroupExtension.cpp | 35 ++++++++++++++++++++++++++--------- src/Mod/Test/Document.py | 28 +++++++++++++++++++++------- 3 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/App/DocumentObject.cpp b/src/App/DocumentObject.cpp index 4198c4242611..6a6156e0f2b2 100644 --- a/src/App/DocumentObject.cpp +++ b/src/App/DocumentObject.cpp @@ -422,10 +422,12 @@ void DocumentObject::onChanged(const Property* prop) if (prop == &Label && _pDoc && oldLabel != Label.getStrValue()) _pDoc->signalRelabelObject(*this); - if (prop->getType() & Prop_Output) - return; - // set object touched - StatusBits.set(ObjectStatus::Touch); + // set object touched if it is a input ptoperty + if (!(prop->getType() & Prop_Output)) + StatusBits.set(0); + + //call the parent for appropriate handling + TransactionalObject::onChanged(prop); } PyObject *DocumentObject::getPyObject(void) diff --git a/src/App/GroupExtension.cpp b/src/App/GroupExtension.cpp index bd092dd8c17b..b34da464b580 100644 --- a/src/App/GroupExtension.cpp +++ b/src/App/GroupExtension.cpp @@ -32,6 +32,7 @@ #include "Document.h" #include "FeaturePythonPyImp.h" #include "GeoFeatureGroupExtension.h" +#include using namespace App; @@ -75,8 +76,14 @@ std::vector GroupExtension::addObject(DocumentObject* obj) //if we are on a geofeaturegroup we need to ensure the object is too auto geogrp = GeoFeatureGroupExtension::getGroupOfObject(getExtendedObject()); - if( geogrp && (geogrp != GeoFeatureGroupExtension::getGroupOfObject(obj)) ) - geogrp->getExtensionByType()->addObject(obj); + auto objgrp = GeoFeatureGroupExtension::getGroupOfObject(obj); + if( geogrp != objgrp ) { + //what to doo depends on if we are in geofeature group or not + if(geogrp) + geogrp->getExtensionByType()->addObject(obj); + else + objgrp->getExtensionByType()->removeObject(obj); + } std::vector grp = Group.getValues(); grp.push_back(obj); @@ -245,19 +252,29 @@ PyObject* GroupExtension::getExtensionPyObject(void) { void GroupExtension::extensionOnChanged(const Property* p) { - //we need to remove all object that have other parent geofeature groups - if(strcmp(p->getName(), "Group")==0) { + //objects are only allowed in a single group. Note that this check must only be done for normal + //groups, not any derived classes + if((this->getExtensionTypeId() == GroupExtension::getExtensionClassTypeId()) && + (strcmp(p->getName(), "Group")==0)) { bool error = false; auto corrected = Group.getValues(); for(auto obj : Group.getValues()) { - auto grp = GroupExtension::getGroupOfObject(obj); - if(grp && (grp != getExtendedObject())) { - error = true; - corrected.erase(std::remove(corrected.begin(), corrected.end(), obj), corrected.end()); + //we have already set the obj into the group, so in a case of multiple groups getGroupOfObject + //would return anyone of it and hence it is possible that we miss an error. We need a custom check + auto list = obj->getInList(); + for (auto in : list) { + if(in->hasExtension(App::GroupExtension::getExtensionClassTypeId(), false) && + in != getExtendedObject()) { + error = true; + corrected.erase(std::remove(corrected.begin(), corrected.end(), obj), corrected.end()); + break; + } } - } + } + + //if an error was found we need to correct the values and inform the user if(error) { Group.setValues(corrected); throw Base::Exception("Object can only be in a single Group"); diff --git a/src/Mod/Test/Document.py b/src/Mod/Test/Document.py index c1a9d051e910..039a9c581c2b 100644 --- a/src/Mod/Test/Document.py +++ b/src/Mod/Test/Document.py @@ -862,14 +862,28 @@ def testGroupAndGeoFeatureGroup(self): self.failUnless(prt1.hasObject(obj1)==False) self.failUnless(prt2.hasObject(grp2)) self.failUnless(prt2.hasObject(obj1)) + + #to test: try add obj to second group by .Group = [] + grp = prt1.Group + grp.append(grp2) + try: + prt1.Group=grp + except: + pass + else: + self.fail("No exception at cross geofeaturegroup links") - #adding the object to a geofeaturegroup, but not its group, should handle it automatically when used - #addObject - - #to test: try add obj to second group, once by addObject, once by .Group = [] - - #test set links over geofeaturegroup borders - + prt2.addObject(grp1) + grp = grp1.Group + grp.append(obj1) + try: + grp1.Group = grp + except: + pass + else: + self.fail("No exception thrown when object is in multiple Groups") + + def tearDown(self): # closing doc From 9e8a030407ef51cb1fbb12bdfc7486b5cf1dd01f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sat, 11 Feb 2017 13:55:20 +0100 Subject: [PATCH 22/29] Fix undo/redo while checking link integrity --- src/App/Document.cpp | 6 ++++++ src/App/Document.h | 2 ++ src/App/GeoFeatureGroupExtension.cpp | 8 ++++++++ src/App/Origin.cpp | 2 +- src/App/Origin.h | 2 +- src/App/OriginGroupExtension.cpp | 16 +++++++++++++--- src/App/PropertyLinks.cpp | 18 +++++++++++++++--- 7 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 7275adc431f9..d9f9bd23a2e0 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -863,6 +863,12 @@ bool Document::redo(void) return false; } +bool Document::performsTransactionOperation() { + + return d->undoing || d->rollback; +} + + std::vector Document::getAvailableUndoNames() const { std::vector vList; diff --git a/src/App/Document.h b/src/App/Document.h index 996d37abf2e9..64b20fa8f4ec 100644 --- a/src/App/Document.h +++ b/src/App/Document.h @@ -306,6 +306,8 @@ class AppExport Document : public App::PropertyContainer std::vector getAvailableRedoNames() const; /// Will REDO one step, returns False if no redo was done (Redos == 0). bool redo() ; + /// returns true if the document is in an Transaction phase, e.g. currently performing a redo/undo or rollback + bool performsTransactionOperation(); //@} /** @name dependency stuff */ diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 12333c56f9f5..19d88828f707 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -32,6 +32,7 @@ #include "GeoFeatureGroupExtension.h" #include "OriginFeature.h" #include "Origin.h" +#include "OriginGroupExtension.h" //#include "GeoFeatureGroupPy.h" //#include "FeaturePythonPyImp.h" @@ -81,6 +82,13 @@ void GeoFeatureGroupExtension::transformPlacement(const Base::Placement &transfo DocumentObject* GeoFeatureGroupExtension::getGroupOfObject(const DocumentObject* obj) { + if(!obj) + return nullptr; + + //we will find origins, but not origin features + if(obj->isDerivedFrom(App::OriginFeature::getClassTypeId())) + return OriginGroupExtension::getGroupOfObject(obj); + //compared to GroupExtension we do return here all geofeaturegroups including all extensions erived from it //like origingroup. That is needed as we use this function to get all local coordinate systems. Also there //is no reason to distuinguish between geofeatuergroups, there is only between group/geofeaturegroup diff --git a/src/App/Origin.cpp b/src/App/Origin.cpp index f3e41287e33c..b506619eecc8 100644 --- a/src/App/Origin.cpp +++ b/src/App/Origin.cpp @@ -99,7 +99,7 @@ App::Plane *Origin::getPlane( const char *role ) const { } } -bool Origin::hasObject (DocumentObject *obj) const { +bool Origin::hasObject (const DocumentObject *obj) const { const auto & features = OriginFeatures.getValues (); return std::find (features.begin(), features.end(), obj) != features.end (); } diff --git a/src/App/Origin.h b/src/App/Origin.h index 1a7d0c5ab425..a36a6626f8f4 100644 --- a/src/App/Origin.h +++ b/src/App/Origin.h @@ -106,7 +106,7 @@ class AppExport Origin : public App::DocumentObject ///@} /// Returns true if the given object is part of the origin - bool hasObject (DocumentObject *obj) const; + bool hasObject (const DocumentObject *obj) const; /// Returns the default bounding box of the origin (use this if you confused what should be s ) // TODO Delete me if not really needed (2015-09-01, Fat-Zer) diff --git a/src/App/OriginGroupExtension.cpp b/src/App/OriginGroupExtension.cpp index a7b9ca550228..55418ebce588 100644 --- a/src/App/OriginGroupExtension.cpp +++ b/src/App/OriginGroupExtension.cpp @@ -67,10 +67,20 @@ App::Origin *OriginGroupExtension::getOrigin () const { App::DocumentObject *OriginGroupExtension::getGroupOfObject (const DocumentObject* obj) { + if(!obj) + return nullptr; + + bool isOriginFeature = obj->isDerivedFrom(App::OriginFeature::getClassTypeId()); + auto list = obj->getInList(); - for (auto obj : list) { - if(obj->hasExtension(App::OriginGroupExtension::getExtensionClassTypeId())) - return obj; + for (auto o : list) { + if(o->hasExtension(App::OriginGroupExtension::getExtensionClassTypeId())) + return o; + else if (isOriginFeature && o->isDerivedFrom(App::Origin::getClassTypeId())) { + auto result = getGroupOfObject(o); + if(result) + return result; + } } return nullptr; diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index 62838f2d0ecb..af835ad33e17 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -51,8 +51,14 @@ void ensureDAG(PropertyContainer* container, App::DocumentObject* object) { if(!container || !object) return; + //document containers and other non-object things don't need to be handled if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) - throw Base::Exception("Only DocumentObjects are allowed to use PropertyLinks"); + return; + + //undo and redo do not need to be handled as they can only go to already checked stated (the link + //state during those actions can get messed up, we really don't want to check for that) + if(object->getDocument()->performsTransactionOperation()) + return; auto cont = static_cast(container); @@ -71,15 +77,21 @@ void ensureDAG(PropertyContainer* container, App::DocumentObject* object) { }; //helper functions to ensure correct geofeaturegroups. Each object is only allowed to be in a -//single group, and links are not allowed to cross GeoFeatureGroup borders +//single geofeatueregroup, and links are not allowed to cross GeoFeatureGroup borders void ensureCorrectGroups(PropertyContainer* container, App::DocumentObject* object) { //on object creation the container may be null, and linked objects may be null anyhow if(!container || !object) return; + //document containers and other non-object things don't need to be handled if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) - throw Base::Exception("Only DocumentObjects are allowed to use PropertyLinks"); + return; + + //undo and redo do not need to be handled as they can only go to already checked stated (the link + //state during those actions can get messed up, we really don't want to check for that) + if(object->getDocument()->performsTransactionOperation()) + return; auto cont = static_cast(container); From d5593559ed061ec2daffca887aa685fd91dca6b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sun, 12 Feb 2017 09:01:22 +0100 Subject: [PATCH 23/29] Extend python interface for groups and fix test cases --- src/App/DocumentObjectPy.xml | 14 ++ src/App/DocumentObjectPyImp.cpp | 38 ++++ src/App/ExtensionContainer.cpp | 2 +- src/App/PropertyLinks.cpp | 9 + src/Mod/PartDesign/App/Body.cpp | 5 + src/Mod/PartDesign/Gui/CommandBody.cpp | 5 +- src/Mod/PartDesign/TestPartDesignGui.py | 226 ++++++++++++------------ src/Mod/Test/Document.py | 4 + 8 files changed, 188 insertions(+), 115 deletions(-) diff --git a/src/App/DocumentObjectPy.xml b/src/App/DocumentObjectPy.xml index 156d6e6c49bc..31e009cbbe70 100644 --- a/src/App/DocumentObjectPy.xml +++ b/src/App/DocumentObjectPy.xml @@ -55,6 +55,20 @@ Recomputes this object + + + Returns the group the object is in or None if it is not part of a group. + Note that an object can only be in a single group, hence only a single return + value. + + + + + Returns the GeoFeatureGroup, and hence the local coorinate system, the object + is in or None if it is not part of a group. Note that an object can only be + in a single group, hence only a single return value. + + A list of all objects this object links to. diff --git a/src/App/DocumentObjectPyImp.cpp b/src/App/DocumentObjectPyImp.cpp index da62d024d20a..cb5835d7eda4 100644 --- a/src/App/DocumentObjectPyImp.cpp +++ b/src/App/DocumentObjectPyImp.cpp @@ -25,6 +25,8 @@ #include "DocumentObject.h" #include "Document.h" #include "Expression.h" +#include "GroupExtension.h" +#include "GeoFeatureGroupExtension.h" // inclusion of the generated files (generated out of DocumentObjectPy.xml) #include @@ -315,6 +317,42 @@ PyObject* DocumentObjectPy::recompute(PyObject *args) } } +PyObject* DocumentObjectPy::getParentGroup(PyObject *args) +{ + if (!PyArg_ParseTuple(args, "")) + return NULL; + + try { + auto grp = GroupExtension::getGroupOfObject(getDocumentObjectPtr()); + if(!grp) { + Py_INCREF(Py_None); + return Py_None; + } + return grp->getPyObject(); + } + catch (const Base::Exception& e) { + throw Py::RuntimeError(e.what()); + } +} + +PyObject* DocumentObjectPy::getParentGeoFeatureGroup(PyObject *args) +{ + if (!PyArg_ParseTuple(args, "")) + return NULL; + + try { + auto grp = GeoFeatureGroupExtension::getGroupOfObject(getDocumentObjectPtr()); + if(!grp) { + Py_INCREF(Py_None); + return Py_None; + } + return grp->getPyObject(); + } + catch (const Base::Exception& e) { + throw Py::RuntimeError(e.what()); + } +} + PyObject *DocumentObjectPy::getCustomAttributes(const char* attr) const { // search for dynamic property diff --git a/src/App/ExtensionContainer.cpp b/src/App/ExtensionContainer.cpp index 64ff37606c9c..cbc203a76bd1 100644 --- a/src/App/ExtensionContainer.cpp +++ b/src/App/ExtensionContainer.cpp @@ -80,7 +80,7 @@ bool ExtensionContainer::hasExtension(Base::Type t, bool derived) const { } return false; } - return true; + return found; } bool ExtensionContainer::hasExtension(const std::string& name) const { diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index af835ad33e17..d7320b47d7a1 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -40,6 +40,7 @@ #include "PropertyLinks.h" #include "GeoFeatureGroupExtension.h" +#include "OriginFeature.h" using namespace App; using namespace Base; @@ -88,6 +89,14 @@ void ensureCorrectGroups(PropertyContainer* container, App::DocumentObject* obje if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) return; + //links to origin feature can go over CS borders, as they are the same everywere anyway. This is + //a workaround to allow moving of objects between GeoFeatureGroups that link to origin features. + //During movement there is always a link in to the wron CS and the error would occure. If we + //surpress the error at least the origin links can be fixed afterwards + //TODO: Find a more elegant solution + if(object->isDerivedFrom(App::OriginFeature::getClassTypeId())) + return; + //undo and redo do not need to be handled as they can only go to already checked stated (the link //state during those actions can get messed up, we really don't want to check for that) if(object->getDocument()->performsTransactionOperation()) diff --git a/src/Mod/PartDesign/App/Body.cpp b/src/Mod/PartDesign/App/Body.cpp index 7ec008b9aa46..ca713c557110 100644 --- a/src/Mod/PartDesign/App/Body.cpp +++ b/src/Mod/PartDesign/App/Body.cpp @@ -283,6 +283,9 @@ std::vector Body::addObject(App::DocumentObject *feature) if (isSolidFeature(feature)) { Tip.setValue (feature); } + + std::vector result = {feature}; + return result; } @@ -380,6 +383,8 @@ std::vector Body::removeObject(App::DocumentObject* featur model.erase(it); Group.setValues(model); } + std::vector result = {feature}; + return result; } diff --git a/src/Mod/PartDesign/Gui/CommandBody.cpp b/src/Mod/PartDesign/Gui/CommandBody.cpp index e19488bcfbbb..4b1df7b51c26 100644 --- a/src/Mod/PartDesign/Gui/CommandBody.cpp +++ b/src/Mod/PartDesign/Gui/CommandBody.cpp @@ -615,6 +615,9 @@ CmdPartDesignMoveFeature::CmdPartDesignMoveFeature() void CmdPartDesignMoveFeature::activated(int iMsg) { + QMessageBox::warning(Gui::getMainWindow(), QObject::tr("Features moving is diabled"), + QObject::tr("Moving features is currently disabled as there is no way of handling origin connected moves")); + /* Q_UNUSED(iMsg); std::vector features = getSelection().getObjectsOfType(Part::Feature::getClassTypeId()); if (features.empty()) return; @@ -731,7 +734,7 @@ void CmdPartDesignMoveFeature::activated(int iMsg) PartDesignGui::relinkToOrigin(feat, target); } - updateActive(); + updateActive();*/ } bool CmdPartDesignMoveFeature::isActive(void) diff --git a/src/Mod/PartDesign/TestPartDesignGui.py b/src/Mod/PartDesign/TestPartDesignGui.py index cac9c5180344..5daa2d4db0db 100644 --- a/src/Mod/PartDesign/TestPartDesignGui.py +++ b/src/Mod/PartDesign/TestPartDesignGui.py @@ -86,121 +86,121 @@ class PartDesignGuiTestCases(unittest.TestCase): def setUp(self): self.Doc = FreeCAD.newDocument("SketchGuiTest") - def testRefuseToMoveSingleFeature(self): - FreeCAD.Console.PrintMessage('Testing refuse to move the feature with dependecies from one body to another\n') - self.BodySource = self.Doc.addObject('PartDesign::Body','Body') - Gui.activeView().setActiveObject('pdbody', self.BodySource) - - self.BoxObj = self.Doc.addObject('PartDesign::AdditiveBox','Box') - self.BoxObj.Length=10.0 - self.BoxObj.Width=10.0 - self.BoxObj.Height=10.0 - self.BodySource.addObject(self.BoxObj) - - App.ActiveDocument.recompute() + #def testRefuseToMoveSingleFeature(self): + #FreeCAD.Console.PrintMessage('Testing refuse to move the feature with dependecies from one body to another\n') + #self.BodySource = self.Doc.addObject('PartDesign::Body','Body') + #Gui.activeView().setActiveObject('pdbody', self.BodySource) + + #self.BoxObj = self.Doc.addObject('PartDesign::AdditiveBox','Box') + #self.BoxObj.Length=10.0 + #self.BoxObj.Width=10.0 + #self.BoxObj.Height=10.0 + #self.BodySource.addObject(self.BoxObj) + + #App.ActiveDocument.recompute() + + #self.Sketch = self.Doc.addObject('Sketcher::SketchObject','Sketch') + #self.Sketch.Support = (self.BoxObj, ('Face3',)) + #self.Sketch.MapMode = 'FlatFace' + #self.BodySource.addObject(self.Sketch) + + #geoList = [] + #geoList.append(Part.LineSegment(App.Vector(2.0,8.0,0),App.Vector(8.0,8.0,0))) + #geoList.append(Part.LineSegment(App.Vector(8.0,8.0,0),App.Vector(8.0,2.0,0))) + #geoList.append(Part.LineSegment(App.Vector(8.0,2.0,0),App.Vector(2.0,2.0,0))) + #geoList.append(Part.LineSegment(App.Vector(2.0,2.0,0),App.Vector(2.0,8.0,0))) + #self.Sketch.addGeometry(geoList,False) + #conList = [] + #conList.append(Sketcher.Constraint('Coincident',0,2,1,1)) + #conList.append(Sketcher.Constraint('Coincident',1,2,2,1)) + #conList.append(Sketcher.Constraint('Coincident',2,2,3,1)) + #conList.append(Sketcher.Constraint('Coincident',3,2,0,1)) + #conList.append(Sketcher.Constraint('Horizontal',0)) + #conList.append(Sketcher.Constraint('Horizontal',2)) + #conList.append(Sketcher.Constraint('Vertical',1)) + #conList.append(Sketcher.Constraint('Vertical',3)) + #self.Sketch.addConstraint(conList) + + #self.Pad = self.Doc.addObject("PartDesign::Pad","Pad") + #self.Pad.Profile = self.Sketch + #self.Pad.Length = 10.000000 + #self.Pad.Length2 = 100.000000 + #self.Pad.Type = 0 + #self.Pad.UpToFace = None + #self.Pad.Reversed = 0 + #self.Pad.Midplane = 0 + #self.Pad.Offset = 0.000000 + + #self.BodySource.addObject(self.Pad) + + #self.Doc.recompute() + #Gui.SendMsgToActiveView("ViewFit") + + #self.BodyTarget = self.Doc.addObject('PartDesign::Body','Body') + + #Gui.Selection.addSelection(App.ActiveDocument.Pad) + #cobj = CallableCheckWarning(self) + #QtCore.QTimer.singleShot(500, cobj) + #Gui.runCommand('PartDesign_MoveFeature') + ##assert depenedencies of the Sketch + #self.assertEqual(len(self.BodySource.Group), 3, "Source body feature count is wrong") + #self.assertEqual(len(self.BodyTarget.Group), 0, "Target body feature count is wrong") + + #def testMoveSingleFeature(self): + #FreeCAD.Console.PrintMessage('Testing moving one feature from one body to another\n') + #self.BodySource = self.Doc.addObject('PartDesign::Body','Body') + #Gui.activeView().setActiveObject('pdbody', self.BodySource) + + #self.Sketch = self.Doc.addObject('Sketcher::SketchObject','Sketch') + #self.BodySource.addObject(self.Sketch) + #self.Sketch.Support = (self.BodySource.Origin.OriginFeatures[3], ['']) + #self.Sketch.MapMode = 'FlatFace' + - self.Sketch = self.Doc.addObject('Sketcher::SketchObject','Sketch') - self.Sketch.Support = (self.BoxObj, ('Face3',)) - self.Sketch.MapMode = 'FlatFace' - self.BodySource.addObject(self.Sketch) - - geoList = [] - geoList.append(Part.LineSegment(App.Vector(2.0,8.0,0),App.Vector(8.0,8.0,0))) - geoList.append(Part.LineSegment(App.Vector(8.0,8.0,0),App.Vector(8.0,2.0,0))) - geoList.append(Part.LineSegment(App.Vector(8.0,2.0,0),App.Vector(2.0,2.0,0))) - geoList.append(Part.LineSegment(App.Vector(2.0,2.0,0),App.Vector(2.0,8.0,0))) - self.Sketch.addGeometry(geoList,False) - conList = [] - conList.append(Sketcher.Constraint('Coincident',0,2,1,1)) - conList.append(Sketcher.Constraint('Coincident',1,2,2,1)) - conList.append(Sketcher.Constraint('Coincident',2,2,3,1)) - conList.append(Sketcher.Constraint('Coincident',3,2,0,1)) - conList.append(Sketcher.Constraint('Horizontal',0)) - conList.append(Sketcher.Constraint('Horizontal',2)) - conList.append(Sketcher.Constraint('Vertical',1)) - conList.append(Sketcher.Constraint('Vertical',3)) - self.Sketch.addConstraint(conList) - - self.Pad = self.Doc.addObject("PartDesign::Pad","Pad") - self.Pad.Profile = self.Sketch - self.Pad.Length = 10.000000 - self.Pad.Length2 = 100.000000 - self.Pad.Type = 0 - self.Pad.UpToFace = None - self.Pad.Reversed = 0 - self.Pad.Midplane = 0 - self.Pad.Offset = 0.000000 - - self.BodySource.addObject(self.Pad) - - self.Doc.recompute() - Gui.SendMsgToActiveView("ViewFit") - - self.BodyTarget = self.Doc.addObject('PartDesign::Body','Body') - - Gui.Selection.addSelection(App.ActiveDocument.Pad) - cobj = CallableCheckWarning(self) - QtCore.QTimer.singleShot(500, cobj) - Gui.runCommand('PartDesign_MoveFeature') - #assert depenedencies of the Sketch - self.assertEqual(len(self.BodySource.Group), 3, "Source body feature count is wrong") - self.assertEqual(len(self.BodyTarget.Group), 0, "Target body feature count is wrong") - - def testMoveSingleFeature(self): - FreeCAD.Console.PrintMessage('Testing moving one feature from one body to another\n') - self.BodySource = self.Doc.addObject('PartDesign::Body','Body') - Gui.activeView().setActiveObject('pdbody', self.BodySource) - - self.Sketch = self.Doc.addObject('Sketcher::SketchObject','Sketch') - self.Sketch.Support = (self.Doc.XY_Plane, ['']) - self.Sketch.MapMode = 'FlatFace' - self.BodySource.addObject(self.Sketch) - - geoList = [] - geoList.append(Part.LineSegment(App.Vector(-10.000000,10.000000,0),App.Vector(10.000000,10.000000,0))) - geoList.append(Part.LineSegment(App.Vector(10.000000,10.000000,0),App.Vector(10.000000,-10.000000,0))) - geoList.append(Part.LineSegment(App.Vector(10.000000,-10.000000,0),App.Vector(-10.000000,-10.000000,0))) - geoList.append(Part.LineSegment(App.Vector(-10.000000,-10.000000,0),App.Vector(-10.000000,10.000000,0))) - self.Sketch.addGeometry(geoList,False) - conList = [] - conList.append(Sketcher.Constraint('Coincident',0,2,1,1)) - conList.append(Sketcher.Constraint('Coincident',1,2,2,1)) - conList.append(Sketcher.Constraint('Coincident',2,2,3,1)) - conList.append(Sketcher.Constraint('Coincident',3,2,0,1)) - conList.append(Sketcher.Constraint('Horizontal',0)) - conList.append(Sketcher.Constraint('Horizontal',2)) - conList.append(Sketcher.Constraint('Vertical',1)) - conList.append(Sketcher.Constraint('Vertical',3)) - self.Sketch.addConstraint(conList) - - self.Pad = self.Doc.addObject("PartDesign::Pad","Pad") - self.Pad.Profile = self.Sketch - self.Pad.Length = 10.000000 - self.Pad.Length2 = 100.000000 - self.Pad.Type = 0 - self.Pad.UpToFace = None - self.Pad.Reversed = 0 - self.Pad.Midplane = 0 - self.Pad.Offset = 0.000000 - - self.BodySource.addObject(self.Pad) - - self.Doc.recompute() - Gui.SendMsgToActiveView("ViewFit") - - self.BodyTarget = self.Doc.addObject('PartDesign::Body','Body') - - Gui.Selection.addSelection(App.ActiveDocument.Pad) - cobj = CallableComboBox(self) - QtCore.QTimer.singleShot(500, cobj) - Gui.runCommand('PartDesign_MoveFeature') - #assert depenedencies of the Sketch - self.Doc.recompute() + #geoList = [] + #geoList.append(Part.LineSegment(App.Vector(-10.000000,10.000000,0),App.Vector(10.000000,10.000000,0))) + #geoList.append(Part.LineSegment(App.Vector(10.000000,10.000000,0),App.Vector(10.000000,-10.000000,0))) + #geoList.append(Part.LineSegment(App.Vector(10.000000,-10.000000,0),App.Vector(-10.000000,-10.000000,0))) + #geoList.append(Part.LineSegment(App.Vector(-10.000000,-10.000000,0),App.Vector(-10.000000,10.000000,0))) + #self.Sketch.addGeometry(geoList,False) + #conList = [] + #conList.append(Sketcher.Constraint('Coincident',0,2,1,1)) + #conList.append(Sketcher.Constraint('Coincident',1,2,2,1)) + #conList.append(Sketcher.Constraint('Coincident',2,2,3,1)) + #conList.append(Sketcher.Constraint('Coincident',3,2,0,1)) + #conList.append(Sketcher.Constraint('Horizontal',0)) + #conList.append(Sketcher.Constraint('Horizontal',2)) + #conList.append(Sketcher.Constraint('Vertical',1)) + #conList.append(Sketcher.Constraint('Vertical',3)) + #self.Sketch.addConstraint(conList) + + #self.Pad = self.Doc.addObject("PartDesign::Pad","Pad") + #self.BodySource.addObject(self.Pad) + #self.Pad.Profile = self.Sketch + #self.Pad.Length = 10.000000 + #self.Pad.Length2 = 100.000000 + #self.Pad.Type = 0 + #self.Pad.UpToFace = None + #self.Pad.Reversed = 0 + #self.Pad.Midplane = 0 + #self.Pad.Offset = 0.000000 + + #self.Doc.recompute() + #Gui.SendMsgToActiveView("ViewFit") + + #self.BodyTarget = self.Doc.addObject('PartDesign::Body','Body') + + #Gui.Selection.addSelection(App.ActiveDocument.Pad) + #cobj = CallableComboBox(self) + #QtCore.QTimer.singleShot(500, cobj) + #Gui.runCommand('PartDesign_MoveFeature') + ##assert depenedencies of the Sketch + #self.Doc.recompute() - self.assertFalse(self.Sketch.Support[0][0] in self.BodySource.Origin.OriginFeatures) - self.assertTrue(self.Sketch.Support[0][0] in self.BodyTarget.Origin.OriginFeatures) - self.assertEqual(len(self.BodySource.Group), 0, "Source body feature count is wrong") - self.assertEqual(len(self.BodyTarget.Group), 2, "Target body feature count is wrong") + #self.assertFalse(self.Sketch.Support[0][0] in self.BodySource.Origin.OriginFeatures) + #self.assertTrue(self.Sketch.Support[0][0] in self.BodyTarget.Origin.OriginFeatures) + #self.assertEqual(len(self.BodySource.Group), 0, "Source body feature count is wrong") + #self.assertEqual(len(self.BodyTarget.Group), 2, "Target body feature count is wrong") def tearDown(self): FreeCAD.closeDocument("SketchGuiTest") diff --git a/src/Mod/Test/Document.py b/src/Mod/Test/Document.py index 039a9c581c2b..0413e9db7fbf 100644 --- a/src/Mod/Test/Document.py +++ b/src/Mod/Test/Document.py @@ -841,6 +841,8 @@ def testGroupAndGeoFeatureGroup(self): grp1 = self.Doc.addObject("App::DocumentObjectGroup","Group1") grp2 = self.Doc.addObject("App::DocumentObjectGroup","Group2") grp1.addObject(obj1) + self.failUnless(obj1.getParentGroup()==grp1) + self.failUnless(obj1.getParentGeoFeatureGroup()==None) self.failUnless(grp1.hasObject(obj1)) grp2.addObject(obj1) self.failUnless(grp1.hasObject(obj1)==False) @@ -851,6 +853,8 @@ def testGroupAndGeoFeatureGroup(self): prt2 = self.Doc.addObject("App::Part","Part2") prt1.addObject(grp2) + self.failUnless(grp2.getParentGeoFeatureGroup()==prt1) + self.failUnless(grp2.getParentGroup()==None) self.failUnless(grp2.hasObject(obj1)) self.failUnless(prt1.hasObject(grp2)) self.failUnless(prt1.hasObject(obj1)) From f2d1302610a877481723d60773c9708cc357fe30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Fri, 17 Feb 2017 06:51:46 +0100 Subject: [PATCH 24/29] Allow add/remove multiple objects in a group --- src/App/GeoFeatureGroupExtension.cpp | 72 ++++++++++++++---------- src/App/GeoFeatureGroupExtension.h | 4 +- src/App/GroupExtension.cpp | 83 ++++++++++++++++++---------- src/App/GroupExtension.h | 7 ++- src/App/GroupExtensionPy.xml | 28 +++++++--- src/App/GroupExtensionPyImp.cpp | 74 +++++++++++++++++++++++++ src/App/OriginGroupExtension.cpp | 9 ++- src/App/OriginGroupExtension.h | 2 +- 8 files changed, 205 insertions(+), 74 deletions(-) diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 19d88828f707..87bee84eb240 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -119,47 +119,61 @@ Base::Placement GeoFeatureGroupExtension::recursiveGroupPlacement(GeoFeatureGrou return group->placement().getValue(); } -std::vector GeoFeatureGroupExtension::addObject(App::DocumentObject* object) { +std::vector GeoFeatureGroupExtension::addObjects(std::vector objects) { - if(!allowObject(object)) - return std::vector(); - - //cross CoordinateSystem links are not allowed, so we need to move the whole link group - auto links = getCSRelevantLinks(object); - links.push_back(object); - - auto ret = links; std::vector grp = Group.getValues(); - for( auto obj : links) { - //only one geofeaturegroup per object. - auto *group = App::GeoFeatureGroupExtension::getGroupOfObject(obj); - if(group && group != getExtendedObject()) - group->getExtensionByType()->removeObject(obj); + std::vector ret; + + for(auto object : objects) { - if (!hasObject(obj)) - grp.push_back(obj); - else - ret.erase(std::remove(ret.begin(), ret.end(), obj), ret.end()); + if(!allowObject(object)) + continue; + + //cross CoordinateSystem links are not allowed, so we need to move the whole link group + auto links = getCSRelevantLinks(object); + links.push_back(object); + + for( auto obj : links) { + //only one geofeaturegroup per object. + auto *group = App::GeoFeatureGroupExtension::getGroupOfObject(obj); + if(group && group != getExtendedObject()) + group->getExtensionByType()->removeObject(obj); + + if (!hasObject(obj)) { + grp.push_back(obj); + ret.push_back(obj); + } + } } Group.setValues(grp); return ret; } - -std::vector GeoFeatureGroupExtension::removeObject(App::DocumentObject* object) { - - //cross CoordinateSystem links are not allowed, so we need to remove the whole link group - auto links = getCSRelevantLinks(object); - links.push_back(object); +std::vector GeoFeatureGroupExtension::removeObjects(std::vector objects) { - //remove all links out of group + std::vector removed; std::vector grp = Group.getValues(); - for(auto link : links) - grp.erase(std::remove(grp.begin(), grp.end(), link), grp.end()); - Group.setValues(grp); - return links; + for(auto object : objects) { + //cross CoordinateSystem links are not allowed, so we need to remove the whole link group + auto links = getCSRelevantLinks(object); + links.push_back(object); + + //remove all links out of group + for(auto link : links) { + auto end = std::remove(grp.begin(), grp.end(), link); + if(end != grp.end()) { + grp.erase(end, grp.end()); + removed.push_back(link); + } + } + } + + if(!removed.empty()) + Group.setValues(grp); + + return removed; } std::vector< DocumentObject* > GeoFeatureGroupExtension::getObjectsFromLinks(DocumentObject* obj) { diff --git a/src/App/GeoFeatureGroupExtension.h b/src/App/GeoFeatureGroupExtension.h index d4ec727b67b2..522b32103b64 100644 --- a/src/App/GeoFeatureGroupExtension.h +++ b/src/App/GeoFeatureGroupExtension.h @@ -96,8 +96,8 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension !obj->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()); } - virtual std::vector addObject(DocumentObject* obj) override; - virtual std::vector removeObject(DocumentObject* obj) override; + virtual std::vector< DocumentObject* > addObjects(std::vector< DocumentObject* > obj) override; + virtual std::vector< DocumentObject* > removeObjects(std::vector< DocumentObject* > obj) override; /// returns GeoFeatureGroup relevant objects that are linked from the given one. That meas all linked objects /// including their linkes (recursively) except GeoFeatureGroups, where the recursion stops. Expressions diff --git a/src/App/GroupExtension.cpp b/src/App/GroupExtension.cpp index b34da464b580..bd3b123d665a 100644 --- a/src/App/GroupExtension.cpp +++ b/src/App/GroupExtension.cpp @@ -62,35 +62,46 @@ DocumentObject* GroupExtension::addObject(const char* sType, const char* pObject std::vector GroupExtension::addObject(DocumentObject* obj) { - if(!allowObject(obj)) - return std::vector(); + std::vector vec = {obj}; + return addObjects(vec); +} + +std::vector< DocumentObject* > GroupExtension::addObjects(std::vector< DocumentObject* > objs) { - if (hasObject(obj)) - return std::vector(); + std::vector added; + std::vector grp = Group.getValues(); + for(auto obj : objs) { + + if(!allowObject(obj)) + continue; - //only one group per object. Note that it is allowed to be in a group and geofeaturegroup. However, - //getGroupOfObject() returns only normal groups, no GeoFeatureGroups. Hence this works. - auto *group = App::GroupExtension::getGroupOfObject(obj); - if(group && group != getExtendedObject()) - group->getExtensionByType()->removeObject(obj); - - //if we are on a geofeaturegroup we need to ensure the object is too - auto geogrp = GeoFeatureGroupExtension::getGroupOfObject(getExtendedObject()); - auto objgrp = GeoFeatureGroupExtension::getGroupOfObject(obj); - if( geogrp != objgrp ) { - //what to doo depends on if we are in geofeature group or not - if(geogrp) - geogrp->getExtensionByType()->addObject(obj); - else - objgrp->getExtensionByType()->removeObject(obj); + if (hasObject(obj)) + continue; + + //only one group per object. Note that it is allowed to be in a group and geofeaturegroup. However, + //getGroupOfObject() returns only normal groups, no GeoFeatureGroups. Hence this works. + auto *group = App::GroupExtension::getGroupOfObject(obj); + if(group && group != getExtendedObject()) + group->getExtensionByType()->removeObject(obj); + + //if we are in a geofeaturegroup we need to ensure the object is too + auto geogrp = GeoFeatureGroupExtension::getGroupOfObject(getExtendedObject()); + auto objgrp = GeoFeatureGroupExtension::getGroupOfObject(obj); + if( geogrp != objgrp ) { + //what to doo depends on if we are in geofeature group or not + if(geogrp) + geogrp->getExtensionByType()->addObject(obj); + else + objgrp->getExtensionByType()->removeObject(obj); + } + + grp.push_back(obj); + added.push_back(obj); } - std::vector grp = Group.getValues(); - grp.push_back(obj); Group.setValues(grp); - std::vector vec = {obj}; - return vec; + return added; } void GroupExtension::addObjects(const std::vector& objs) @@ -118,18 +129,34 @@ void GroupExtension::addObjects(const std::vector& objs) std::vector GroupExtension::removeObject(DocumentObject* obj) { - const std::vector & grp = Group.getValues(); - std::vector newGrp; + std::vector vec = {obj}; + return removeObjects(vec); +} + +std::vector< DocumentObject* > GroupExtension::removeObjects(std::vector< DocumentObject* > objs) { - std::remove_copy (grp.begin(), grp.end(), std::back_inserter (newGrp), obj); + const std::vector & grp = Group.getValues(); + std::vector newGrp = grp; + std::vector removed; + + std::vector::iterator end = newGrp.end(); + for(auto obj : objs) { + auto res = std::remove(newGrp.begin(), end, obj); + if(res != end) { + end = res; + removed.push_back(obj); + } + } + + newGrp.erase(end, newGrp.end()); if (grp.size() != newGrp.size()) { Group.setValues (newGrp); } - std::vector vec = {obj}; - return vec; + return removed; } + void GroupExtension::removeObjectsFromDocument() { const std::vector & grp = Group.getValues(); diff --git a/src/App/GroupExtension.h b/src/App/GroupExtension.h index 0dd0e9141320..9332512d8d65 100644 --- a/src/App/GroupExtension.h +++ b/src/App/GroupExtension.h @@ -53,9 +53,9 @@ class AppExport GroupExtension : public DocumentObjectExtension /* Adds the object \a obj to this group. Returns all objects that have been added. */ virtual std::vector addObject(DocumentObject* obj); - /* Adds an array of object \a objs to this group. + /* Adds the objects \a objs to this group. Returns all objects that have been added. */ - virtual void addObjects(const std::vector& objs); + virtual std::vector addObjects(std::vector obj); /*override this function if you want only special objects */ virtual bool allowObject(DocumentObject* ) {return true;} @@ -63,6 +63,9 @@ class AppExport GroupExtension : public DocumentObjectExtension /** Removes an object from this group. Returns all objects that have been removed. */ virtual std::vector removeObject(DocumentObject* obj); + /** Removes objects from this group. Returns all objects that have been removed. + */ + virtual std::vector removeObjects(std::vector obj); /** Removes all children objects from this group and the document. */ virtual void removeObjectsFromDocument(); diff --git a/src/App/GroupExtensionPy.xml b/src/App/GroupExtensionPy.xml index 703d310cb797..252ce45ccfde 100644 --- a/src/App/GroupExtensionPy.xml +++ b/src/App/GroupExtensionPy.xml @@ -18,15 +18,25 @@ Create and add an object with given type and name to the group - - - Add an object to the group - - - - - Remove an object from the group - + + + Add an object to the group + + + + + Adds multiple objects to the group. Expects a list. + + + + + Remove an object from the group + + + + + Remove multiple objects from the group. Expects a list. + diff --git a/src/App/GroupExtensionPyImp.cpp b/src/App/GroupExtensionPyImp.cpp index 4d3fd7aa3eb6..c9cb181150f3 100644 --- a/src/App/GroupExtensionPyImp.cpp +++ b/src/App/GroupExtensionPyImp.cpp @@ -94,6 +94,43 @@ PyObject* GroupExtensionPy::addObject(PyObject *args) return Py::new_reference_to(list); } +PyObject* GroupExtensionPy::addObjects(PyObject *args) { + + PyObject *object; + if (!PyArg_ParseTuple(args, "O", &object)) // convert args: Python->C + return NULL; // NULL triggers exception + + if (PyTuple_Check(object) || PyList_Check(object)) { + Py::Sequence list(object); + Py::Sequence::size_type size = list.size(); + std::vector values; + values.resize(size); + + for (Py::Sequence::size_type i = 0; i < size; i++) { + Py::Object item = list[i]; + if (!PyObject_TypeCheck(*item, &(DocumentObjectPy::Type))) { + std::string error = std::string("type in list must be 'DocumentObject', not "); + error += (*item)->ob_type->tp_name; + throw Base::TypeError(error); + } + + values[i] = static_cast(*item)->getDocumentObjectPtr(); + } + + GroupExtension* grp = getGroupExtensionPtr(); + auto vec = grp->addObjects(values); + Py::List result; + for (App::DocumentObject* obj : vec) + result.append(Py::asObject(obj->getPyObject())); + + return Py::new_reference_to(result); + } + + std::string error = std::string("type must be list of 'DocumentObject', not "); + error += object->ob_type->tp_name; + throw Base::TypeError(error); +}; + PyObject* GroupExtensionPy::removeObject(PyObject *args) { PyObject *object; @@ -120,6 +157,43 @@ PyObject* GroupExtensionPy::removeObject(PyObject *args) return Py::new_reference_to(list); } +PyObject* GroupExtensionPy::removeObjects(PyObject *args) { + + PyObject *object; + if (!PyArg_ParseTuple(args, "O", &object)) // convert args: Python->C + return NULL; // NULL triggers exception + + if (PyTuple_Check(object) || PyList_Check(object)) { + Py::Sequence list(object); + Py::Sequence::size_type size = list.size(); + std::vector values; + values.resize(size); + + for (Py::Sequence::size_type i = 0; i < size; i++) { + Py::Object item = list[i]; + if (!PyObject_TypeCheck(*item, &(DocumentObjectPy::Type))) { + std::string error = std::string("type in list must be 'DocumentObject', not "); + error += (*item)->ob_type->tp_name; + throw Base::TypeError(error); + } + + values[i] = static_cast(*item)->getDocumentObjectPtr(); + } + + GroupExtension* grp = getGroupExtensionPtr(); + auto vec = grp->removeObjects(values); + Py::List result; + for (App::DocumentObject* obj : vec) + result.append(Py::asObject(obj->getPyObject())); + + return Py::new_reference_to(result); + } + + std::string error = std::string("type must be list of 'DocumentObject', not "); + error += object->ob_type->tp_name; + throw Base::TypeError(error); +}; + PyObject* GroupExtensionPy::removeObjectsFromDocument(PyObject *args) { if (!PyArg_ParseTuple(args, "")) // convert args: Python->C diff --git a/src/App/OriginGroupExtension.cpp b/src/App/OriginGroupExtension.cpp index 55418ebce588..ca4b694bea88 100644 --- a/src/App/OriginGroupExtension.cpp +++ b/src/App/OriginGroupExtension.cpp @@ -184,9 +184,12 @@ void OriginGroupExtension::relinkToOrigin(App::DocumentObject* obj) } } -std::vector< DocumentObject* > OriginGroupExtension::addObject(DocumentObject* obj) { - relinkToOrigin(obj); - return App::GeoFeatureGroupExtension::addObject(obj); +std::vector< DocumentObject* > OriginGroupExtension::addObjects(std::vector objs) { + + for(auto obj : objs) + relinkToOrigin(obj); + + return App::GeoFeatureGroupExtension::addObjects(objs); } diff --git a/src/App/OriginGroupExtension.h b/src/App/OriginGroupExtension.h index c802308e2921..d79ed2d2f75a 100644 --- a/src/App/OriginGroupExtension.h +++ b/src/App/OriginGroupExtension.h @@ -66,7 +66,7 @@ class AppExport OriginGroupExtension : public App::GeoFeatureGroupExtension //changes all links of obj to a origin to point to this groupes origin void relinkToOrigin(App::DocumentObject* obj); - virtual std::vector addObject(DocumentObject* obj) override; + virtual std::vector addObjects(std::vector obj) override; protected: /// Checks integrity of the Origin From 7ce514bdca005217497889223a59fb2a2d6d21bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sat, 18 Feb 2017 12:26:23 +0100 Subject: [PATCH 25/29] Partially fix PartDesign move test --- src/Mod/PartDesign/App/Body.cpp | 17 +++- src/Mod/PartDesign/App/Body.h | 1 + src/Mod/PartDesign/Gui/CommandBody.cpp | 34 ++++++-- src/Mod/PartDesign/TestPartDesignGui.py | 104 ++++++++++++------------ 4 files changed, 92 insertions(+), 64 deletions(-) diff --git a/src/Mod/PartDesign/App/Body.cpp b/src/Mod/PartDesign/App/Body.cpp index ca713c557110..7e4f67b644b1 100644 --- a/src/Mod/PartDesign/App/Body.cpp +++ b/src/Mod/PartDesign/App/Body.cpp @@ -271,12 +271,12 @@ std::vector Body::addObject(App::DocumentObject *feature) throw Base::Exception("Body: object is not allowed"); //TODO: features should not add all links - /* - //only one group per object + + //only one group per object. If it is in a body the single feature will be removed auto *group = App::GroupExtension::getGroupOfObject(feature); if(group && group != getExtendedObject()) - group->getExtensionByType()->removeObject(feature); - */ + group->getExtensionByType()->removeObject(feature); + insertObject (feature, getNextSolidFeature (), /*after = */ false); // Move the Tip if we added a solid @@ -288,6 +288,15 @@ std::vector Body::addObject(App::DocumentObject *feature) return result; } +std::vector< App::DocumentObject* > Body::addObjects(std::vector< App::DocumentObject* > objs) { + + for(auto obj : objs) + addObject(obj); + + return objs; +} + + void Body::insertObject(App::DocumentObject* feature, App::DocumentObject* target, bool after) { diff --git a/src/Mod/PartDesign/App/Body.h b/src/Mod/PartDesign/App/Body.h index 04efa9a88bdc..f623bce19621 100644 --- a/src/Mod/PartDesign/App/Body.h +++ b/src/Mod/PartDesign/App/Body.h @@ -69,6 +69,7 @@ class PartDesignExport Body : public Part::BodyBase * The insertion poin is the before next solid after the Tip feature */ virtual std::vector addObject(App::DocumentObject*) override; + virtual std::vector< DocumentObject* > addObjects(std::vector< DocumentObject* > obj) override; /** * Insert the feature into the body after the given feature. diff --git a/src/Mod/PartDesign/Gui/CommandBody.cpp b/src/Mod/PartDesign/Gui/CommandBody.cpp index 4b1df7b51c26..cc890c44c186 100644 --- a/src/Mod/PartDesign/Gui/CommandBody.cpp +++ b/src/Mod/PartDesign/Gui/CommandBody.cpp @@ -614,10 +614,7 @@ CmdPartDesignMoveFeature::CmdPartDesignMoveFeature() } void CmdPartDesignMoveFeature::activated(int iMsg) -{ - QMessageBox::warning(Gui::getMainWindow(), QObject::tr("Features moving is diabled"), - QObject::tr("Moving features is currently disabled as there is no way of handling origin connected moves")); - /* +{ Q_UNUSED(iMsg); std::vector features = getSelection().getObjectsOfType(Part::Feature::getClassTypeId()); if (features.empty()) return; @@ -644,6 +641,15 @@ void CmdPartDesignMoveFeature::activated(int iMsg) PartDesign::Body* source = PartDesign::Body::findBodyOf(feat); source_bodies.insert(static_cast(source)); } + + if(source_bodies.size() != 1) { + //show messagebox and cancel + QMessageBox::warning(Gui::getMainWindow(), QObject::tr("Features cannot be moved"), + QObject::tr("Only features of a single source Body can be moved")); + return; + } + + auto source_body = *source_bodies.begin(); std::vector target_bodies; for (auto body : bodies) { @@ -675,8 +681,20 @@ void CmdPartDesignMoveFeature::activated(int iMsg) PartDesign::Body* target = static_cast(target_bodies[index]); openCommand("Move an object"); - - for (auto feat: features) { + + std::stringstream stream; + stream << "features_ = [App.ActiveDocument." << features.back()->getNameInDocument(); + features.pop_back(); + + for (auto feat: features) + stream << ", App.ActiveDocument." << feat->getNameInDocument(); + + stream << "]"; + doCommand(Doc, stream.str().c_str()); + doCommand(Doc, "App.ActiveDocument.%s.removeObjects(features_)", source_body->getNameInDocument()); + doCommand(Doc, "App.ActiveDocument.%s.addObjects(features_)", target->getNameInDocument()); + /* + // Find body of this feature Part::BodyBase* source = PartDesign::Body::findBodyOf(feat); bool featureWasTip = false; @@ -732,9 +750,9 @@ void CmdPartDesignMoveFeature::activated(int iMsg) //relink origin for sketches and datums (coordinates) PartDesignGui::relinkToOrigin(feat, target); - } + }*/ - updateActive();*/ + updateActive(); } bool CmdPartDesignMoveFeature::isActive(void) diff --git a/src/Mod/PartDesign/TestPartDesignGui.py b/src/Mod/PartDesign/TestPartDesignGui.py index 5daa2d4db0db..87e5b6769e0d 100644 --- a/src/Mod/PartDesign/TestPartDesignGui.py +++ b/src/Mod/PartDesign/TestPartDesignGui.py @@ -146,61 +146,61 @@ def setUp(self): #self.assertEqual(len(self.BodySource.Group), 3, "Source body feature count is wrong") #self.assertEqual(len(self.BodyTarget.Group), 0, "Target body feature count is wrong") - #def testMoveSingleFeature(self): - #FreeCAD.Console.PrintMessage('Testing moving one feature from one body to another\n') - #self.BodySource = self.Doc.addObject('PartDesign::Body','Body') - #Gui.activeView().setActiveObject('pdbody', self.BodySource) - - #self.Sketch = self.Doc.addObject('Sketcher::SketchObject','Sketch') - #self.BodySource.addObject(self.Sketch) - #self.Sketch.Support = (self.BodySource.Origin.OriginFeatures[3], ['']) - #self.Sketch.MapMode = 'FlatFace' + def testMoveSingleFeature(self): + FreeCAD.Console.PrintMessage('Testing moving one feature from one body to another\n') + self.BodySource = self.Doc.addObject('PartDesign::Body','Body') + Gui.activeView().setActiveObject('pdbody', self.BodySource) + + self.Sketch = self.Doc.addObject('Sketcher::SketchObject','Sketch') + self.BodySource.addObject(self.Sketch) + self.Sketch.Support = (self.BodySource.Origin.OriginFeatures[3], ['']) + self.Sketch.MapMode = 'FlatFace' - #geoList = [] - #geoList.append(Part.LineSegment(App.Vector(-10.000000,10.000000,0),App.Vector(10.000000,10.000000,0))) - #geoList.append(Part.LineSegment(App.Vector(10.000000,10.000000,0),App.Vector(10.000000,-10.000000,0))) - #geoList.append(Part.LineSegment(App.Vector(10.000000,-10.000000,0),App.Vector(-10.000000,-10.000000,0))) - #geoList.append(Part.LineSegment(App.Vector(-10.000000,-10.000000,0),App.Vector(-10.000000,10.000000,0))) - #self.Sketch.addGeometry(geoList,False) - #conList = [] - #conList.append(Sketcher.Constraint('Coincident',0,2,1,1)) - #conList.append(Sketcher.Constraint('Coincident',1,2,2,1)) - #conList.append(Sketcher.Constraint('Coincident',2,2,3,1)) - #conList.append(Sketcher.Constraint('Coincident',3,2,0,1)) - #conList.append(Sketcher.Constraint('Horizontal',0)) - #conList.append(Sketcher.Constraint('Horizontal',2)) - #conList.append(Sketcher.Constraint('Vertical',1)) - #conList.append(Sketcher.Constraint('Vertical',3)) - #self.Sketch.addConstraint(conList) - - #self.Pad = self.Doc.addObject("PartDesign::Pad","Pad") - #self.BodySource.addObject(self.Pad) - #self.Pad.Profile = self.Sketch - #self.Pad.Length = 10.000000 - #self.Pad.Length2 = 100.000000 - #self.Pad.Type = 0 - #self.Pad.UpToFace = None - #self.Pad.Reversed = 0 - #self.Pad.Midplane = 0 - #self.Pad.Offset = 0.000000 - - #self.Doc.recompute() - #Gui.SendMsgToActiveView("ViewFit") - - #self.BodyTarget = self.Doc.addObject('PartDesign::Body','Body') - - #Gui.Selection.addSelection(App.ActiveDocument.Pad) - #cobj = CallableComboBox(self) - #QtCore.QTimer.singleShot(500, cobj) - #Gui.runCommand('PartDesign_MoveFeature') - ##assert depenedencies of the Sketch - #self.Doc.recompute() + geoList = [] + geoList.append(Part.LineSegment(App.Vector(-10.000000,10.000000,0),App.Vector(10.000000,10.000000,0))) + geoList.append(Part.LineSegment(App.Vector(10.000000,10.000000,0),App.Vector(10.000000,-10.000000,0))) + geoList.append(Part.LineSegment(App.Vector(10.000000,-10.000000,0),App.Vector(-10.000000,-10.000000,0))) + geoList.append(Part.LineSegment(App.Vector(-10.000000,-10.000000,0),App.Vector(-10.000000,10.000000,0))) + self.Sketch.addGeometry(geoList,False) + conList = [] + conList.append(Sketcher.Constraint('Coincident',0,2,1,1)) + conList.append(Sketcher.Constraint('Coincident',1,2,2,1)) + conList.append(Sketcher.Constraint('Coincident',2,2,3,1)) + conList.append(Sketcher.Constraint('Coincident',3,2,0,1)) + conList.append(Sketcher.Constraint('Horizontal',0)) + conList.append(Sketcher.Constraint('Horizontal',2)) + conList.append(Sketcher.Constraint('Vertical',1)) + conList.append(Sketcher.Constraint('Vertical',3)) + self.Sketch.addConstraint(conList) + + self.Pad = self.Doc.addObject("PartDesign::Pad","Pad") + self.BodySource.addObject(self.Pad) + self.Pad.Profile = self.Sketch + self.Pad.Length = 10.000000 + self.Pad.Length2 = 100.000000 + self.Pad.Type = 0 + self.Pad.UpToFace = None + self.Pad.Reversed = 0 + self.Pad.Midplane = 0 + self.Pad.Offset = 0.000000 + + self.Doc.recompute() + Gui.SendMsgToActiveView("ViewFit") + + self.BodyTarget = self.Doc.addObject('PartDesign::Body','Body') + + Gui.Selection.addSelection(App.ActiveDocument.Pad) + cobj = CallableComboBox(self) + QtCore.QTimer.singleShot(500, cobj) + Gui.runCommand('PartDesign_MoveFeature') + #assert depenedencies of the Sketch + self.Doc.recompute() - #self.assertFalse(self.Sketch.Support[0][0] in self.BodySource.Origin.OriginFeatures) - #self.assertTrue(self.Sketch.Support[0][0] in self.BodyTarget.Origin.OriginFeatures) - #self.assertEqual(len(self.BodySource.Group), 0, "Source body feature count is wrong") - #self.assertEqual(len(self.BodyTarget.Group), 2, "Target body feature count is wrong") + self.assertFalse(self.Sketch.Support[0][0] in self.BodySource.Origin.OriginFeatures) + self.assertTrue(self.Sketch.Support[0][0] in self.BodyTarget.Origin.OriginFeatures) + self.assertEqual(len(self.BodySource.Group), 0, "Source body feature count is wrong") + self.assertEqual(len(self.BodyTarget.Group), 2, "Target body feature count is wrong") def tearDown(self): FreeCAD.closeDocument("SketchGuiTest") From 699abe9123f5e626ddb5876463b0d1ee351c39c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sat, 3 Jun 2017 15:18:40 +0200 Subject: [PATCH 26/29] GeoFeatureGroup: Make link collection non-DAG save --- src/App/GeoFeatureGroupExtension.cpp | 89 +++++++++++---------- src/App/GeoFeatureGroupExtension.h | 18 ++--- src/App/GroupExtension.cpp | 23 ------ src/App/OriginGroupExtension.h | 2 - src/App/Part.h | 2 - src/App/PropertyLinks.cpp | 3 +- src/Mod/Sketcher/App/SketchObject.cpp | 4 +- src/Mod/Sketcher/Gui/ViewProviderSketch.cpp | 6 +- 8 files changed, 60 insertions(+), 87 deletions(-) diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 87bee84eb240..233dd58aeaf2 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -130,7 +130,8 @@ std::vector GeoFeatureGroupExtension::addObjects(std::vector links; + getCSRelevantLinks(object, links); links.push_back(object); for( auto obj : links) { @@ -157,7 +158,8 @@ 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 - auto links = getCSRelevantLinks(object); + std::vector< DocumentObject* > links; + getCSRelevantLinks(object, links); links.push_back(object); //remove all links out of group @@ -206,14 +208,14 @@ std::vector< DocumentObject* > GeoFeatureGroupExtension::getObjectsFromLinks(Doc } -std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSOutList(App::DocumentObject* obj) { +void GeoFeatureGroupExtension::getCSOutList(App::DocumentObject* obj, std::vector< DocumentObject* >& vec) { if(!obj) - return std::vector< App::DocumentObject* >(); + return; //if the object is a geofeaturegroup than all dependencies belong to that CS, we don't want them if(obj->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) - return std::vector< App::DocumentObject* >(); + return; //we get all linked objects. We can't use outList() as this includes the links from expressions auto result = getObjectsFromLinks(obj); @@ -224,37 +226,43 @@ std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSOutList(App::Docum obj->isDerivedFrom(App::Origin::getClassTypeId())); }), result.end()); - //collect all dependencies of those objects - std::vector< App::DocumentObject* > links; + //collect all dependencies of those objects and store them in the result vector for(App::DocumentObject *obj : result) { - auto vec = getCSOutList(obj); - links.insert(links.end(), vec.begin(), vec.end()); - } - - if (!links.empty()) { - result.insert(result.end(), links.begin(), links.end()); - std::sort(result.begin(), result.end()); - result.erase(std::unique(result.begin(), result.end()), result.end()); + + //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()); } - return result; + //post process the vector + std::sort(vec.begin(), vec.end()); + vec.erase(std::unique(vec.begin(), vec.end()), vec.end()); } -std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSInList(DocumentObject* obj) { +void GeoFeatureGroupExtension::getCSInList(DocumentObject* obj, std::vector< DocumentObject* >& vec) { if(!obj) - return std::vector< App::DocumentObject* >(); + 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 or if it is a expression one (could also be both, so it is not //enough to check the expressions) auto res = getObjectsFromLinks(parent); @@ -262,46 +270,39 @@ std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSInList(DocumentObj result.push_back(parent); } - //clear all douplicates + //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 - std::vector< App::DocumentObject* > links; for(App::DocumentObject *obj : result) { - auto vec = getCSInList(obj); - links.insert(links.end(), vec.begin(), vec.end()); + vec.push_back(obj); + getCSInList(obj, vec); } - if (!links.empty()) { - result.insert(result.end(), links.begin(), links.end()); - std::sort(result.begin(), result.end()); - result.erase(std::unique(result.begin(), result.end()), result.end()); - } + std::sort(vec.begin(), vec.end()); + vec.erase(std::unique(vec.begin(), vec.end()), vec.end()); - return result; } -std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSRelevantLinks(DocumentObject* obj) { +void GeoFeatureGroupExtension::getCSRelevantLinks(DocumentObject* obj, std::vector< DocumentObject* >& vec ) { - //we need to get the outlist of all inlist objects and ourself. This is needed to handle things + //get all out links + getCSOutList(obj, 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. - auto in = getCSInList(obj); - in.push_back(obj); //there may be nothing in inlist - std::vector result; - for(auto o : in) { - - auto out = getCSOutList(o); - result.insert(result.end(), out.begin(), out.end()); + std::vector< DocumentObject* > in; + getCSInList(obj, in); + for(auto o : in) { + vec.push_back(o); + getCSOutList(o, vec); } - //there will be many douplicates and also the passed obj in - std::sort(result.begin(), result.end()); - result.erase(std::unique(result.begin(), result.end()), result.end()); - result.erase(std::remove(result.begin(), result.end(), obj), result.end()); - - return result; + //post process + std::sort(vec.begin(), vec.end()); + vec.erase(std::unique(vec.begin(), vec.end()), vec.end()); } // Python feature --------------------------------------------------------- diff --git a/src/App/GeoFeatureGroupExtension.h b/src/App/GeoFeatureGroupExtension.h index 522b32103b64..1718c7149add 100644 --- a/src/App/GeoFeatureGroupExtension.h +++ b/src/App/GeoFeatureGroupExtension.h @@ -73,8 +73,6 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension * In case this object is not part of any geoFeatureGroup 0 is returned. * Unlike DocumentObjectGroup::getGroupOfObject serches only for GeoFeatureGroups * @param obj the object to search for - * @param indirect if true return if the group that so-called geoHas the object, @see geoHasObject() - * default is true */ static DocumentObject* getGroupOfObject(const DocumentObject* obj); @@ -99,18 +97,18 @@ class AppExport GeoFeatureGroupExtension : public App::GroupExtension virtual std::vector< DocumentObject* > addObjects(std::vector< DocumentObject* > obj) override; virtual std::vector< DocumentObject* > removeObjects(std::vector< DocumentObject* > obj) override; - /// returns GeoFeatureGroup relevant objects that are linked from the given one. That meas all linked objects + /// Collects GeoFeatureGroup relevant objects that are linked from the given one. That meas all linked objects /// including their linkes (recursively) except GeoFeatureGroups, where the recursion stops. Expressions - /// links are ignored. - static std::vector getCSOutList(App::DocumentObject* obj); - ///returns GeoFeatureGroup relevant objects that link to the given one. That meas all objects + /// links are ignored. A exception is thrown when there are dependency loops. + static void getCSOutList(App::DocumentObject* obj, std::vector& vec); + /// Collects GeoFeatureGroup relevant objects that link to the given one. That meas all objects /// including their parents (recursively) except GeoFeatureGroups, where the recursion stops. Expression - /// links are ignored - static std::vector getCSInList(App::DocumentObject* obj); - /// Returns all links that are relevant for the coordinate system, meaning all recursive links to + /// links are ignored. A exception is thrown when there are dependency loops. + static void getCSInList(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 std::vector getCSRelevantLinks(App::DocumentObject* obj); + static void getCSRelevantLinks(App::DocumentObject* obj, std::vector& vec); private: Base::Placement recursiveGroupPlacement(GeoFeatureGroupExtension* group); diff --git a/src/App/GroupExtension.cpp b/src/App/GroupExtension.cpp index bd3b123d665a..09b21ec83863 100644 --- a/src/App/GroupExtension.cpp +++ b/src/App/GroupExtension.cpp @@ -104,29 +104,6 @@ std::vector< DocumentObject* > GroupExtension::addObjects(std::vector< DocumentO return added; } -void GroupExtension::addObjects(const std::vector& objs) -{ - bool objectAdded = false; - std::vector grp = Group.getValues(); - for (auto obj : objs) { - if (allowObject(obj)) { - - //only one group per object - auto *group = App::GroupExtension::getGroupOfObject(obj); - if (group && group != getExtendedObject()) - group->getExtensionByType()->removeObject(obj); - - if (std::find(grp.begin(), grp.end(), obj) == grp.end()) { - grp.push_back(obj); - objectAdded = true; - } - } - } - - if (objectAdded) - Group.setValues(grp); -} - std::vector GroupExtension::removeObject(DocumentObject* obj) { std::vector vec = {obj}; diff --git a/src/App/OriginGroupExtension.h b/src/App/OriginGroupExtension.h index d79ed2d2f75a..eecaef29d05d 100644 --- a/src/App/OriginGroupExtension.h +++ b/src/App/OriginGroupExtension.h @@ -52,8 +52,6 @@ class AppExport OriginGroupExtension : public App::GeoFeatureGroupExtension * Returns the origin group which contains this object. * In case this object is not part of any geoFeatureGroup 0 is returned. * @param obj the object to search for - * @param indirect if true return if the group that so-called geoHas the object, @see geoHasObject() - * default is true */ static DocumentObject* getGroupOfObject (const DocumentObject* obj); diff --git a/src/App/Part.h b/src/App/Part.h index a3726251a10e..3f4ed3c97778 100644 --- a/src/App/Part.h +++ b/src/App/Part.h @@ -88,8 +88,6 @@ class AppExport Part : public App::GeoFeature, public App::OriginGroupExtension * Returns the part which contains this object. * In case this object is not belongs to any Part 0 is returned. * @param obj the object to search for - * @param indirect if true return if the part that so-called geoHas the object, @see geoHasObject() - * default is true */ static App::Part* getPartOfObject (const DocumentObject* obj); diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index d7320b47d7a1..d437f67fb7bb 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -108,7 +108,8 @@ void ensureCorrectGroups(PropertyContainer* container, App::DocumentObject* obje if(cont->isRestoring() || object->isRestoring()) return; - auto objs = GeoFeatureGroupExtension::getCSRelevantLinks(object); + std::vector< DocumentObject* > objs; + GeoFeatureGroupExtension::getCSRelevantLinks(object, objs); objs.push_back(object); for(auto obj : objs) { diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 6d660de879ed..3023459e27cb 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -2053,8 +2053,8 @@ bool SketchObject::isCarbonCopyAllowed(App::Document *pDoc, App::DocumentObject //App::DocumentObject *support = this->Support.getValue(); Part::BodyBase* body_this = Part::BodyBase::findBodyOf(this); Part::BodyBase* body_obj = Part::BodyBase::findBodyOf(pObj); - App::Part* part_this = App::Part::getPartOfObject(this, true); - App::Part* part_obj = App::Part::getPartOfObject(pObj, true); + App::Part* part_this = App::Part::getPartOfObject(this); + App::Part* part_obj = App::Part::getPartOfObject(pObj); if (part_this == part_obj){ //either in the same part, or in the root of document if (body_this != NULL) { if ((body_this != body_obj) && !this->allowOtherBody) { diff --git a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp index 368b3b866f38..22bca62d1cd3 100644 --- a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp +++ b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp @@ -5818,9 +5818,9 @@ bool ViewProviderSketch::onDelete(const std::vector &subList) return PartGui::ViewProviderPart::onDelete(subList); } -} - void ViewProviderSketch::showRestoreInformationLayer() { visibleInformationChanged = true ; - draw(false,false); \ No newline at end of file + draw(false,false); + +} \ No newline at end of file From c9b1b83e9dd073f67071098143f93d751f5076da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sat, 3 Jun 2017 15:27:24 +0200 Subject: [PATCH 27/29] Revert link integrity checks in properties The DAG test is not needed anymore as the relevant functions are non-DAG save now, and the other check will be moved to the recompute as it is not efficient or save to do it in the links itself. --- src/App/PropertyLinks.cpp | 119 -------------------------------------- src/Mod/Test/Document.py | 14 +++-- 2 files changed, 8 insertions(+), 125 deletions(-) diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index d437f67fb7bb..cb92d0621cec 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -39,101 +39,11 @@ #include "Document.h" #include "PropertyLinks.h" -#include "GeoFeatureGroupExtension.h" -#include "OriginFeature.h" using namespace App; using namespace Base; using namespace std; - -void ensureDAG(PropertyContainer* container, App::DocumentObject* object) { - - if(!container || !object) - return; - - //document containers and other non-object things don't need to be handled - if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) - return; - - //undo and redo do not need to be handled as they can only go to already checked stated (the link - //state during those actions can get messed up, we really don't want to check for that) - if(object->getDocument()->performsTransactionOperation()) - return; - - auto cont = static_cast(container); - - //on restore we don't need to do anything - if(cont->isRestoring() || object->isRestoring()) - return; - - //recursively traverse the object links and see if we reach the container, which would be a - //cyclic graph --> not allowed - for(auto obj : object->getOutList()) { - if(obj == cont) - throw Base::Exception("This link would create a cyclic dependency, hence it is rejected"); - - ensureDAG(container, obj); - } -}; - -//helper functions to ensure correct geofeaturegroups. Each object is only allowed to be in a -//single geofeatueregroup, and links are not allowed to cross GeoFeatureGroup borders -void ensureCorrectGroups(PropertyContainer* container, App::DocumentObject* object) { - - //on object creation the container may be null, and linked objects may be null anyhow - if(!container || !object) - return; - - //document containers and other non-object things don't need to be handled - if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) - return; - - //links to origin feature can go over CS borders, as they are the same everywere anyway. This is - //a workaround to allow moving of objects between GeoFeatureGroups that link to origin features. - //During movement there is always a link in to the wron CS and the error would occure. If we - //surpress the error at least the origin links can be fixed afterwards - //TODO: Find a more elegant solution - if(object->isDerivedFrom(App::OriginFeature::getClassTypeId())) - return; - - //undo and redo do not need to be handled as they can only go to already checked stated (the link - //state during those actions can get messed up, we really don't want to check for that) - if(object->getDocument()->performsTransactionOperation()) - return; - - auto cont = static_cast(container); - - //on restore we don't need to do anything - if(cont->isRestoring() || object->isRestoring()) - return; - - std::vector< DocumentObject* > objs; - GeoFeatureGroupExtension::getCSRelevantLinks(object, objs); - objs.push_back(object); - - for(auto obj : objs) { - App::DocumentObject* grp = GeoFeatureGroupExtension::getGroupOfObject(obj); - - //if the container is a GeoFeatureGroup there are two allowed possibilites: - //1. the objet is in no group, hence in a single one after adding - //2. the object is already in the container group - if(cont->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) { - - if(!grp || (grp == cont)) - return; - } - //if the container is a normal object there is only a single allowed possibility: - //1. The object has the exact same GeoFeatureGrou as the container (null included) - else if(grp == GeoFeatureGroupExtension::getGroupOfObject(cont)) { - return; - } - - //if we arrive here the container and the object don't have compatible GeoFeatureGroups - throw Base::Exception("Links are only allowed within a GeoFeatureGroup"); - } -}; - //************************************************************************** //************************************************************************** // PropertyLink @@ -162,8 +72,6 @@ PropertyLink::~PropertyLink() void PropertyLink::setValue(App::DocumentObject * lValue) { - ensureDAG(getContainer(), lValue); - ensureCorrectGroups(getContainer(), lValue); aboutToSetValue(); #ifndef USE_OLD_DAG // maintain the back link in the DocumentObject class @@ -297,9 +205,6 @@ int PropertyLinkList::getSize(void) const void PropertyLinkList::setValue(DocumentObject* lValue) { - ensureDAG(getContainer(), lValue); - ensureCorrectGroups(getContainer(), lValue); - #ifndef USE_OLD_DAG //maintain the back link in the DocumentObject class for(auto *obj : _lValueList) @@ -323,11 +228,6 @@ void PropertyLinkList::setValue(DocumentObject* lValue) void PropertyLinkList::setValues(const std::vector& lValue) { - for(auto obj : lValue) { - ensureDAG(getContainer(), obj); - ensureCorrectGroups(getContainer(), obj); - } - aboutToSetValue(); #ifndef USE_OLD_DAG //maintain the back link in the DocumentObject class @@ -480,9 +380,6 @@ PropertyLinkSub::~PropertyLinkSub() void PropertyLinkSub::setValue(App::DocumentObject * lValue, const std::vector &SubList) { - ensureDAG(getContainer(), lValue); - ensureCorrectGroups(getContainer(), lValue); - aboutToSetValue(); #ifndef USE_OLD_DAG if (_pcLinkSub) @@ -679,9 +576,6 @@ int PropertyLinkSubList::getSize(void) const void PropertyLinkSubList::setValue(DocumentObject* lValue,const char* SubName) { - ensureDAG(getContainer(), lValue); - ensureCorrectGroups(getContainer(), lValue); - #ifndef USE_OLD_DAG //maintain backlinks for(auto *obj : _lValueList) @@ -711,11 +605,6 @@ void PropertyLinkSubList::setValues(const std::vector& lValue,c if (lValue.size() != lSubNames.size()) throw Base::ValueError("PropertyLinkSubList::setValues: size of subelements list != size of objects list"); - for(auto obj : lValue) { - ensureDAG(getContainer(), obj); - ensureCorrectGroups(getContainer(), obj); - } - #ifndef USE_OLD_DAG //maintain backlinks. _lValueList can contain items multiple times, but we trust the document //object to ensure that this works @@ -742,11 +631,6 @@ void PropertyLinkSubList::setValues(const std::vector& lValue,c if (lValue.size() != lSubNames.size()) throw Base::ValueError("PropertyLinkSubList::setValues: size of subelements list != size of objects list"); - for(auto obj : lValue) { - ensureDAG(getContainer(), obj); - ensureCorrectGroups(getContainer(), obj); - } - #ifndef USE_OLD_DAG //maintain backlinks. _lValueList can contain items multiple times, but we trust the document //object to ensure that this works @@ -767,9 +651,6 @@ void PropertyLinkSubList::setValues(const std::vector& lValue,c void PropertyLinkSubList::setValue(DocumentObject* lValue, const std::vector &SubList) { - ensureDAG(getContainer(), lValue); - ensureCorrectGroups(getContainer(), lValue); - #ifndef USE_OLD_DAG //maintain backlinks. _lValueList can contain items multiple times, but we trust the document //object to ensure that this works diff --git a/src/Mod/Test/Document.py b/src/Mod/Test/Document.py index 0413e9db7fbf..44b56b86ca8b 100644 --- a/src/Mod/Test/Document.py +++ b/src/Mod/Test/Document.py @@ -870,12 +870,14 @@ def testGroupAndGeoFeatureGroup(self): #to test: try add obj to second group by .Group = [] grp = prt1.Group grp.append(grp2) - try: - prt1.Group=grp - except: - pass - else: - self.fail("No exception at cross geofeaturegroup links") + + #to test: check if cross CS link works + #try: + # prt1.Group=grp + #except: + # pass + #else: + # self.fail("No exception at cross geofeaturegroup links") prt2.addObject(grp1) grp = grp1.Group From 4163282a1d0c0c760a846d95e073ce693e359693 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sat, 3 Jun 2017 15:38:47 +0200 Subject: [PATCH 28/29] DependencyGraph: Ensure OriginFeatures get in the correct subgraph --- src/App/Document.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index d9f9bd23a2e0..fcdf62c7947f 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -399,8 +399,12 @@ void Document::exportGraphviz(std::ostream& out) const if(CSSubgraphs) { if(!sgraph) { auto group = GeoFeatureGroupExtension::getGroupOfObject(docObj); - if(group) - sgraph = GraphList[group]; + if(group) { + if(docObj->isDerivedFrom(App::OriginFeature::getClassTypeId())) + sgraph = GraphList[group->getExtensionByType()->Origin.getValue()]; + else + sgraph = GraphList[group]; + } } if(!sgraph) { if(docObj->isDerivedFrom(OriginFeature::getClassTypeId())) From 3687f01c5fb783884f004c2cc889ca6be8146c0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sat, 3 Jun 2017 16:38:45 +0200 Subject: [PATCH 29/29] Add missing header for random numbers --- src/App/Document.cpp | 1 + src/App/PreCompiled.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index fcdf62c7947f..b8ec2d9dc91a 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -56,6 +56,7 @@ recompute path. Also enables more complicated dependencies beyond trees. # include # include # include +# include #endif #include diff --git a/src/App/PreCompiled.h b/src/App/PreCompiled.h index 3729ed29d641..6cd3a881eb4d 100644 --- a/src/App/PreCompiled.h +++ b/src/App/PreCompiled.h @@ -68,6 +68,7 @@ #include #include #include +#include // Boost #include