kindred/create
1
1
Fork 0
Code Issues 44 Pull Requests Actions Packages Projects 9 Releases 2 Wiki Activity

[TD] fix centerline crashes and bugs

Browse Source 
the PR fixes the following things:
- change the way centerlines between 2 lines are calculated. The current implementation leads to many bugs and even crashes (e.g. when the 2 selected lines are parallel ones of a square). There are different forum threads from the last 1.5 years.
The new endpoint line sorting is the one described here: https://forum.freecadweb.org/viewtopic.php?f=35&start=20&t=44255&sid=989a668890f954c13ef81e4a04ee6912#p501179

- as consequence the erroneous and misleading line end flipping can go and is removed (only used internally)

- when creating a new centerline, you see it immediately

- when creating a new or editing an existing centerline and press Cancel, the creation/editing is aborted

- fix crash when the 2 selected lines are bot horizontal and one tries to create a vertical centerline

- fix crash when changing the orientation  of an existing centerline and the result would be invalid

- cleanup the code a bit
This commit is contained in:
donovaly
2021-05-14 23:48:12 +02:00
parent bb434d3ff2
commit 2bc5ac2e4b
5 changed files with 173 additions and 155 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
src/Mod/TechDraw/App/Cosmetic.cpp
View File

@@ -39,6 +39,7 @@
#include <Base/Console.h>
#include <Base/Exception.h>
#include <Base/Matrix.h>
#include <Base/Parameter.h>
#include <Base/Reader.h>
#include <Base/Tools.h>
@@ -977,6 +978,20 @@ std::pair<Base::Vector3d, Base::Vector3d> CenterLine::calcEndPoints(DrawViewPart
return result;
}
bool CenterLine::Circulation(Base::Vector3d A, Base::Vector3d B, Base::Vector3d C)
{
Base::Matrix4D CircMatrix(
A.x, A.y, 1, 0,
B.x, B.y, 1, 0,
C.x, C.y, 1, 0,
0, 0, 0, 1);
if (CircMatrix.determinant() > 0)
return true;
else
return false;
}
std::pair<Base::Vector3d, Base::Vector3d> CenterLine::calcEndPoints2Lines(DrawViewPart* partFeat,
std::vector<std::string> edgeNames,
int mode, double ext,
@@ -1018,17 +1033,21 @@ std::pair<Base::Vector3d, Base::Vector3d> CenterLine::calcEndPoints2Lines(DrawVi
Base::Vector3d l2p1 = edges.back()->getStartPoint();
Base::Vector3d l2p2 = edges.back()->getEndPoint();
if (flip) { //reverse line 2
Base::Vector3d temp;
temp = l2p1;
l2p1 = l2p2;
l2p2 = temp;
// The centerline is drawn using the midpoints of the two lines that connect l1p1-l2p1 and l1p2-l2p2.
// However, we don't know which point should be l1p1 to get a geometrically correct result.
// Thus we test this by a circulation test.
if (Circulation(l1p1, l1p2, l2p1) != Circulation(l1p2, l2p2, l2p1)) {
Base::Vector3d temp; // reverse line 1
temp = l1p1;
l1p1 = l1p2;
l1p2 = temp;
}
Base::Vector3d p1 = (l1p1 + l2p1) / 2.0;
Base::Vector3d p2 = (l1p2 + l2p2) / 2.0;
Base::Vector3d mid = (p1 + p2) / 2.0;
//orientation
if (mode == 0) { //Vertical
p1.x = mid.x;
p2.x = mid.x;

1
src/Mod/TechDraw/App/Cosmetic.h
View File

@@ -220,6 +220,7 @@ public:
int mode, double ext,
double m_hShift, double m_vShift,
double rotate);
static bool Circulation(Base::Vector3d A, Base::Vector3d B, Base::Vector3d C);
static std::pair<Base::Vector3d, Base::Vector3d> calcEndPoints2Lines(
TechDraw::DrawViewPart* partFeat,
std::vector<std::string> faceNames,

266
src/Mod/TechDraw/Gui/TaskCenterLine.cpp
View File

@@ -86,15 +86,14 @@ TaskCenterLine::TaskCenterLine(TechDraw::DrawViewPart* partFeat,
m_edgeName(edgeName),
m_extendBy(0.0),
m_clIdx(0),
m_type(0), //0 - Face, 1 - 2 Lines, 2 - 2 points
m_mode(0), //0 - vertical, 1 - horizontal, 2 - aligned
m_type(0), // 0 - Face, 1 - Lines, 2 - Points
m_mode(0), // 0 - vertical, 1 - horizontal, 2 - aligned
m_editMode(editMode)
{
// Base::Console().Message("TCL::TCL() - edit mode\n");
ui->setupUi(this);
m_geomIndex = DrawUtil::getIndexFromName(m_edgeName);
const std::vector<TechDraw::BaseGeom *> &geoms = partFeat->getEdgeGeometry();
const std::vector<TechDraw::BaseGeom*> &geoms = partFeat->getEdgeGeometry();
BaseGeom* bg = geoms.at(m_geomIndex);
std::string tag = bg->getCosmeticTag();
m_cl = partFeat->getCenterLine(tag);
@@ -107,6 +106,10 @@ TaskCenterLine::TaskCenterLine(TechDraw::DrawViewPart* partFeat,
}
setUiEdit();
// connect the dialog objects
setUiConnect();
// save the existing centerline to restore in in case the user rejects the changes
orig_cl = *m_cl;
}
//ctor for creation
@@ -125,11 +128,10 @@ TaskCenterLine::TaskCenterLine(TechDraw::DrawViewPart* partFeat,
m_geomIndex(0),
m_cl(nullptr),
m_clIdx(0),
m_type(0), //0 - Face, 1 - 2 Lines, 2 - 2 points
m_mode(0), //0 - vertical, 1 - horizontal, 2 - aligned
m_type(0), // 0 - Face, 1 - Lines, 2 - Points
m_mode(0), // 0 - vertical, 1 - horizontal, 2 - aligned
m_editMode(editMode)
{
// Base::Console().Message("TCL::TCL() - create mode\n");
if ( (m_basePage == nullptr) ||
(m_partFeat == nullptr) ) {
//should be caught in CMD caller
@@ -151,7 +153,12 @@ TaskCenterLine::TaskCenterLine(TechDraw::DrawViewPart* partFeat,
return;
}
// setup the Ui using (user-defined) default values
setUiPrimary();
// connect the dialog objects
setUiConnect();
// now create the centerline
createCenterLine();
}
TaskCenterLine::~TaskCenterLine()
@@ -160,9 +167,6 @@ TaskCenterLine::~TaskCenterLine()
void TaskCenterLine::updateTask()
{
// blockUpdate = true;
// blockUpdate = false;
}
void TaskCenterLine::changeEvent(QEvent *e)
@@ -172,6 +176,25 @@ void TaskCenterLine::changeEvent(QEvent *e)
}
}
void TaskCenterLine::setUiConnect()
{
// first enabling/disabling
if (m_type == 0) // if face, then aligned is not possible
ui->rbAligned->setEnabled(false);
else
ui->rbAligned->setEnabled(true);
// now connection
connect(ui->cpLineColor, SIGNAL(changed()), this, SLOT(onColorChanged()));
connect(ui->dsbWeight, SIGNAL(valueChanged(double)), this, SLOT(onWeightChanged()));
connect(ui->cboxStyle, SIGNAL(currentIndexChanged(int)), this, SLOT(onStyleChanged()));
connect(ui->qsbVertShift, SIGNAL(valueChanged(double)), this, SLOT(onShiftVertChanged()));
connect(ui->qsbHorizShift, SIGNAL(valueChanged(double)), this, SLOT(onShiftHorizChanged()));
connect(ui->qsbExtend, SIGNAL(valueChanged(double)), this, SLOT(onExtendChanged()));
connect(ui->qsbRotate, SIGNAL(valueChanged(double)), this, SLOT(onRotationChanged()));
connect(ui->bgOrientation, SIGNAL(buttonClicked(int)), this, SLOT(onOrientationChanged()));
}
void TaskCenterLine::setUiPrimary()
{
setWindowTitle(QObject::tr("Create Center Line"));
@@ -199,17 +222,7 @@ void TaskCenterLine::setUiPrimary()
qAngle.setUnit(Base::Unit::Angle);
ui->qsbRotate->setValue(qAngle);
int precision = Base::UnitsApi::getDecimals();
ui->qsbRotate->setDecimals(precision);
if (m_type == 0) // if face, then aligned is not possible
ui->rbAligned->setEnabled(false);
else
ui->rbAligned->setEnabled(true);
if (m_type == 1) // only if line, feature is enabled
ui->cbFlip->setEnabled(true);
else
ui->cbFlip->setEnabled(false);
ui->qsbRotate->setDecimals(precision);
}
void TaskCenterLine::setUiEdit()
@@ -222,11 +235,8 @@ void TaskCenterLine::setUiEdit()
ui->lstSubList->addItem(listItem);
}
ui->cpLineColor->setColor(m_cl->m_format.m_color.asValue<QColor>());
connect(ui->cpLineColor, SIGNAL(changed()), this, SLOT(onColorChanged()));
ui->dsbWeight->setValue(m_cl->m_format.m_weight);
connect(ui->dsbWeight, SIGNAL(valueChanged(double)), this, SLOT(onWeightChanged()));
ui->cboxStyle->setCurrentIndex(m_cl->m_format.m_style - 1);
connect(ui->cboxStyle, SIGNAL(currentIndexChanged(int)), this, SLOT(onStyleChanged()));
ui->rbVertical->setChecked(false);
ui->rbHorizontal->setChecked(false);
@@ -237,44 +247,22 @@ void TaskCenterLine::setUiEdit()
ui->rbHorizontal->setChecked(true);
else if (m_cl->m_mode ==2)
ui->rbAligned->setChecked(true);
if (m_cl->m_type == 0) // if face, then aligned is not possible
ui->rbAligned->setEnabled(false);
else
ui->rbAligned->setEnabled(true);
Base::Quantity qVal;
qVal.setUnit(Base::Unit::Length);
qVal.setValue(m_cl->m_vShift);
ui->qsbVertShift->setValue(qVal);
connect(ui->qsbVertShift, SIGNAL(valueChanged(double)), this, SLOT(onShiftVertChanged()));
qVal.setValue(m_cl->m_hShift);
ui->qsbHorizShift->setValue(qVal);
connect(ui->qsbHorizShift, SIGNAL(valueChanged(double)), this, SLOT(onShiftHorizChanged()));
qVal.setValue(m_cl->m_extendBy);
ui->qsbExtend->setValue(qVal);
connect(ui->qsbExtend, SIGNAL(valueChanged(double)), this, SLOT(onExtendChanged()));
Base::Quantity qAngle;
qAngle.setUnit(Base::Unit::Angle);
ui->qsbRotate->setValue(qAngle);
int precision = Base::UnitsApi::getDecimals();
ui->qsbRotate->setDecimals(precision);
ui->qsbRotate->setValue(m_cl->m_rotate);
connect(ui->qsbRotate, SIGNAL(valueChanged(double)), this, SLOT(onRotationChanged()));
if (m_cl->m_flip2Line)
ui->cbFlip->setChecked(true);
else
ui->cbFlip->setChecked(false);
if (m_cl->m_type == 1) // only if line, feature is enabled
ui->cbFlip->setEnabled(true);
else
ui->cbFlip->setEnabled(false);
connect(ui->cbFlip, SIGNAL(toggled(bool)), this, SLOT(onFlipChanged()));
// connect the Orientation radio group box
connect(ui->bgOrientation, SIGNAL(buttonClicked(int)), this, SLOT(onOrientationChanged()));
ui->qsbRotate->setValue(m_cl->m_rotate);
}
void TaskCenterLine::onOrientationChanged()
@@ -285,7 +273,12 @@ void TaskCenterLine::onOrientationChanged()
m_cl->m_mode = CenterLine::CLMODE::HORIZONTAL;
else if (ui->rbAligned->isChecked())
m_cl->m_mode = CenterLine::CLMODE::ALIGNED;
m_partFeat->recomputeFeature();
// for centerlines between 2 lines we cannot just recompute
// because the new orientation might lead to an invalid centerline
if (m_type == 1)
updateOrientation();
else
m_partFeat->recomputeFeature();
}
void TaskCenterLine::onShiftHorizChanged()
@@ -332,43 +325,35 @@ void TaskCenterLine::onStyleChanged()
m_partFeat->recomputeFeature();
}
void TaskCenterLine::onFlipChanged()
{
m_cl->m_flip2Line = ui->cbFlip->isChecked();
m_partFeat->recomputeFeature();
}
//******************************************************************************
void TaskCenterLine::createCenterLine(void)
{
// Base::Console().Message("TCL::createCenterLine() - m_type: %d\n", m_type);
Gui::Command::openCommand(QT_TRANSLATE_NOOP("Command", "Create CenterLine"));
// bool vertical = false;
double hShift = ui->qsbHorizShift->rawValue();
double vShift = ui->qsbVertShift->rawValue();
double rotate = ui->qsbRotate->rawValue();
double extendBy = ui->qsbExtend->rawValue();
std::pair<Base::Vector3d, Base::Vector3d> ends;
if (ui->rbVertical->isChecked()) {
m_mode = CenterLine::CLMODE::VERTICAL;
// vertical = true;
} else if (ui->rbHorizontal->isChecked()) {
m_mode = CenterLine::CLMODE::HORIZONTAL;
} else if (ui->rbAligned->isChecked()) {
m_mode = CenterLine::CLMODE::ALIGNED;
CenterLine* cl = CenterLine::CenterLineBuilder(m_partFeat, m_subNames, m_mode, false);
// the centerline creation can fail if m_type is edge and both selected edges are horizontal
// because we attempt by default to create a vertical centerline
if (cl == nullptr) { // try a horizontal line
cl = CenterLine::CenterLineBuilder(m_partFeat, m_subNames, CenterLine::CLMODE::HORIZONTAL, false);
if (cl != nullptr) {
m_mode = CenterLine::CLMODE::HORIZONTAL;
ui->rbHorizontal->blockSignals(true);
ui->rbHorizontal->setChecked(true);
ui->rbHorizontal->blockSignals(false);
}
}
bool flip = ui->cbFlip->isChecked();
TechDraw::CenterLine* cl = CenterLine::CenterLineBuilder(m_partFeat,
m_subNames,
m_mode,
flip);
if (cl != nullptr) {
double hShift = ui->qsbHorizShift->rawValue();
double vShift = ui->qsbVertShift->rawValue();
double rotate = ui->qsbRotate->rawValue();
double extendBy = ui->qsbExtend->rawValue();
cl->setShifts(hShift, vShift);
cl->setExtend(extendBy);
cl->setRotate(rotate);
cl->m_flip2Line = ui->cbFlip->isChecked();
cl->m_flip2Line = false;
App::Color ac;
ac.setValue<QColor>(ui->cpLineColor->color());
cl->m_format.m_color = ac;
@@ -378,41 +363,67 @@ void TaskCenterLine::createCenterLine(void)
m_partFeat->addCenterLine(cl);
} else {
Base::Console().Log("TCL::createCenterLine - CenterLine creation failed!\n");
Gui::Command::abortCommand();
return;
}
m_partFeat->recomputeFeature();
Gui::Command::updateActive();
Gui::Command::commitCommand();
// entering the edit mode
m_editMode = true;
m_cl = cl;
}
void TaskCenterLine::updateCenterLine(void)
void TaskCenterLine::updateOrientation(void)
{
// Base::Console().Message("TCL::updateCenterLine()\n");
Gui::Command::openCommand(QT_TRANSLATE_NOOP("Command", "Edit CenterLine"));
m_cl->m_format.m_color.setValue<QColor>(ui->cpLineColor->color() );
m_cl->m_format.m_weight = ui->dsbWeight->value().getValue();
m_cl->m_format.m_style = ui->cboxStyle->currentIndex() + 1;
m_cl->m_format.m_visible = true;
if (ui->rbVertical->isChecked()) {
m_mode = CenterLine::CLMODE::VERTICAL;
} else if (ui->rbHorizontal->isChecked()) {
m_mode = CenterLine::CLMODE::HORIZONTAL;
} else if (ui->rbAligned->isChecked()) {
m_mode = CenterLine::CLMODE::ALIGNED;
// When the orientation was changed, it can be that the centerline becomes invalid
// this can lead to a crash, see e.g.
// https://forum.freecadweb.org/viewtopic.php?f=35&t=44255&start=20#p503220
// The centerline creation can fail if m_type is edge and both selected edges are vertical or horizontal.
// To test the validity before an existing centerline is changed, we create a new one with the desired parameters.
int orientation = m_cl->m_mode;
if (!m_edgeName.empty()) { // we have an existing centerline, not a freshly created one
// since m_subNames is then empty, fill it with two times the centerline
// because the result of CenterLineBuilder will then in case of success again be the centerline
m_subNames.resize(2);
m_subNames[0] = m_edgeName;
m_subNames[1] = m_edgeName;
}
m_cl->m_mode = m_mode;
m_cl->m_rotate = ui->qsbRotate->rawValue();
m_cl->m_vShift = ui->qsbVertShift->rawValue();
m_cl->m_hShift = ui->qsbHorizShift->rawValue();
m_cl->m_extendBy = ui->qsbExtend->rawValue();
m_cl->m_type = m_type;
m_cl->m_flip2Line = ui->cbFlip->isChecked();
m_partFeat->refreshCLGeoms();
m_partFeat->requestPaint();
Gui::Command::updateActive();
Gui::Command::commitCommand();
CenterLine* cl = CenterLine::CenterLineBuilder(m_partFeat, m_subNames, orientation, m_cl->m_flip2Line);
if (cl == nullptr) { // try another orientation
if (orientation == CenterLine::CLMODE::VERTICAL)
orientation = CenterLine::CLMODE::HORIZONTAL;
else if (orientation == CenterLine::CLMODE::HORIZONTAL)
orientation = CenterLine::CLMODE::VERTICAL;
cl = CenterLine::CenterLineBuilder(m_partFeat, m_subNames, orientation, m_cl->m_flip2Line);
if (cl != nullptr) {
if (orientation == CenterLine::CLMODE::VERTICAL) {
m_cl->m_mode = CenterLine::CLMODE::VERTICAL;
ui->rbVertical->blockSignals(true);
ui->rbVertical->setChecked(true);
// we know now that only vertical is possible
ui->rbHorizontal->setEnabled(false);
ui->rbVertical->blockSignals(false);
}
else if (orientation == CenterLine::CLMODE::HORIZONTAL) {
m_cl->m_mode = CenterLine::CLMODE::HORIZONTAL;
ui->rbHorizontal->blockSignals(true);
ui->rbHorizontal->setChecked(true);
ui->rbVertical->setEnabled(false);
ui->rbHorizontal->blockSignals(false);
}
}
}
if (cl != nullptr) { // we succeeded
// reset the flip for existing centerline that might use the flip feature (when created with FC 0.19)
m_cl->m_flip2Line = false;
m_partFeat->recomputeFeature();
}
}
void TaskCenterLine::saveButtons(QPushButton* btnOK,
@@ -468,42 +479,49 @@ double TaskCenterLine::getExtendBy(void)
bool TaskCenterLine::accept()
{
// Base::Console().Message("TCL::accept()\n");
Gui::Document* doc = Gui::Application::Instance->getDocument(m_basePage->getDocument());
if (!doc) return false;
if (!doc)
return false;
if (!getCreateMode()) {
updateCenterLine();
} else {
createCenterLine();
}
Gui::Command::doCommand(Gui::Command::Gui,"Gui.ActiveDocument.resetEdit()");
Gui::Command::updateActive();
Gui::Command::commitCommand();
doc->resetEdit();
return true;
}
bool TaskCenterLine::reject()
{
Gui::Command::abortCommand();
Gui::Document* doc = Gui::Application::Instance->getDocument(m_basePage->getDocument());
if (!doc) return false;
if (!doc)
return false;
if (getCreateMode() &&
(m_partFeat != nullptr) ) {
// Base::Console().Message("TCL::reject - create Mode!!\n");
//nothing to remove.
if (getCreateMode() && m_partFeat) {
// undo the centerline creation
doc->undo(1);
}
else if (!getCreateMode() && m_partFeat) {
// restore the initial centerline
m_cl->m_format.m_color = (&orig_cl)->m_format.m_color;
m_cl->m_format.m_weight = (&orig_cl)->m_format.m_weight;
m_cl->m_format.m_style = (&orig_cl)->m_format.m_style;
m_cl->m_format.m_visible = (&orig_cl)->m_format.m_visible;
m_cl->m_mode = (&orig_cl)->m_mode;
m_cl->m_rotate = (&orig_cl)->m_rotate;
m_cl->m_vShift = (&orig_cl)->m_vShift;
m_cl->m_hShift = (&orig_cl)->m_hShift;
m_cl->m_extendBy = (&orig_cl)->m_extendBy;
m_cl->m_type = (&orig_cl)->m_type;
}
if (!getCreateMode() &&
(m_partFeat != nullptr) ) {
// Base::Console().Message("TCL::reject - edit Mode!!\n");
//nothing to un-update
}
if (m_partFeat)
m_partFeat->recomputeFeature();
Gui::Command::doCommand(Gui::Command::Gui, "App.activeDocument().recompute()");
doc->resetEdit();
//make sure any dangling objects are cleaned up
Gui::Command::doCommand(Gui::Command::Gui,"App.activeDocument().recompute()");
Gui::Command::doCommand(Gui::Command::Gui,"Gui.ActiveDocument.resetEdit()");
return false;
return true;
}
@@ -519,6 +537,7 @@ TaskDlgCenterLine::TaskDlgCenterLine(TechDraw::DrawViewPart* partFeat,
widget->windowTitle(), true, 0);
taskbox->groupLayout()->addWidget(widget);
Content.push_back(taskbox);
setAutoCloseOnTransactionChange(true);
}
TaskDlgCenterLine::TaskDlgCenterLine(TechDraw::DrawViewPart* partFeat,
@@ -532,6 +551,7 @@ TaskDlgCenterLine::TaskDlgCenterLine(TechDraw::DrawViewPart* partFeat,
widget->windowTitle(), true, 0);
taskbox->groupLayout()->addWidget(widget);
Content.push_back(taskbox);
setAutoCloseOnTransactionChange(true);
}
TaskDlgCenterLine::~TaskDlgCenterLine()

14
src/Mod/TechDraw/Gui/TaskCenterLine.h
View File

@@ -94,24 +94,16 @@ public:
void saveButtons(QPushButton* btnOK,
QPushButton* btnCancel);
void enableTaskButtons(bool b);
void setFlipped(bool b);
protected Q_SLOTS:
protected:
void changeEvent(QEvent *e);
void blockButtons(bool b);
void setUiConnect(void);
void setUiPrimary(void);
void setUiEdit(void);
void createCenterLine(void);
void create2Lines(void);
void create2Points(void);
void updateCenterLine(void);
void update2Lines(void);
void update2Points(void);
void updateOrientation(void);
double getCenterWidth();
QColor getCenterColor();
@@ -127,7 +119,6 @@ private Q_SLOTS:
void onColorChanged();
void onWeightChanged();
void onStyleChanged();
void onFlipChanged();
private:
std::unique_ptr<Ui_TaskCenterLine> ui;
@@ -144,6 +135,7 @@ private:
double m_extendBy;
int m_geomIndex;
TechDraw::CenterLine* m_cl;
TechDraw::CenterLine orig_cl;
int m_clIdx;
int m_type;
int m_mode;

18
src/Mod/TechDraw/Gui/TaskCenterLine.ui
View File

@@ -9,8 +9,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>360</width>
<height>385</height>
<width>250</width>
<height>352</height>
</rect>
</property>
<property name="sizePolicy">
@@ -371,20 +371,6 @@
</item>
</layout>
</item>
<item>
<widget class="QCheckBox" name="cbFlip">
<property name="enabled">
<bool>false</bool>
</property>
<property name="toolTip">
<string>Flips endpoints of selected lines for centerline creation,
see the FreeCAD Wiki '2LineCenterLine' for a description</string>
</property>
<property name="text">
<string>Flip Ends</string>
</property>
</widget>
</item>
</layout>
</widget>
<customwidgets>