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

[przanoni at gmail.com]

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 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?

diff --git a/src/cairo-path-stroke-boxes.c b/src/cairo-path-stroke-boxes.c
index 794250b..265e761 100644
--- a/src/cairo-path-stroke-boxes.c
+++ b/src/cairo-path-stroke-boxes.c
@@ -598,6 +598,22 @@ _cairo_rectilinear_stroker_close_path (void *closure)
     return CAIRO_STATUS_SUCCESS;
 }
 
+static inline void
+_canonicalize_box (cairo_box_t *box)
+{
+    cairo_fixed_t tmp;
+    if (box->p1.x > box->p2.x) {
+	tmp = box->p1.x;
+	box->p1.x = box->p2.x;
+	box->p2.x = tmp;
+    }
+    if (box->p1.y > box->p2.y) {
+	tmp = box->p1.y;
+	box->p1.y = box->p2.y;
+	box->p2.y = tmp;
+    }
+}
+
 cairo_int_status_t
 _cairo_path_fixed_stroke_rectilinear_to_boxes (const cairo_path_fixed_t	*path,
 					       const cairo_stroke_style_t	*stroke_style,
@@ -623,11 +639,15 @@ _cairo_path_fixed_stroke_rectilinear_to_boxes (const cairo_path_fixed_t	*path,
     {
 	cairo_box_t b;
 
+	/* If rectilinear_stroker.half_line_width is too big, we might generate
+	 * a non-canonical box "b", so we need to fix it. */
+
 	/* top */
 	b.p1.x = box.p1.x - rectilinear_stroker.half_line_width;
 	b.p2.x = box.p2.x + rectilinear_stroker.half_line_width;
 	b.p1.y = box.p1.y - rectilinear_stroker.half_line_width;
 	b.p2.y = box.p1.y + rectilinear_stroker.half_line_width;
+	_canonicalize_box(&b);
 	status = _cairo_boxes_add (boxes, antialias, &b);
 	assert (status == CAIRO_INT_STATUS_SUCCESS);
 
@@ -636,6 +656,7 @@ _cairo_path_fixed_stroke_rectilinear_to_boxes (const cairo_path_fixed_t	*path,
 	b.p2.x = box.p1.x + rectilinear_stroker.half_line_width;
 	b.p1.y = box.p1.y + rectilinear_stroker.half_line_width;
 	b.p2.y = box.p2.y - rectilinear_stroker.half_line_width;
+	_canonicalize_box(&b);
 	status = _cairo_boxes_add (boxes, antialias, &b);
 	assert (status == CAIRO_INT_STATUS_SUCCESS);
 
@@ -644,6 +665,7 @@ _cairo_path_fixed_stroke_rectilinear_to_boxes (const cairo_path_fixed_t	*path,
 	b.p2.x = box.p2.x + rectilinear_stroker.half_line_width;
 	b.p1.y = box.p1.y + rectilinear_stroker.half_line_width;
 	b.p2.y = box.p2.y - rectilinear_stroker.half_line_width;
+	_canonicalize_box(&b);
 	status = _cairo_boxes_add (boxes, antialias, &b);
 	assert (status == CAIRO_INT_STATUS_SUCCESS);
 
@@ -652,6 +674,7 @@ _cairo_path_fixed_stroke_rectilinear_to_boxes (const cairo_path_fixed_t	*path,
 	b.p2.x = box.p2.x + rectilinear_stroker.half_line_width;
 	b.p1.y = box.p2.y - rectilinear_stroker.half_line_width;
 	b.p2.y = box.p2.y + rectilinear_stroker.half_line_width;
+	_canonicalize_box(&b);
 	status = _cairo_boxes_add (boxes, antialias, &b);
 	assert (status == CAIRO_INT_STATUS_SUCCESS);
 
-- 
1.7.4.1