Closed Bug 1168879 Opened 4 years ago Closed 8 months ago
Drawing linear-gradients with Skia as the content backend can cause noticeable banding
When creating CSS linear-gradients, in some case noticeable banding can result if Skia is the content backend. This problem also appears on the test case in Chromium. It does not occur if any other content backend is used. Therefor, the problem seems confined to Skia's gradient rendering. See testcase: http://m8y.org/tmp/testcase429.xhtml
Oh neat. Didn't even notice you'd filed this. Yeah, I was wondering why people were saying the demo looked ugly. Seemed fine for me. Guess they were Chrome users.
A nice person on #blink linked me to: https://code.google.com/p/skia/issues/detail?id=1077 which seems to be this exact problem. Looks like it hasn't gotten any love for years though. Unfortunate, since right now the colour shift effect in testcase has to be simulated in JS for Chrome. Constantly regenerating linear gradients many times a second. Very inefficient.
When looking into bug 1013540, it was discovered that the half-texel (or rather, here, texel = gradient cache index step) offset is required if bilinear filtering is used to force the chosen indexes to round correctly. This offset should not be used in cases where point filtering is used. In the cases of vertical gradients, linear filtering was used since a single color could be chosen for a whole row, and color between rows blended once and applied to the whole row. In non-vertical cases, it would have had to do this at every pixel... so it didn't. Thus, it used point filtering to index into the gradient table, causing drastically noticeable banding in the above testcase. Gradients in Skia uses a dithering scheme that will try to help reproduce color values themselves that are not integral, since the cache only stores 32bpp colors, so without the dithering a value like 210.75 could not be reproduced, but the dithering allows it to be approximated. But this dithering does not apply to the indexes into the gradient cache themselves, so does nothing to alleviate banding. However, with a modification of the dithering scheme already in place for the gradient cache, we use the existing dither toggle (a value from 0..3) to add toggle*0.25 to the gradient cache index, thus effectively dithering the gradient index and giving us an effective resolution 4 times higher in the gradient cache. Overall, this drastically reduces observable banding in the test case, and only costs us one bit shift and an add per pixel. Since bilinear filtering is emulated in all cases, this patch also helps fix the issue noted in bug 1013540.
Comment on attachment 8645501 [details] [diff] [review] implement dithering of the gradient index to emulate bilinear filtering Review of attachment 8645501 [details] [diff] [review]: ----------------------------------------------------------------- Have you looked at how much this code has changed upstream and how easy it would be to upstream. I have no problem taking it if you're willing to maintain it but it would be good to file an upstream review to try to get an opinion on how likely they are to take it.
Just figured I'd mention. Without your patch, enabling skia in Firefox causes CPU usage to jump like 5 fold. So Cairo is I guess hardware acelerated here? That is, Chromium, with Skia I get: 1976 root 20 0 667848 305544 110300 S 77.0 1.9 588:10.34 Xorg 19816 nemo 20 0 1530472 572288 556420 S 63.1 3.5 0:37.22 chromium-b+ 19646 nemo 20 0 1803948 170536 101148 R 44.8 1.0 0:27.46 chromium-b+ Total, ~185% - almost 2 cores. Firefox, with Skia: 19870 nemo 20 0 1194440 294252 72080 S 39.9 1.8 0:14.48 firefox 19961 nemo 20 0 550604 89396 47396 S 35.2 0.5 0:07.77 Web Content 1976 root 20 0 690696 316972 121728 R 17.3 1.9 588:30.83 Xorg Total, ~92% - almost 1 core. Firefox, with Cairo: 20589 nemo 20 0 505316 75012 31604 S 6.6 0.5 0:07.46 Web Content 1976 root 20 0 675184 308020 112776 S 5.7 1.9 589:47.66 Xorg 20501 nemo 20 0 1135932 237968 47216 S 5.0 1.5 0:12.27 firefox Total, ~17% (!) Overall seems to me Cairo has been better behaved lately too... Wonder if someone's been working on Cairo's performance.
Has any progress been made on this issue?
Skia upstream has done some work in this area on certain types of clamped gradients, in which cases it uses linear interpolation with SIMD acceleration. This work we already benefit from in recent Skia updates incorporated into our tree. The testcase in this bug however uses a repeating gradient, which still causes Skia to fall back to the old dithered indexing scheme. I think we would want to just convince upstream to further modify their code to support the linear interpolation strategy for the repeating gradient case as well, rather than the original patch I proposed. If nothing else, they have at least put a framework in place for us to do it ourselves as a local patch if they are not interested. For now I think I will just deprecate my originally proposed patch, as it does not seem to represent the direction Skia is going.
Assignee: lsalzman → nobody
Status: ASSIGNED → NEW
Just FYI, Skia seems to now be default on my Linux nightlies. CPU usage is now through the roof on http://m8y.org/tmp/testcase429.xhtml (instead of 10-20% of one core as before I have: 25261 nemo 20 0 1487516 634248 79780 R 60.5 3.9 28:55.62 Web Content 1879 root 20 0 616124 273936 181376 S 36.2 1.7 355:13.51 Xorg 31962 nemo 20 0 1562496 366124 107348 S 32.9 2.2 11985:51 firefox about 150% - same as chrome. Also the banding reported in this bug is still there. So I guess it hasn't been fixed upstream after all.
Ok... Nevermind... $ chromium-browser --version Chromium 53.0.2785.143 Built on Ubuntu , running on Ubuntu 14.04 Has no banding at all. Latest firefox nightly does. So I guess it's just that upstream changes aren't incorporated in firefox yet.
So... I figured I'd follow up on this to mention that latest nightly Firefox still has banding using skia while all Chrome browsers I've used have had this fixed in stable like... last year. This is odd since there was a proposed fix in this bug first, and, from question asked on IRC it seems that Firefox should be on same version of skia as recent Chrome browsers. Could it be that the fix for this is flagged off and the Firefox build config is not picking it up?
2 years ago
Priority: -- → P3
Assignee: nobody → lsalzman
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.