Don't repeat a Quartz gradient more times than necessary, to avoid Quartz quality problems when there are lots of repeated color stops.

-- Add a parameter to _cairo_quartz_setup_source so we can pass down the
extents of the object we're drawing
-- Compute fill/stroke/glyph extents and pass them down in the cases we need to
(repeating/reflecting gradients)
-- Pass those extents on down to where we set up the gradients
-- Make _cairo_quartz_setup_linear_source fall back to pixman for the
degenerate case where the linear gradient vector has no length
-- In CreateRepeatingRadialGradientFunction and
CreateRepeatingLinearGradientFunction, use the object extents (or surface
extents, for the paint() case) instead of the clip box to calculate the
parameters for the gradient
-- I've changed the way CreateRepeatingLinearGradientFunction calculates the
repetition count. The new approach gives much more precise bounds on the number
of repetitions needed (and is very similar to what we do for radial gradients).
This is important because if we specify a much larger input range than we
really need for our gradient color function, Quartz samples it too coarsely
over the range we actually care about, and the gradients look bad.

For example, suppose start = (5,0), end = (6,10), the CTM is identity and the
bounds we want to cover is (0,0)-(10,10). I think the current approach sets up
the gradient to be repeated 10 times. In fact only 3 repetitions are needed.

Also, using 'width' here didn't look right:
-       y_rep_end = (int) ceil((surface->extents.width - MAX(mstart.y, mend.y))
/ dy

From https://bugzilla.mozilla.org/show_bug.cgi?id=508730

1 files changed, 179 insertions, 82 deletions

diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c
index 8fbb6709..ffcc41d4 100644
--- a/src/cairo-quartz-surface.c
+++ b/src/cairo-quartz-surface.c
@@ -820,72 +820,90 @@ CreateGradientFunction (const cairo_gradient_pattern_t *gpat)
 			     &gradient_callbacks);
 }
 
