From ad48dd2538269825d446a6efacf299a867c986cc Mon Sep 17 00:00:00 2001 From: wmayer Date: Wed, 14 Oct 2020 15:20:52 +0200 Subject: [PATCH] PartDesign: [skip ci] Improve revolution function + Fix hard crash when selecting an edge or face of the created feature before selecting option to choose a reference. + In ProfileBased::getAxis() convert OCCT into FreeCAD exception to simplify handling of calling instances. + Change return value of getReferencedSelection() from void to bool to mkae it easier for calling instance to detect if the selection failed. This is needed to avoid to add invalid items to the combo box of the revolution task panel --- src/Mod/PartDesign/App/FeatureSketchBased.cpp | 9 +++++- src/Mod/PartDesign/Gui/ReferenceSelection.cpp | 25 +++++++++-------- src/Mod/PartDesign/Gui/ReferenceSelection.h | 2 +- .../Gui/TaskRevolutionParameters.cpp | 28 +++++++++++-------- 4 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/Mod/PartDesign/App/FeatureSketchBased.cpp b/src/Mod/PartDesign/App/FeatureSketchBased.cpp index e177b7ffbc..8fe9f1dc58 100644 --- a/src/Mod/PartDesign/App/FeatureSketchBased.cpp +++ b/src/Mod/PartDesign/App/FeatureSketchBased.cpp @@ -1092,7 +1092,14 @@ void ProfileBased::getAxis(const App::DocumentObject *pcReferenceAxis, const std throw Base::ValueError("No rotation axis reference specified"); const Part::Feature* refFeature = static_cast(pcReferenceAxis); Part::TopoShape refShape = refFeature->Shape.getShape(); - TopoDS_Shape ref = refShape.getSubShape(subReferenceAxis[0].c_str()); + TopoDS_Shape ref; + try { + // if an exception is raised then convert it into a FreeCAD-specific exception + ref = refShape.getSubShape(subReferenceAxis[0].c_str()); + } + catch (const Standard_Failure& e) { + throw Base::RuntimeError(e.GetMessageString()); + } if (ref.ShapeType() == TopAbs_EDGE) { TopoDS_Edge refEdge = TopoDS::Edge(ref); diff --git a/src/Mod/PartDesign/Gui/ReferenceSelection.cpp b/src/Mod/PartDesign/Gui/ReferenceSelection.cpp index ee4ab46526..431bb690d9 100644 --- a/src/Mod/PartDesign/Gui/ReferenceSelection.cpp +++ b/src/Mod/PartDesign/Gui/ReferenceSelection.cpp @@ -195,21 +195,22 @@ bool CombineSelectionFilterGates::allow(App::Document* pDoc, App::DocumentObject namespace PartDesignGui { -void getReferencedSelection(const App::DocumentObject* thisObj, const Gui::SelectionChanges& msg, +bool getReferencedSelection(const App::DocumentObject* thisObj, const Gui::SelectionChanges& msg, App::DocumentObject*& selObj, std::vector& selSub) { + selObj = nullptr; if (!thisObj) - return; + return false; if (strcmp(thisObj->getDocument()->getName(), msg.pDocName) != 0) - return; - + return false; + selObj = thisObj->getDocument()->getObject(msg.pObjectName); if (selObj == thisObj) - return; - + return false; + std::string subname = msg.pSubName; - + //check if the selection is an external reference and ask the user what to do //of course only if thisObj is in a body, as otherwise the old workflow would not //be supported @@ -224,10 +225,11 @@ void getReferencedSelection(const App::DocumentObject* thisObj, const Gui::Selec dia.setModal(true); int result = dia.exec(); if (result == QDialog::DialogCode::Rejected) { - selObj = NULL; - return; + selObj = nullptr; + return false; } - else if (!dlg.radioXRef->isChecked()) { + + if (!dlg.radioXRef->isChecked()) { App::Document* document = thisObj->getDocument(); document->openTransaction("Make copy"); auto copy = PartDesignGui::TaskFeaturePick::makeCopy(selObj, subname, dlg.radioIndependent->isChecked()); @@ -237,7 +239,6 @@ void getReferencedSelection(const App::DocumentObject* thisObj, const Gui::Selec subname.erase(std::remove_if(subname.begin(), subname.end(), &isdigit), subname.end()); subname.append("1"); } - } } @@ -247,6 +248,8 @@ void getReferencedSelection(const App::DocumentObject* thisObj, const Gui::Selec } selSub = std::vector(1,subname); + + return true; } QString getRefStr(const App::DocumentObject* obj, const std::vector& sub) diff --git a/src/Mod/PartDesign/Gui/ReferenceSelection.h b/src/Mod/PartDesign/Gui/ReferenceSelection.h index a2feed0f7b..11cf85f29d 100644 --- a/src/Mod/PartDesign/Gui/ReferenceSelection.h +++ b/src/Mod/PartDesign/Gui/ReferenceSelection.h @@ -88,7 +88,7 @@ public: }; // Convenience methods /// Extract reference from Selection -void getReferencedSelection(const App::DocumentObject* thisObj, const Gui::SelectionChanges& msg, +bool getReferencedSelection(const App::DocumentObject* thisObj, const Gui::SelectionChanges& msg, App::DocumentObject*& selObj, std::vector& selSub); /// Return reference as string for UI elements (format : QString getRefStr(const App::DocumentObject* obj, const std::vector& sub); diff --git a/src/Mod/PartDesign/Gui/TaskRevolutionParameters.cpp b/src/Mod/PartDesign/Gui/TaskRevolutionParameters.cpp index 267d167866..83c7f8448a 100644 --- a/src/Mod/PartDesign/Gui/TaskRevolutionParameters.cpp +++ b/src/Mod/PartDesign/Gui/TaskRevolutionParameters.cpp @@ -172,9 +172,7 @@ void TaskRevolutionParameters::fillAxisCombo(bool forceRefill) } //add part axes - App::DocumentObject* obj = vp->getObject(); - - PartDesign::Body * body = PartDesign::Body::findBodyOf ( obj ); + PartDesign::Body * body = PartDesign::Body::findBodyOf ( pcFeat ); if (body) { try { App::Origin* orig = body->getOrigin(); @@ -243,13 +241,12 @@ void TaskRevolutionParameters::onSelectionChanged(const Gui::SelectionChanges& m exitSelectionMode(); std::vector axis; App::DocumentObject* selObj; - getReferencedSelection(vp->getObject(), msg, selObj, axis); - if(!selObj) - return; - propReferenceAxis->setValue(selObj, axis); + if (getReferencedSelection(vp->getObject(), msg, selObj, axis) && selObj) { + propReferenceAxis->setValue(selObj, axis); - recomputeFeature(); - updateUI(); + recomputeFeature(); + updateUI(); + } } } @@ -272,6 +269,9 @@ void TaskRevolutionParameters::onAxisChanged(int num) App::DocumentObject *oldRefAxis = propReferenceAxis->getValue(); std::vector oldSubRefAxis = propReferenceAxis->getSubValues(); + std::string oldRefName; + if (!oldSubRefAxis.empty()) + oldRefName = oldSubRefAxis.front(); App::PropertyLinkSub &lnk = *(axesInList[num]); if (lnk.getValue() == 0) { @@ -289,13 +289,17 @@ void TaskRevolutionParameters::onAxisChanged(int num) try { App::DocumentObject *newRefAxis = propReferenceAxis->getValue(); const std::vector &newSubRefAxis = propReferenceAxis->getSubValues(); + std::string newRefName; + if (!newSubRefAxis.empty()) + newRefName = newSubRefAxis.front(); + if (oldRefAxis != newRefAxis || oldSubRefAxis.size() != newSubRefAxis.size() || - oldSubRefAxis[0] != newSubRefAxis[0]) { + oldRefName != newRefName) { bool reversed = propReversed->getValue(); - if(pcRevolution->isDerivedFrom(PartDesign::Revolution::getClassTypeId())) + if (pcRevolution->isDerivedFrom(PartDesign::Revolution::getClassTypeId())) reversed = static_cast(pcRevolution)->suggestReversed(); - if(pcRevolution->isDerivedFrom(PartDesign::Groove::getClassTypeId())) + if (pcRevolution->isDerivedFrom(PartDesign::Groove::getClassTypeId())) reversed = static_cast(pcRevolution)->suggestReversed(); if (reversed != propReversed->getValue()) {