LibreCAD › LibreCAD-dev

[Bug] rs_actiondrawellipsecenter3point

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

15 messages Options

cantcode

[Bug] rs_actiondrawellipsecenter3point

Hey guys cause I would like to learn how to code, I was trying to find the cause of a bug.
When drawing an ellipse from the center and three points (rs_actiondrawellipsecenter3points)
there seems to be a bug.
Trying to figure out what causes the crash I ended up in
LibreCAD-master/librecad/src/lib/math/lc_quadratic.cpp
at the line 416
return getIntersection(p1->flipXY(),p2->flipXY()).flipXY();

this line calls
LC_Quadratic LC_Quadratic::flipXY(void) const
and in here, a new LC_Quadratic object is created
LC_Quadratic qf(*this);

the problem is the above line calls the copyconstructor and it seems like the constructor
does not copy/create the m_mQuad attribute when the old m_mQuad is not quadratic ->size of m_mQuad is 0!!!! .
that's no problem until the line 422 (in lc_quadratic.cpp)
fabs(p2->m_mQuad(0,0))<RS_TOLERANCE && fabs(p2->m_mQuad(0,1))<RS_TOLERANCE
in RS_VectorSolutions LC_Quadratic::getIntersection(const LC_Quadratic& l1, const LC_Quadratic& l2)

because the size of m_mQuad is 0, boost throws an exception and LCAD crashes.

dxli

Re: [Bug] rs_actiondrawellipsecenter3point

nice job!

I remember the LC_Quadratic is based on boost stuff, might be the reason.

Can you fix this (and go ahead to review the LC_Quadratic class)?

I moved all getIntersection() methods to be based on quadratic. so removed logic like:

if ( line && circle) getIntersectionLineCircle();
if ( circle && circle) getIntersectionCircleCircle();
etc.

cantcode

Re: [Bug] rs_actiondrawellipsecenter3point

well,
I tried to "fix" that but got even more errors :)
I think I have not enough knowledge to make a good fix. So if someone want to do this, he/she can do that.
I will try to "fix" this and tell what I find out.
But I guess before I can "fix" this I will have to ask dozens of questions first ;) cause I just started pocking in the code and don't understand a lot.

dxli

Re: [Bug] rs_actiondrawellipsecenter3point

Hi,

It's a mistake in line 383 of lc_quadratic.cpp RS_VectorSolutions LC_Quadratic::getIntersection(const LC_Quadratic& l1, const LC_Quadratic& l2);

We only go ahead to find intersections when both quadratic forms are valid.

I pushed the fix to master branch, commit: fb5712b

The steps to reproduce the crash:

0, enable snap to grid only.
1, start draw ellipse by center and 3 points;
2, choose any center;
3, choose another point at one grid point as the first point;
4, when proposed to choose the second point, move to the same grid point chosen in step 3.

Results: LibreCAD crashes.

a patch to fix it:

