Last Comment Bug 768775 - Gradient perf is unusably slow on test case
: Gradient perf is unusably slow on test case
Status: RESOLVED FIXED
[Snappy:p1]
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal with 1 vote (vote)
: mozilla17
Assigned To: Robert O'Callahan (:roc) (email my personal email if necessary)
:
:
Mentors:
Depends on: 777755 779399 1224761
Blocks: 644444
  Show dependency treegraph
 
Reported: 2012-06-26 23:01 PDT by Benoit Girard (:BenWa)
Modified: 2015-11-23 18:35 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
A reduced test case (12.74 KB, text/html)
2012-06-27 08:21 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details
fix (5.16 KB, patch)
2012-06-27 16:10 PDT, Robert O'Callahan (:roc) (email my personal email if necessary)
jmuizelaar: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
Improve precision of bounds for cairo-D2D repeating stops (10.53 KB, patch)
2012-07-24 16:46 PDT, Robert O'Callahan (:roc) (email my personal email if necessary)
bas: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
add some fuzziness/fail for reftests (3.06 KB, patch)
2012-07-24 16:52 PDT, Robert O'Callahan (:roc) (email my personal email if necessary)
bas: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Comment 1 Jeff Muizelaar [:jrmuizel] 2012-06-27 08:21:06 PDT
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.
Comment 2 Jeff Muizelaar [:jrmuizel] 2012-06-27 15:15:33 PDT
FWIW, the webkit code is:
  WebCore::GeneratedImage::drawPattern()
Comment 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-06-27 15:18:52 PDT
The problem is getting pixel-snapping and joins between tiles right.
Comment 4 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-06-27 16:10:45 PDT
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.
Comment 5 Benoit Girard (:BenWa) 2012-06-27 16:31:41 PDT
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.
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-03 19:54:32 PDT
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
Comment 7 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-03 20:05:36 PDT
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.
Comment 8 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-10 19:55:19 PDT
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.
Comment 9 Binyamin 2012-07-24 02:20:04 PDT
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.
Comment 10 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-24 16:46:59 PDT
Created attachment 645581 [details] [diff] [review]
Improve precision of bounds for cairo-D2D repeating stops

This reduces the effects of bug 582236.
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-24 16:52:29 PDT
Created attachment 645587 [details] [diff] [review]
add some fuzziness/fail for reftests
Comment 12 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-24 16:54:17 PDT
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.
Comment 14 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-25 22:28:57 PDT
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.
Comment 15 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-25 22:29:01 PDT
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.
Comment 16 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-25 22:29:06 PDT
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.
Comment 18 Alex Keybl [:akeybl] 2012-07-27 16:40:34 PDT
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.
Comment 19 Binyamin 2012-07-29 03:34:38 PDT
The bug still appears on Nightly 17.0a1 (2012-07-28). Tested on Win7 64-bit.
Comment 20 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-29 15:16:04 PDT
On this page? http://estelle.github.com/mobileperf/#slide14
Comment 21 Marco Castelluccio [:marco] 2012-08-18 17:34:21 PDT
Did the patches land on Aurora?
Comment 22 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-08-20 04:14:35 PDT
Not yet. I'm waiting for Aurora approval for the regression fix in bug 779399.
Comment 24 Binyamin 2012-08-20 22:20:32 PDT
It still has not been fixed on Nightly 17.0a1 (2012-08-20).
Comment 25 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-08-20 23:04:40 PDT
On which page do you still see the bug? http://estelle.github.com/mobileperf/#slide14?
Comment 26 Binyamin 2012-08-21 00:19:32 PDT
@Robert, http://estelle.github.com/mobileperf/#slide14 `-moz-linear-gradient` seems rendering fine, but http://laukstein.com `repeating-linear-gradient` still is unusable slow.
Comment 27 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-08-21 17:32:32 PDT
I think that's a completely different problem. Please file it as a separate bug.
Comment 28 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-08-21 17:32:55 PDT
(And when you do, please specify your platform and about:support data.)
Comment 29 Binyamin 2012-08-21 22:20:12 PDT
@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.
Comment 30 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-08-21 23:13:54 PDT
OK, thanks. I reopened bug 644444.

Note You need to log in before you can comment on or make changes to this bug.