From 28282e680039be67af065c10e849fc45295fd06f Mon Sep 17 00:00:00 2001 From: wmayer Date: Sun, 27 Oct 2024 14:08:26 +0100 Subject: [PATCH] PD: Cleanup code of TaskRevolutionParameters --- .../Gui/TaskRevolutionParameters.cpp | 158 ++++++++++-------- .../PartDesign/Gui/TaskRevolutionParameters.h | 26 +-- 2 files changed, 102 insertions(+), 82 deletions(-) diff --git a/src/Mod/PartDesign/Gui/TaskRevolutionParameters.cpp b/src/Mod/PartDesign/Gui/TaskRevolutionParameters.cpp index 8e2a1bc588..9fb29e7f28 100644 --- a/src/Mod/PartDesign/Gui/TaskRevolutionParameters.cpp +++ b/src/Mod/PartDesign/Gui/TaskRevolutionParameters.cpp @@ -40,6 +40,7 @@ #include "ui_TaskRevolutionParameters.h" #include "TaskRevolutionParameters.h" #include "ViewProviderGroove.h" +#include "ViewProviderRevolution.h" #include "ReferenceSelection.h" using namespace PartDesignGui; @@ -47,12 +48,15 @@ using namespace Gui; /* TRANSLATOR PartDesignGui::TaskRevolutionParameters */ -TaskRevolutionParameters::TaskRevolutionParameters(PartDesignGui::ViewProvider* RevolutionView, const char* pixname, QString title, QWidget *parent) - : TaskSketchBasedParameters(RevolutionView, parent, pixname, title), - ui(new Ui_TaskRevolutionParameters), - proxy(new QWidget(this)), - selectionFace(false), - isGroove(false) +TaskRevolutionParameters::TaskRevolutionParameters(PartDesignGui::ViewProvider* RevolutionView, + const char* pixname, + const QString& title, + QWidget *parent) + : TaskSketchBasedParameters(RevolutionView, parent, pixname, title) + , ui(new Ui_TaskRevolutionParameters) + , proxy(new QWidget(this)) + , selectionFace(false) + , isGroove(false) { // we need a separate container widget to add all controls to ui->setupUi(proxy); @@ -94,18 +98,27 @@ TaskRevolutionParameters::TaskRevolutionParameters(PartDesignGui::ViewProvider* setFocus(); // show the parts coordinate system axis for selection - PartDesign::Body * body = PartDesign::Body::findBodyOf(getObject()); - if (body) { - try { - App::Origin *origin = body->getOrigin(); - auto *vpOrigin = static_cast( - Gui::Application::Instance->getViewProvider(origin)); + try { + if (auto vpOrigin = getOriginView()) { vpOrigin->setTemporaryVisibility(true, false); } - catch (const Base::Exception &ex) { - ex.ReportException(); - } + } + catch (const Base::Exception &ex) { + ex.ReportException(); + } +} + +Gui::ViewProviderOrigin* TaskRevolutionParameters::getOriginView() const +{ + // show the parts coordinate system axis for selection + PartDesign::Body * body = PartDesign::Body::findBodyOf(getObject()); + if (body) { + App::Origin *origin = body->getOrigin(); + return dynamic_cast( + Gui::Application::Instance->getViewProvider(origin)); } + + return nullptr; } void TaskRevolutionParameters::setupDialog() @@ -123,8 +136,9 @@ void TaskRevolutionParameters::setupDialog() int faceId = -1; if (obj && !subStrings.empty()) { upToFace = subStrings.front(); - if (upToFace.compare(0, 4, "Face") == 0) + if (upToFace.compare(0, 4, "Face") == 0) { faceId = std::atoi(&upToFace[4]); + } } // Set object labels @@ -154,7 +168,7 @@ void TaskRevolutionParameters::setupDialog() ui->revolveAngle2->setMaximum(propAngle2->getMaximum()); ui->revolveAngle2->setMinimum(propAngle2->getMinimum()); - index = rev->Type.getValue(); + index = int(rev->Type.getValue()); } else { auto rev = getObject(); @@ -162,7 +176,7 @@ void TaskRevolutionParameters::setupDialog() ui->revolveAngle2->setMaximum(propAngle2->getMaximum()); ui->revolveAngle2->setMinimum(propAngle2->getMinimum()); - index = rev->Type.getValue(); + index = int(rev->Type.getValue()); } translateModeList(index); @@ -188,8 +202,10 @@ void TaskRevolutionParameters::fillAxisCombo(bool forceRefill) { Base::StateLocker lock(getUpdateBlockRef(), true); - if (axesInList.empty()) - forceRefill = true;//not filled yet, full refill + if (axesInList.empty()) { + // not filled yet, full refill + forceRefill = true; + } if (forceRefill) { ui->axis->clear(); @@ -234,31 +250,34 @@ void TaskRevolutionParameters::fillAxisCombo(bool forceRefill) App::DocumentObject* ax = propReferenceAxis->getValue(); const std::vector &subList = propReferenceAxis->getSubValues(); for (size_t i = 0; i < axesInList.size(); i++) { - if (ax == axesInList[i]->getValue() && subList == axesInList[i]->getSubValues()) - indexOfCurrent = i; + if (ax == axesInList[i]->getValue() && subList == axesInList[i]->getSubValues()) { + indexOfCurrent = int(i); + } } if (indexOfCurrent == -1 && ax) { assert(subList.size() <= 1); std::string sub; - if (!subList.empty()) + if (!subList.empty()) { sub = subList[0]; + } addAxisToCombo(ax, sub, getRefStr(ax, subList)); - indexOfCurrent = axesInList.size()-1; + indexOfCurrent = int(axesInList.size()) - 1; } //highlight current. - if (indexOfCurrent != -1) + if (indexOfCurrent != -1) { ui->axis->setCurrentIndex(indexOfCurrent); + } } void TaskRevolutionParameters::addAxisToCombo(App::DocumentObject* linkObj, - std::string linkSubname, - QString itemText) + const std::string& linkSubname, + const QString& itemText) { this->ui->axis->addItem(itemText); this->axesInList.emplace_back(new App::PropertyLinkSub()); App::PropertyLinkSub &lnk = *(axesInList[axesInList.size()-1]); - lnk.setValue(linkObj,std::vector(1,linkSubname)); + lnk.setValue(linkObj, std::vector(1, linkSubname)); } void TaskRevolutionParameters::setCheckboxes(PartDesign::Revolution::RevolMethod mode) @@ -286,9 +305,6 @@ void TaskRevolutionParameters::setCheckboxes(PartDesign::Revolution::RevolMethod isMidplaneVisible = true; isReversedEnabled = !ui->checkBoxMidplane->isChecked(); } - else if (mode == PartDesign::Revolution::RevolMethod::ToLast && !isGroove) { - isReversedEnabled = true; - } else if (mode == PartDesign::Revolution::RevolMethod::ToFirst) { isReversedEnabled = true; } @@ -297,8 +313,9 @@ void TaskRevolutionParameters::setCheckboxes(PartDesign::Revolution::RevolMethod isFaceEditEnabled = true; QMetaObject::invokeMethod(ui->lineFaceName, "setFocus", Qt::QueuedConnection); // Go into reference selection mode if no face has been selected yet - if (ui->lineFaceName->property("FeatureName").isNull()) + if (ui->lineFaceName->property("FeatureName").isNull()) { ui->buttonFace->setChecked(true); + } } else if (mode == PartDesign::Revolution::RevolMethod::TwoDimensions) { isRevolveAngleVisible = true; @@ -330,6 +347,7 @@ void TaskRevolutionParameters::setCheckboxes(PartDesign::Revolution::RevolMethod void TaskRevolutionParameters::connectSignals() { + // clang-format off connect(ui->revolveAngle, qOverload(&Gui::QuantitySpinBox::valueChanged), this, &TaskRevolutionParameters::onAngleChanged); connect(ui->revolveAngle2, qOverload(&Gui::QuantitySpinBox::valueChanged), @@ -348,12 +366,15 @@ void TaskRevolutionParameters::connectSignals() this, &TaskRevolutionParameters::onButtonFace); connect(ui->lineFaceName, &QLineEdit::textEdited, this, &TaskRevolutionParameters::onFaceName); + // clang-format on } void TaskRevolutionParameters::updateUI(int index) { - if (isUpdateBlocked()) + if (isUpdateBlocked()) { return; + } + Base::StateLocker lock(getUpdateBlockRef(), true); fillAxisCombo(); setCheckboxes(static_cast(index)); @@ -380,7 +401,7 @@ void TaskRevolutionParameters::onSelectionChanged(const Gui::SelectionChanges& m else { exitSelectionMode(); std::vector axis; - App::DocumentObject* selObj; + App::DocumentObject* selObj {}; if (getReferencedSelection(getObject(), msg, selObj, axis) && selObj) { propReferenceAxis->setValue(selObj, axis); @@ -394,7 +415,7 @@ void TaskRevolutionParameters::onSelectionChanged(const Gui::SelectionChanges& m } } -void TaskRevolutionParameters::onButtonFace(const bool pressed) +void TaskRevolutionParameters::onButtonFace(bool pressed) { // to distinguish that this is NOT the axis selection selectionFace = pressed; @@ -443,8 +464,7 @@ void TaskRevolutionParameters::translateFaceName() if (ok) { ui->lineFaceName->setText(QString::fromLatin1("%1:%2%3") - .arg(parts[0]) - .arg(tr("Face")) + .arg(parts[0], tr("Face")) .arg(faceId)); } else { @@ -453,7 +473,7 @@ void TaskRevolutionParameters::translateFaceName() } } -QString TaskRevolutionParameters::getFaceName(void) const +QString TaskRevolutionParameters::getFaceName() const { QVariant featureName = ui->lineFaceName->property("FeatureName"); if (featureName.isValid()) { @@ -494,18 +514,21 @@ void TaskRevolutionParameters::onAngle2Changed(double len) void TaskRevolutionParameters::onAxisChanged(int num) { - if (isUpdateBlocked()) + if (isUpdateBlocked()) { return; + } auto pcRevolution = getObject(); - if (axesInList.empty()) + if (axesInList.empty()) { return; + } App::DocumentObject *oldRefAxis = propReferenceAxis->getValue(); std::vector oldSubRefAxis = propReferenceAxis->getSubValues(); std::string oldRefName; - if (!oldSubRefAxis.empty()) + if (!oldSubRefAxis.empty()) { oldRefName = oldSubRefAxis.front(); + } App::PropertyLinkSub &lnk = *(axesInList[num]); if (!lnk.getValue()) { @@ -529,17 +552,20 @@ void TaskRevolutionParameters::onAxisChanged(int num) App::DocumentObject *newRefAxis = propReferenceAxis->getValue(); const std::vector &newSubRefAxis = propReferenceAxis->getSubValues(); std::string newRefName; - if (!newSubRefAxis.empty()) + if (!newSubRefAxis.empty()) { newRefName = newSubRefAxis.front(); + } if (oldRefAxis != newRefAxis || oldSubRefAxis.size() != newSubRefAxis.size() || 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()) { propReversed->setValue(reversed); @@ -574,30 +600,24 @@ void TaskRevolutionParameters::onReversed(bool on) void TaskRevolutionParameters::onModeChanged(int index) { - App::PropertyEnumeration* pcType {}; - if (!isGroove) - pcType = &(getObject()->Type); - else - pcType = &(getObject()->Type); + App::PropertyEnumeration* propEnum = isGroove ? &(getObject()->Type) + : &(getObject()->Type); switch (static_cast(index)) { case PartDesign::Revolution::RevolMethod::Dimension: - pcType->setValue("Angle"); + propEnum->setValue("Angle"); break; case PartDesign::Revolution::RevolMethod::ToLast: - if (!isGroove) - pcType->setValue("UpToLast"); - else - pcType->setValue("ThroughAll"); + propEnum->setValue(isGroove ? "ThroughAll" : "UpToLast"); break; case PartDesign::Revolution::RevolMethod::ToFirst: - pcType->setValue("UpToFirst"); + propEnum->setValue("UpToFirst"); break; case PartDesign::Revolution::RevolMethod::ToFace: - pcType->setValue("UpToFace"); + propEnum->setValue("UpToFace"); break; case PartDesign::Revolution::RevolMethod::TwoDimensions: - pcType->setValue("TwoAngles"); + propEnum->setValue("TwoAngles"); break; } @@ -616,15 +636,14 @@ void TaskRevolutionParameters::getReferenceAxis(App::DocumentObject*& obj, std:: if (!lnk.getValue()) { throw Base::RuntimeError("Still in reference selection mode; reference wasn't selected yet"); } - else { - auto revolution = getObject(); - if (!revolution->getDocument()->isIn(lnk.getValue())){ - throw Base::RuntimeError("Object was deleted"); - } - obj = lnk.getValue(); - sub = lnk.getSubValues(); + auto revolution = getObject(); + if (!revolution->getDocument()->isIn(lnk.getValue())){ + throw Base::RuntimeError("Object was deleted"); } + + obj = lnk.getValue(); + sub = lnk.getSubValues(); } bool TaskRevolutionParameters::getMidplane() const @@ -639,14 +658,9 @@ bool TaskRevolutionParameters::getReversed() const TaskRevolutionParameters::~TaskRevolutionParameters() { + //hide the parts coordinate system axis for selection try { - //hide the parts coordinate system axis for selection - auto obj = getObject(); - PartDesign::Body * body = obj ? PartDesign::Body::findBodyOf(obj) : nullptr; - if (body) { - App::Origin *origin = body->getOrigin(); - ViewProviderOrigin* vpOrigin; - vpOrigin = static_cast(Gui::Application::Instance->getViewProvider(origin)); + if (auto vpOrigin = getOriginView()) { vpOrigin->resetTemporaryVisibility(); } } @@ -674,7 +688,7 @@ void TaskRevolutionParameters::apply() ui->revolveAngle->apply(); ui->revolveAngle2->apply(); std::vector sub; - App::DocumentObject* obj; + App::DocumentObject* obj {}; getReferenceAxis(obj, sub); std::string axis = buildLinkSingleSubPythonStr(obj, sub); auto tobj = getObject(); diff --git a/src/Mod/PartDesign/Gui/TaskRevolutionParameters.h b/src/Mod/PartDesign/Gui/TaskRevolutionParameters.h index 3bc9066796..87df41a7c5 100644 --- a/src/Mod/PartDesign/Gui/TaskRevolutionParameters.h +++ b/src/Mod/PartDesign/Gui/TaskRevolutionParameters.h @@ -26,8 +26,6 @@ #include #include #include "TaskSketchBasedParameters.h" -#include "ViewProviderRevolution.h" -#include "ViewProviderGroove.h" class Ui_TaskRevolutionParameters; @@ -38,19 +36,22 @@ class Property; namespace Gui { class ViewProvider; +class ViewProviderOrigin; } namespace PartDesignGui { +class ViewProviderRevolution; +class ViewProviderGroove; class TaskRevolutionParameters : public TaskSketchBasedParameters { Q_OBJECT public: - explicit TaskRevolutionParameters(ViewProvider* RevolutionView, - const char *pixname, - QString title = tr("Revolution parameters"), - QWidget* parent = nullptr); + TaskRevolutionParameters(ViewProvider* RevolutionView, + const char *pixname, + const QString& title, + QWidget* parent = nullptr); ~TaskRevolutionParameters() override; void apply() override; @@ -63,7 +64,9 @@ public: * list (if necessary), and selected. If the list is empty, it will be refilled anyway. */ void fillAxisCombo(bool forceRefill = false); - void addAxisToCombo(App::DocumentObject *linkObj, std::string linkSubname, QString itemText); + void addAxisToCombo(App::DocumentObject *linkObj, + const std::string& linkSubname, + const QString& itemText); private Q_SLOTS: void onAngleChanged(double); @@ -72,7 +75,7 @@ private Q_SLOTS: void onMidplane(bool); void onReversed(bool); void onModeChanged(int); - void onButtonFace(const bool pressed = true); + void onButtonFace(bool pressed = true); void onFaceName(const QString& text); protected: @@ -82,9 +85,10 @@ protected: bool getMidplane() const; bool getReversed() const; QString getFaceName() const; - void setupDialog(void); + void setupDialog(); void setCheckboxes(PartDesign::Revolution::RevolMethod mode); +private: //mirrors of revolution's or groove's properties //should have been done by inheriting revolution and groove from common class... App::PropertyAngle* propAngle; @@ -101,6 +105,7 @@ private: // TODO: This is common with extrude. Maybe send to superclass. void translateFaceName(); void clearFaceName(); + Gui::ViewProviderOrigin* getOriginView() const; private: std::unique_ptr ui; @@ -119,7 +124,6 @@ private: std::vector> axesInList; }; -/// simulation dialog for the TaskView class TaskDlgRevolutionParameters : public TaskDlgSketchBasedParameters { Q_OBJECT @@ -127,6 +131,7 @@ class TaskDlgRevolutionParameters : public TaskDlgSketchBasedParameters public: explicit TaskDlgRevolutionParameters(PartDesignGui::ViewProviderRevolution *RevolutionView); }; + class TaskDlgGrooveParameters : public TaskDlgSketchBasedParameters { Q_OBJECT @@ -134,6 +139,7 @@ class TaskDlgGrooveParameters : public TaskDlgSketchBasedParameters public: explicit TaskDlgGrooveParameters(PartDesignGui::ViewProviderGroove *GrooveView); }; + } //namespace PartDesignGui #endif // GUI_TASKVIEW_TASKAPPERANCE_H