Bug 4137

Created attachment 2926 [details] [review]
change cairo_rectangle to use cairo_line_to instead of cairo_rel_line_to

This fixes the first problem; I guess it's too late for an API change now.

The bug here is in cairo_rel_line_to which loses precision when converting
the relative value to fixed-point. It should instead only convert to fixed-
point after the aboslute position is computed.

When that bug is fixed, the two versions of cairo_rectangle as seen in the
patch above should be equivalent.

(patch) the patch has a typo, y+width should be y+height (my test program only
draws squares, apparently).

(cworth) If cairo_rel_line_to is fixed that way, that will definitely improve
the situation a lot. There will still be (much smaller) rounding errors due to
the limited precision of double though.

If all rounding is done correctly, there is no need to worry about epsilon
errors ... as long as they are less than 1 fixed point device unit, they won't
make any difference to the output, things will still be pixel aligned in
the end.

There were some problems earlier where we were truncating converting
to ints rather than rounding - see bug 2379. Don't know if we still
have these.

(otaylor) #2739 is a different issue, it is concerned with drawing primitives
with slightly varying coordinates, and expecting the result to be the same.
The issue in this report is that when an application specifies two polygons with
an identical lines, the respective border should be exactly the same for the two
polygons. To achieve this, you have to round everything in exactly the same way;
it actually does not matter much how.
NB. this is primarily a non-antialiasing issue, because without antialiasing,
the smallest error can eventually lead to missing whole pixels, if it happens at
a rounding boundary. With antialiasing, this will hardly ever produce visible
differences.

If you do rounding correctly, then the cases that you care about (pixel
aligned primitives are robust) -

 x,y,width,height = 10,10,40,40

Gives the same as

  x,y,width,height = 9.9999,10.0001,39.99999,40.0001

With antialiasing off, you *can* construct cases that trigger on epsilon
changes, but you still get robustness in the normal cases.

Making the assumption that it is ever possible to get the *same* double
values in two different code paths is not a good idea.

To be a little more explicit, if everything rounds correctly, then to introduce
seems as you do, then you need to end up with numbers that are 
(N+0.5/66536) +/- epsilon 



Ending up with something that is at 

 (N+0.5) +/- epsilon 

As in your test case is much more likely.

 x' = ((x + 1) * 677 / 26) - 300 

gives x' = 38.5 for x = 12.

I think the problem is likely cairo_fixed_from_double() which doesn't
round correctly.

Changing cairo_fixed_from_double does indeed fix this test case.
Now committed to CVS:

 cairo_fixed_t
 _cairo_fixed_from_double (double d)
 {
-    return (cairo_fixed_t) (d * 65536);
+    return (cairo_fixed_t) floor (d * 65536 + 0.5);
 }

Though I would still like to clean up the precision problem with
cairo_rel_line_to and friends, (eg. make cairo store the current
point with double-precision instead of just in fixed-point).

PS. Bertram, if you could send the test case with appropriate licensing
information I would definitely like to include it in the test suite.
Thanks!

Created attachment 2937 [details] [review]
testcase

(cworth) is this what you had in mind?

Created attachment 2938 [details] [review]
testcase - with corrected license

(cworth) I forgot to replace a few occurrences of 'Billy Biggs'; I also had not
read your followup email. Sorry for this. (And I think using 'the author' is a
very good idea)

i feel so unloved...

Thanks for the test case! All committed now.

Move bugs against "cvs" version to "0.9.3" so we can remove the "cvs" version.