From 7683bb2b4f3c81cddca480efdb6bce132c659b00 Mon Sep 17 00:00:00 2001 From: wmayer Date: Mon, 3 Dec 2018 17:54:05 +0100 Subject: [PATCH] Fix usage of Reference dialog: + fix memory leak by using QDialog on the stack + open a transaction where needed when making a copy to avoid mess with undo/redo + set proper title and use main window as parent + replace class name Dialog with PartDesignGui::DlgReference --- src/Mod/PartDesign/Gui/Command.cpp | 76 ++++++++++--------- src/Mod/PartDesign/Gui/DlgReference.ui | 10 +-- src/Mod/PartDesign/Gui/ReferenceSelection.cpp | 36 ++++----- .../PartDesign/Gui/TaskDatumParameters.cpp | 48 ++++++------ src/Mod/PartDesign/Gui/TaskPipeParameters.cpp | 26 +++---- 5 files changed, 99 insertions(+), 97 deletions(-) diff --git a/src/Mod/PartDesign/Gui/Command.cpp b/src/Mod/PartDesign/Gui/Command.cpp index 474b479637..d02dbe679d 100644 --- a/src/Mod/PartDesign/Gui/Command.cpp +++ b/src/Mod/PartDesign/Gui/Command.cpp @@ -487,30 +487,32 @@ void CmdPartDesignNewSketch::activated(int iMsg) //check the prerequisites for the selected objects //the user has to decide which option we should take if external references are used // TODO share this with UnifiedDatumCommand() (2015-10-20, Fat-Zer) - QDialog* dia = new QDialog; - Ui_Dialog dlg; - dlg.setupUi(dia); - dia->setModal(true); - int result = dia->exec(); - if(result == QDialog::DialogCode::Rejected) + QDialog dia(Gui::getMainWindow()); + PartDesignGui::Ui_DlgReference dlg; + dlg.setupUi(&dia); + dia.setModal(true); + int result = dia.exec(); + if (result == QDialog::DialogCode::Rejected) return; - else if(!dlg.radioXRef->isChecked()) { - + else if (!dlg.radioXRef->isChecked()) { + openCommand("Make copy"); std::string sub; if(FaceFilter.match()) sub = FaceFilter.Result[0][0].getSubNames()[0]; auto copy = PartDesignGui::TaskFeaturePick::makeCopy(obj, sub, dlg.radioIndependent->isChecked()); - if(pcActiveBody) + if (pcActiveBody) pcActiveBody->addObject(copy); else if (pcActivePart) pcActivePart->addObject(copy); - if(PlaneFilter.match()) + if (PlaneFilter.match()) supportString = std::string("(App.activeDocument().") + copy->getNameInDocument() + ", '')"; else //it is ensured that only a single face is selected, hence it must always be Face1 of the shapebinder supportString = std::string("(App.activeDocument().") + copy->getNameInDocument() + ", 'Face1')"; + + commitCommand(); } } } @@ -842,7 +844,7 @@ void prepareProfileBased(Gui::Command* cmd, const std::string& which, Gui::Command::openCommand((std::string("Make ") + which).c_str()); Gui::Command::doCommand(Gui::Command::Doc,"App.activeDocument().%s.newObject(\"PartDesign::%s\",\"%s\")", PartDesignGui::getBody(false)->getNameInDocument(), which.c_str(), FeatName.c_str()); - + if (feature->isDerivedFrom(Part::Part2DObject::getClassTypeId())) { Gui::Command::doCommand(Gui::Command::Doc,"App.activeDocument().%s.Profile = App.activeDocument().%s", FeatName.c_str(), feature->getNameInDocument()); @@ -850,11 +852,11 @@ void prepareProfileBased(Gui::Command* cmd, const std::string& which, else { Gui::Command::doCommand(Gui::Command::Doc,"App.activeDocument().%s.Profile = (App.activeDocument().%s, [\"%s\"])", FeatName.c_str(), feature->getNameInDocument(), sub.c_str()); - } + } func(static_cast(feature), FeatName); }; - + //if a profile is selected we can make our life easy and fast std::vector selection = cmd->getSelection().getSelectionEx(); if (!selection.empty() && selection.front().hasSubNames()) { @@ -872,7 +874,7 @@ void prepareProfileBased(Gui::Command* cmd, const std::string& which, sketches = cmd->getDocument()->getObjectsOfType(Part::Part2DObject::getClassTypeId()); bNoSketchWasSelected = true; } - + if (sketches.empty()) { QMessageBox::warning(Gui::getMainWindow(), QObject::tr("No sketch to work on"), QObject::tr("No sketch is available in the document")); @@ -890,52 +892,52 @@ void prepareProfileBased(Gui::Command* cmd, const std::string& which, return true; }; - + auto sketch_worker = [&, base_worker](std::vector features) { - base_worker(features.front(), ""); }; - + //if there is a sketch selected which is from another body or part we need to bring up the //pick task dialog to decide how those are handled - bool ext = std::find_if( status.begin(), status.end(), + bool extReference = std::find_if( status.begin(), status.end(), [] (const PartDesignGui::TaskFeaturePick::featureStatus& s) { return s == PartDesignGui::TaskFeaturePick::otherBody || s == PartDesignGui::TaskFeaturePick::otherPart || s == PartDesignGui::TaskFeaturePick::notInBody; } ) != status.end(); - + // TODO Clean this up (2015-10-20, Fat-Zer) auto* pcActiveBody = PartDesignGui::getBody(false); - if (pcActiveBody && !bNoSketchWasSelected && ext) { + if (pcActiveBody && !bNoSketchWasSelected && extReference) { auto* pcActivePart = PartDesignGui::getPartFor(pcActiveBody, true); // getPartFor() already has reported an error if (!pcActivePart) return; - QDialog* dia = new QDialog; - Ui_Dialog dlg; - dlg.setupUi(dia); - dia->setModal(true); - int result = dia->exec(); - if(result == QDialog::DialogCode::Rejected) + QDialog dia(Gui::getMainWindow()); + PartDesignGui::Ui_DlgReference dlg; + dlg.setupUi(&dia); + dia.setModal(true); + int result = dia.exec(); + if (result == QDialog::DialogCode::Rejected) return; - else if(!dlg.radioXRef->isChecked()) { - auto copy = PartDesignGui::TaskFeaturePick::makeCopy(sketches[0], "", dlg.radioIndependent->isChecked()); - auto oBody = PartDesignGui::getBodyFor(sketches[0], false); - if(oBody) - pcActiveBody->addObject(copy); - else - pcActivePart->addObject(copy); + if (!dlg.radioXRef->isChecked()) { + Gui::Command::openCommand("Make copy"); + auto copy = PartDesignGui::TaskFeaturePick::makeCopy(sketches[0], "", dlg.radioIndependent->isChecked()); + auto oBody = PartDesignGui::getBodyFor(sketches[0], false); + if (oBody) + pcActiveBody->addObject(copy); + else + pcActivePart->addObject(copy); - sketches[0] = copy; - firstFreeSketch = sketches.begin(); + sketches[0] = copy; + firstFreeSketch = sketches.begin(); } } - + // Show sketch choose dialog and let user pick sketch if no sketch was selected and no free one available or // multiple free ones are available if (bNoSketchWasSelected && (freeSketches != 1) ) { @@ -960,7 +962,7 @@ void prepareProfileBased(Gui::Command* cmd, const std::string& which, Gui::Selection().clearSelection(); pickDlg = new PartDesignGui::TaskDlgFeaturePick(sketches, status, accepter, sketch_worker); - if (!bNoSketchWasSelected && ext) + if (!bNoSketchWasSelected && extReference) pickDlg->showExternal(true); Gui::Control().showDialog(pickDlg); diff --git a/src/Mod/PartDesign/Gui/DlgReference.ui b/src/Mod/PartDesign/Gui/DlgReference.ui index 41b80824b7..9d2bd11b90 100644 --- a/src/Mod/PartDesign/Gui/DlgReference.ui +++ b/src/Mod/PartDesign/Gui/DlgReference.ui @@ -1,7 +1,7 @@ - Dialog - + PartDesignGui::DlgReference + 0 @@ -11,7 +11,7 @@ - Dialog + Reference @@ -93,7 +93,7 @@ buttonBox accepted() - Dialog + PartDesignGui::DlgReference accept() @@ -109,7 +109,7 @@ buttonBox rejected() - Dialog + PartDesignGui::DlgReference reject() diff --git a/src/Mod/PartDesign/Gui/ReferenceSelection.cpp b/src/Mod/PartDesign/Gui/ReferenceSelection.cpp index 62e572b930..064b4cfa31 100644 --- a/src/Mod/PartDesign/Gui/ReferenceSelection.cpp +++ b/src/Mod/PartDesign/Gui/ReferenceSelection.cpp @@ -28,7 +28,7 @@ # include # include # include -#include +# include #endif #include @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -213,32 +214,33 @@ void getReferencedSelection(const App::DocumentObject* thisObj, const Gui::Selec //be supportet PartDesign::Body* body = PartDesignGui::getBodyFor(thisObj, false); bool originfeature = selObj->isDerivedFrom(App::OriginFeature::getClassTypeId()); - if(!originfeature && body) { + if (!originfeature && body) { PartDesign::Body* selBody = PartDesignGui::getBodyFor(selObj, false); if(!selBody || body != selBody) { auto* pcActivePart = PartDesignGui::getPartFor(body, false); - QDialog* dia = new QDialog; - Ui_Dialog dlg; - dlg.setupUi(dia); - dia->setModal(true); - int result = dia->exec(); - if(result == QDialog::DialogCode::Rejected) { + QDialog dia(Gui::getMainWindow()); + Ui_DlgReference dlg; + dlg.setupUi(&dia); + dia.setModal(true); + int result = dia.exec(); + if (result == QDialog::DialogCode::Rejected) { selObj = NULL; return; } else if(!dlg.radioXRef->isChecked()) { + App::Document* document = thisObj->getDocument(); + document->openTransaction("Make copy"); + auto copy = PartDesignGui::TaskFeaturePick::makeCopy(selObj, subname, dlg.radioIndependent->isChecked()); + if (body) + body->addObject(copy); + else if (pcActivePart) + pcActivePart->addObject(copy); - auto copy = PartDesignGui::TaskFeaturePick::makeCopy(selObj, subname, dlg.radioIndependent->isChecked()); - if(selBody) - body->addObject(copy); - else - pcActivePart->addObject(copy); - - selObj = copy; - subname.erase(std::remove_if(subname.begin(), subname.end(), &isdigit), subname.end()); - subname.append("1"); + selObj = copy; + subname.erase(std::remove_if(subname.begin(), subname.end(), &isdigit), subname.end()); + subname.append("1"); } } diff --git a/src/Mod/PartDesign/Gui/TaskDatumParameters.cpp b/src/Mod/PartDesign/Gui/TaskDatumParameters.cpp index a132c292e1..7d7d5e46dc 100644 --- a/src/Mod/PartDesign/Gui/TaskDatumParameters.cpp +++ b/src/Mod/PartDesign/Gui/TaskDatumParameters.cpp @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -109,7 +110,6 @@ bool TaskDlgDatumParameters::reject() { bool TaskDlgDatumParameters::accept() { - std::string name = ViewProvider->getObject()->getNameInDocument(); Part::Datum* pcDatum = static_cast(ViewProvider->getObject()); auto pcActiveBody = PartDesignGui::getBodyFor(pcDatum, false); auto pcActivePart = PartDesignGui::getPartFor(pcActiveBody, false); @@ -135,34 +135,34 @@ bool TaskDlgDatumParameters::accept() { //see what to do with external references //check the prerequisites for the selected objects //the user has to decide which option we should take if external references are used - bool ext = false; - for(App::DocumentObject* obj : pcDatum->Support.getValues()) { - if(!pcActiveBody->hasObject(obj) && !pcActiveBody->getOrigin()->hasObject(obj)) - ext = true; + bool extReference = false; + for (App::DocumentObject* obj : pcDatum->Support.getValues()) { + if (!pcActiveBody->hasObject(obj) && !pcActiveBody->getOrigin()->hasObject(obj)) + extReference = true; } - if(ext) { - // TODO: rewrite this to be shared with CmdPartDesignNewSketch::activated() (2015-10-20, Fat-Zer) - QDialog* dia = new QDialog; - Ui_Dialog dlg; - dlg.setupUi(dia); - dia->setModal(true); - int result = dia->exec(); - if(result == QDialog::DialogCode::Rejected) - return false; - else if(!dlg.radioXRef->isChecked()) { + if(extReference) { + // TODO: rewrite this to be shared with CmdPartDesignNewSketch::activated() (2015-10-20, Fat-Zer) + QDialog dia(Gui::getMainWindow()); + PartDesignGui::Ui_DlgReference dlg; + dlg.setupUi(&dia); + dia.setModal(true); + int result = dia.exec(); + if (result == QDialog::DialogCode::Rejected) + return false; + else if (!dlg.radioXRef->isChecked()) { std::vector objs; std::vector subs = pcDatum->Support.getSubValues(); int index = 0; - for(App::DocumentObject* obj : pcDatum->Support.getValues()) { - - if(!pcActiveBody->hasObject(obj) && !pcActiveBody->getOrigin()->hasObject(obj)) { + for (App::DocumentObject* obj : pcDatum->Support.getValues()) { + if (!pcActiveBody->hasObject(obj) && !pcActiveBody->getOrigin()->hasObject(obj)) { objs.push_back(PartDesignGui::TaskFeaturePick::makeCopy(obj, subs[index], dlg.radioIndependent->isChecked())); copies.push_back(objs.back()); subs[index] = ""; } - else + else { objs.push_back(obj); + } index++; } @@ -170,18 +170,18 @@ bool TaskDlgDatumParameters::accept() { pcDatum->Support.setValues(objs, subs); } } - - if(!PartGui::TaskDlgAttacher::accept()) + + if (!PartGui::TaskDlgAttacher::accept()) return false; - + //we need to add the copied features to the body after the command action, as otherwise FreeCAD crashes unexplainably for(auto obj : copies) { - if(pcActiveBody) + if (pcActiveBody) pcActiveBody->addObject(obj); else if (pcActivePart) pcActivePart->addObject(obj); } - + return true; } diff --git a/src/Mod/PartDesign/Gui/TaskPipeParameters.cpp b/src/Mod/PartDesign/Gui/TaskPipeParameters.cpp index 801603e84f..ab4fa5b03a 100644 --- a/src/Mod/PartDesign/Gui/TaskPipeParameters.cpp +++ b/src/Mod/PartDesign/Gui/TaskPipeParameters.cpp @@ -50,6 +50,7 @@ #include #include #include +#include #include #include #include @@ -805,9 +806,6 @@ TaskDlgPipeParameters::~TaskDlgPipeParameters() bool TaskDlgPipeParameters::accept() { - std::string name = vp->getObject()->getNameInDocument(); - - //see what to do with external references //check the prerequisites for the selected objects //the user has to decide which option we should take if external references are used @@ -816,26 +814,26 @@ bool TaskDlgPipeParameters::accept() //auto pcActivePart = PartDesignGui::getPartFor(pcActiveBody, false); std::vector copies; - bool ext = false; + bool extReference = false; if(!pcActiveBody->hasObject(pcPipe->Spine.getValue()) && !pcActiveBody->getOrigin()->hasObject(pcPipe->Spine.getValue())) - ext = true; + extReference = true; else if(pcPipe->AuxillerySpine.getValue() && !pcActiveBody->hasObject(pcPipe->AuxillerySpine.getValue()) && !pcActiveBody->getOrigin()->hasObject(pcPipe->AuxillerySpine.getValue())) - ext = true; + extReference = true; else { for(App::DocumentObject* obj : pcPipe->Sections.getValues()) { if(!pcActiveBody->hasObject(obj) && !pcActiveBody->getOrigin()->hasObject(obj)) - ext = true; + extReference = true; } } - if(ext) { - QDialog* dia = new QDialog; - Ui_Dialog dlg; - dlg.setupUi(dia); - dia->setModal(true); - int result = dia->exec(); - if(result == QDialog::DialogCode::Rejected) + if (extReference) { + QDialog dia(Gui::getMainWindow()); + Ui_DlgReference dlg; + dlg.setupUi(&dia); + dia.setModal(true); + int result = dia.exec(); + if (result == QDialog::DialogCode::Rejected) return false; else if(!dlg.radioXRef->isChecked()) {