Add new gradient to support HTML Canvas2D two-point-radial-gradient. Ours is

Add new gradient to support HTML Canvas2D two-point-radial-gradient. Ours is
called (for now at least) TwoPointConical, to distinguish it from the
existing one (TwoPointRadial), and to reflect its odd behavior where it draws
transparent outside the boundary of the "cone" between the two circles.

This impl is unsupported by GPU and Printing at the moment, and Chrome does not
yet invoke it.
Committed: https://code.google.com/p/skia/source/detail?r=4197

[TomH, 2012-06-06 18:51:29]

Was I too slow?

[TomH, 2012-06-06 19:05:12]

https://codereview.appspot.com/6299051/diff/1/gm/twopointradial.cpp
File gm/twopointradial.cpp (right):

https://codereview.appspot.com/6299051/diff/1/gm/twopointradial.cpp#newcode60
gm/twopointradial.cpp:60: return SkString("twopointconical");
Getting rid of the old GM? This new definition is weird enough  - can we verify
that we're really going to match other browsers, or compare against the HTML/CSS
compliance tests?

https://codereview.appspot.com/6299051/diff/1/src/effects/SkGradientShader.cpp
File src/effects/SkGradientShader.cpp (right):

https://codereview.appspot.com/6299051/diff/1/src/effects/SkGradientShader.cp...
src/effects/SkGradientShader.cpp:2131: return (int)(r - roots);
Do we really need to do this with pointer math? Or could we use an index?
int r = 0; r += valid_divide(...); if r == 2 ... else ...; return r;

https://codereview.appspot.com/6299051/diff/1/src/effects/SkGradientShader.cp...
src/effects/SkGradientShader.cpp:2224: *dstC++ = 0;
So for the whole "2pt radial gradients don't effect the rest of the world" spec,
we're counting on a blend mode post-hoc?

[reed1, 2012-06-06 19:16:57]

https://codereview.appspot.com/6299051/diff/1/gm/twopointradial.cpp
File gm/twopointradial.cpp (right):

https://codereview.appspot.com/6299051/diff/1/gm/twopointradial.cpp#newcode60
gm/twopointradial.cpp:60: return SkString("twopointconical");
There were no images checked in before. The rename is just to reflect the new
name of the feature (SkGradientShader.h)

We do match Safari now (at least for the examples in the GM)

On 2012/06/06 19:05:12, TomH wrote:
> Getting rid of the old GM? This new definition is weird enough  - can we
verify
> that we're really going to match other browsers, or compare against the
HTML/CSS
> compliance tests?

https://codereview.appspot.com/6299051/diff/1/src/effects/SkGradientShader.cpp
File src/effects/SkGradientShader.cpp (right):

https://codereview.appspot.com/6299051/diff/1/src/effects/SkGradientShader.cp...
src/effects/SkGradientShader.cpp:2131: return (int)(r - roots);
This code was extracted (and modified) from SkGeometry.cpp, which had this ptr
technique. As written, this is easier to compare against that version, unless we
rewrite both to use indices.

On 2012/06/06 19:05:12, TomH wrote:
> Do we really need to do this with pointer math? Or could we use an index?
> int r = 0; r += valid_divide(...); if r == 2 ... else ...; return r;

https://codereview.appspot.com/6299051/diff/1/src/effects/SkGradientShader.cp...
src/effects/SkGradientShader.cpp:2224: *dstC++ = 0;
The spec said return transparent-black.

On 2012/06/06 19:05:12, TomH wrote:
> So for the whole "2pt radial gradients don't effect the rest of the world"
spec,
> we're counting on a blend mode post-hoc?

[TomH, 2012-06-06 19:36:58]

https://codereview.appspot.com/6299051/diff/1/src/effects/SkGradientShader.cpp
File src/effects/SkGradientShader.cpp (right):

https://codereview.appspot.com/6299051/diff/1/src/effects/SkGradientShader.cp...
src/effects/SkGradientShader.cpp:2131: return (int)(r - roots);
On 2012/06/06 19:16:57, reed1 wrote:
> This code was extracted (and modified) from SkGeometry.cpp, which had this ptr
> technique. As written, this is easier to compare against that version, unless
we
> rewrite both to use indices.

Sounds to me like you're volunteering to fix SkGeometry, too?

[bsalomon, 2012-06-06 19:44:32]

On 2012/06/06 19:36:58, TomH wrote:
> https://codereview.appspot.com/6299051/diff/1/src/effects/SkGradientShader.cpp
> File src/effects/SkGradientShader.cpp (right):
> 
>
https://codereview.appspot.com/6299051/diff/1/src/effects/SkGradientShader.cp...
> src/effects/SkGradientShader.cpp:2131: return (int)(r - roots);
> On 2012/06/06 19:16:57, reed1 wrote:
> > This code was extracted (and modified) from SkGeometry.cpp, which had this
ptr
> > technique. As written, this is easier to compare against that version,
unless
> we
> > rewrite both to use indices.
> 
> Sounds to me like you're volunteering to fix SkGeometry, too?

I don't find the pointer math offensive. I'll leave that to you guys to fight
out. Otherwise, LGTM.

[reed1, 2012-06-06 19:44:41]

I find the current form readable. If you want it changed, I think you could
propose a CL that changes both instances, since that sort of change I view as
orthogonal to this CL.

[TomH, 2012-06-06 19:45:38]

OK, OK, LGTM.

I'll think about an orthogonal CL, but if I want to go windmill tilting I'll
probably try to bring SkNEW to Ganesh first.

[bsalomon, 2012-06-06 19:51:13]

On 2012/06/06 19:45:38, TomH wrote:
> OK, OK, LGTM.
> 
> I'll think about an orthogonal CL, but if I want to go windmill tilting I'll
> probably try to bring SkNEW to Ganesh first.

SkWHATEVER