Avoid hitting D2D slow path when drawing radial gradients from css

RESOLVED FIXED in Firefox 20



7 years ago
6 years ago


(Reporter: jrmuizel, Assigned: jrmuizel)


(Depends on 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(firefox18 wontfix, firefox19- wontfix, firefox20+ fixed, firefox21 fixed, firefox-esr17- wontfix, b2g18 unaffected)



(1 attachment, 2 obsolete attachments)



7 years ago
This seems to happen quite a bit because we don't have an inner radius of 0

Comment 1

7 years ago
Assignee: nobody → jmuizelaar


7 years ago
Blocks: 823141

Comment 2

7 years ago

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.
Blocks: 832374

Comment 6

6 years ago
Posted patch A new approach (obsolete) — Splinter Review
Attachment #693989 - Attachment is obsolete: true

Comment 7

6 years ago
This avoids moving the start position of non-repeating radial gradients so that they will go down the D2D fast path.
Attachment #704092 - Attachment is obsolete: true
Attachment #704226 - Flags: review?(roc)
Blocks: 833586
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21


6 years ago
Blocks: 834019
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.

Comment 12

6 years ago
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.
Flags: needinfo?(jmuizelaar)

Comment 15

6 years ago
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
Attachment #704226 - Flags: approval-mozilla-aurora?
Flags: needinfo?(jmuizelaar)
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.
Attachment #704226 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.