Hi,
I am using inkscape-0.41 at an Athlon64 machine. The program was crashing whenever I used some tool which uses bezier curves (caligraphic tool, free hand tool (i think), etc.). The programa failed on a assertion, requiring the last member of a double array to be 1.0, like in: g_assert("bla".., u[len-1] == 1.0)
This kind of comparision (equals) on double is unreliable, since rounding errors may introduce subtle differences between two values which we would expect to be equal.
The correct way to verify this should be like: g_assert("blas",u[len-1]>=0.999 && [len-1]<=1.001).
Where the constants are chosen accordingly.
The code that creates this array is in the function chord_length_parameterize
This is a snipped of chord_length_parameterize:
gdouble tot_len = u[len - 1]; g_return_if_fail( tot_len != 0 ); if (finite(tot_len)) { for (unsigned i = 1; i < len; ++i) { u[i] /= tot_len; } } else { /* We could do better, but this probably never happens anyway. */ for (unsigned i = 1; i < len; ++i) { u[i] = i / (gdouble) ( len - 1 ); } }
In both cases, u[len-1] should be very near 1.0, but not necessarily equals 1.0. I added the line u[len-1] = 1.0; after the loop. And the problem stopped. However, the true bug lies in the comparision (u[len-1]==1.0)
By the way. Thanks for developing this excellent program. I am now deploying it at an workplace in place of Corel Draw.
Yours,
João Rafael Moraes Nicola
On Mon, Jul 11, 2005 at 10:11:44PM -0300, João Rafael Nicola wrote:
This is a snipped of chord_length_parameterize:
gdouble tot_len = u[len - 1]; g_return_if_fail( tot_len != 0 ); if (finite(tot_len)) { for (unsigned i = 1; i < len; ++i) { u[i] /= tot_len; } } else { /* We could do better, but this probably never happens anyway. */ for (unsigned i = 1; i < len; ++i) { u[i] = i / (gdouble) ( len - 1 ); } }
In both cases, u[len-1] should be very near 1.0, but not necessarily equals 1.0.
Surely x / x is exactly equal to 1.0 when x is a positive integer? On what platform does this `else' case fail?
The `if' case is another `x / x' case, but with weaker constraints on x: we've tested above that it's non-zero and (in the 0.41 version you've pasted above) not +/-inf.
tot_len is read from u[], which is presumably in memory, so we should be safe from issues from architectures like x86 where registers are wider than `double's in memory.
I would guess that the bug is rather that someone has erroneously replaced the original `isfinite' test with `finite', which differs for NaN. This bug has already been corrected in CVS.
I have previously come across a problem under windows where there was a number with strange properties; I think the windows libc printf showed it as something like `0.#' or something like that (involving a zero and I think a hash character).
João, rather than forcing the last element to be 1.0, can you change the assertion code to something like:
if (u[len - 1] != 1) { g_warning("expecting last element to equal 1, but found %17g"); }
and report here what it gives? Btw, I suggest trying current CVS, which is near to being released as 0.42.
Meanwhile, I'll try to check that the current code still respects documented preconditions.
pjrm.
Using floating point number the following 'then' branch will not necessarily execute:
{ double x,y; x = 5.0; y = sqrt(25.0); //or another operation resulting in '5' if ( (x/y) == 1.0) { printf("This message may appear or may not.\n"); } }
The reason is due to the imprecision in floating point number representation. Technicaly, the real numbers numbers 0.99999....... and 1.0 are the same, but their finite representation as floating point numbers differ and thus would not pass the test above. For a detailed explanation see: http://citeseer.ist.psu.edu/goldberg91what.html I added the g_warning code, as suggested, and this is the result (which I expected):
** (inkscape:10949): WARNING **: expecting element to be equal to 1, but found 1
After that I added the following warning, to print the difference between the two numbers:
g_warning("\tdifference is %30g\n", 1.0 - u[last]);
The result is:
** (inkscape:13405): WARNING **: difference is 1.11022e-16
So there is the imprecision.
I still recommend against using == to compare floating point numbers.
By the way, I forgot to describe my environment:
Athlon 64, Linux 2.6.11, gcc 3.4.3
Hope this clears things a bit.
João
On 7/12/05, Peter Moulder <Peter.Moulder@...38...> wrote:
On Mon, Jul 11, 2005 at 10:11:44PM -0300, João Rafael Nicola wrote:
This is a snipped of chord_length_parameterize:
gdouble tot_len = u[len - 1]; g_return_if_fail( tot_len != 0 ); if (finite(tot_len)) { for (unsigned i = 1; i < len; ++i) { u[i] /= tot_len; } } else { /* We could do better, but this probably never happens anyway. */ for (unsigned i = 1; i < len; ++i) { u[i] = i / (gdouble) ( len - 1 ); } }
In both cases, u[len-1] should be very near 1.0, but not necessarily equals 1.0.
Surely x / x is exactly equal to 1.0 when x is a positive integer? On what platform does this `else' case fail?
The `if' case is another `x / x' case, but with weaker constraints on x: we've tested above that it's non-zero and (in the 0.41 version you've pasted above) not +/-inf.
tot_len is read from u[], which is presumably in memory, so we should be safe from issues from architectures like x86 where registers are wider than `double's in memory.
I would guess that the bug is rather that someone has erroneously replaced the original `isfinite' test with `finite', which differs for NaN. This bug has already been corrected in CVS.
I have previously come across a problem under windows where there was a number with strange properties; I think the windows libc printf showed it as something like `0.#' or something like that (involving a zero and I think a hash character).
João, rather than forcing the last element to be 1.0, can you change the assertion code to something like:
if (u[len - 1] != 1) { g_warning("expecting last element to equal 1, but found %17g"); }
and report here what it gives? Btw, I suggest trying current CVS, which is near to being released as 0.42.
Meanwhile, I'll try to check that the current code still respects documented preconditions.
pjrm.
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual core and dual graphics technology at this free one hour event hosted by HP, AMD, and NVIDIA. To register visit http://www.hp.com/go/dualwebinar _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
On Tue, Jul 12, 2005 at 12:50:35PM -0300, João Rafael Nicola wrote:
Using floating point number the following 'then' branch will not necessarily execute:
{ double x,y; x = 5.0; y = sqrt(25.0); //or another operation resulting in '5' if ( (x/y) == 1.0) { printf("This message may appear or may not.\n"); } }
The above code isn't a good example because the most likely reason for inequality would be sqrt resulting in a number not exactly equal to 5.
Anyway, I've now added some code to bezier-utils.cpp intended to deal with the case that x / x != 1.0 (despite x being isFinite and non-zero).
Can you cvs up and tell me if it still produces any messages (and what the message says, if it does produce a message) ?
Thanks,
pjrm.
I'll try.
Thanks,
João
On 7/14/05, Peter Moulder <Peter.Moulder@...38...> wrote:
On Tue, Jul 12, 2005 at 12:50:35PM -0300, João Rafael Nicola wrote:
Using floating point number the following 'then' branch will not necessarily execute:
{ double x,y; x = 5.0; y = sqrt(25.0); //or another operation resulting in '5' if ( (x/y) == 1.0) { printf("This message may appear or may not.\n"); } }
The above code isn't a good example because the most likely reason for inequality would be sqrt resulting in a number not exactly equal to 5.
Anyway, I've now added some code to bezier-utils.cpp intended to deal with the case that x / x != 1.0 (despite x being isFinite and non-zero).
Can you cvs up and tell me if it still produces any messages (and what the message says, if it does produce a message) ?
Thanks,
pjrm.
participants (2)
-
João Rafael Nicola
-
Peter Moulder