comment face computations, check for reflecting transformation to select correct face orientations

check for reflecting transform when computing pen to ensure consistent pen orientation

5 files changed, 146 insertions, 78 deletions

diff --git a/ChangeLog b/ChangeLog
index fc9e8c19..33bdbcd0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2003-09-05  Keith Packard <keithp@keithp.com>
+
+	* src/cairo_path_stroke.c: comment face computations, check for
+	reflecting transformation to select correct face orientations
+	
+	* src/cairo_pen.c: check for reflecting transform when computing
+	pen to ensure consistent pen orientation
+
 2003-09-05  Carl Worth  <cworth@east.isi.edu>
 
 	* configure.in: Bumped version to 0.1.2 for new cairo_copy
diff --git a/src/cairo-path-stroke.c b/src/cairo-path-stroke.c
index 5b2ac33a..04c7f694 100644
--- a/src/cairo-path-stroke.c
+++ b/src/cairo-path-stroke.c
@@ -171,26 +171,22 @@ _cairo_stroker_join (cairo_stroker_t *stroker, cairo_stroke_face_t *in, cairo_st
     case CAIRO_LINE_JOIN_ROUND: {
 	int i;
 	int start, step, stop;
-	cairo_point_t tri[3], initial, final;
+	cairo_point_t tri[3];
 	cairo_pen_t *pen = &gstate->pen_regular;
 
 	tri[0] = in->pt;
 	if (clockwise) {
-	    initial = in->ccw;
 	    _cairo_pen_find_active_ccw_vertex_index (pen, &in->dev_vector, &start);
 	    step = -1;
 	    _cairo_pen_find_active_ccw_vertex_index (pen, &out->dev_vector, &stop);
-	    final = out->ccw;
 	} else {
-	    initial = in->cw;
 	    _cairo_pen_find_active_cw_vertex_index (pen, &in->dev_vector, &start);
 	    step = +1;
 	    _cairo_pen_find_active_cw_vertex_index (pen, &out->dev_vector, &stop);
-	    final = out->cw;
 	}
 
 	i = start;
-	tri[1] = initial;
+	tri[1] = *inpt;
 	while (i != stop) {
 	    tri[2] = in->pt;
 	    _translate_point (&tri[2], &pen->vertex[i].pt);
@@ -203,7 +199,7 @@ _cairo_stroker_join (cairo_stroker_t *stroker, cairo_stroke_face_t *in, cairo_st
 		i = 0;
 	}
 
-	tri[2] = final;
+	tri[2] = *outpt;
 
 	return _cairo_traps_tessellate_triangle (stroker->traps, tri);
     }
@@ -388,36 +384,55 @@ _cairo_stroker_cap (cairo_stroker_t *stroker, cairo_stroke_face_t *f)
 static void
 _compute_face (cairo_point_t *pt, cairo_slope_t *slope, cairo_gstate_t *gstate, cairo_stroke_face_t *face)
 {
-    double mag, tmp;
-    double dx, dy;
+    double mag, det;
+    double line_dx, line_dy;
+    double face_dx, face_dy;
     XPointDouble usr_vector;
     cairo_point_t offset_ccw, offset_cw;
 
-    dx = cairo_fixed_to_double (slope->dx);
-    dy = cairo_fixed_to_double (slope->dy);
+    line_dx = cairo_fixed_to_double (slope->dx);
+    line_dy = cairo_fixed_to_double (slope->dy);
 
-    cairo_matrix_transform_distance (&gstate->ctm_inverse, &dx, &dy);
+    /* faces are normal in user space, not device space */
+    cairo_matrix_transform_distance (&gstate->ctm_inverse, &line_dx, &line_dy);
 
-    mag = sqrt (dx * dx + dy * dy);
+    mag = sqrt (line_dx * line_dx + line_dy * line_dy);
     if (mag == 0) {
 	/* XXX: Can't compute other face points. Do we want a tag in the face for this case? */
 	return;
     }
 
-    dx /= mag;
-    dy /= mag;
-
-    usr_vector.x = dx;
-    usr_vector.y = dy;
-
-    tmp = dx;
-    dx = - dy * (gstate->line_width / 2.0);
-    dy = tmp * (gstate->line_width / 2.0);
+    /* normalize to unit length */
+    line_dx /= mag;
+    line_dy /= mag;
+
+    usr_vector.x = line_dx;
+    usr_vector.y = line_dy;
+
+    /* 
+     * rotate to get a line_width/2 vector along the face, note that
+     * the vector must be rotated the right direction in device space,
+     * but by 90° in user space. So, the rotation depends on
+     * whether the ctm reflects or not, and that can be determined
+     * by looking at the determinant of the matrix.
+     */
+    _cairo_matrix_compute_determinant (&gstate->ctm, &det);
+    if (det >= 0)
+    {
+	face_dx = - line_dy * (gstate->line_width / 2.0);
+	face_dy = line_dx * (gstate->line_width / 2.0);
+    }
+    else
+    {
+	face_dx = line_dy * (gstate->line_width / 2.0);
+	face_dy = - line_dx * (gstate->line_width / 2.0);
+    }
 
-    cairo_matrix_transform_distance (&gstate->ctm, &dx, &dy);
+    /* back to device space */
+    cairo_matrix_transform_distance (&gstate->ctm, &face_dx, &face_dy);
 
-    offset_ccw.x = cairo_double_to_fixed (dx);
-    offset_ccw.y = cairo_double_to_fixed (dy);
+    offset_ccw.x = cairo_double_to_fixed (face_dx);
+    offset_ccw.y = cairo_double_to_fixed (face_dy);
     offset_cw.x = -offset_ccw.x;
     offset_cw.y = -offset_ccw.y;
 
diff --git a/src/cairo-pen.c b/src/cairo-pen.c
index c8a1c839..f46bf00e 100644
--- a/src/cairo-pen.c
+++ b/src/cairo-pen.c
@@ -28,7 +28,7 @@
 #include "cairoint.h"
 
 static int
-_cairo_pen_vertices_needed (double radius, double tolerance, cairo_matrix_t *matrix);
+_cairo_pen_vertices_needed (double radius, double tolerance, double expansion);
 
 static void
 _cairo_pen_compute_slopes (cairo_pen_t *pen);
@@ -54,6 +54,8 @@ cairo_status_t
 _cairo_pen_init (cairo_pen_t *pen, double radius, cairo_gstate_t *gstate)
 {
     int i;
+    int reflect;
+    double det, expansion;
 
     if (pen->num_vertices) {
 	/* XXX: It would be nice to notice that the pen is already properly constructed.
@@ -68,7 +70,20 @@ _cairo_pen_init (cairo_pen_t *pen, double radius, cairo_gstate_t *gstate)
     pen->radius = radius;
     pen->tolerance = gstate->tolerance;
 
-    pen->num_vertices = _cairo_pen_vertices_needed (radius, gstate->tolerance, &gstate->ctm);
+    /* The determinant represents the area expansion factor of the
+       transform. In the worst case, this is entirely in one
+       dimension, which is what we assume here. */
+
+    _cairo_matrix_compute_determinant (&gstate->ctm, &det);
+    if (det >= 0) {
+	reflect = 0;
+	expansion = det;
+    } else {
+	reflect = 1;
+	expansion = -det;
+    }
+    
+    pen->num_vertices = _cairo_pen_vertices_needed (radius, gstate->tolerance, expansion);
     /* number of vertices must be even */
     if (pen->num_vertices % 2)
 	pen->num_vertices++;
@@ -78,14 +93,20 @@ _cairo_pen_init (cairo_pen_t *pen, double radius, cairo_gstate_t *gstate)
 	return CAIRO_STATUS_NO_MEMORY;
     }
 
+    /*
+     * Compute pen coordinates.  To generate the right ellipse, compute points around
+     * a circle in user space and transform them to device space.  To get a consistent
+     * orientation in device space, flip the pen if the transformation matrix
+     * is reflecting
+     */
     for (i=0; i < pen->num_vertices; i++) {
 	double theta = 2 * M_PI * i / (double) pen->num_vertices;
-	double dx = radius * cos (theta);
-	double dy = radius * sin (theta);
+	double dx = radius * cos (reflect ? -theta : theta);
+	double dy = radius * sin (reflect ? -theta : theta);
 	cairo_pen_vertex_t *v = &pen->vertex[i];
 	cairo_matrix_transform_distance (&gstate->ctm, &dx, &dy);
-	v->pt.x = XDoubleToFixed (dx);
-	v->pt.y = XDoubleToFixed (dy);
+	v->pt.x = cairo_double_to_fixed (dx);
+	v->pt.y = cairo_double_to_fixed (dy);
     }
 
     _cairo_pen_compute_slopes (pen);
@@ -157,15 +178,9 @@ _cairo_pen_add_points (cairo_pen_t *pen, cairo_point_t *pt, int num_pts)
 }
 
 static int
-_cairo_pen_vertices_needed (double radius, double tolerance, cairo_matrix_t *matrix)
+_cairo_pen_vertices_needed (double radius, double tolerance, double expansion)
 {
-    double expansion, theta;
-
-    /* The determinant represents the area expansion factor of the
-       transform. In the worst case, this is entirely in one
-       dimension, which is what we assume here. */
-
-    _cairo_matrix_compute_determinant (matrix, &expansion);
+    double theta;
 
     if (tolerance > expansion*radius) {
 	return 4;
diff --git a/src/cairo_path_stroke.c b/src/cairo_path_stroke.c
index 5b2ac33a..04c7f694 100644
--- a/src/cairo_path_stroke.c
+++ b/src/cairo_path_stroke.c
@@ -171,26 +171,22 @@ _cairo_stroker_join (cairo_stroker_t *stroker, cairo_stroke_face_t *in, cairo_st
     case CAIRO_LINE_JOIN_ROUND: {
 	int i;
 	int start, step, stop;
-	cairo_point_t tri[3], initial, final;
+	cairo_point_t tri[3];
 	cairo_pen_t *pen = &gstate->pen_regular;
 
 	tri[0] = in->pt;
 	if (clockwise) {
-	    initial = in->ccw;
 	    _cairo_pen_find_active_ccw_vertex_index (pen, &in->dev_vector, &start);
 	    step = -1;
 	    _cairo_pen_find_active_ccw_vertex_index (pen, &out->dev_vector, &stop);
-	    final = out->ccw;
 	} else {
-	    initial = in->cw;
 	    _cairo_pen_find_active_cw_vertex_index (pen, &in->dev_vector, &start);
 	    step = +1;
 	    _cairo_pen_find_active_cw_vertex_index (pen, &out->dev_vector, &stop);
-	    final = out->cw;
 	}
 
 	i = start;
-	tri[1] = initial;
+	tri[1] = *inpt;
 	while (i != stop) {
 	    tri[2] = in->pt;
 	    _translate_point (&tri[2], &pen->vertex[i].pt);
@@ -203,7 +199,7 @@ _cairo_stroker_join (cairo_stroker_t *stroker, cairo_stroke_face_t *in, cairo_st
 		i = 0;
 	}
 
-	tri[2] = final;
+	tri[2] = *outpt;
 
 	return _cairo_traps_tessellate_triangle (stroker->traps, tri);
     }
@@ -388,36 +384,55 @@ _cairo_stroker_cap (cairo_stroker_t *stroker, cairo_stroke_face_t *f)
 static void
 _compute_face (cairo_point_t *pt, cairo_slope_t *slope, cairo_gstate_t *gstate, cairo_stroke_face_t *face)
 {
-    double mag, tmp;
-    double dx, dy;
+    double mag, det;
+    double line_dx, line_dy;
+    double face_dx, face_dy;
     XPointDouble usr_vector;
     cairo_point_t offset_ccw, offset_cw;
 
-    dx = cairo_fixed_to_double (slope->dx);
-    dy = cairo_fixed_to_double (slope->dy);
+    line_dx = cairo_fixed_to_double (slope->dx);
+    line_dy = cairo_fixed_to_double (slope->dy);
 
-    cairo_matrix_transform_distance (&gstate->ctm_inverse, &dx, &dy);
+    /* faces are normal in user space, not device space */
+    cairo_matrix_transform_distance (&gstate->ctm_inverse, &line_dx, &line_dy);
 
-    mag = sqrt (dx * dx + dy * dy);
+    mag = sqrt (line_dx * line_dx + line_dy * line_dy);
     if (mag == 0) {
 	/* XXX: Can't compute other face points. Do we want a tag in the face for this case? */
 	return;
     }
 
-    dx /= mag;
-    dy /= mag;
-
-    usr_vector.x = dx;
-    usr_vector.y = dy;
-
-    tmp = dx;
-    dx = - dy * (gstate->line_width / 2.0);
-    dy = tmp * (gstate->line_width / 2.0);
+    /* normalize to unit length */
+    line_dx /= mag;
+    line_dy /= mag;
+
+    usr_vector.x = line_dx;
+    usr_vector.y = line_dy;
+
+    /* 
+     * rotate to get a line_width/2 vector along the face, note that
+     * the vector must be rotated the right direction in device space,
+     * but by 90° in user space. So, the rotation depends on
+     * whether the ctm reflects or not, and that can be determined
+     * by looking at the determinant of the matrix.
+     */
+    _cairo_matrix_compute_determinant (&gstate->ctm, &det);
+    if (det >= 0)
+    {
+	face_dx = - line_dy * (gstate->line_width / 2.0);
+	face_dy = line_dx * (gstate->line_width / 2.0);
+    }
+    else
+    {
+	face_dx = line_dy * (gstate->line_width / 2.0);
+	face_dy = - line_dx * (gstate->line_width / 2.0);
+    }
 
-    cairo_matrix_transform_distance (&gstate->ctm, &dx, &dy);
+    /* back to device space */
+    cairo_matrix_transform_distance (&gstate->ctm, &face_dx, &face_dy);
 
-    offset_ccw.x = cairo_double_to_fixed (dx);
-    offset_ccw.y = cairo_double_to_fixed (dy);
+    offset_ccw.x = cairo_double_to_fixed (face_dx);
+    offset_ccw.y = cairo_double_to_fixed (face_dy);
     offset_cw.x = -offset_ccw.x;
     offset_cw.y = -offset_ccw.y;
 
diff --git a/src/cairo_pen.c b/src/cairo_pen.c
index c8a1c839..f46bf00e 100644
--- a/src/cairo_pen.c
+++ b/src/cairo_pen.c
@@ -28,7 +28,7 @@
 #include "cairoint.h"
 
 static int
-_cairo_pen_vertices_needed (double radius, double tolerance, cairo_matrix_t *matrix);
+_cairo_pen_vertices_needed (double radius, double tolerance, double expansion);
 
 static void
 _cairo_pen_compute_slopes (cairo_pen_t *pen);
@@ -54,6 +54,8 @@ cairo_status_t
 _cairo_pen_init (cairo_pen_t *pen, double radius, cairo_gstate_t *gstate)
 {
     int i;
+    int reflect;
+    double det, expansion;
 
     if (pen->num_vertices) {
 	/* XXX: It would be nice to notice that the pen is already properly constructed.
@@ -68,7 +70,20 @@ _cairo_pen_init (cairo_pen_t *pen, double radius, cairo_gstate_t *gstate)
     pen->radius = radius;
     pen->tolerance = gstate->tolerance;
 
-    pen->num_vertices = _cairo_pen_vertices_needed (radius, gstate->tolerance, &gstate->ctm);
+    /* The determinant represents the area expansion factor of the
+       transform. In the worst case, this is entirely in one
+       dimension, which is what we assume here. */
+
+    _cairo_matrix_compute_determinant (&gstate->ctm, &det);
+    if (det >= 0) {
+	reflect = 0;
+	expansion = det;
+    } else {
+	reflect = 1;
+	expansion = -det;
+    }
+    
+    pen->num_vertices = _cairo_pen_vertices_needed (radius, gstate->tolerance, expansion);
     /* number of vertices must be even */
     if (pen->num_vertices % 2)
 	pen->num_vertices++;
@@ -78,14 +93,20 @@ _cairo_pen_init (cairo_pen_t *pen, double radius, cairo_gstate_t *gstate)
 	return CAIRO_STATUS_NO_MEMORY;
     }
 
+    /*
+     * Compute pen coordinates.  To generate the right ellipse, compute points around
+     * a circle in user space and transform them to device space.  To get a consistent
+     * orientation in device space, flip the pen if the transformation matrix
+     * is reflecting
+     */
     for (i=0; i < pen->num_vertices; i++) {
 	double theta = 2 * M_PI * i / (double) pen->num_vertices;
-	double dx = radius * cos (theta);
-	double dy = radius * sin (theta);
+	double dx = radius * cos (reflect ? -theta : theta);
+	double dy = radius * sin (reflect ? -theta : theta);
 	cairo_pen_vertex_t *v = &pen->vertex[i];
 	cairo_matrix_transform_distance (&gstate->ctm, &dx, &dy);
-	v->pt.x = XDoubleToFixed (dx);
-	v->pt.y = XDoubleToFixed (dy);
+	v->pt.x = cairo_double_to_fixed (dx);
+	v->pt.y = cairo_double_to_fixed (dy);
     }
 
     _cairo_pen_compute_slopes (pen);
@@ -157,15 +178,9 @@ _cairo_pen_add_points (cairo_pen_t *pen, cairo_point_t *pt, int num_pts)
 }
 
 static int
-_cairo_pen_vertices_needed (double radius, double tolerance, cairo_matrix_t *matrix)
+_cairo_pen_vertices_needed (double radius, double tolerance, double expansion)
 {
-    double expansion, theta;
-
-    /* The determinant represents the area expansion factor of the
-       transform. In the worst case, this is entirely in one
-       dimension, which is what we assume here. */
-
-    _cairo_matrix_compute_determinant (matrix, &expansion);
+    double theta;
 
     if (tolerance > expansion*radius) {
 	return 4;