Comment 17 for bug 1391857

Revision history for this message
In , Psychon-d (psychon-d) wrote :

(In reply to comment #7)
[...]
> That's my only worry, I am trying to remember all the call paths that enter
> here and why we have empty boxes in the first place. In this case, the empty
> boxes seem to be part of the clip, which is worrisome. All the zero height
> boxes should have been prefiltered...
>
> diff --git a/src/cairo-boxes.c b/src/cairo-boxes.c
> index 63b68dd..90afdbd 100644
> --- a/src/cairo-boxes.c
> +++ b/src/cairo-boxes.c
> @@ -139,6 +139,8 @@ _cairo_boxes_add_internal (cairo_boxes_t *boxes,
> if (unlikely (boxes->status))
> return;
>
> + assert(box->x2 > box->x1 && box->y2 > box->y1);
> +
> chunk = boxes->tail;
> if (unlikely (chunk->count == chunk->size)) {
> int size;
>
> So yes, this suggests a far deeper problem than just the tesselate failure.

I guess you meant this: assert(box->p2.x > box->p1.x && box->p2.y > box->p1.y);

That assert triggers for 61 test cases in the test suite. Most of these are due to boxes likes this (this code appears in different places inside of cairo, e.g. _cairo_xcb_surface_fixup_unbounded_boxes and the span compositor's fixup_unbounded_boxes):

    box.p1.x = _cairo_fixed_from_int (extents->unbounded.x + extents->unbounded.width);
    box.p1.y = _cairo_fixed_from_int (extents->unbounded.y);
    box.p2.x = _cairo_fixed_from_int (extents->unbounded.x);
    box.p2.y = _cairo_fixed_from_int (extents->unbounded.y + extents->unbounded.height);

I guess that means that this code is wrong and should be fixed? Perhaps we should even commit this assert to cairo?

At least I didn't find anything generating zero-height boxes.

List of tests: big-empty-box big-empty-triangle big-little-box bug-40410 bug-bo-collins bug-bo-rectangular clip-complex-bug61492 clip-complex-shape-eo-aa clip-complex-shape-eo-mono clip-fill clip-fill-eo-unbounded clip-fill-nz-unbounded clip-group-shapes-unaligned-rectangles clip-mixed-antialias clip-nesting clip-operator clip-shape clip-stroke-unbounded clip-text clip-twice copy-disjoint fill-disjoint get-path-extents hatchings image-surface-source mask operator operator-alpha operator-alpha-alpha paint-with-alpha-clip-mask pdf-surface-source ps-surface-source random-clip record-paint-alpha-clip-mask record-self-intersecting record1414x-self-intersecting record2x-self-intersecting record90-self-intersecting recordflip-self-intersecting rectilinear-fill rotated-clip self-copy self-copy-overlap self-intersecting subsurface-image-repeat subsurface-modify-child subsurface-modify-parent subsurface-pad subsurface-reflect subsurface-repeat surface-pattern-operator svg-surface-source text-glyph-range tighten-bounds trap-clip unantialiased-shapes unbounded-operator white-in-noop xcb-surface-source xlib-surface-source zero-mask

Oh and the assert does not trigger for the test case attached to this bug report (except for the "test-traps" (pseudo-)backend, which doesn't count, I guess). But that test case doesn't crash here either...?