From b33f92898b4a1154071831d664ae870968242d09 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 e177b7ffbce6..8fe9f1dc589e 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 ee4ab465260c..431bb690d9a4 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 a2feed0f7bce..11cf85f29d63 100644 --- a/src/Mod/PartDesign/Gui/ReferenceSelection.h +++ b/src/Mod/PartDesign/Gui/ReferenceSelection.h @@ -88,7 +88,7 @@ class CombineSelectionFilterGates: public Gui::SelectionFilterGate }; // 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 267d167866bc..83c7f8448a67 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()) {