[cairo] [PATCH] stroke: canonicalize the boxes we create

[Chris Wilson]

On Wed, 28 Sep 2011 18:21:01 -0300, przanoni at gmail.com wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> If the rectilinear_stroker.half_line_width variable is too big, the
> boxes we generate (just called "b" in that function) might not be
> "canonical" (their points are not the top-left and the bottom-right
> points, but the bottom-left and top-right).
> 
> This fixes the line-width-overlap tests.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  src/cairo-path-stroke-boxes.c |   23 +++++++++++++++++++++++
>  1 files changed, 23 insertions(+), 0 deletions(-)
> 
> Hello again
> 
> So I've decided to try to fix one of the test suite failures with the goal of
> learn more about cairo. I chose the "line-width-overlap" test for the "image"
> backend.
> 
> The test consists of creating a small rectangle, a big line_width and a
> cairo_stroke. If the line width is too big we get a segfault. After some
> investigation, it seems we are passing to pixman a rectangle with a negative
> height, and pixman does "while (height--) { something }".
> 
> It seems the culprit is _cairo_path_fixed_stroke_rectilinear_to_boxes. If the
> rectilinear_stroker.half_line_width variable is too big, the boxes we generate
> (just called "b" in that function) might not be "canonical" (their points are
> not the top-left and the bottom-right points, but the bottom-left and
> top-right). In order to fix that problem I created a small "_canonicalize_box"
> function and fixed the boxes. The tests now seem to pass.
> 
> I'm still learning cairo's internals (and externals too) so I'm not sure this is
> the right fix... Comments?

It's not. ;-)

Add the test case that triggered the segfault (line-width wider than box)
since that is an interesting bit of state not yet tested.

Then simply do not proceed done that fast path if the boxes may overlap
i.e. width(box) < 2 * half_line_width || height(box) < 2 * half_line_width

You need to run the resultant set of boxes through the tessellator to
correctly reduce them to cannonical form.

> It's hard for me to check whether this patch creates a regression or not because
> many of the other tests were failing before this patch, so it's hard to compare.
>
> Any tip? What's the "best known method" for checking patch regressions?

Think the patch through. This should be possible to thoroughly test using
image, so make sure it is. Otherwise you can use the cairo-test-suite to
compare before and after results of a patch to look for changes and decide
if they are correct.  E.g.
$ ./cairo-test-suite
$ mv output/ before
# apply-path
$ CAIRO_REF_DIR=before ./cairo-test-suite
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre