Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1457783005:
Enable high-quality linear gradients (Closed)
DescriptionEnable high-quality linear gradients
(remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE)
Start using Skia's analytical linear gradients (software-only).
The new implementation addresses a couple of fundamental issues present
in the old/LUT-based approach:
* inexact (or blurred) hard color stops
* banding artifacts
BUG=558622, 41756, 98436, 117140, 135568, 140208, 152706, 177293, 180783, 229561, 233879, 271552, 317502, 375630, 403959, 414254, 419344, 476548, 486063, 543625, skia:1077
R=reed@google.com,chrishtr@chromium.org
Committed: https://crrev.com/32158b3da9a7a21c44698862fc5370c2ed735299
Cr-Commit-Position: refs/heads/master@{#361476}
Patch Set 1 #Patch Set 2 : layout tests pls #Patch Set 3 : expectations #Patch Set 4 : drop expectations for new results #Patch Set 5 : expectations #Patch Set 6 : one more rebaseline #Patch Set 7 : drop expectations for updated layout test run #Patch Set 8 : expectations, no whitespace #
Messages
Total messages: 40 (26 generated)
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG= R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,152706,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,152706,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,152706,177293,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,152706,177293,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,419344,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,476548,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,476548,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,476548,486063,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,476548,486063,skia:1077 R=reed@google.com ========== to ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,476548,486063,543625,skia:1077 R=reed@google.com ==========
Description was changed from ========== Enable awesome Skia gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Switch Chromium to the analytical gradient impl. BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,476548,486063,543625,skia:1077 R=reed@google.com ========== to ========== Enable high-quality linear gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Start using Skia's analytical linear gradients (software-only). The new implementation addresses a couple of fundamental issues present in the old/LUT-based approach: * inexact (or blurred) hard color stops * banding artifacts BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,476548,486063,543625,skia:1077 R=reed@google.com,chrishtr@chromium.org ==========
fmalita@chromium.org changed reviewers: + chrishtr@chromium.org
fmalita@chromium.org changed reviewers: + fs@opera.com
~170 rebaselines: https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... Most are trivial. Some notable diffs - 1) hard stop accuracy progressions: https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... 2) no moar LUT banding! https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... 3) svg/batik/paints/gradientLimit.svg https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... ^^ this is actually a progression: the gradient should clamp to crimson on both ends. There is still one linear case where we don't do that (plus the radial line is also incorrect but unaffected by this change).
Forgot to mention: the ref test update is to increase numerical stability by ensuring both cases get the same layerization/gradient offset. Previously: test gradient draws into own layer, ref gradient draws into inherited layer. Now: both draw into own layer.
On 2015/11/24 at 20:07:53, fmalita wrote: > ~170 rebaselines: https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... > > Most are trivial. Some notable diffs - > > 1) hard stop accuracy progressions: > https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... > https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... > https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... > > 2) no moar LUT banding! > https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... > https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... > > > 3) svg/batik/paints/gradientLimit.svg > https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... > > ^^ this is actually a progression: the gradient should clamp to crimson on both ends. There is still one linear case where we don't do that (plus the radial line is also incorrect but unaffected by this change). This is great! Thanks reed(?)! LGTM.
Cool! What is the performance impact? When you say software only, do you mean it doesn't do anything in Ganesh mode?
On 2015/11/24 21:19:03, chrishtr wrote: > Cool! > > What is the performance impact? I don't recall the exact numbers (Mike?), but Skia has already switched internally and we haven't had any noticeable perf alerts AFAIK. We may see some minor regressions, for alpha gradients in particular. Hopefully nothing that would override the quality improvements. > When you say software only, do you mean it doesn't do anything in Ganesh mode? Right. My understanding is that a general/analytic impl in GLSL is next to impossible. People are thinking about it though, so there's some hope - at least for special cases.
On 2015/11/24 21:41:58, f(malita) wrote: > On 2015/11/24 21:19:03, chrishtr wrote: > > Cool! > > > > What is the performance impact? > > I don't recall the exact numbers (Mike?) "faster or the same on SIMD platforms, slower for non-SIMD"
On 2015/11/24 21:41:58, f(malita) wrote: > On 2015/11/24 21:19:03, chrishtr wrote: > > Cool! > > > > What is the performance impact? > > I don't recall the exact numbers (Mike?), but Skia has already switched > internally and we haven't had any noticeable perf alerts AFAIK. > > We may see some minor regressions, for alpha gradients in particular. Hopefully > nothing that would override the quality improvements. > > > > When you say software only, do you mean it doesn't do anything in Ganesh mode? > > Right. (this is mitigated by Ganesh's current color interpolation => the existing GPU impl doesn't exhibit banding artifacts - but it does have the hard stop positioning/blurriness problem)
The CQ bit was checked by chrishtr@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1457783005/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1457783005/140001
Message was sent while issue was closed.
Committed patchset #8 (id:140001)
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/32158b3da9a7a21c44698862fc5370c2ed735299 Cr-Commit-Position: refs/heads/master@{#361476}
Message was sent while issue was closed.
On 2015/11/24 at 21:57:34, fmalita wrote: > On 2015/11/24 21:41:58, f(malita) wrote: > > On 2015/11/24 21:19:03, chrishtr wrote: > > > Cool! > > > > > > What is the performance impact? > > > > I don't recall the exact numbers (Mike?), but Skia has already switched > > internally and we haven't had any noticeable perf alerts AFAIK. > > > > We may see some minor regressions, for alpha gradients in particular. Hopefully > > nothing that would override the quality improvements. > > > > > > > When you say software only, do you mean it doesn't do anything in Ganesh mode? > > > > Right. > > (this is mitigated by Ganesh's current color interpolation => the existing GPU impl doesn't exhibit banding artifacts - but it does have the hard stop positioning/blurriness problem) For my education, could you explain how the Ganesh color interpolation works?
Message was sent while issue was closed.
Description was changed from ========== Enable high-quality linear gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Start using Skia's analytical linear gradients (software-only). The new implementation addresses a couple of fundamental issues present in the old/LUT-based approach: * inexact (or blurred) hard color stops * banding artifacts BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,476548,486063,543625,skia:1077 R=reed@google.com,chrishtr@chromium.org Committed: https://crrev.com/32158b3da9a7a21c44698862fc5370c2ed735299 Cr-Commit-Position: refs/heads/master@{#361476} ========== to ========== Enable high-quality linear gradients (remove SK_SUPPORT_LEGACY_LINEAR_GRADIENT_TABLE) Start using Skia's analytical linear gradients (software-only). The new implementation addresses a couple of fundamental issues present in the old/LUT-based approach: * inexact (or blurred) hard color stops * banding artifacts BUG=558622,41756,98436,117140,135568,140208,152706,177293,180783,229561,233879,271552,317502,375630,403959,414254,419344,476548,486063,543625,skia:1077 R=reed@google.com,chrishtr@chromium.org Committed: https://crrev.com/32158b3da9a7a21c44698862fc5370c2ed735299 Cr-Commit-Position: refs/heads/master@{#361476} ==========
Message was sent while issue was closed.
On 2015/11/25 01:05:44, chrishtr wrote: > On 2015/11/24 at 21:57:34, fmalita wrote: > > On 2015/11/24 21:41:58, f(malita) wrote: > > > On 2015/11/24 21:19:03, chrishtr wrote: > > > > When you say software only, do you mean it doesn't do anything in Ganesh > mode? > > > > > > Right. > > > > (this is mitigated by Ganesh's current color interpolation => the existing GPU > impl doesn't exhibit banding artifacts - but it does have the hard stop > positioning/blurriness problem) > > For my education, could you explain how the Ganesh color interpolation works? Robert or Brian can explain this more articulately, since I'm not familiar with the actual implementation details. I think Ganesh starts off with the same 256x1 texture as the old raster impl, but applies some magic during resampling which smoothens color transitions (stretched portions are not limited to the discrete colors present in the input texture but are effectively interpolated).
Message was sent while issue was closed.
Right. Ganesh uses linear filtering when sampling the 1D gradient texture. This mitigates some of the artifacts the software path's point sampling was showing. Note that we do hope to have a Ganesh version that searches through the stops but we will only want to use it in cases that require it (e.g., two stops at the same point) since it will be slower. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
