Initial conic-gradient() implementation

Initial conic-gradient() implementation

Introduce a new Gradient specialization (ConicGradient), and wire it to Skia's
SkSweepGradient.

Only non-repeating conic gradients are supported for now.

Tests based on Lea Verou's excellent https://leaverou.github.io/conic-gradient/.

BUG=706973
Review-Url: https://codereview.chromium.org/2787113002
Cr-Commit-Position: refs/heads/master@{#461213}
Committed: https://chromium.googlesource.com/chromium/src/+/d25c21ff39a3dfc462ac0bbbb402f8e314acf576

[f(malita), 2017-03-30 19:51:15]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 19:52:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[f(malita), 2017-03-30 19:58:23]

Description was changed from

==========
Initial conic-gradient() implementation

WIP

BUG=
==========

to

==========
Initial conic-gradient() implementation

Introduce a new Gradient specialization (ConicGradient), and wire it to Skia's
SkSweepGradient.

Only non-repeating conic gradients are supported for now.

BUG=
==========

[f(malita), 2017-03-30 20:11:08]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 20:12:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[f(malita), 2017-03-30 20:12:53]

Description was changed from

==========
Initial conic-gradient() implementation

Introduce a new Gradient specialization (ConicGradient), and wire it to Skia's
SkSweepGradient.

Only non-repeating conic gradients are supported for now.

BUG=
==========

to

==========
Initial conic-gradient() implementation

Introduce a new Gradient specialization (ConicGradient), and wire it to Skia's
SkSweepGradient.

Only non-repeating conic gradients are supported for now.

BUG=706973
==========

[f(malita), 2017-03-30 20:16:18]

Description was changed from

==========
Initial conic-gradient() implementation

Introduce a new Gradient specialization (ConicGradient), and wire it to Skia's
SkSweepGradient.

Only non-repeating conic gradients are supported for now.

BUG=706973
==========

to

==========
Initial conic-gradient() implementation

Introduce a new Gradient specialization (ConicGradient), and wire it to Skia's
SkSweepGradient.

Only non-repeating conic gradients are supported for now.

Tests based on Lea Verou's excellent https://leaverou.github.io/conic-gradient/.

BUG=706973
==========

[f(malita), 2017-03-30 20:27:15]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 20:27:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[f(malita), 2017-03-30 20:32:59]

fmalita@chromium.org changed reviewers:
+ fs@opera.com, schenney@chromium.org

[f(malita), 2017-03-30 20:33:00]

Depends on https://codereview.chromium.org/2785203002/, and still needs
non-linux baselines.  But other than that, I think it's good to go.

[fs, 2017-03-30 22:09:11]

LGTM w/ a (trivial) issue and some nits.

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSGradientValue.cpp (right):

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.cpp:461: bool hasHints =
false;
Nit: Maybe move this down to just before the for() while we're at it.

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.cpp:464: switch
(getClassType()) {
(The old code here had me scratch my head...)

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.cpp:603: }
Strange indent (here and next line)? (Or late hour?)

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.cpp:1399: RefPtr<Gradient>
gradient = Gradient::createConic(position, angle);
position, angle, ..., Gradient::ColorInterpolation::Premultiplied

right? Whether ... is desc.spreadMethod or not I guess doesn't matter much for
now, but maybe best to use that.

(Maybe this also means we should have a test for stops with alpha? or
'transparent' stops)

[commit-bot: I haz the power, 2017-03-30 22:43:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-30 22:43:37]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[f(malita), 2017-03-31 13:54:00]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 13:54:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[f(malita), 2017-03-31 13:56:23]

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSGradientValue.cpp (right):

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.cpp:461: bool hasHints =
false;
On 2017/03/30 22:09:11, fs wrote:
> Nit: Maybe move this down to just before the for() while we're at it.

Done.

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.cpp:603: }
On 2017/03/30 22:09:11, fs wrote:
> Strange indent (here and next line)? (Or late hour?)

