Improved main path validations.

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.

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<VPointF> firstPoint;
+        QSharedPointer<VPointF> secondPoint;
+
+        try
+        {
+            firstPoint = data->GeometricObject<VPointF>(firstNode.GetId());
+            secondPoint = data->GeometricObject<VPointF>(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
+        return firstPoint->toQPointF() == secondPoint->toQPointF();
-        {
-            const QSharedPointer<VPointF> firstPoint = data->GeometricObject<VPointF>(firstNode.GetId());
-            const QSharedPointer<VPointF> secondPoint = data->GeometricObject<VPointF>(secondNode.GetId());
-
-            return firstPoint->toQPointF() == secondPoint->toQPointF();
-        }
-        catch(const VExceptionBadId &)
-        {
-            return true;
-        }
     }
 
     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<VGObject> curve1 = data->GetGObject(firstNode.GetId());
+            const QSharedPointer<VGObject> 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<sz; ++i)
@@ -437,7 +463,7 @@ auto DoubleCurves(QListWidget *listWidget) -> 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<quint32> pointLabels;
@@ -455,8 +481,8 @@ bool EachPointLabelIsUnique(QListWidget *listWidget)
     {
         const QListWidgetItem *rowItem = listWidget->item(i);
         SCASSERT(rowItem != nullptr);
-        const VPieceNode rowNode = qvariant_cast<VPieceNode>(rowItem->data(Qt::UserRole));
+        const auto rowNode = qvariant_cast<VPieceNode>(rowItem->data(Qt::UserRole));
-        if (rowNode.GetTypeTool() == Tool::NodePoint)
+        if (rowNode.GetTypeTool() == Tool::NodePoint && not rowNode.IsExcluded())
         {
             ++countPoints;
             pointLabels.insert(rowNode.GetId());

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);

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->listWidget->model(), &QAbstractItemModel::rowsMoved, this, [this]()
-    connect(ui->toolButtonUp, &QToolButton::clicked, this, [this](){MoveListRowUp(ui->listWidget);});
+    {
-    connect(ui->toolButtonDown, &QToolButton::clicked, this, [this](){MoveListRowDown(ui->listWidget);});
+        ValidObjects(PathIsValid());
-    connect(ui->toolButtonBottom, &QToolButton::clicked, this, [this](){MoveListRowBottom(ui->listWidget);});
+    });
+
+    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(DialogWarningIcon() + tr("You need more points!"));
-        ui->helpLabel->setText(url);
         return false;
     }
-    else
+
+    if (GetType() == PiecePathType::CustomSeamAllowance && FirstPointEqualLast(ui->listWidget, data))
     {
-        if (GetType() == PiecePathType::CustomSeamAllowance && FirstPointEqualLast(ui->listWidget, data))
+        ui->helpLabel->setText(DialogWarningIcon() +
-        {
+                               tr("First point of <b>custom seam allowance</b> cannot be equal to the last point!"));
-            url += tr("First point of <b>custom seam allowance</b> cannot be equal to the last point!");
+        return false;
-            ui->helpLabel->setText(url);
+    }
-            return false;
+
-        }
+    if (DoublePoints(ui->listWidget, data))
-        else if (DoublePoints(ui->listWidget, data))
+    {
-        {
+        ui->helpLabel->setText(DialogWarningIcon() + tr("You have double points!"));
-            url += tr("You have double points!");
+        return false;
-            ui->helpLabel->setText(url);
+    }
-            return false;
+
-        }
+    if (DoubleCurves(ui->listWidget, data))
-        else if (DoubleCurves(ui->listWidget))
+    {
-        {
+        ui->helpLabel->setText(DialogWarningIcon() + tr("The same curve repeats twice!"));
-            url += tr("The same curve repeats twice!");
+        return false;
-            ui->helpLabel->setText(url);
+    }
-            return false;
+
-        }
+    if (GetType() == PiecePathType::CustomSeamAllowance && not EachPointLabelIsUnique(ui->listWidget))
-        else if (GetType() == PiecePathType::CustomSeamAllowance && not EachPointLabelIsUnique(ui->listWidget))
+    {
-        {
+        ui->helpLabel->setText(DialogWarningIcon() +
-            url += tr("Each point in the <b>custom seam allowance</b> path must be unique!");
+                               tr("Each point in the <b>custom seam allowance</b> path must be unique!"));
-            ui->helpLabel->setText(url);
+        return false;
-            return false;
-        }
     }
 
     if (not m_showMode && ui->comboBoxPiece->count() <= 0)
     {
-        url += tr("List of details is empty!");
+        ui->helpLabel->setText(DialogWarningIcon() + tr("List of details is empty!"));
-        ui->helpLabel->setText(url);
         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(DialogWarningIcon() + tr("Please, select a detail to insert into!"));
-        ui->helpLabel->setText(url);
         return false;
     }
 

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(DialogWarningIcon() + tr("You need more points!"));
-        uiTabPaths->helpLabel->setText(url);
+        return false;
-        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;
-        }
     }
 
-    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,
+    connect(uiTabPaths->listWidgetMainPath->model(), &QAbstractItemModel::rowsMoved, this, [this]()
-            [this](){MoveListRowTop(uiTabPaths->listWidgetMainPath);});
+    {
-    connect(uiTabPaths->toolButtonUp, &QToolButton::clicked, this,
+        ValidObjects(MainPathIsValid());
-            [this](){MoveListRowUp(uiTabPaths->listWidgetMainPath);});
+    });
-    connect(uiTabPaths->toolButtonDown, &QToolButton::clicked, this,
+
-            [this](){MoveListRowDown(uiTabPaths->listWidgetMainPath);});
+    connect(uiTabPaths->toolButtonTop, &QToolButton::clicked, this, [this]()
-    connect(uiTabPaths->toolButtonBottom, &QToolButton::clicked, this,
+    {
-            [this](){MoveListRowBottom(uiTabPaths->listWidgetMainPath);});
+        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());
+    });
 }
 
 //---------------------------------------------------------------------------------------------------------------------