1. Roman Telezhynskyi Avatar Roman Telezhynskyi
  2. Untitled project
  3. Valentina
  4. Issues

Error when drawing a curved path

Issue #485 resolved
Barbara Weberkind created an issue

.val file, .vit file and a screenshot of the error is attached. Open the .val file with the measurements. Try to draw a curved path: Curve tool menu -> click 'curved path' -> Click point 'HalsansatzVorne', then click point 'AusschnittSchulter'. The application displays a warning 'QPainterPath:lineTo: Adding point where x or y is NaN or Inf, ignoring call'. When the warning is OK'ed, the application crashes. In fact, this might be reproducible with any point. Drawing paths does not seem to be possible.

Comments (69)

  3. Barbara Weberkind reporter

    Hi Roman, I am observing this in the develop branch, hence 0.5.0 . I have debugged it and have tracked it down to DialogSpline.cpp and VisToolSpline.cpp. The dialog gets the spline from the VisTool like so:

    spl = VSpline(*GetP1(), path->GetP2(), path->GetP3(), *GetP4());

    where path is the VisTool. The VisTool is supposed to have the second and third defining point, p2 and p3, but they get set all wrong in RefreshGeomentry(). They are actually the same as p1 and p4.

    When I create a curve interactively in the VisTool, I click one point and then the next point. Between the clicks, the line is completely straight anyway, so it is not really difficult to determine p2 and p3. E.g., I can make P2 and P3 sit at 30% and 70% of the line with brute force (just calculating that as constant offsets along the line), and then the crash doesn't happen.

    But that only fixes it for the curve tool, the crash also happens for the curved path (multi-spline), and I haven't even tried the various other curve tools or an arc.

    I believe one should rather fix the geometry calculation of p2 and p3, are you planning to do that?

    Thanks.

  4. Roman Telezhynskyi repo owner

    I can't reproduce. I tried all curve tools and many different points. All is fine. I don't see the warning message.

  5. Roman Telezhynskyi repo owner

    I believe one should rather fix the geometry calculation of p2 and p3, are you planning to do that?

    Now i see about what you are talking. I recommend you to try click on a point, hold mouse button and move it. And you will see. So, no we do not have plans to "fix" it.

    But can you find which data cause the error message? I still can't get your result.

  7. Barbara Weberkind reporter

    Only contains four points and one valid spline -- and an invalid spline. For me, valentina crashes with the described error when opening that file. The invalid spline with zero length control vectors is at fault.

  8. Barbara Weberkind reporter

    Hi, I attached a much simpler file, fourPoints.val, but this one for me reproduces the error already when loading. It contains a spline with zero length control vectors. Furthermore, when I open this file, I now even get a more detailed error position somewhere in the Qt library, and the beauty of it is that it shows me the version of the Qt library, which I think is the difference. This is Qt 5.6.0, msvc2015_64. I will attach the error message. It was extremely hard to debug because it doesn't throw at the position where the error is made, but in a different thread when Qt tries to display the line.

  10. Barbara Weberkind reporter

    Then, to avoid this error, I have made and committed a fix for this at the point where the file is loaded . Note that the diff viewer on that page can be slightly misleading, I haven't removed any code, only moved it into the else branch of the test for the error condition.

    I am not creating a pull request of this one yet. The solution is to omit the faulty spline from the pattern, mark the pattern as modified and log the fact. I wonder whether also a message box should be displayed informing the user. If so, I haven't worked out yet how to create translations for the messages. I have put a TODO there for the message box code.

  13. Barbara Weberkind reporter

    This file is contrived, but I actually had a pattern file created by valentina which had such a spline.

    The point is that the curve creation tool for me creates such splines. It creates splines from 4 points where p1 = p2 and p3 = p4, i.e. with zero control vector length and angle. And then, when displaying that, my Qt version crashes.

    It so happened that one time only, valentina stayed stable enough after that that I could still save the file. Then I had a file that already crashed on startup. That helped me going after this error.

  14. Barbara Weberkind reporter

    I have looked at the logfile a lot, I even created more logs (which I have removed again). The crash actually happens outside of user code and has no proper trace. It happens when the file is finished loading and Qt is about to display the file. Qt doesn't log. The exception message is

    "WARNING:painting\qpainterpath.cpp(724)] void __cdecl QPainterPath::lineTo(const class QPointF &): default: QPainterPath::lineTo: Adding point where x or y is NaN or Inf, ignoring call"

    .. but this is none of the lineTo commands that the code contains explicitly, I have tracked them all. It appears to be a call by Qt internally when it wants to draw that degenerate spline

  17. Barbara Weberkind reporter

    One question: You are saying you can't reproduce. But it prevents me completely from working. I can't draw any curve. If I finally track this down and get it fixed (for my version of the library), will you accept the pull request?

  19. Roman Telezhynskyi repo owner

    One question: You are saying you can't reproduce. But it prevents me completely from working. I can't draw any curve. If I finally track this down and get it fixed (for my version of the library), will you accept the pull request?

    Yes! :)

  20. Barbara Weberkind reporter

    I can of course delete the incorrect spline. And even better, look at the change I did -- in the code change I propose, valentina will do that automatically when opening the file.

    But the point is that I can't draw any new curves at all, because they all come out degenerate like this.

  21. Barbara Weberkind reporter

    I now have a clue, I will try debug this a little more and see whether I can nail it down.

  22. Roman Telezhynskyi repo owner

    No, let try to find what's wrong. Delete correct spline from file and start debugging in VPattern::ParseToolSpline.

  23. Roman Telezhynskyi repo owner

    Then you will go to VToolSpline::Create. What you are looking is string

    auto spline = new VSpline(*p1, *p4, calcAngle1, a1, calcAngle2, a2, calcLength1, l1, calcLength2, l2);
  24. Roman Telezhynskyi repo owner

    Check if spline is ok. And all values are correct and do not contain Nan or Inf values.

  27. Barbara Weberkind reporter

    I don't understand. VPattern::ParseToolSpline isn't broken. That's the function that gets called when loading the file, not when creating a curve. It didn't have an error. I have already debugged that one last night. It reads the correct and the degenerate spline correctly, exactly as they are in the file. What else should it do? And the spline is actually degenerate, why bother debugging how it gets created? My fix only makes it reject the degenerate spline, and not call new VToolSpline::Create() on it at all.

  28. Barbara Weberkind reporter

    A spline with zero control point length is mathematically degenerate, underdefined.

  29. Roman Telezhynskyi repo owner

    I understand what you are trying to do, but i am not satisfied with this solution. What i want is actual place where "0" values create problems. "0" should be correct value for a curve.

  30. Roman Telezhynskyi repo owner

    A spline with zero control point length is mathematically degenerate, underdefined.

    Yes, but if user want to why not?

  31. Roman Telezhynskyi repo owner

    What else should it do? And the spline is actually degenerate, why bother debugging how it gets created?

    Because you problem is value Nan or Inf that you get. Since you get such a value it is like a bomb. It is only matter of time that such bomb will blow. I want to know where exactly this happens. Because for degenerate spline is just a theory. I want a prove that you are right. And we shouldn't allow a user to have such a spline.

  32. Roman Telezhynskyi repo owner

    A spline with zero control point length is mathematically degenerate, underdefined.

    Sorry, did not read carefully. So prove me that such data generate Nan or Inf result.

  34. Barbara Weberkind reporter

    I don't get NaN or Inf in our code. I just get 0s. As I said, the lineTo that throws, and the NaN, is not in valentina code, it is in Qt code.

    I suspect that Qt is wanting to draw the curve by deriving a derivative or direction somehow. And you do derivatives or directions computationally with approximations, taking a small enough step and dividing the function difference by the length of the step.

    I imagine that it wants to step along the control vector that way. But because its length is zero, when you try to divide by zero, you get NaN or Inf. The common-world description for that problem is, when the direction is 0, you don't know which direction to turn.

  35. Barbara Weberkind reporter

    That's why I think, at least my version of the Qt library is not able to handle a degenerate spline like that, so the user can't have it.

  36. Barbara Weberkind reporter

    You can load the curve. What does that look like when you see it with your version of Qt? Does it come up as a straight line?

  37. Barbara Weberkind reporter

    Because that's really what the spline degenerates to in that case. A workable solution would be to replace the spline, if it is discovered to be degenerate like that, by a straight line.

  38. Roman Telezhynskyi repo owner

    You can load the curve. What does that look like when you see it with your version of Qt? Does it come up as a straight line?

    It is a straight line.

  39. Roman Telezhynskyi repo owner

    The second place you should check is VAbstractSpline::ToolPath(PathDirection direction).

  40. Roman Telezhynskyi repo owner

    Go to VAbstractCurve::GetPath(PathDirection direction) and check string

    QVector<QPointF> points = GetPoints();
  42. Barbara Weberkind reporter

    Hi, done this now. For that curve, the path has 3 points, and the last 2 are equal. But no NaN or Inf values. I still think Qt generates them internally.

  43. Roman Telezhynskyi repo owner

    Ok, that is good. What left is full backtrace to error. Show me if you can. Even is the error in internal Qt's code.

  44. Roman Telezhynskyi repo owner

    I mean run the application in a debugger. It will show you the place where error occured. And i ask you to show me a stack trace. Do you use Qt Creator or Visual Studio?

  46. Roman Telezhynskyi repo owner

    The backtrace shows that this is issue with QGraphicsLineItem::shape. There is only one place with line. This is line to point label.

  47. Barbara Weberkind reporter

    Yes. The error obviously occurred in QGraphicsLineItem::shape. I only don't understand what you mean by 'This is line to point label.' -- I don't see anything about a point label in the backtrace.

  48. Roman Telezhynskyi repo owner

    Right now i will try build the code in VirtualBox with MSVC. Hope it will help more.

  49. Roman Telezhynskyi repo owner

    I don't see anything about a point label in the backtrace.

    I either. But i see QGraphicsLineItem, and spline is based on QGraphicsPathItem. There are not so many places where we use lines, especially in this case. The line from point to label is one of the cases.

  50. Barbara Weberkind reporter

    The control line of a point in vcontrolpointspline is a QGraphicsLineItem. And that's exactly the beast that the problem is with....

  52. Barbara Weberkind reporter

    Trying to get my head around this, but 

    void VControlPointSpline::SetCtrlLine(const QPointF &controlPoint, const QPointF &splinePoint)
{
    QPointF p1, p2;
    VGObject::LineIntersectCircle(QPointF(), radius, QLineF( QPointF(), splinePoint-controlPoint), p1, p2);
    controlLine->setLine(QLineF(splinePoint-controlPoint, p1));
}

    assuming that the controlpoint is the same as p1, the line would never intersect the circle... isn't that right? maybe here p1 or p2 are becoming NaN?

    OK, but that doesn't seem to be the case, the two points are not the same.

  56. Barbara Weberkind reporter

    yep.

    VGObject::LineIntersectCircle() in vgObject.cpp does 

        const qreal t = QLineF (QPointF (0, 0), QPointF (b, - a)).length();
    // add to projection a vectors aimed to points of intersection
    p1 = addVector (p, QPointF (0, 0), QPointF (- b, a), k / t);
    p2 = addVector (p, QPointF (0, 0), QPointF (b, - a), k / t);

    and does not check whether t is 0.

    Here is the relevant stack trace:

    000000ba`30556c90 00007ff7`6aafc286 valentina!VGObject::LineIntersectCircle(class QPointF * center = 0x000000ba`30556ee0, double radius = 5.6692913385826778, class QLineF * line = 0x000000ba`30556f20, class QPointF * p1 = 0x000000ba`30556e50, class QPointF * p2 = 0x000000ba`30556eb0)+0x12b [d:\users\barbara\projects\valentina-develop\valentina\src\libs\vgeometry\vgobject.cpp @ 381]
