This seems to happen quite a bit because we don't have an inner radius of 0
Created attachment 693989 [details] [diff] [review] This random collection of removals of firstStop might do the trick.
Roc, D2D doesn't support inner radii != 0. What are the reasons we need to adjust the gradient to start at the first stop? Should we only avoid that in the radial case, or can we get away with not adjusting it the rest of the time too.
// Cairo gradients must have stop positions in the range [0, 1]. So, // stop positions will be normalized below by subtracting firstStop and then // multiplying by stopScale. So, for radial gradients it's OK to map lastStop to 1 and map 0 to 0, since firstStop must be >= 0. However, for linear gradients we could have a negative firstStop and cairo doesn't support that so we have to remap it at least in the case where it's negative.
Created attachment 704092 [details] [diff] [review] A new approach
Created attachment 704226 [details] [diff] [review] Avoid moving the start location of non-repeating radial gradients This avoids moving the start position of non-repeating radial gradients so that they will go down the D2D fast path.
Can we uplift this? This makes pages like http://www.psfk.com/2013/01/honda-gear-concept-is-brands-return-to-cool.html not DoS my browser.
I think https://bugzilla.mozilla.org/show_bug.cgi?id=813957 may report the same problem. I'm not able to tell. But I point at it so you can close it if it's indeed related and fixed by this bug.
We'll track and uplift for Aurora, but it's too late to take this on beta and it doesn't meet ESR criteria so minusing for those.
Can we get an uplift nomination here? Merge Day is next Tuesday and it would be great to have this land while 20 is still on Aurora and before FF 20 beta 1.
Comment on attachment 704226 [details] [diff] [review] Avoid moving the start location of non-repeating radial gradients [Approval Request Comment] Bug caused by (feature/regressing bug #): Not a regression User impact if declined: Really bad gradient performance on some pages. Without this, we basically denial of service some machines. Testing completed (on m-c, etc.): It has been on m-c since 1-23 (19 days ago) Risk to taking this patch (and alternatives if risky): Some gradients will draw wrong. However, this is very easy to back out, and we're more likely to find any problems on release. String or UUID changes made by this patch: None
Comment on attachment 704226 [details] [diff] [review] Avoid moving the start location of non-repeating radial gradients Glad to hear this is easy to back out, so let's go ahead with uplift and watch for regressions while 20 is on Beta.