823147 - Avoid hitting D2D slow path when drawing radial gradients from css

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

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

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Attachment: This random collection of removals of firstStop might do the trick.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

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.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

    // 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.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

https://tbpl.mozilla.org/?tree=Try&rev=0b379e8acd18

Comment 5

[Jeff Muizelaar [:jrmuizel]]

https://tbpl.mozilla.org/?tree=Try&rev=aff3fbe7f96a

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Attachment: A new approach

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Attachment: 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.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

https://tbpl.mozilla.org/?tree=Try&rev=9a6701a80638

Comment 9

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/76611c3de0d7

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/76611c3de0d7

Comment 11

[(dormant account)]

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

[godotcg-rg]

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.

Comment 13

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

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 15

[Jeff Muizelaar [:jrmuizel]]

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 16

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/28cfe506a7d9