000000ba`30556e20 00007ff7`6aafb22a valentina!VControlPointSpline::SetCtrlLine(class QPointF * controlPoint = 0x000000ba`30557390, class QPointF * splinePoint = 0x000000ba`30557380)+0xc6 [d:\users\barbara\projects\valentina-develop\valentina\src\libs\vwidgets\vcontrolpointspline.cpp @ 259]
000000ba`30556f90 00007ff7`6a9524f8 valentina!VControlPointSpline::VControlPointSpline(int * indexSpline = 0x000000ba`30557048, SplinePointPosition position = FirstPoint (0n0), class QPointF * controlPoint = 0x000000ba`30557390, class QPointF * splinePoint = 0x000000ba`30557380, Unit patternUnit = Cm (0n1), bool freeAngle = true, bool freeLength = true, class QGraphicsItem * parent = 0x000001d5`bcdb8158)+0x14a [d:\users\barbara\projects\valentina-develop\valentina\src\libs\vwidgets\vcontrolpointspline.cpp @ 83]
000000ba`30556fd0 00007ff7`6a9536b6 valentina!VToolSpline::VToolSpline(class VAbstractPattern * doc = 0x000001d5`bc85e4d0, class VContainer * data = 0x000001d5`bc6527c0, unsigned int id = 6, class QString * color = 0x000000ba`30557790, Source * typeCreation = 0x000000ba`30557750, class QGraphicsItem * parent = 0x00000000`00000000)+0x438 [d:\users\barbara\projects\valentina-develop\valentina\src\libs\vtools\tools\drawtools\toolcurve\vtoolspline.cpp @ 72]
000000ba`305573e0 00007ff7`6a952fbc valentina!VToolSpline::Create(unsigned int _id = 0, class VSpline * spline = 0x000001d5`bcce2200, class QString * color = 0x000000ba`30557790, class VMainGraphicsScene * scene = 0x000001d5`bc7f0f00, class VAbstractPattern * doc = 0x000001d5`bc85e4d0, class VContainer * data = 0x000001d5`bc6527c0, Document * parse = 0x000000ba`30557751, Source * typeCreation = 0x000000ba`30557750)+0x6b6 [d:\users\barbara\projects\valentina-develop\valentina\src\libs\vtools\tools\drawtools\toolcurve\vtoolspline.cpp @ 184]
000000ba`30557710 00007ff7`6a8978dc valentina!VToolSpline::Create(class DialogTool * dialog = 0x000001d5`bcccccf0, class VMainGraphicsScene * scene = 0x000001d5`bc7f0f00, class VAbstractPattern * doc = 0x000001d5`bc85e4d0, class VContainer * data = 0x000001d5`bc6527c0)+0x1ac [d:\users\barbara\projects\valentina-develop\valentina\src\libs\vtools\tools\drawtools\toolcurve\vtoolspline.cpp @ 134]
000000ba`305577f0 00007ff7`6a8ba3c2 valentina!MainWindow::ClosedDialogWithApply<VToolSpline>(int result = 0n1)+0x14c [d:\users\barbara\projects\valentina-develop\valentina\src\app\valentina\mainwindow.cpp @ 667]
000000ba`30557890 00007ff7`6a8a96ed valentina!QtPrivate::FunctorCall<QtPrivate::IndexesList<0>,QtPrivate::List<int>,void,void (<function> * f = 0x000000ba`305578f0, class MainWindow * o = 0x000000ba`3055f430, void ** arg = 0x000000ba`30557c10)+0x42 [c:\qt\qt5.6.0\5.6\msvc2015_64\include\qtcore\qobjectdefs_impl.h @ 501]
000000ba`305578d0 00007ff7`6a8bcdb6 valentina!QtPrivate::FunctionPointer<void (<function> * f = 0x000000ba`30557970, class MainWindow * o = 0x000000ba`3055f430, void ** arg = 0x000000ba`30557c10)+0x3d [c:\qt\qt5.6.0\5.6\msvc2015_64\include\qtcore\qobjectdefs_impl.h @ 521]
000000ba`30557920 00000000`56199868 valentina!QtPrivate::QSlotObject<void (int which = 0n1, class QtPrivate::QSlotObjectBase * this_ = 0x000001d5`bcc6a2e0, class QObject * r = 0x000000ba`3055f430, void ** a = 0x000000ba`30557c10, bool * ret = 0x00000000`00000000)+0xb6 [c:\qt\qt5.6.0\5.6\msvc2015_64\include\qtcore\qobject_impl.h @ 144]
000000ba`305579a0 00000000`5618845a Qt5Cored!QtPrivate::QSlotObjectBase::call(class QObject * r = 0x000000ba`3055f430, void ** a = 0x000000ba`30557c10)+0x38 [c:\users\qt\work\qt\qtbase\src\corelib\kernel\qobject_impl.h @ 124]
000000ba`305579e0 00000000`56187d18 Qt5Cored!QMetaObject::activate(class QObject * sender = 0x000001d5`bcccccf0, int signalOffset = 0n10, int local_signal_index = 0n0, void ** argv = 0x000000ba`30557c10)+0x72a [c:\users\qt\work\qt\qtbase\src\corelib\kernel\qobject.cpp @ 3720]
000000ba`30557bc0 00007ff7`6a8f819a Qt5Cored!QMetaObject::activate(class QObject * sender = 0x000001d5`bcccccf0, struct QMetaObject * m = 0x00007ff7`6b1932c8, int local_signal_index = 0n0, void ** argv = 0x000000ba`30557c10)+0x38 [c:\users\qt\work\qt\qtbase\src\corelib\kernel\qobject.cpp @ 3596]
000000ba`30557bf0 00007ff7`6a8efc4b valentina!DialogTool::DialogClosed(int _t1 = 0n1)+0x3a [d:\users\barbara\projects\valentina-develop\build-valentina-windows_kits-debug\src\libs\vtools\moc\moc_dialogtool.cpp @ 245]
000000ba`30557c30 00007ff7`6aa210a7 valentina!DialogTool::DialogAccepted(void)+0x2b [d:\users\barbara\projects\valentina-develop\valentina\src\libs\vtools\dialogs\tools\dialogtool.cpp @ 749]
000000ba`30557c60 00007ff7`6a880fc2 valentina!DialogSpline::ShowDialog(bool click = true)+0x977 [d:\users\barbara\projects\valentina-develop\valentina\src\libs\vtools\dialogs\tools\dialogspline.cpp @ 531]

    How I did this: Create a curve, but only click on the second point, don't drag it.

  59. Barbara Weberkind reporter

    valentina definitely doing a division by zero in this case. Who should fix, and how?

  63. Barbara Weberkind reporter

    Cool, thank you! In fact, I am now curious why it didn't occur on your system... do you work on Linux? Because it did a division by 0 -- should not have been different... did the Qt library in your case tolerate the NaN?

    Also good that you do the fuzzy comparison to 0, because on some rare cases I have ended up with incredibly long control lines, obviously having divided by something very close to 0.

  64. Roman Telezhynskyi repo owner

    do you work on Linux?

    Yes, i use Ubuntu 14.04.

    Because it did a division by 0 -- should not have been different...

    I also get nan, but

    did the Qt library in your case tolerate the NaN?

    Looks like.

    Also good that you do the fuzzy comparison to 0, because on some rare cases I have ended up with incredibly long control lines, obviously having divided by something very close to 0.

    Yes, i know. We always use fuzzy comparison in the code. GCC warns about such cases.

  65. Susan Spencer

    It is entirely possible that in Qt's implementation of b-splines and bezier curves there is a problem for any curve (P0, c1, c2, P1) where (P0==c1) or (P1==c2).

    Where (P0==c1), length P0 to c1 might be stored as NaN instead of 0, so that de Castlejau's algorithm bombs out when dividing NaN (length from P1 to c1) by t to find t1.

    I vote that we prevent the user from defining these curves. If the user needs a line then the user should create a line, all other curves can be adjusted so that P0<>c1 and P1<>c2.

  66. Roman Telezhynskyi repo owner

    It is entirely possible that in Qt's implementation of b-splines and bezier curves there is a problem for any curve (P0, c1, c2, P1) where (P0==c1) or (P1==c2).

    We don't use Qt's implementation.

    Where (P0==c1), length P0 to c1 might be stored as NaN instead of 0, so that de Castlejau's algorithm bombs out when dividing NaN (length from P1 to c1) by t to find t1.

    No, we checked. All is fine. The length is 0.

    I vote that we prevent the user from defining these curves. If the user needs a line then the user should create a line, all other curves can be adjusted so that P0<>c1 and P1<>c2.

    I don't agree. I don't see any reason why we should prevent the user from defining these curves. No one still can't prove me that this case call a crash.

  67. Barbara Weberkind reporter

    Hi Susan,

    I first was of the same opinion, but Roman has convinced me, because there is an elegant solution to it -- make it 0, not NaN.

    It makes mathematical sense. Imagine a perfect S curve. Now, let the two control lengths get shorter and shorter. The shorter they get, the more the 'S' becomes a straight line. Imagine the length going towards zero. At the same time, the curve converges to a straight line. As a curve, it is degenerate, but it can be drawn.

    So, it makes sense to accept that as the limit case. We don't know in how many ways users may arrive at a 0 control point length, but they might, and if we allow the case, then the experience stays smooth.

  70. Log in to comment
Assignee
–
Type
bug
Priority
major
Status
resolved
Component
Draw mode
Milestone
–
Version
–
Votes
0
Watchers
1
Jira: the preferred issue tracker for Bitbucket. Join the team!