Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[PD] Pad/Pocket: sort out duplicated code #5355

Closed
wants to merge 1 commit into from
Closed

[PD] Pad/Pocket: sort out duplicated code #5355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
80 changes: 78 additions & 2 deletions src/Mod/PartDesign/Gui/TaskExtrudeParameters.cpp
View file
Open in desktop
Expand Up @@ -25,10 +25,10 @@

#ifndef _PreComp_
# include <sstream>
# include <QRegExp>
# include <QTextStream>
# include <Precision.hxx>
# include <QRegExp>
# include <QSignalBlocker>
# include <QTextStream>
#endif

#include "ui_TaskPadPocketParameters.h"
Expand Down Expand Up @@ -389,6 +389,82 @@ void TaskExtrudeParameters::addAxisToCombo(App::DocumentObject* linkObj, std::st
lnk.setValue(linkObj, std::vector<std::string>(1, linkSubname));
}

void TaskExtrudeParameters::setCheckboxes(int index, Modes mode)
{
// disable/hide everything unless we are sure we don't need it
// exception: the direction parameters are in any case visible
bool isLengthEditVisible = false;
bool isLengthEdit2Visible = false;
bool isOffsetEditVisible = false;
bool isOffsetEditEnabled = true;
bool isMidplaneEnabled = false;
bool isMidplaneVisible = false;
bool isReversedEnabled = false;
bool isFaceEditEnabled = false;

if (mode == Modes::Dimension) {
isLengthEditVisible = true;
ui->lengthEdit->selectNumber();
QMetaObject::invokeMethod(ui->lengthEdit, "setFocus", Qt::QueuedConnection);
isMidplaneVisible = true;
isMidplaneEnabled = true;
// Reverse only makes sense if Midplane is not true
isReversedEnabled = !ui->checkBoxMidplane->isChecked();
}
else if (mode == Modes::ThroughAll) {
isOffsetEditVisible = true;
isOffsetEditEnabled = false; // offset may have some meaning for through all but it doesn't work
isMidplaneEnabled = true;
isReversedEnabled = !ui->checkBoxMidplane->isChecked();
}
else if (mode == Modes::ToFirst) {
isOffsetEditVisible = true;
isReversedEnabled = true;
}
else if (mode == Modes::ToLast) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToLast is equal to ThroughAll and thus this else block is dead code. So, what's still needed is the extrude type (Pad or Pocket) to make sure ThroughAll is only handled for Pockets and ToLast only for Pads.

isOffsetEditVisible = true;
isReversedEnabled = true;
}
else if (mode == Modes::ToFace) {
isOffsetEditVisible = true;
isReversedEnabled = true;
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())
onButtonFace(true);
}
else if (mode == Modes::TwoDimensions) {
isLengthEditVisible = true;
isLengthEdit2Visible = true;
isReversedEnabled = true;
}

ui->lengthEdit->setVisible(isLengthEditVisible);
ui->lengthEdit->setEnabled(isLengthEditVisible);
ui->labelLength->setVisible(isLengthEditVisible);
ui->checkBoxAlongDirection->setVisible(isLengthEditVisible);

ui->lengthEdit2->setVisible(isLengthEdit2Visible);
ui->lengthEdit2->setEnabled(isLengthEdit2Visible);
ui->labelLength2->setVisible(isLengthEdit2Visible);

ui->offsetEdit->setVisible(isOffsetEditVisible);
ui->offsetEdit->setEnabled(isOffsetEditVisible && isOffsetEditEnabled);
ui->labelOffset->setVisible(isOffsetEditVisible);

ui->checkBoxMidplane->setEnabled(isMidplaneEnabled);
ui->checkBoxMidplane->setVisible(isMidplaneVisible);

ui->checkBoxReversed->setEnabled(isReversedEnabled);

ui->buttonFace->setEnabled(isFaceEditEnabled);
ui->lineFaceName->setEnabled(isFaceEditEnabled);
if (!isFaceEditEnabled) {
onButtonFace(false);
}
}

void TaskExtrudeParameters::onDirectionCBChanged(int num)
{
PartDesign::FeatureExtrude* extrude = static_cast<PartDesign::FeatureExtrude*>(vp->getObject());
Expand Down
10 changes: 10 additions & 0 deletions src/Mod/PartDesign/Gui/TaskExtrudeParameters.h
View file
Open in desktop
Expand Up @@ -67,6 +67,16 @@ class TaskExtrudeParameters : public TaskSketchBasedParameters
bool hasSketch = true);
void applyParameters(QString facename);

