From e876a4d61131659388f74b60c2842bc3762cb63e Mon Sep 17 00:00:00 2001 From: Roman Telezhynskyi Date: Thu, 3 Feb 2022 15:48:52 +0200 Subject: [PATCH] Fix issue in tool cut spline. Infinite loop while calculating a t parameter. --- ChangeLog.txt | 1 + src/libs/vgeometry/vabstractcubicbezier.cpp | 28 +++++++---- src/libs/vgeometry/vabstractcubicbezier.h | 3 +- src/libs/vgeometry/vcubicbezier.cpp | 7 +++ src/libs/vgeometry/vcubicbezier.h | 1 + src/libs/vgeometry/vspline.cpp | 7 +++ src/libs/vgeometry/vspline.h | 1 + src/test/ValentinaTest/tst_vspline.cpp | 53 ++++++++++++++++++--- src/test/ValentinaTest/tst_vspline.h | 1 + 9 files changed, 85 insertions(+), 17 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 7581f80b6..869b36166 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -29,6 +29,7 @@ - New path validation Invalid segment. - [smart-pattern/valentina#43] Background image. - Fix alias for tool Cut Arc. +- Fix infinite loop in tool cut spline. # Valentina 0.7.49 July 1, 2021 - Fix crash. diff --git a/src/libs/vgeometry/vabstractcubicbezier.cpp b/src/libs/vgeometry/vabstractcubicbezier.cpp index f6769426e..4bda1d047 100644 --- a/src/libs/vgeometry/vabstractcubicbezier.cpp +++ b/src/libs/vgeometry/vabstractcubicbezier.cpp @@ -503,28 +503,38 @@ QString VAbstractCubicBezier::NameForHistory(const QString &toolName) const } //--------------------------------------------------------------------------------------------------------------------- -qreal VAbstractCubicBezier::GetParmT(qreal length) const +auto VAbstractCubicBezier::GetParmT(qreal length) const -> qreal { + const qreal base = GetRealLength(); if (length < 0) { return 0; } - else if (length > GetLength()) + + if (length > base) { - length = GetLength(); + length = base; } - const qreal eps = 0.001 * length; + constexpr qreal eps = UnitConvertor(0.001, Unit::Mm, Unit::Px); qreal parT = 0.5; qreal step = parT; - qreal splLength = LengthT(parT); + qreal splLength = 0; - while (qAbs(splLength - length) > eps) + do { + splLength = RealLengthByT(parT); step /= 2.0; + + if (qFuzzyIsNull(step)) + { + break; + } + splLength > length ? parT -= step : parT += step; - splLength = LengthT(parT); } + while (qAbs(splLength - length) > eps); + return parT; } @@ -591,7 +601,7 @@ qreal VAbstractCubicBezier::LengthBezier(const QPointF &p1, const QPointF &p2, c } //--------------------------------------------------------------------------------------------------------------------- -qreal VAbstractCubicBezier::LengthT(qreal t) const +qreal VAbstractCubicBezier::RealLengthByT(qreal t) const { if (t < 0 || t > 1) { @@ -622,5 +632,5 @@ qreal VAbstractCubicBezier::LengthT(qreal t) const seg123_234.setLength(seg123_234.length () * t); const QPointF p1234 = seg123_234.p2(); - return LengthBezier ( static_cast(GetP1()), p12, p123, p1234, GetApproximationScale()); + return LengthBezier ( static_cast(GetP1()), p12, p123, p1234, maxCurveApproximationScale); } diff --git a/src/libs/vgeometry/vabstractcubicbezier.h b/src/libs/vgeometry/vabstractcubicbezier.h index c740ab90a..5c501b831 100644 --- a/src/libs/vgeometry/vabstractcubicbezier.h +++ b/src/libs/vgeometry/vabstractcubicbezier.h @@ -62,7 +62,7 @@ public: virtual QString NameForHistory(const QString &toolName) const override; qreal GetParmT(qreal length) const; - qreal LengthT(qreal t) const; + qreal RealLengthByT(qreal t) const; protected: virtual void CreateName() override; @@ -75,6 +75,7 @@ protected: virtual QPointF GetControlPoint1() const =0; virtual QPointF GetControlPoint2() const =0; + virtual auto GetRealLength() const -> qreal =0; }; QT_WARNING_POP diff --git a/src/libs/vgeometry/vcubicbezier.cpp b/src/libs/vgeometry/vcubicbezier.cpp index 2f2303f2a..45d861b39 100644 --- a/src/libs/vgeometry/vcubicbezier.cpp +++ b/src/libs/vgeometry/vcubicbezier.cpp @@ -252,3 +252,10 @@ QPointF VCubicBezier::GetControlPoint2() const { return static_cast(GetP3()); } + +//--------------------------------------------------------------------------------------------------------------------- +qreal VCubicBezier::GetRealLength() const +{ + return LengthBezier (static_cast(GetP1()), static_cast(GetP2()), + static_cast(GetP3()), static_cast(GetP4()), maxCurveApproximationScale); +} diff --git a/src/libs/vgeometry/vcubicbezier.h b/src/libs/vgeometry/vcubicbezier.h index aacdd2197..b4647fb96 100644 --- a/src/libs/vgeometry/vcubicbezier.h +++ b/src/libs/vgeometry/vcubicbezier.h @@ -84,6 +84,7 @@ public: protected: virtual QPointF GetControlPoint1() const override; virtual QPointF GetControlPoint2() const override; + auto GetRealLength() const -> qreal override; private: QSharedDataPointer d; diff --git a/src/libs/vgeometry/vspline.cpp b/src/libs/vgeometry/vspline.cpp index a9c6eaea9..b9f209398 100644 --- a/src/libs/vgeometry/vspline.cpp +++ b/src/libs/vgeometry/vspline.cpp @@ -611,3 +611,10 @@ QPointF VSpline::GetControlPoint2() const { return static_cast(GetP3 ()); } + +//--------------------------------------------------------------------------------------------------------------------- +auto VSpline::GetRealLength() const -> qreal +{ + return LengthBezier(static_cast(GetP1()), static_cast(GetP2()), static_cast(GetP3()), + static_cast(GetP4()), maxCurveApproximationScale); +} diff --git a/src/libs/vgeometry/vspline.h b/src/libs/vgeometry/vspline.h index 1f701a909..8bb4dfaa7 100644 --- a/src/libs/vgeometry/vspline.h +++ b/src/libs/vgeometry/vspline.h @@ -117,6 +117,7 @@ public: protected: virtual QPointF GetControlPoint1() const override; virtual QPointF GetControlPoint2() const override; + auto GetRealLength () const -> qreal override; private: QSharedDataPointer d; QVector CalcT(qreal curveCoord1, qreal curveCoord2, qreal curveCoord3, qreal curveCoord4, diff --git a/src/test/ValentinaTest/tst_vspline.cpp b/src/test/ValentinaTest/tst_vspline.cpp index 7363302a1..b660fd6c9 100644 --- a/src/test/ValentinaTest/tst_vspline.cpp +++ b/src/test/ValentinaTest/tst_vspline.cpp @@ -28,6 +28,7 @@ #include "tst_vspline.h" #include "../vgeometry/vspline.h" +#include "def.h" #include @@ -717,18 +718,56 @@ void TST_VSpline::CompareThreeWays() CompareSplines(spl3, spl1); } +//--------------------------------------------------------------------------------------------------------------------- +void TST_VSpline::TestParametrT_data() +{ + QTest::addColumn("spl"); + QTest::addColumn("length"); + + VPointF p1(30, 39.999874015748034, QStringLiteral("p1"), 15, 30); + VPointF p4(2883.86674323853821, 805.33182541168674, QStringLiteral("p4"), 9.9999874015748045, 15); + + VSpline spl(p1, p4, 240.60499999999999, QStringLiteral("240.605"), 260.36399999999998, QStringLiteral("260.364"), + 6614.8535433070883, QStringLiteral("175.018"), 10695.382677165355, QStringLiteral("282.982")); + + qreal base = spl.GetLength(); + + qreal step = 0; + do + { + QTest::newRow(qUtf8Printable(QStringLiteral("Curve 1. Step = %1").arg(step))) << spl << base * step; + step += 0.001; + } + while(step <= 1); + + p1 = VPointF(3476.6410658375944, 334.08136371135333, QStringLiteral("p1"), -141.34148031496062, 109.00951181102363); + p4 = VPointF(3483.5141183177025, 333.72231804361377, QStringLiteral("p4"), 48, 102.99930708661418); + + spl = VSpline(p1, p4, 3.1976200000000001, QStringLiteral("3.19762"), 175.24700000000001, QStringLiteral("175.247"), + 7.5076913385826796, QStringLiteral("0.198641"), 9.7351181102362219, QStringLiteral("0.257575")); + + base = spl.GetLength(); + + step = 0; + do + { + QTest::newRow(qUtf8Printable(QStringLiteral("Curve 2. Step = %1").arg(step))) << spl << base * step; + step += 0.001; + } + while(step <= 1); + + QTest::newRow(qUtf8Printable(QStringLiteral("Curve 2. Bug.").arg(step))) << spl << 4.7813492845686536; +} + //--------------------------------------------------------------------------------------------------------------------- void TST_VSpline::TestParametrT() { - VPointF p1(1168.8582803149607, 39.999874015748034, "p1", 5.0000125984251973, 9.9999874015748045); - VPointF p4(681.33729132409951, 1815.7969526662778, "p4", 5.0000125984251973, 9.9999874015748045); + QFETCH(VSpline, spl); + QFETCH(qreal, length); - VSpline spl(p1, p4, 229.381, 41.6325, 0.96294100000000005, 1.00054, 1); + const qreal resLength = spl.RealLengthByT(spl.GetParmT(length)); - const qreal halfLength = spl.GetLength()/2.0; - const qreal resLength = spl.LengthT(spl.GetParmT(halfLength)); - - QVERIFY(qAbs(halfLength - resLength) < UnitConvertor(0.5, Unit::Mm, Unit::Px)); + QVERIFY(qAbs(length - resLength) < UnitConvertor(1, Unit::Mm, Unit::Px)); } //--------------------------------------------------------------------------------------------------------------------- diff --git a/src/test/ValentinaTest/tst_vspline.h b/src/test/ValentinaTest/tst_vspline.h index a35570e33..38e39a47e 100644 --- a/src/test/ValentinaTest/tst_vspline.h +++ b/src/test/ValentinaTest/tst_vspline.h @@ -49,6 +49,7 @@ private slots: void GetSegmentPoints_RotateTool(); void GetSegmentPoints_issue767(); void CompareThreeWays(); + void TestParametrT_data(); void TestParametrT(); void TestLengthByPoint_data(); void TestLengthByPoint();