[cairo] Assertion `i < pen->num_vertices' failed in 1.4.10

[Carl Worth]

On Mon, 27 Aug 2007 10:41:03 +0100, Chris Wilson wrote:
> Benjamin Otte (otte at gnome.org) said:
> > I just found a crasher in cairo 1.4.10 while trying to watch the
> > presentation Miguel linked to on Planet Gnome [1] in Swfdec.
> > So I grabbed libcairowrap and threw it at the problem. After throwing
> > away everything that did not relate to the crash, I got a small enough
> > result. It's attached.

Hi Benjamin,

Thanks for the report, and particularly for the nice, little test
case. I know you've been suffering from this bug for a long time, but
it looks like your waiting is finally over!

> For the record the failure message is:
> cairo-pen.c:324: _cairo_pen_find_active_cw_vertex_index: Assertion `i < pen->num_vertices' failed.
>
> The issue appears to be that the rectangle is projected onto a 1D line.
> Unfortunately _cairo_slope_compare() has a big WARNING that it will not
> work correctly on antiparallel lines.

Yes, that's the problem. I've now got a nice little "degenerate-pen"
test case that probes the problem by transforming the pen to a line in
three directions, (horizontally, vertically, and at 45 degrees).

> My random hack of the day is:
> diff --git a/src/cairo-slope.c b/src/cairo-slope.c
> index 78664c5..00650b9 100644
> --- a/src/cairo-slope.c
> +++ b/src/cairo-slope.c
> @@ -78,6 +78,9 @@ _cairo_slope_compare (cairo_slope_t *a, cairo_slope_t
> *b)
>      if (b->dx == 0 && b->dy ==0)
>         return -1;
>
> +    if (a->dy == 0 && b->dy == 0)
> +       return a->dx * b->dx;
> +

I think that's definitely a random hack. This makes the comparison
function now show any preference for a vs. b. So if you imagine the
caller switching the order of the parameters, it will still return the
same result, (which isn't at all good for a comparison function).

That said, with the above patch, plus a similar case for a->dx == 0 &&
b->dx == 0 returning a->dy & b->dy, I can actually get my test case to
pass. I'm not actually sure why that works, but I think it must at
least rely on the callers being careful about the order they are
using.

Meanwhile, here's a patch that I believe is more clearly correct. It's
nice in that it removes the triggering assertion altogether, replacing
it with behavior that's fairly easy to defend, and allows the
comparison function to continue to return 0 for the truly ambiguous,
parallel-lines cases.

I'll go ahead and push this out now, (just before 1.5.2), which should
make Benjamin happy. But I wanted to present it on the list for
review.

-Carl

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-degenerate-pen-test-case-by-removing-the-trigger.patch
Type: application/octet-stream
Size: 0 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20071030/fa2923ab/attachment.obj