Done.

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.cpp:1399: RefPtr<Gradient>
gradient = Gradient::createConic(position, angle);
On 2017/03/30 22:09:11, fs wrote:
> position, angle, ..., Gradient::ColorInterpolation::Premultiplied
> 
> right? Whether ... is desc.spreadMethod or not I guess doesn't matter much for
> now, but maybe best to use that.

Good catch, done.

> 
> (Maybe this also means we should have a test for stops with alpha? or
> 'transparent' stops)

The last conic-gradient.html example uses some translucent colors and does catch
this diff.

[fs, 2017-03-31 14:05:43]

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSGradientValue.cpp (right):

https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.cpp:1399: RefPtr<Gradient>
gradient = Gradient::createConic(position, angle);
On 2017/03/31 at 13:56:23, f(malita) wrote:
> On 2017/03/30 22:09:11, fs wrote:
> > position, angle, ..., Gradient::ColorInterpolation::Premultiplied
> > 
> > right? Whether ... is desc.spreadMethod or not I guess doesn't matter much
for
> > now, but maybe best to use that.
> 
> Good catch, done.
> 
> > 
> > (Maybe this also means we should have a test for stops with alpha? or
> > 'transparent' stops)
> 
> The last conic-gradient.html example uses some translucent colors and does
catch this diff.

Yeah, I saw that... it's very subtle though. So maybe a more "plain" test would
be good to have - like "conic-gradient(blue, transparent)", where the effect
ought to be fairly noticable.

[f(malita), 2017-03-31 14:34:25]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 14:34:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[f(malita), 2017-03-31 14:36:10]

On 2017/03/31 14:05:43, fs wrote:
>
https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/css/CSSGradientValue.cpp (right):
> 
>
https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/CSSGradientValue.cpp:1399: RefPtr<Gradient>
> gradient = Gradient::createConic(position, angle);
> On 2017/03/31 at 13:56:23, f(malita) wrote:
> > On 2017/03/30 22:09:11, fs wrote:
> > > position, angle, ..., Gradient::ColorInterpolation::Premultiplied
> > > 
> > > right? Whether ... is desc.spreadMethod or not I guess doesn't matter much
> for
> > > now, but maybe best to use that.
> > 
> > Good catch, done.
> > 
> > > 
> > > (Maybe this also means we should have a test for stops with alpha? or
> > > 'transparent' stops)
> > 
> > The last conic-gradient.html example uses some translucent colors and does
> catch this diff.
> 
> Yeah, I saw that... it's very subtle though. So maybe a more "plain" test
would
> be good to have - like "conic-gradient(blue, transparent)", where the effect
> ought to be fairly noticable.

Ack, updated.  I went with transparent/yellow, 'cause I think it shows the gray
unpremul anomaly even better.

[f(malita), 2017-03-31 14:39:00]

On 2017/03/31 14:36:10, f(malita) wrote:
> On 2017/03/31 14:05:43, fs wrote:
> >
>
https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/css/CSSGradientValue.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/css/CSSGradientValue.cpp:1399:
RefPtr<Gradient>
> > gradient = Gradient::createConic(position, angle);
> > On 2017/03/31 at 13:56:23, f(malita) wrote:
> > > On 2017/03/30 22:09:11, fs wrote:
> > > > position, angle, ..., Gradient::ColorInterpolation::Premultiplied
> > > > 
> > > > right? Whether ... is desc.spreadMethod or not I guess doesn't matter
much
> > for
> > > > now, but maybe best to use that.
> > > 
> > > Good catch, done.
> > > 
> > > > 
> > > > (Maybe this also means we should have a test for stops with alpha? or
> > > > 'transparent' stops)
> > > 
> > > The last conic-gradient.html example uses some translucent colors and does
> > catch this diff.
> > 
> > Yeah, I saw that... it's very subtle though. So maybe a more "plain" test
> would
> > be good to have - like "conic-gradient(blue, transparent)", where the effect
> > ought to be fairly noticable.
> 
> Ack, updated.  I went with transparent/yellow, 'cause I think it shows the
gray
> unpremul anomaly even better.

