Bug 7245

Summary:	cairo_stroke_extents() gives wrong result for arcs in some cases
Product:	cairo	Reporter:	Olexiy Avramchenko <aolexiy>
Component:	general	Assignee:	Carl Worth <cworth>
Status:	RESOLVED FIXED	QA Contact:	cairo-bugs mailing list <cairo-bugs>
Severity:	normal
Priority:	high
Version:	1.1.8
Hardware:	x86 (IA32)
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:
Attachments:	small test case, requires GTK Correct the mitre limit derivation

Description Olexiy Avramchenko 2006-06-16 01:31:13 UTC

cairo_stroke_extents() gives a wrong result (!) for arcs in some specific cases.
Example:

cairo_arc(cr, 250.0, 250.0, 157.0, 5.147, 3.432);
cairo_set_line_width(cr, 154.0);
cairo_stroke_extents(cr, &x1,&y1, &x2,&y2);

cairo_stroke_extents gives (!) x1=-229.47 (!), y1==37.75, x2=484.00, y2=483.90,
but stroked arc looks good, however.

This behaviour can be reproduced only for some specific cases - small changes of
the arc radius/angle1/angle2 or the line width hide this beast
(cairo_set_line_width(cr, 153.99) fixes the example above).

Comment 1 Olexiy Avramchenko 2006-06-16 01:35:47 UTC

Created attachment 5928 [details]
small test case, requires GTK

The small test case, GTK2 required, compile:
cc -Wall `pkg-config --cflags --libs gtk+-2.0` cairo-test.c -o cairo-test

It shows the window with the arc, red half-toned rectangle shows the area
reported by cairo_stroke_extents().

Comment 2 Chris Wilson 2007-09-26 03:36:28 UTC

Created attachment 11758 [details] [review]
Correct the mitre limit derivation

I believe the bug is in the derivation of the mitre limit conditional

Comment 3 Chris Wilson 2007-10-02 15:09:30 UTC

(In reply to comment #2)
> I believe the bug is in the derivation of the mitre limit conditional

[chorus] Oh no it isn't!

The final equation is correct, it is just a misleading comment about the condition  being secant(phi/2) <= ml (instead of 1 <= ml sin(phi/2) or cosecant(phi/2) <= ml). So the bug appears related to incorrect intersection vectors of the curves.

Comment 4 Chris Wilson 2007-10-27 09:38:02 UTC

I've worked around this problem by relaxing the check that the in and out faces are exactly coincident, and instead just checking that if the two faces are within stoker->tolerance of each other, then we need not do any work to join the two faces and return SUCCESS.

@@ -205,17 +210,28 @@ _cairo_stroker_face_clockwise (cairo_stroke_face_t *in, ca
     return _cairo_slope_clockwise (&in_slope, &out_slope);
 }
 
+static double
+_PointDistanceSquaredToPoint (const cairo_point_t *a, const cairo_point_t *b)
+{
+    double dx = _cairo_fixed_to_double (b->x - a->x);
+    double dy = _cairo_fixed_to_double (b->y - a->y);
+
+    return dx*dx + dy*dy;
+}
 static cairo_status_t
 _cairo_stroker_join (cairo_stroker_t *stroker, cairo_stroke_face_t *in, cairo_s
 {
     int                        clockwise = _cairo_stroker_face_clockwise (out, 
     cairo_point_t      *inpt, *outpt;
     cairo_status_t status;
+    double tolerance_sqr = stroker->tolerance * stroker->tolerance;
 
-    if (in->cw.x == out->cw.x
+    if ((in->cw.x == out->cw.x
        && in->cw.y == out->cw.y
        && in->ccw.x == out->ccw.x
        && in->ccw.y == out->ccw.y)
+       || (_PointDistanceSquaredToPoint (&in->cw, &out->cw) < tolerance_sqr &&
+           _PointDistanceSquaredToPoint (&in->ccw, &out->ccw) < tolerance_sqr))
     {
        return CAIRO_STATUS_SUCCESS;
     }

I'm a little hesitant to apply this as I'm uncertain as to whether this is just papering over a deeper bug.

Comment 5 Carl Worth 2007-10-30 08:45:56 UTC

I've fixed this bug now here:

http://gitweb.freedesktop.org/?p=cairo;a=commitdiff;h=00d701ff7de68609aa8cec7871d93b27a108fd14

The fix will appear in the imminent 1.5.2 snapshot of cairo.

-Carl

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.