-static CGFunctionRef
-CreateRepeatingLinearGradientFunction (cairo_quartz_surface_t *surface,
-				       const cairo_gradient_pattern_t *gpat,
-				       CGPoint *start, CGPoint *end,
-				       CGAffineTransform matrix)
+static void
+UpdateLinearParametersToIncludePoint(double *min_t, double *max_t, CGPoint *start,
+                                     double dx, double dy,
+                                     double x, double y)
 {
-    cairo_pattern_t *pat;
-    cairo_quartz_float_t input_value_range[2];
-
-    CGPoint mstart, mend;
+    /* Compute a parameter t such that a line perpendicular to the (dx,dy)
+       vector, passing through (start->x + dx*t, start->y + dy*t), also
+       passes through (x,y).
 
-    double dx, dy;
-    int x_rep_start = 0, x_rep_end = 0;
-    int y_rep_start = 0, y_rep_end = 0;
+       Let px = x - start->x, py = y - start->y.
+       t is given by
+         (px - dx*t)*dx + (py - dy*t)*dy = 0
 
-    int rep_start, rep_end;
-
-    // figure out how many times we'd need to repeat the gradient pattern
-    // to cover the whole (transformed) surface area
-    mstart = CGPointApplyAffineTransform (*start, matrix);
-    mend = CGPointApplyAffineTransform (*end, matrix);
-
-    dx = fabs (mend.x - mstart.x);
-    dy = fabs (mend.y - mstart.y);
+       Solving for t we get
+         numerator = dx*px + dy*py
+         denominator = dx^2 + dy^2
+         t = numerator/denominator
 
-    if (dx > 1e-6) {
-	x_rep_start = ceil(MIN(mstart.x, mend.x) / dx);
-	x_rep_end = ceil((surface->extents.width - MAX(mstart.x, mend.x)) / dx);
+       In CreateRepeatingLinearGradientFunction we know the length of (dx,dy)
+       is not zero. (This is checked in _cairo_quartz_setup_linear_source.)
+    */
+    double px = x - start->x;
+    double py = y - start->y;
+    double numerator = dx*px + dy*py;
+    double denominator = dx*dx + dy*dy;
+    double t = numerator/denominator;
 
-	if (mend.x < mstart.x) {
-	    int swap = x_rep_end;
-	    x_rep_end = x_rep_start;
-	    x_rep_start = swap;
-	}
+    if (*min_t > t) {
+        *min_t = t;
     }
-
-    if (dy > 1e-6) {
-	y_rep_start = ceil(MIN(mstart.y, mend.y) / dy);
-	y_rep_end = ceil((surface->extents.width - MAX(mstart.y, mend.y)) / dy);
-
-	if (mend.y < mstart.y) {
-	    int swap = y_rep_end;
-	    y_rep_end = y_rep_start;
-	    y_rep_start = swap;
-	}
+    if (*max_t < t) {
+        *max_t = t;
     }
+}
 
-    rep_start = MAX(x_rep_start, y_rep_start);
-    rep_end = MAX(x_rep_end, y_rep_end);
-
-    // extend the line between start and end by rep_start times from the start
-    // and rep_end times from the end
-
-    dx = end->x - start->x;
-    dy = end->y - start->y;
-
-    start->x = start->x - dx * rep_start;
-    start->y = start->y - dy * rep_start;
-
-    end->x = end->x + dx * rep_end;
-    end->y = end->y + dy * rep_end;
+static CGFunctionRef
+CreateRepeatingLinearGradientFunction (cairo_quartz_surface_t *surface,
+				       const cairo_gradient_pattern_t *gpat,
+				       CGPoint *start, CGPoint *end,
+				       cairo_rectangle_int_t *extents)
+{
+    cairo_pattern_t *pat;
+    cairo_quartz_float_t input_value_range[2];
+    double t_min = 0.;
+    double t_max = 0.;
+    double dx = end->x - start->x;
+    double dy = end->y - start->y;
+    double bounds_x1, bounds_x2, bounds_y1, bounds_y2;
+
+    if (!extents) {
+        extents = &surface->extents;
+    }
+    bounds_x1 = extents->x;
+    bounds_y1 = extents->y;
+    bounds_x2 = extents->x + extents->width;
+    bounds_y2 = extents->y + extents->height;
+    _cairo_matrix_transform_bounding_box (&gpat->base.matrix,
+                                          &bounds_x1, &bounds_y1,
+                                          &bounds_x2, &bounds_y2,
+                                          NULL);
+
+    UpdateLinearParametersToIncludePoint(&t_min, &t_max, start, dx, dy,
+                                         bounds_x1, bounds_y1);
+    UpdateLinearParametersToIncludePoint(&t_min, &t_max, start, dx, dy,
+                                         bounds_x2, bounds_y1);
+    UpdateLinearParametersToIncludePoint(&t_min, &t_max, start, dx, dy,
+                                         bounds_x2, bounds_y2);
+    UpdateLinearParametersToIncludePoint(&t_min, &t_max, start, dx, dy,
+                                         bounds_x1, bounds_y2);
+
+    /* Move t_min and t_max to the nearest usable integer to try to avoid
+       subtle variations due to numerical instability, especially accidentally
+       cutting off a pixel. Extending the gradient repetitions is always safe. */
+    t_min = floor (t_min);
+    t_max = ceil (t_max);
+    end->x = start->x + dx*t_max;
+    end->y = start->y + dy*t_max;
+    start->x = start->x + dx*t_min;
+    start->y = start->y + dy*t_min;
 
     // set the input range for the function -- the function knows how to
     // map values outside of 0.0 .. 1.0 to that range for REPEAT/REFLECT.
-    input_value_range[0] = 0.0 - 1.0 * rep_start;
-    input_value_range[1] = 1.0 + 1.0 * rep_end;
+    input_value_range[0] = t_min;
+    input_value_range[1] = t_max;
 
     if (_cairo_pattern_create_copy (&pat, &gpat->base))
 	/* quartz doesn't deal very well with malloc failing, so there's
@@ -950,10 +968,9 @@ static CGFunctionRef
 CreateRepeatingRadialGradientFunction (cairo_quartz_surface_t *surface,
                                        const cairo_gradient_pattern_t *gpat,
                                        CGPoint *start, double *start_radius,
-                                       CGPoint *end, double *end_radius)
+                                       CGPoint *end, double *end_radius,
+                                       cairo_rectangle_int_t *extents)
 {
-    CGRect clip = CGContextGetClipBoundingBox (surface->cgContext);
-    CGAffineTransform transform;
     cairo_pattern_t *pat;
     float input_value_range[2];
     CGPoint *inner;
@@ -965,10 +982,19 @@ CreateRepeatingRadialGradientFunction (cairo_quartz_surface_t *surface,
     double t_min, t_max, t_temp;
     /* outer minus inner */
     double dr, dx, dy;
+    double bounds_x1, bounds_x2, bounds_y1, bounds_y2;
 
-    _cairo_quartz_cairo_matrix_to_quartz (&gpat->base.matrix, &transform);
-    /* clip is in cairo device coordinates; get it into cairo user space */
-    clip = CGRectApplyAffineTransform (clip, transform);
+    if (!extents) {
+        extents = &surface->extents;
+    }
+    bounds_x1 = extents->x;
+    bounds_y1 = extents->y;
+    bounds_x2 = extents->x + extents->width;
+    bounds_y2 = extents->y + extents->height;
+    _cairo_matrix_transform_bounding_box (&gpat->base.matrix,
+                                          &bounds_x1, &bounds_y1,
+                                          &bounds_x2, &bounds_y2,
+                                          NULL);
 
     if (*start_radius < *end_radius) {
         /* end circle contains start circle */
@@ -988,6 +1014,7 @@ CreateRepeatingRadialGradientFunction (cairo_quartz_surface_t *surface,
     dx = outer->x - inner->x;
     dy = outer->y - inner->y;
 
+    /* We can't round or fudge t_min here, it has to be as accurate as possible. */
     t_min = -(*inner_radius/dr);
     inner->x += t_min*dx;
     inner->y += t_min*dy;
@@ -995,19 +1022,19 @@ CreateRepeatingRadialGradientFunction (cairo_quartz_surface_t *surface,
 
     t_temp = 0.;
     UpdateRadialParameterToIncludePoint(&t_temp, inner, dr, dx, dy,
-                                        clip.origin.x, clip.origin.y);
+                                        bounds_x1, bounds_y1);
     UpdateRadialParameterToIncludePoint(&t_temp, inner, dr, dx, dy,
-                                        clip.origin.x + clip.size.width, clip.origin.y);
+                                        bounds_x2, bounds_y1);
     UpdateRadialParameterToIncludePoint(&t_temp, inner, dr, dx, dy,
-                                        clip.origin.x + clip.size.width, clip.origin.y + clip.size.height);
+                                        bounds_x2, bounds_y2);
     UpdateRadialParameterToIncludePoint(&t_temp, inner, dr, dx, dy,
-                                        clip.origin.x, clip.origin.y + clip.size.height);
+                                        bounds_x1, bounds_y2);
     /* UpdateRadialParameterToIncludePoint assumes t=0 means radius 0.
        But for the parameter values we use with Quartz, t_min means radius 0.
-       Also, add a small fudge factor to avoid rounding issues. Since the
-       circles are alway expanding and containing the earlier circles, this is
-       OK. */
-    t_temp += 1e-6;
+       Since the circles are alway expanding and contain the earlier circles,
+       it's safe to extend t_max/t_temp as much as we want, so round t_temp up
+       to the nearest integer. This may help us give stable results. */
+    t_temp = ceil (t_temp);
     t_max = t_min + t_temp;
     outer->x = inner->x + t_temp*dx;
     outer->y = inner->y + t_temp*dy;
@@ -1346,9 +1373,23 @@ _cairo_quartz_setup_fallback_source (cairo_quartz_surface_t *surface,
     return DO_IMAGE;
 }
 
+/*
+Quartz does not support repeating radients. We handle repeating gradients
+by manually extending the gradient and repeating color stops. We need to
+minimize the number of repetitions since Quartz seems to sample our color
+function across the entire range, even if part of that range is not needed
+for the visible area of the gradient, and it samples with some fixed resolution,
+so if the gradient range is too large it samples with very low resolution and
+the gradient is very coarse. CreateRepeatingLinearGradientFunction and
+CreateRepeatingRadialGradientFunction compute the number of repetitions needed
+based on the extents of the object (the clip region cannot be used here since
+we don't want the rasterization of the entire gradient to depend on the
+clip region).
+*/
 static cairo_quartz_action_t
 _cairo_quartz_setup_linear_source (cairo_quartz_surface_t *surface,
-				   const cairo_linear_pattern_t *lpat)
+				   const cairo_linear_pattern_t *lpat,
+				   cairo_rectangle_int_t *extents)
 {
     const cairo_pattern_t *abspat = &lpat->base.base;
     cairo_matrix_t mat;
@@ -1363,6 +1404,16 @@ _cairo_quartz_setup_linear_source (cairo_quartz_surface_t *surface,
 	return DO_SOLID;
     }
 
+    if (lpat->p1.x == lpat->p2.x &&
+        lpat->p1.y == lpat->p2.y) {
+	/* Quartz handles cases where the vector has no length very
+	 * differently from pixman.
+	 * Whatever the correct behaviour is, let's at least have only pixman's
+	 * implementation to worry about.
+	 */
+	return _cairo_quartz_setup_fallback_source (surface, abspat);
+    }
+
     mat = abspat->matrix;
     cairo_matrix_invert (&mat);
     _cairo_quartz_cairo_matrix_to_quartz (&mat, &surface->sourceTransform);
@@ -1382,7 +1433,7 @@ _cairo_quartz_setup_linear_source (cairo_quartz_surface_t *surface,
 	gradFunc = CreateRepeatingLinearGradientFunction (surface,
 						          &lpat->base,
 						          &start, &end,
-						          surface->sourceTransform);
+						          extents);
     }
 
     surface->sourceShading = CGShadingCreateAxial (rgb,
@@ -1398,7 +1449,8 @@ _cairo_quartz_setup_linear_source (cairo_quartz_surface_t *surface,
 
 static cairo_quartz_action_t
 _cairo_quartz_setup_radial_source (cairo_quartz_surface_t *surface,
-				   const cairo_radial_pattern_t *rpat)
+				   const cairo_radial_pattern_t *rpat,
+				   cairo_rectangle_int_t *extents)
 {
     const cairo_pattern_t *abspat = &rpat->base.base;
     cairo_matrix_t mat;
@@ -1450,7 +1502,8 @@ _cairo_quartz_setup_radial_source (cairo_quartz_surface_t *surface,
 	gradFunc = CreateRepeatingRadialGradientFunction (surface,
 						          &rpat->base,
 						          &start, &r1,
-						          &end, &r2);
+						          &end, &r2,
+						          extents);
     }
 
     surface->sourceShading = CGShadingCreateRadial (rgb,
@@ -1469,7 +1522,8 @@ _cairo_quartz_setup_radial_source (cairo_quartz_surface_t *surface,
 
 static cairo_quartz_action_t
 _cairo_quartz_setup_source (cairo_quartz_surface_t *surface,
-			    const cairo_pattern_t *source)
+			    const cairo_pattern_t *source,
+			    cairo_rectangle_int_t *extents)
 {
     assert (!(surface->sourceImage || surface->sourceShading || surface->sourcePattern));
 
@@ -1495,14 +1549,12 @@ _cairo_quartz_setup_source (cairo_quartz_surface_t *surface,
 
     if (source->type == CAIRO_PATTERN_TYPE_LINEAR) {
 	const cairo_linear_pattern_t *lpat = (const cairo_linear_pattern_t *)source;
-	return _cairo_quartz_setup_linear_source (surface, lpat);
-
+	return _cairo_quartz_setup_linear_source (surface, lpat, extents);
     }
 
     if (source->type == CAIRO_PATTERN_TYPE_RADIAL) {
 	const cairo_radial_pattern_t *rpat = (const cairo_radial_pattern_t *)source;
-	return _cairo_quartz_setup_radial_source (surface, rpat);
-
+	return _cairo_quartz_setup_radial_source (surface, rpat, extents);
     }
 
     if (source->type == CAIRO_PATTERN_TYPE_SURFACE &&
@@ -2025,7 +2077,7 @@ _cairo_quartz_surface_paint_cg (void *abstract_surface,
     if (unlikely (rv))
 	return rv == CAIRO_INT_STATUS_NOTHING_TO_DO ? CAIRO_STATUS_SUCCESS : rv;
 
-    action = _cairo_quartz_setup_source (surface, source);
+    action = _cairo_quartz_setup_source (surface, source, NULL);
 
     if (action == DO_SOLID || action == DO_PATTERN) {
 	CGContextFillRect (surface->cgContext, CGRectMake(surface->extents.x,
@@ -2051,6 +2103,25 @@ _cairo_quartz_surface_paint_cg (void *abstract_surface,
     return rv;
 }
 
+static cairo_bool_t
+_cairo_quartz_source_needs_extents (const cairo_pattern_t *source)
+{
+    /* For repeating gradients we need to manually extend the gradient and
+       repeat stops, since Quartz doesn't support repeating gradients natively.
+       We need to minimze the number of repeated stops, and since rasterization
+       depends on the number of repetitions we use (even if some of the
+       repetitions go beyond the extents of the object or outside the clip
+       region), it's important to use the same number of repetitions when
+       rendering an object no matter what the clip region is. So the
+       computation of the repetition count cannot depended on the clip region,
+       and should only depend on the object extents, so we need to compute
+       the object extents for repeating gradients. */
+    return (source->type == CAIRO_PATTERN_TYPE_LINEAR ||
+            source->type == CAIRO_PATTERN_TYPE_RADIAL) &&
+           (source->extend == CAIRO_EXTEND_REPEAT ||
+            source->extend == CAIRO_EXTEND_REFLECT);
+}
+
 static cairo_int_status_t
 _cairo_quartz_surface_paint (void *abstract_surface,
 			     cairo_operator_t op,
@@ -2110,7 +2181,17 @@ _cairo_quartz_surface_fill_cg (void *abstract_surface,
 
     CGContextSetShouldAntialias (surface->cgContext, (antialias != CAIRO_ANTIALIAS_NONE));
 
-    action = _cairo_quartz_setup_source (surface, source);
+    if (_cairo_quartz_source_needs_extents (source))
+    {
+        /* We don't need precise extents since these are only used to
+           compute the number of gradient reptitions needed to cover the
+           object. */
+        cairo_rectangle_int_t path_extents;
+        _cairo_path_fixed_approximate_fill_extents (path, &path_extents);
+        action = _cairo_quartz_setup_source (surface, source, &path_extents);
+    } else {
+        action = _cairo_quartz_setup_source (surface, source, NULL);
+    }
 
     _cairo_quartz_cairo_path_to_quartz_context (path, surface->cgContext);
 
@@ -2267,7 +2348,14 @@ _cairo_quartz_surface_stroke_cg (void *abstract_surface,
     CGContextSaveGState (surface->cgContext);
 
 
-    action = _cairo_quartz_setup_source (surface, source);
+    if (_cairo_quartz_source_needs_extents (source))
+    {
+        cairo_rectangle_int_t path_extents;
+        _cairo_path_fixed_approximate_stroke_extents (path, style, ctm, &path_extents);
+        action = _cairo_quartz_setup_source (surface, source, &path_extents);
+    } else {
+        action = _cairo_quartz_setup_source (surface, source, NULL);
+    }
 
     _cairo_quartz_cairo_path_to_quartz_context (path, surface->cgContext);
 
@@ -2406,7 +2494,16 @@ _cairo_quartz_surface_show_glyphs_cg (void *abstract_surface,
 
     CGContextSaveGState (surface->cgContext);
 
-    action = _cairo_quartz_setup_source (surface, source);
+    if (_cairo_quartz_source_needs_extents (source))
+    {
+        cairo_rectangle_int_t glyph_extents;
+        _cairo_scaled_font_glyph_device_extents (scaled_font, glyphs, num_glyphs,
+                                                 &glyph_extents, NULL);
+        action = _cairo_quartz_setup_source (surface, source, &glyph_extents);
+    } else {
+        action = _cairo_quartz_setup_source (surface, source, NULL);
+    }
+
     if (action == DO_SOLID || action == DO_PATTERN) {
 	CGContextSetTextDrawingMode (surface->cgContext, kCGTextFill);
     } else if (action == DO_IMAGE || action == DO_TILED_IMAGE || action == DO_SHADING) {