Skip to content

Commit

Permalink
PartDesign: [skip ci] Improve revolution function
Browse files Browse the repository at this point in the history
+ 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
  • Loading branch information
wwmayer committed Oct 14, 2020
1 parent 3edbcb7 commit b33f928
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 25 deletions.
9 changes: 8 additions & 1 deletion src/Mod/PartDesign/App/FeatureSketchBased.cpp
Expand Up @@ -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<const Part::Feature*>(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);
Expand Down
25 changes: 14 additions & 11 deletions src/Mod/PartDesign/Gui/ReferenceSelection.cpp
Expand Up @@ -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<std::string>& 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
Expand All @@ -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());
Expand All @@ -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");
}

}
}

Expand All @@ -247,6 +248,8 @@ void getReferencedSelection(const App::DocumentObject* thisObj, const Gui::Selec
}

selSub = std::vector<std::string>(1,subname);

return true;
}

QString getRefStr(const App::DocumentObject* obj, const std::vector<std::string>& sub)
Expand Down
2 changes: 1 addition & 1 deletion src/Mod/PartDesign/Gui/ReferenceSelection.h
Expand Up @@ -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<std::string>& selSub);
/// Return reference as string for UI elements (format <obj>:<subelement>
QString getRefStr(const App::DocumentObject* obj, const std::vector<std::string>& sub);
Expand Down
28 changes: 16 additions & 12 deletions src/Mod/PartDesign/Gui/TaskRevolutionParameters.cpp
Expand Up @@ -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();
Expand Down Expand Up @@ -243,13 +241,12 @@ void TaskRevolutionParameters::onSelectionChanged(const Gui::SelectionChanges& m
exitSelectionMode();
std::vector<std::string> 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();
}
}
}

Expand All @@ -272,6 +269,9 @@ void TaskRevolutionParameters::onAxisChanged(int num)

App::DocumentObject *oldRefAxis = propReferenceAxis->getValue();
std::vector<std::string> oldSubRefAxis = propReferenceAxis->getSubValues();
std::string oldRefName;
if (!oldSubRefAxis.empty())
oldRefName = oldSubRefAxis.front();

App::PropertyLinkSub &lnk = *(axesInList[num]);
if (lnk.getValue() == 0) {
Expand All @@ -289,13 +289,17 @@ void TaskRevolutionParameters::onAxisChanged(int num)
try {
App::DocumentObject *newRefAxis = propReferenceAxis->getValue();
const std::vector<std::string> &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<PartDesign::Revolution*>(pcRevolution)->suggestReversed();
if(pcRevolution->isDerivedFrom(PartDesign::Groove::getClassTypeId()))
if (pcRevolution->isDerivedFrom(PartDesign::Groove::getClassTypeId()))
reversed = static_cast<PartDesign::Groove*>(pcRevolution)->suggestReversed();

if (reversed != propReversed->getValue()) {
Expand Down

0 comments on commit b33f928

Please sign in to comment.