enum class Modes {
Dimension,
ThroughAll,
ToLast = ThroughAll,
ToFirst,
ToFace,
TwoDimensions
};
void setCheckboxes(int index, Modes mode);

protected Q_SLOTS:
void onLengthChanged(double);
void onLength2Changed(double);
Expand Down
66 changes: 2 additions & 64 deletions src/Mod/PartDesign/Gui/TaskPadParameters.cpp
View file
Open in desktop
Expand Up @@ -80,70 +80,8 @@ void TaskPadParameters::updateUI(int index)
{
// update direction combobox
fillDirectionCombo();

// disable/hide everything unless we are sure we don't need it
// exception: the direction parameters are in any case visible
bool isLengthEditVisible = false;
bool isLengthEdit2Visible = false;
bool isOffsetEditVisible = false;
bool isMidplaneEnabled = false;
bool isMidplaneVisible = false;
bool isReversedEnabled = false;
bool isFaceEditEnabled = false;

Modes mode = static_cast<Modes>(index);

if (mode == Modes::Dimension) {
isLengthEditVisible = true;
ui->lengthEdit->selectNumber();
QMetaObject::invokeMethod(ui->lengthEdit, "setFocus", Qt::QueuedConnection);
isMidplaneEnabled = !ui->checkBoxReversed->isChecked();
isMidplaneVisible = true;
// Reverse only makes sense if Midplane is not true
isReversedEnabled = !ui->checkBoxMidplane->isChecked();
}
else if (mode == Modes::ToLast || mode == Modes::ToFirst) {
isOffsetEditVisible = true;
isReversedEnabled = true;
}
else if (mode == Modes::ToFace) {
isOffsetEditVisible = true;
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())
onButtonFace(true);
isReversedEnabled = true;
}
else if (mode == Modes::TwoDimensions) {
isLengthEditVisible = true;
isLengthEdit2Visible = true;
isReversedEnabled = true;
}

ui->lengthEdit->setVisible( isLengthEditVisible );
ui->lengthEdit->setEnabled( isLengthEditVisible );
ui->labelLength->setVisible( isLengthEditVisible );
ui->checkBoxAlongDirection->setVisible( isLengthEditVisible );

ui->offsetEdit->setVisible( isOffsetEditVisible );
ui->offsetEdit->setEnabled( isOffsetEditVisible );
ui->labelOffset->setVisible( isOffsetEditVisible );

ui->checkBoxMidplane->setEnabled( isMidplaneEnabled );
ui->checkBoxMidplane->setVisible( isMidplaneVisible );

ui->checkBoxReversed->setEnabled( isReversedEnabled );

ui->lengthEdit2->setVisible( isLengthEdit2Visible );
ui->lengthEdit2->setEnabled( isLengthEdit2Visible );
ui->labelLength2->setVisible( isLengthEdit2Visible );

ui->buttonFace->setEnabled( isFaceEditEnabled );
ui->lineFaceName->setEnabled( isFaceEditEnabled );
if (!isFaceEditEnabled) {
onButtonFace(false);
}
// set and enable checkboxes
setCheckboxes(index, static_cast<Modes>(index));
Copy link
Contributor

@wwmayer wwmayer Jan 14, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't make much sense. Why is index passed twice now?

In my code example the second argument was supposed to be new enum class type:

enum class Type {
  Pad,
  Pocket
};

The called function would then be: setCheckboxes(index, Type::Pad); and of course the signature of setCheckboxes is: void setCheckboxes(int, Type);

Copy link
Member Author

@donovaly donovaly Jan 16, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't make much sense. Why is index passed twice now?

Good question. I have no clue why I force-pushed this. Before my last push the enum was correctly passed.

I see now that you already fixed and merged. Many thanks.

}