(btw, if you're wondering why the hard/axis-aligned edges are not smooth, so am
I - https://bugs.chromium.org/p/skia/issues/detail?id=6452)

[commit-bot: I haz the power, 2017-03-31 15:45:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 15:45:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fs, 2017-03-31 18:34:30]

On 2017/03/31 at 14:39:00, fmalita wrote:
> On 2017/03/31 14:36:10, f(malita) wrote:
> > On 2017/03/31 14:05:43, fs wrote:
> > >
> >
https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/core/css/CSSGradientValue.cpp (right):
> > > 
> > >
> >
https://codereview.chromium.org/2787113002/diff/40001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/core/css/CSSGradientValue.cpp:1399:
RefPtr<Gradient>
> > > gradient = Gradient::createConic(position, angle);
> > > On 2017/03/31 at 13:56:23, f(malita) wrote:
> > > > On 2017/03/30 22:09:11, fs wrote:
> > > > > position, angle, ..., Gradient::ColorInterpolation::Premultiplied
> > > > > 
> > > > > right? Whether ... is desc.spreadMethod or not I guess doesn't matter
much
> > > for
> > > > > now, but maybe best to use that.
> > > > 
> > > > Good catch, done.
> > > > 
> > > > > 
> > > > > (Maybe this also means we should have a test for stops with alpha? or
> > > > > 'transparent' stops)
> > > > 
> > > > The last conic-gradient.html example uses some translucent colors and
does
> > > catch this diff.
> > > 
> > > Yeah, I saw that... it's very subtle though. So maybe a more "plain" test
> > would
> > > be good to have - like "conic-gradient(blue, transparent)", where the
effect
> > > ought to be fairly noticable.
> > 
> > Ack, updated.  I went with transparent/yellow, 'cause I think it shows the
gray
> > unpremul anomaly even better.
> 
> (btw, if you're wondering why the hard/axis-aligned edges are not smooth, so
am I - https://bugs.chromium.org/p/skia/issues/detail?id=6452)

Right, I was wondering, but figured it was a Skia issue =)

[f(malita), 2017-03-31 19:07:20]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 19:07:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 20:31:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 20:31:57]

Dry run: This issue passed the CQ dry run.

[f(malita), 2017-03-31 20:32:15]

The CQ bit was checked by fmalita@chromium.org

[f(malita), 2017-03-31 20:32:16]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/2787113002/#ps100001
(title: "baselines")

[commit-bot: I haz the power, 2017-03-31 20:33:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 20:40:57]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1490992335963710,
"parent_rev": "31a445667f4fddb5c78a464a4de52f000777a74a", "commit_rev":
"d25c21ff39a3dfc462ac0bbbb402f8e314acf576"}

[commit-bot: I haz the power, 2017-03-31 20:42:22]

Description was changed from

==========
Initial conic-gradient() implementation

Introduce a new Gradient specialization (ConicGradient), and wire it to Skia's
SkSweepGradient.

Only non-repeating conic gradients are supported for now.

Tests based on Lea Verou's excellent https://leaverou.github.io/conic-gradient/.

BUG=706973
==========

to

==========
Initial conic-gradient() implementation

Introduce a new Gradient specialization (ConicGradient), and wire it to Skia's
SkSweepGradient.

Only non-repeating conic gradients are supported for now.

Tests based on Lea Verou's excellent https://leaverou.github.io/conic-gradient/.

BUG=706973

Review-Url: https://codereview.chromium.org/2787113002
Cr-Commit-Position: refs/heads/master@{#461213}
Committed:
https://chromium.googlesource.com/chromium/src/+/d25c21ff39a3dfc462ac0bbbb402...
==========

[commit-bot: I haz the power, 2017-03-31 20:42:24]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/d25c21ff39a3dfc462ac0bbbb402...