Closed Bug 1168879 Opened 4 years ago Closed 8 months ago

Drawing linear-gradients with Skia as the content backend can cause noticeable banding

Categories

(Core :: Graphics, defect, P3)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: lsalzman, Assigned: lsalzman)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [gfx-noted])

Attachments

(1 obsolete file)

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
Blocks: 1038800
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.
Blocks: skia-reftest
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.
Blocks: 1013540
Status: NEW → ASSIGNED
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.
Attachment #8645501 - Flags: review?(jmuizelaar)
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.
No longer blocks: 1013540
Has any progress been made on this issue?
Flags: needinfo?(lsalzman)
Whiteboard: [gfx-noted]
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.
Flags: needinfo?(lsalzman)
Attachment #8645501 - Attachment is obsolete: true
Attachment #8645501 - Flags: review?(jmuizelaar)
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?

Skia no longer uses a lookup table for gradients, so this is fixed.

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.