void TaskPadParameters::onModeChanged(int index)
Expand Down
8 changes: 0 additions & 8 deletions src/Mod/PartDesign/Gui/TaskPadParameters.h
View file
Open in desktop
Expand Up @@ -46,14 +46,6 @@ class TaskPadParameters : public TaskExtrudeParameters
{
Q_OBJECT

enum class Modes {
Dimension,
ToLast,
ToFirst,
ToFace,
TwoDimensions
};

public:
TaskPadParameters(ViewProviderPad *PadView, QWidget *parent = 0, bool newObj=false);
~TaskPadParameters();
Expand Down
76 changes: 2 additions & 74 deletions src/Mod/PartDesign/Gui/TaskPocketParameters.cpp
View file
Open in desktop
Expand Up @@ -81,80 +81,8 @@ void TaskPocketParameters::updateUI(int index)
{
// update direction combobox
fillDirectionCombo();

// disable/hide everything unless we are sure we don't need it
// exception: the direction parameters are in any case visible
bool isLengthEditVisible = false;
bool isLengthEdit2Visible = false;
bool isOffsetEditVisible = false;
bool isOffsetEditEnabled = true;
bool isMidplaneEnabled = false;
bool isMidplaneVisible = false;
bool isReversedEnabled = false;
bool isFaceEditEnabled = false;

Modes mode = static_cast<Modes>(index);

if (mode == Modes::Dimension) {
isLengthEditVisible = true;
ui->lengthEdit->selectNumber();
QMetaObject::invokeMethod(ui->lengthEdit, "setFocus", Qt::QueuedConnection);
isMidplaneVisible = true;
isMidplaneEnabled = true;
// Reverse only makes sense if Midplane is not true
isReversedEnabled = !ui->checkBoxMidplane->isChecked();
}
else if (mode == Modes::ThroughAll) {
isOffsetEditVisible = true;
isOffsetEditEnabled = false; // offset may have some meaning for through all but it doesn't work
isMidplaneEnabled = true;
isReversedEnabled = !ui->checkBoxMidplane->isChecked();
}
else if (mode == Modes::ToFirst) {
isOffsetEditVisible = true;
isReversedEnabled = true; // Will change the direction it seeks for its first face?
// It may work not quite as expected but useful if sketch oriented upside-down.
// (may happen in bodies)
// FIXME: Fix probably lies somewhere in IF block on line 125 of FeaturePocket.cpp
}
else if (mode == Modes::ToFace) {
isOffsetEditVisible = true;
isReversedEnabled = true;
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())
onButtonFace(true);
}
else if (mode == Modes::TwoDimensions) {
isLengthEditVisible = true;
isLengthEdit2Visible = true;
isReversedEnabled = true;
}

ui->lengthEdit->setVisible( isLengthEditVisible );
ui->lengthEdit->setEnabled( isLengthEditVisible );
ui->labelLength->setVisible( isLengthEditVisible );
ui->checkBoxAlongDirection->setVisible(isLengthEditVisible);

ui->lengthEdit2->setVisible( isLengthEdit2Visible );
ui->lengthEdit2->setEnabled( isLengthEdit2Visible );
ui->labelLength2->setVisible( isLengthEdit2Visible );

ui->offsetEdit->setVisible( isOffsetEditVisible );
ui->offsetEdit->setEnabled( isOffsetEditVisible && isOffsetEditEnabled );
ui->labelOffset->setVisible( isOffsetEditVisible );

ui->checkBoxMidplane->setEnabled( isMidplaneEnabled );
ui->checkBoxMidplane->setVisible(isMidplaneVisible);

ui->checkBoxReversed->setEnabled( isReversedEnabled );

ui->buttonFace->setEnabled( isFaceEditEnabled );
ui->lineFaceName->setEnabled( isFaceEditEnabled );
if (!isFaceEditEnabled) {
onButtonFace(false);
}
// set and enable checkboxes
setCheckboxes(index, static_cast<Modes>(index));
}

void TaskPocketParameters::onModeChanged(int index)
Expand Down
8 changes: 0 additions & 8 deletions src/Mod/PartDesign/Gui/TaskPocketParameters.h
View file
Open in desktop
Expand Up @@ -46,14 +46,6 @@ class TaskPocketParameters : public TaskExtrudeParameters
{
Q_OBJECT

enum class Modes {
Dimension,
ThroughAll,
ToFirst,
ToFace,
TwoDimensions
};

public:
TaskPocketParameters(ViewProviderPocket *PocketView, QWidget *parent = 0, bool newObj=false);
~TaskPocketParameters();
Expand Down