diff --git a/librecad/src/lib/math/lc_quadratic.cpp b/librecad/src/lib/math/lc_quadratic.cpp
index 7c5712b..1dcf78a 100644
--- a/librecad/src/lib/math/lc_quadratic.cpp
+++ b/librecad/src/lib/math/lc_quadratic.cpp
@@ -380,7 +380,7 @@ LC_Quadratic LC_Quadratic::flipXY(void) const
RS_VectorSolutions LC_Quadratic::getIntersection(const LC_Quadratic& l1, const LC_Quadratic& l2)
{
RS_VectorSolutions ret;
- if( (l1.isValid() && l2.isValid()) == false ) {
+ if( (l1.isValid()==false && l2.isValid()) == false ) {
// DEBUG_HEADER();
// std::cout<<l1<<std::endl;
// std::cout<<l2<<std::endl;

cantcode

Re: [Bug] rs_actiondrawellipsecenter3point

now that was fast.
thanks for telling the reason, so I can have a look at it and try to understand.

Well at least I was in the same file when searching the bug ;)

dxli

Re: [Bug] rs_actiondrawellipsecenter3point

Just make sure you don't miss this commit:

https://github.com/LibreCAD/LibreCAD/commit/f2b089680b7ccf9f849a894a4feaf94b0800c81f

- if( (l1.isValid()==false && l2.isValid()) == false ) {
+ if( (l1.isValid()==false || l2.isValid()) == false ) {

The logic is clear: do not do intersection if any quadratic is invalid.

cantcode

Re: [Bug] rs_actiondrawellipsecenter3point

"- if( (l1.isValid()==false && l2.isValid()) == false ) {
+ if( (l1.isValid()==false || l2.isValid()) == false ) { "

I think there is a bracket missing or there are too much of them.
trying to fix this bracket issue, I still get a crash.
will try to investigate a litte more.

LordOfBikes

Re: [Bug] rs_actiondrawellipsecenter3point

Administrator

Maybe you are right,
you can try this
if( (l1.isValid()==false || l2.isValid()==false ) ) {
or this
if( l1.isValid()==false || l2.isValid()==false ) {

If this doesn't help, I must have a look at the source file myself.

Armin

^{investing less than half an hour into Search function can save hours or days of waiting for a solution}

dxli

Re: [Bug] rs_actiondrawellipsecenter3point

In reply to this post by cantcode

You are right!

it should read:

if( l1.isValid()==false || || l2.isValid()==false ) {

my bad again :(

Thanks a lot to point out

Dongxu

On Fri, Jun 14, 2013 at 1:22 PM, cantcode [via LibreCAD] <[hidden email]> wrote:

"- if( (l1.isValid()==false && l2.isValid()) == false ) {
+ if( (l1.isValid()==false || l2.isValid()) == false ) { "

I think there is a bracket missing or there are too much of them.
trying to fix this bracket issue, I still get a crash.
will try to investigate a litte more.

If you reply to this email, your message will be added to the discussion below:

http://forum.librecad.org/Bug-rs-actiondrawellipsecenter3point-tp5708194p5708348.html

To start a new topic under LibreCAD-dev, email [hidden email]
To unsubscribe from LibreCAD-dev, click here.
NAML

--
Dongxu Li, Ph.D.
www.librecad.org

dxli

Re: [Bug] rs_actiondrawellipsecenter3point

In reply to this post by LordOfBikes

I still get a crash:

0, start LibreCAD with empty drawing;
1, start drawing ellipse with center and 3 points;
2, select only "snap by grid" in snap toolbar;
3, select center a center at: 100, 100;
4, select the first point at @100, 10;
5, move mouse point to make snap point for the second point at @80, 10;

results:

LibreCAD crashes.

LordOfBikes wrote

Maybe you are right,
you can try this
if( (l1.isValid()==false || l2.isValid()==false ) ) {
or this
if( l1.isValid()==false || l2.isValid()==false ) {

If this doesn't help, I must have a look at the source file myself.

Armin

LordOfBikes

Re: [Bug] rs_actiondrawellipsecenter3point

Administrator

OK, I will sneak a peek at it.

Armin

^{investing less than half an hour into Search function can save hours or days of waiting for a solution}

dxli

Re: [Bug] rs_actiondrawellipsecenter3point

it's a tolerance issue.

to test whether a double is zero, the broken test is:

foo < 1.e-20

looks like it's more reasonable to do:

foo < 1.5e-15

if this happens again, we increase it further

commit 5ecbc49891f81dc76eebd13742e4c4f23911b368

LordOfBikes

Re: [Bug] rs_actiondrawellipsecenter3point

Administrator

Well done!

I reproduced the crash as you described with the previous version.
With your new commit it is OK.

Armin

^{investing less than half an hour into Search function can save hours or days of waiting for a solution}

cantcode

Re: [Bug] rs_actiondrawellipsecenter3point

can someone pleasse tell me why we have to square the vector??

"if( (sol.get(mSize) - sol.get(mSize-1)).squared() < RS_TOLERANCE15 ) {..."

so we take the last two vectors and subtract them to get a new vector.
why do we then use .squared afterwards?
Isn't the RS_TOLERANCE15 the length which tells us if we should consider the next point as a new one?
If so, then one would have to calculate the length of the new vector (coming from the subtraction).
IIRC, the length of a vector is calculated by squaring the x,y,z components, adding them and then square root the result.

dxli

Re: [Bug] rs_actiondrawellipsecenter3point

actually, squared() is a misnomer here. It should be called squaredLength() or lengthSquared():

for a 2D vector, RS_Vector( x, y);

double RS_Vector::squared() const {

        return x*x + y*y;

we use squared length instead of length when we only need to compare length, so, we skip the extra sqrt() here.

On Sat, Jun 15, 2013 at 4:34 PM, cantcode [via LibreCAD] <[hidden email]> wrote:

can someone pleasse tell me why we have to square the vector??

"if( (sol.get(mSize) - sol.get(mSize-1)).squared() < RS_TOLERANCE15 ) {..."

so we take the last two vectors and subtract them to get a new vector.
why do we then use .squared afterwards?
Isn't the RS_TOLERANCE15 the length which tells us if we should consider the next point as a new one?
If so, then one would have to calculate the length of the new vector (coming from the subtraction).
IIRC, the length of a vector is calculated by squaring the x,y,z components, adding them and then square root the result.

If you reply to this email, your message will be added to the discussion below:

http://forum.librecad.org/Bug-rs-actiondrawellipsecenter3point-tp5708194p5708359.html

To start a new topic under LibreCAD-dev, email [hidden email]
To unsubscribe from LibreCAD-dev, click here.
NAML

--
Dongxu Li, Ph.D.
www.librecad.org