I think we can use CGShading to do repeating radial gradients, at least for cases where the outer circle fully encloses the inner circle. If I understand radial gradients correctly, we can just expand the outer circle parameter, setting it to be one of the repeated outer circles that fully encloses the clipBox, and then set the color function to repeat color stops in the right places (like we already do for linear gradients). I may give this a go.

This does it. Figuring out where the outer circle should move to is tricky. I hope the algebra in there is understandable. This is a nice performance improvement, probably because we avoid painting the gradient over the entire clipBox (which is usually the entire surface). I tried to write reftests that compared a repeating radial gradient to a non-repeating gradient with manually repeated stops, but it didn't work because the rasterization was slightly different --- I'm not sure why. This patch also forces us to use pixman for all degenerate cases where the circles intersect. This at least makes us consistent across platforms.

Better fix ... transform clip into cairo user space before we do all these tests that assume it's in user space.

Oops, there was some stuff missing because I mismanaged my patches. I'm going to change the way we compute the extents for the radial gradient, in bug 508730. But that builds on this.

Comment on attachment 392913 [details] [diff] [review] fix v3 I know the style in this file is not that consistent but using camel-case stuck out to me as foreign. I don't feel that strongly about it though. >+static void >+UpdateRadialParameterToIncludePoint(double *max_t, CGPoint *center, >+ double dr, double dx, double dy, >+ double x, double y) >+{ [snip] >@@ -1142,52 +1283,68 @@ _cairo_quartz_setup_radial_source (cairo >+ double c1x = _cairo_fixed_to_double (rpat->c1.x); >+ double c1y = _cairo_fixed_to_double (rpat->c1.y); >+ double c2x = _cairo_fixed_to_double (rpat->c2.x); >+ double c2y = _cairo_fixed_to_double (rpat->c2.y); >+ double r1 = _cairo_fixed_to_double (rpat->r1); >+ double r2 = _cairo_fixed_to_double (rpat->r2); >+ double dx = c1x - c2x; >+ double dy = c1y - c2y; >+ double centerDistance = sqrt (dx*dx + dy*dy); you could use hypot(dx, dy) here depending on how you want to trade off accuracy vs. performance.

Checked in but backed out due to test failures: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1251238468.1251248064.24764.gz

The test failures are because we're now falling back to pixman for degenerate radial gradient cases (where neither circle contains the other), and pixman fails these tests --- they were only run on Mac. I've filed bug 512647 for these pixman failures. I think in the meantime it's reasonable to disable them on Mac as well; at least my patch here makes us consistent across platforms.

http://hg.mozilla.org/mozilla-central/rev/e564f3ab4ea6 http://hg.mozilla.org/mozilla-central/rev/0dc906f51445

Comment on attachment 392913 [details] [diff] [review] fix v3 Makes CSS gradients work better

What prevents this fix from getting lost when we next update cairo? I don't see any changes to gfx/cairo/README or any tests that would catch such a regression.

Sorry, I have a patch in my queue that adds to gfx/cairo/README but I forgot to land it.

That patch landed as changeset 9f811e15ca51.

Do we still want these on 1.9.2?

This already landed on 1.9.2 in commit ebbeedee7a9a.