Bug 4220

Summary: Clip slow when overlapping
Product: cairo Reporter: Richard Stellingwerff <remenic>
Component: xlib backendAssignee: Billy Biggs <billy.biggs>
Status: RESOLVED FIXED QA Contact: cairo-bugs mailing list <cairo-bugs>
Severity: normal    
Priority: high CC: cworth
Version: 0.9.3   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: Benchmark for overlapping vs normal.
Standalone test case that actually stands alone
Proposed patch

Description Richard Stellingwerff 2005-08-24 06:57:16 UTC
When overlapping clip rectangles, drawing operations become very slow.

Take this clipping code for example:
        cairo_identity_matrix (cr);
        cairo_rectangle (cr, 0,   0,   200,       height);
        cairo_rectangle (cr, 300, 0,   width-300, height);
        cairo_rectangle (cr, 200, 0,   100,       200);
        cairo_rectangle (cr, 200, 300, 100,       height-300);
        cairo_clip      (cr);
        cairo_new_path  (cr);

It would appear that the rectangles don't overlap here. However, changing the
width of the two bottom rectangles from '100' to '99' makes drawing operations
fast again.

The problem also happens when creating this clip more intelligently, by creating
two rectangles which overlap, then setting fill rule to even odd.
This code looks like:
        cairo_set_fill_rule (cr, CAIRO_FILL_RULE_EVEN_ODD);
        cairo_rectangle (cr, 0, 0, width, height);
        cairo_rectangle (cr, 200, 200, 100, 100);
Comment 1 Billy Biggs 2005-08-24 07:12:45 UTC
One reason some of this is slow is because the code to composite multiple clip
regions ends up going through the slow path in libpixman.  See bug 4191.  That
said, it seems clear that cairo can be more intelligent about how it interprets
the clipping region.
Comment 2 Owen Taylor 2005-08-24 07:23:04 UTC
Aren't the clips here pixel aligned? If so, we shouldn't be doing
libpixman operations at all.
Comment 3 Owen Taylor 2005-08-24 10:45:12 UTC
Would it be possible to get a standalone test case for this?
Comment 4 Richard Stellingwerff 2005-08-24 11:51:29 UTC
Created attachment 3020 [details]
Benchmark for overlapping vs normal.

To compile and run this, check out cairo-benchmarks from cvs, run make. Then
place the source file in the root of cairo-benchmarks and run:

cc   -Wall `pkg-config --cflags cairo libpng12` `pkg-config --libs cairo
libpng12` tools.o clip-rectangles.c -o clip-rectangles

It can be executed in two ways:
 * NOT overlapping: ./clip-rectangles 
 * overlapping: ./clip-rectangles 1

On my machine, when the rectangles don't overlap, I get on average 3ms.
When the rectangles DO overlap, it takes on average 16ms.
Comment 5 Owen Taylor 2005-08-24 12:29:42 UTC
Created attachment 3021 [details]
Standalone test case that actually stands alone
Comment 6 Owen Taylor 2005-08-24 12:45:10 UTC
The dump of the traps the tesellator generates for this test case are:

                               left                          right
N  top  bottom    p1.x  p1.y      p2.x  p2.y      p1.x p1.y       p2.x  p2.y

0    0   200    (    0     0) - (    0   480)   (  200     0) - (  200   480)
1    0   200    (  200     0) - (  200   200)   (  300     0) - (  300   480)
2    0   200    (  300     0) - (  300   480)   (  300     0) - (  300   200)
3    0   200    (  300     0) - (  300   200)   (  640     0) - (  640   480)
4  200   300    (    0     0) - (    0   480)   (  200     0) - (  200   480)
5  200   300    (  300     0) - (  300   480)   (  640     0) - (  640   480)
6  300   480    (    0     0) - (    0   480)   (  200     0) - (  200   480)
7  300   480    (  200   300) - (  200   480)   (  300     0) - (  300   480)
8  300   480    (  300     0) - (  300   480)   (  300   300) - (  300   480)
9  300   480    (  300   300) - (  300   480)   (  640     0) - (  640   480)

All these traps are rectangular, but they don't match the logic in
cairo_traps_extract_region which is:

        if (!(traps->traps[i].left.p1.x == traps->traps[i].left.p2.x
              && traps->traps[i].right.p1.x == traps->traps[i].right.p2.x
              && traps->traps[i].left.p1.y == traps->traps[i].right.p1.y
              && traps->traps[i].left.p2.y == traps->traps[i].right.p2.y
              && _cairo_fixed_is_integer(traps->traps[i].left.p1.x)
              && _cairo_fixed_is_integer(traps->traps[i].left.p1.y)
              && _cairo_fixed_is_integer(traps->traps[i].left.p2.x)
              && _cairo_fixed_is_integer(traps->traps[i].left.p2.y)
              && _cairo_fixed_is_integer(traps->traps[i].right.p1.x)
              && _cairo_fixed_is_integer(traps->traps[i].right.p1.y)
              && _cairo_fixed_is_integer(traps->traps[i].right.p2.x)
              && _cairo_fixed_is_integer(traps->traps[i].right.p2.y))) {

Because p1.y is different than top in some cases, and p2.y is different
from bottom in some cases.

As far as I can see, changing the logic in that function to

        if (!(traps->traps[i].left.p1.x == traps->traps[i].left.p2.x
              && traps->traps[i].right.p1.x == traps->traps[i].right.p2.x
              && _cairo_fixed_is_integer(traps->traps[i].top)
              && _cairo_fixed_is_integer(traps->traps[i].bottom)
              && _cairo_fixed_is_integer(traps->traps[i].left.p1.x)
              && _cairo_fixed_is_integer(traps->traps[i].right.p1.x))) {

Should work better (and be considerably simpler)
Comment 7 Owen Taylor 2005-08-24 12:52:03 UTC
Created attachment 3022 [details] [review]
Proposed patch
Comment 8 Carl Worth 2005-08-24 17:17:12 UTC
The patch appears correct to me.

Well done!
Comment 9 Owen Taylor 2005-08-27 18:54:13 UTC
2005-08-27  Owen Taylor  <otaylor@redhat.com>

        reviewed by: cworth

        * src/cairo-traps.c (_cairo_traps_extract_region): Make the
        check for rectangular trapezoids simpler and more accurate.
        (#4220, found using test case from Richard Stellingwerff)

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.