From cd23cec411213368efdcb463ce7424ec18b1d0b7 Mon Sep 17 00:00:00 2001 From: Roman Telezhynskyi Date: Wed, 24 Nov 2021 11:27:44 +0200 Subject: [PATCH] Improved main path validations. --- ChangeLog.txt | 1 + src/libs/vtools/dialogs/dialogtoolbox.cpp | 62 +++++++--- src/libs/vtools/dialogs/dialogtoolbox.h | 2 +- .../dialogs/tools/piece/dialogpiecepath.cpp | 94 ++++++++------- .../tools/piece/dialogseamallowance.cpp | 108 ++++++++++-------- 5 files changed, 160 insertions(+), 107 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index decea1fa2..6dafedc44 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -25,6 +25,7 @@ - [smart-pattern/valentina#153] To add text search bar in History window. - Improve for a search bar. - Backport fix vulnerability CVE-2021-21900. +- Improved main path validations. # Valentina 0.7.49 July 1, 2021 - Fix crash. diff --git a/src/libs/vtools/dialogs/dialogtoolbox.cpp b/src/libs/vtools/dialogs/dialogtoolbox.cpp index b3dbe282d..2f3eeeced 100644 --- a/src/libs/vtools/dialogs/dialogtoolbox.cpp +++ b/src/libs/vtools/dialogs/dialogtoolbox.cpp @@ -64,7 +64,7 @@ namespace const int dialogMaxFormulaHeight = 80; //--------------------------------------------------------------------------------------------------------------------- -bool DoublePoint(const VPieceNode &firstNode, const VPieceNode &secondNode, const VContainer *data) +auto DoublePoint(const VPieceNode &firstNode, const VPieceNode &secondNode, const VContainer *data) -> bool { if (firstNode.GetTypeTool() == Tool::NodePoint && not (firstNode.GetId() == NULL_ID) && secondNode.GetTypeTool() == Tool::NodePoint && not (secondNode.GetId() == NULL_ID)) @@ -75,30 +75,40 @@ bool DoublePoint(const VPieceNode &firstNode, const VPieceNode &secondNode, cons return true; } + QSharedPointer firstPoint; + QSharedPointer secondPoint; + + try + { + firstPoint = data->GeometricObject(firstNode.GetId()); + secondPoint = data->GeometricObject(secondNode.GetId()); + } + catch(const VExceptionBadId &) + { + return true; + } + + // The same point, but different modeling objects + if (firstPoint->getIdObject() != NULL_ID && secondPoint->getIdObject() != NULL_ID && + firstPoint->getIdObject() == secondPoint->getIdObject()) + { + return true; + } + // But ignore the same coordinate if a user wants if (not firstNode.IsCheckUniqueness() || not secondNode.IsCheckUniqueness()) { return false; } - try - { - const QSharedPointer firstPoint = data->GeometricObject(firstNode.GetId()); - const QSharedPointer secondPoint = data->GeometricObject(secondNode.GetId()); - - return firstPoint->toQPointF() == secondPoint->toQPointF(); - } - catch(const VExceptionBadId &) - { - return true; - } + return firstPoint->toQPointF() == secondPoint->toQPointF(); } return false; } //--------------------------------------------------------------------------------------------------------------------- -bool DoubleCurve(const VPieceNode &firstNode, const VPieceNode &secondNode) +auto DoubleCurve(const VPieceNode &firstNode, const VPieceNode &secondNode, const VContainer *data) -> bool { if (firstNode.GetTypeTool() != Tool::NodePoint && not (firstNode.GetId() == NULL_ID) && secondNode.GetTypeTool() != Tool::NodePoint && not (secondNode.GetId() == NULL_ID)) @@ -108,6 +118,22 @@ bool DoubleCurve(const VPieceNode &firstNode, const VPieceNode &secondNode) { return true; } + + try + { + // The same curve, but different modeling objects + const QSharedPointer curve1 = data->GetGObject(firstNode.GetId()); + const QSharedPointer curve2 = data->GetGObject(secondNode.GetId()); + + if (curve1->getIdObject() == curve2->getIdObject()) + { + return true; + } + } + catch (const VExceptionBadId &) + { + return false; + } } return false; @@ -428,7 +454,7 @@ bool DoublePoints(QListWidget *listWidget, const VContainer *data) } //--------------------------------------------------------------------------------------------------------------------- -auto DoubleCurves(QListWidget *listWidget) -> bool +auto DoubleCurves(QListWidget *listWidget, const VContainer *data) -> bool { SCASSERT(listWidget != nullptr); for (int i=0, sz = listWidget->count()-1; i bool const VPieceNode firstNode = RowNode(listWidget, firstIndex); const VPieceNode secondNode = RowNode(listWidget, FindNotExcludedNodeDown(listWidget, firstIndex+1)); - if (DoubleCurve(firstNode, secondNode)) + if (DoubleCurve(firstNode, secondNode, data)) { return true; } @@ -446,7 +472,7 @@ auto DoubleCurves(QListWidget *listWidget) -> bool } //--------------------------------------------------------------------------------------------------------------------- -bool EachPointLabelIsUnique(QListWidget *listWidget) +auto EachPointLabelIsUnique(QListWidget *listWidget) -> bool { SCASSERT(listWidget != nullptr); QSet pointLabels; @@ -455,8 +481,8 @@ bool EachPointLabelIsUnique(QListWidget *listWidget) { const QListWidgetItem *rowItem = listWidget->item(i); SCASSERT(rowItem != nullptr); - const VPieceNode rowNode = qvariant_cast(rowItem->data(Qt::UserRole)); - if (rowNode.GetTypeTool() == Tool::NodePoint) + const auto rowNode = qvariant_cast(rowItem->data(Qt::UserRole)); + if (rowNode.GetTypeTool() == Tool::NodePoint && not rowNode.IsExcluded()) { ++countPoints; pointLabels.insert(rowNode.GetId()); diff --git a/src/libs/vtools/dialogs/dialogtoolbox.h b/src/libs/vtools/dialogs/dialogtoolbox.h index 3a8dfcdde..99e4ab41b 100644 --- a/src/libs/vtools/dialogs/dialogtoolbox.h +++ b/src/libs/vtools/dialogs/dialogtoolbox.h @@ -82,7 +82,7 @@ int FindNotExcludedNodeDown(QListWidget *listWidget, int candidate); int FindNotExcludedNodeUp(QListWidget *listWidget, int candidate); bool FirstPointEqualLast(QListWidget *listWidget, const VContainer *data); bool DoublePoints(QListWidget *listWidget, const VContainer *data); -bool DoubleCurves(QListWidget *listWidget); +bool DoubleCurves(QListWidget *listWidget, const VContainer *data); bool EachPointLabelIsUnique(QListWidget *listWidget); QString DialogWarningIcon(); QFont NodeFont(QFont font, bool nodeExcluded = false); diff --git a/src/libs/vtools/dialogs/tools/piece/dialogpiecepath.cpp b/src/libs/vtools/dialogs/tools/piece/dialogpiecepath.cpp index 95f3d0031..88d6e5016 100644 --- a/src/libs/vtools/dialogs/tools/piece/dialogpiecepath.cpp +++ b/src/libs/vtools/dialogs/tools/piece/dialogpiecepath.cpp @@ -1079,10 +1079,31 @@ void DialogPiecePath::InitPathTab() connect(ui->listWidget->model(), &QAbstractItemModel::rowsMoved, this, &DialogPiecePath::ListChanged); connect(ui->listWidget, &QListWidget::itemSelectionChanged, this, &DialogPiecePath::SetMoveControls); - connect(ui->toolButtonTop, &QToolButton::clicked, this, [this](){MoveListRowTop(ui->listWidget);}); - connect(ui->toolButtonUp, &QToolButton::clicked, this, [this](){MoveListRowUp(ui->listWidget);}); - connect(ui->toolButtonDown, &QToolButton::clicked, this, [this](){MoveListRowDown(ui->listWidget);}); - connect(ui->toolButtonBottom, &QToolButton::clicked, this, [this](){MoveListRowBottom(ui->listWidget);}); + connect(ui->listWidget->model(), &QAbstractItemModel::rowsMoved, this, [this]() + { + ValidObjects(PathIsValid()); + }); + + connect(ui->toolButtonTop, &QToolButton::clicked, this, [this]() + { + MoveListRowTop(ui->listWidget); + ValidObjects(PathIsValid()); + }); + connect(ui->toolButtonUp, &QToolButton::clicked, this, [this]() + { + MoveListRowUp(ui->listWidget); + ValidObjects(PathIsValid()); + }); + connect(ui->toolButtonDown, &QToolButton::clicked, this, [this]() + { + MoveListRowDown(ui->listWidget); + ValidObjects(PathIsValid()); + }); + connect(ui->toolButtonBottom, &QToolButton::clicked, this, [this]() + { + MoveListRowBottom(ui->listWidget); + ValidObjects(PathIsValid()); + }); } //--------------------------------------------------------------------------------------------------------------------- @@ -1602,54 +1623,49 @@ VPiecePath DialogPiecePath::CreatePath() const } //--------------------------------------------------------------------------------------------------------------------- -bool DialogPiecePath::PathIsValid() const +auto DialogPiecePath::PathIsValid() const -> bool { - QString url = DialogWarningIcon(); - if(CreatePath().PathPoints(data).count() < 2) { - url += tr("You need more points!"); - ui->helpLabel->setText(url); + ui->helpLabel->setText(DialogWarningIcon() + tr("You need more points!")); return false; } - else + + if (GetType() == PiecePathType::CustomSeamAllowance && FirstPointEqualLast(ui->listWidget, data)) { - if (GetType() == PiecePathType::CustomSeamAllowance && FirstPointEqualLast(ui->listWidget, data)) - { - url += tr("First point of custom seam allowance cannot be equal to the last point!"); - ui->helpLabel->setText(url); - return false; - } - else if (DoublePoints(ui->listWidget, data)) - { - url += tr("You have double points!"); - ui->helpLabel->setText(url); - return false; - } - else if (DoubleCurves(ui->listWidget)) - { - url += tr("The same curve repeats twice!"); - ui->helpLabel->setText(url); - return false; - } - else if (GetType() == PiecePathType::CustomSeamAllowance && not EachPointLabelIsUnique(ui->listWidget)) - { - url += tr("Each point in the custom seam allowance path must be unique!"); - ui->helpLabel->setText(url); - return false; - } + ui->helpLabel->setText(DialogWarningIcon() + + tr("First point of custom seam allowance cannot be equal to the last point!")); + return false; + } + + if (DoublePoints(ui->listWidget, data)) + { + ui->helpLabel->setText(DialogWarningIcon() + tr("You have double points!")); + return false; + } + + if (DoubleCurves(ui->listWidget, data)) + { + ui->helpLabel->setText(DialogWarningIcon() + tr("The same curve repeats twice!")); + return false; + } + + if (GetType() == PiecePathType::CustomSeamAllowance && not EachPointLabelIsUnique(ui->listWidget)) + { + ui->helpLabel->setText(DialogWarningIcon() + + tr("Each point in the custom seam allowance path must be unique!")); + return false; } if (not m_showMode && ui->comboBoxPiece->count() <= 0) { - url += tr("List of details is empty!"); - ui->helpLabel->setText(url); + ui->helpLabel->setText(DialogWarningIcon() + tr("List of details is empty!")); return false; } - else if (not m_showMode && ui->comboBoxPiece->currentIndex() == -1) + + if (not m_showMode && ui->comboBoxPiece->currentIndex() == -1) { - url += tr("Please, select a detail to insert into!"); - ui->helpLabel->setText(url); + ui->helpLabel->setText(DialogWarningIcon() + tr("Please, select a detail to insert into!")); return false; } diff --git a/src/libs/vtools/dialogs/tools/piece/dialogseamallowance.cpp b/src/libs/vtools/dialogs/tools/piece/dialogseamallowance.cpp index f1299eacc..5bdd987f9 100644 --- a/src/libs/vtools/dialogs/tools/piece/dialogseamallowance.cpp +++ b/src/libs/vtools/dialogs/tools/piece/dialogseamallowance.cpp @@ -2684,52 +2684,45 @@ QString DialogSeamAllowance::GetPathName(quint32 path, bool reverse) const } //--------------------------------------------------------------------------------------------------------------------- -bool DialogSeamAllowance::MainPathIsValid() const +auto DialogSeamAllowance::MainPathIsValid() const -> bool { - QString url = DialogWarningIcon(); - bool valid = true; - if(CreatePiece().MainPathPoints(data).count() < 3) { - url += tr("You need more points!"); - uiTabPaths->helpLabel->setText(url); - valid = false; - } - else - { - if(not MainPathIsClockwise()) - { - url += tr("You have to choose points in a clockwise direction!"); - uiTabPaths->helpLabel->setText(url); - valid = false; - } - if (FirstPointEqualLast(uiTabPaths->listWidgetMainPath, data)) - { - url += tr("First point cannot be equal to the last point!"); - uiTabPaths->helpLabel->setText(url); - valid = false; - } - else if (DoublePoints(uiTabPaths->listWidgetMainPath, data)) - { - url += tr("You have double points!"); - uiTabPaths->helpLabel->setText(url); - valid = false; - } - else if (DoubleCurves(uiTabPaths->listWidgetMainPath)) - { - url += tr("The same curve repeats twice!"); - uiTabPaths->helpLabel->setText(url); - valid = false; - } - else if (not EachPointLabelIsUnique(uiTabPaths->listWidgetMainPath)) - { - url += tr("Each point in the path must be unique!"); - uiTabPaths->helpLabel->setText(url); - valid = false; - } + uiTabPaths->helpLabel->setText(DialogWarningIcon() + tr("You need more points!")); + return false; } - return valid; + if(not MainPathIsClockwise()) + { + uiTabPaths->helpLabel->setText(DialogWarningIcon() + tr("You have to choose points in a clockwise direction!")); + return false; + } + + if (FirstPointEqualLast(uiTabPaths->listWidgetMainPath, data)) + { + uiTabPaths->helpLabel->setText(DialogWarningIcon() + tr("First point cannot be equal to the last point!")); + return false; + } + + if (DoublePoints(uiTabPaths->listWidgetMainPath, data)) + { + uiTabPaths->helpLabel->setText(DialogWarningIcon() + tr("You have double points!")); + return false; + } + + if (DoubleCurves(uiTabPaths->listWidgetMainPath, data)) + { + uiTabPaths->helpLabel->setText(DialogWarningIcon() + tr("The same curve repeats twice!")); + return false; + } + + if (not EachPointLabelIsUnique(uiTabPaths->listWidgetMainPath)) + { + uiTabPaths->helpLabel->setText(DialogWarningIcon() + tr("Each point in the path must be unique!")); + return false; + } + + return true; } //--------------------------------------------------------------------------------------------------------------------- @@ -2972,14 +2965,31 @@ void DialogSeamAllowance::InitMainPathTab() connect(uiTabPaths->listWidgetMainPath, &QListWidget::itemSelectionChanged, this, &DialogSeamAllowance::SetMoveControls); - connect(uiTabPaths->toolButtonTop, &QToolButton::clicked, this, - [this](){MoveListRowTop(uiTabPaths->listWidgetMainPath);}); - connect(uiTabPaths->toolButtonUp, &QToolButton::clicked, this, - [this](){MoveListRowUp(uiTabPaths->listWidgetMainPath);}); - connect(uiTabPaths->toolButtonDown, &QToolButton::clicked, this, - [this](){MoveListRowDown(uiTabPaths->listWidgetMainPath);}); - connect(uiTabPaths->toolButtonBottom, &QToolButton::clicked, this, - [this](){MoveListRowBottom(uiTabPaths->listWidgetMainPath);}); + connect(uiTabPaths->listWidgetMainPath->model(), &QAbstractItemModel::rowsMoved, this, [this]() + { + ValidObjects(MainPathIsValid()); + }); + + connect(uiTabPaths->toolButtonTop, &QToolButton::clicked, this, [this]() + { + MoveListRowTop(uiTabPaths->listWidgetMainPath); + ValidObjects(MainPathIsValid()); + }); + connect(uiTabPaths->toolButtonUp, &QToolButton::clicked, this, [this]() + { + MoveListRowUp(uiTabPaths->listWidgetMainPath); + ValidObjects(MainPathIsValid()); + }); + connect(uiTabPaths->toolButtonDown, &QToolButton::clicked, this, [this]() + { + MoveListRowDown(uiTabPaths->listWidgetMainPath); + ValidObjects(MainPathIsValid()); + }); + connect(uiTabPaths->toolButtonBottom, &QToolButton::clicked, this, [this]() + { + MoveListRowBottom(uiTabPaths->listWidgetMainPath); + ValidObjects(MainPathIsValid()); + }); } //---------------------------------------------------------------------------------------------------------------------