Gradient perf is unusably slow on test case

RESOLVED FIXED in Firefox 16

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: BenWa, Assigned: roc)

Tracking

unspecified
mozilla17
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox16 fixed)

Details

(Whiteboard: [Snappy:p1])

Attachments

(4 attachments)

(Reporter)

Description

5 years ago
Test case:
http://estelle.github.com/mobileperf/#slide14

Profile:
http://people.mozilla.org/~bgirard/cleopatra/?report=AMIfv96oQne_-iA9yvusK2c_xPq2XCLcoJk67PE7MsQMtNOphnzr3DxVtg7gKhAcrajRN0kiqtlfGG1z_3h66deiYF8fBZ_wqOJatXDfu3160FzwxCg3HzMk8HIfKsqE8HK_VrhkOc85Yh9nrPKpGgvLSHAhuaVeRg

We're orders of magnitude slower then chrome. There's certainly some low hanging fruits here.

Updated

5 years ago
Whiteboard: [Snappy]
Created attachment 637125 [details]
A reduced test case

The reason webkit is so much faster is because it draws the gradient to an image and tiles that instead of drawing a repeating gradient over the size of the entire screen.
Assignee: nobody → roc
FWIW, the webkit code is:
  WebCore::GeneratedImage::drawPattern()
The problem is getting pixel-snapping and joins between tiles right.
Created attachment 637302 [details] [diff] [review]
fix

This should make things a lot better. In all the cases of small gradients being repeated that I've seen, they have been axis-aligned linear gradients, and this optimization should kick in.
Attachment #637302 - Flags: review?(jmuizelaar)
(Reporter)

Comment 5

5 years ago
This patch makes a huge improvement! Not quite as good as WebKit on this page but at least the page works :).

I notice that when I'm spanning right with the arrow keys some slides disappear.
Attachment #637302 - Flags: review?(jmuizelaar) → review+
I had tried to push this, but there were reftest failures so I backed it out:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=61b94ce72be4
https://hg.mozilla.org/integration/mozilla-inbound/rev/61b94ce72be4 is my attempt to fix some OSX failures, but wasn't quite enough:
https://tbpl.mozilla.org/php/getParsedLog.php?id=13153677&tree=Mozilla-Inbound&full=1

There were other failures on Windows D2D:
https://tbpl.mozilla.org/php/getParsedLog.php?id=13155509&tree=Mozilla-Inbound&full=1
These Windows failures are concerning. The basic issue is sampling. It's clearest in the linear-vertical*.html tests. The first row of pixels is generated partially by sampling the last stop of the gradient. In linear-repeat-* it's also quite obvious; there the last row of pixels is generated partially by sampling the first stop of the gradient. These artifacts are quite obvious and need to be fixed somehow.
The test failures are because Windows D2D is currently using Thebes to draw content in reftests :-(. I have a fix for the cairo-D2D code that would probably fix that, but a better approach is to fix bug 772726 so that on Tinderbox Windows D2D uses Azure to draw content in reftests.

Updated

5 years ago
Whiteboard: [Snappy] → [Snappy:p1]
Blocks: 644444

Comment 9

5 years ago
Need to be fixed before Firefox 16 release what will support unprefixed "repeating-linear-gradient" http://hacks.mozilla.org/2012/07/aurora-16-is-out/ otherwise it will bring Firefox bad experience and tons of failures.
Created attachment 645581 [details] [diff] [review]
Improve precision of bounds for cairo-D2D repeating stops

This reduces the effects of bug 582236.
Attachment #645581 - Flags: review?(bas.schouten)
Created attachment 645587 [details] [diff] [review]
add some fuzziness/fail for reftests
Attachment #645587 - Flags: review?(bas.schouten)
Note that the additional fixes here are only needed because of bug 772726. I decided not to block relanding this on bug 772726, partly because I want to uplift the fixes here to Aurora and I don't want to land bug 772726 on Aurora.
Attachment #645587 - Flags: review?(bas.schouten) → review+
Attachment #645581 - Flags: review?(bas.schouten) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee21073be123
https://hg.mozilla.org/integration/mozilla-inbound/rev/874a4984caf2
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e637d1f8ec6
Comment on attachment 637302 [details] [diff] [review]
fix

Review of attachment 637302 [details] [diff] [review]:
-----------------------------------------------------------------

This is a performance improvement that makes some pages *much* better. We should take it on Aurora because we have unprefixed CSS gradients there and we don't want to be stuck with super-bad performance in some cases.
Attachment #637302 - Flags: approval-mozilla-aurora?
Comment on attachment 645581 [details] [diff] [review]
Improve precision of bounds for cairo-D2D repeating stops

Review of attachment 645581 [details] [diff] [review]:
-----------------------------------------------------------------

This is a performance improvement that makes some pages *much* better. We should take it on Aurora because we have unprefixed CSS gradients there and we don't want to be stuck with super-bad performance in some cases.
Attachment #645581 - Flags: approval-mozilla-aurora?
Comment on attachment 645587 [details] [diff] [review]
add some fuzziness/fail for reftests

Review of attachment 645587 [details] [diff] [review]:
-----------------------------------------------------------------

This is a performance improvement that makes some pages *much* better. We should take it on Aurora because we have unprefixed CSS gradients there and we don't want to be stuck with super-bad performance in some cases.
Attachment #645587 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/ee21073be123
https://hg.mozilla.org/mozilla-central/rev/874a4984caf2
https://hg.mozilla.org/mozilla-central/rev/5e637d1f8ec6
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Depends on: 777755
Comment on attachment 637302 [details] [diff] [review]
fix

[Triage Comment]
Given where we are in the cycle, approving for Aurora 16 in support of new unprefixed CSS changes. We can always back out if this causes new regressions.
Attachment #637302 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

5 years ago
Attachment #645581 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

5 years ago
Attachment #645587 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 19

5 years ago
The bug still appears on Nightly 17.0a1 (2012-07-28). Tested on Win7 64-bit.
On this page? http://estelle.github.com/mobileperf/#slide14

Updated

5 years ago
Depends on: 779399
Did the patches land on Aurora?
Not yet. I'm waiting for Aurora approval for the regression fix in bug 779399.
https://hg.mozilla.org/releases/mozilla-aurora/rev/5e4a0ba8918d
https://hg.mozilla.org/releases/mozilla-aurora/rev/68bc6eeb7b14
https://hg.mozilla.org/releases/mozilla-aurora/rev/d25d95292cad
status-firefox16: --- → fixed

Comment 24

5 years ago
It still has not been fixed on Nightly 17.0a1 (2012-08-20).
On which page do you still see the bug? http://estelle.github.com/mobileperf/#slide14?

Comment 26

5 years ago
@Robert, http://estelle.github.com/mobileperf/#slide14 `-moz-linear-gradient` seems rendering fine, but http://laukstein.com `repeating-linear-gradient` still is unusable slow.
I think that's a completely different problem. Please file it as a separate bug.
(And when you do, please specify your platform and about:support data.)

Comment 29

5 years ago
@Robert, `repeating-linear-gradient` and `background-size` bug 644444 https://bugzilla.mozilla.org/show_bug.cgi?id=644444 reported on 2011/03/23, the current status: RESOLVED FIXED.
I just added comment to update status and fix the bug.
Your mentioned patch in bug 768775 unfortunately did not fix the bug 644444.
OK, thanks. I reopened bug 644444.

Updated

5 years ago
Depends on: 792903

Updated

5 years ago
No longer depends on: 792903
Depends on: 1224761
You need to log in before you can comment on or make changes to this bug.