Added tests for radial gradients with shader matrices

Added tests for radial gradients with shader matrices


Fixed a bug with linear gradient PDF matrices and added test cases

Committed: http://code.google.com/p/skia/source/detail?r=9554

[vandebo (ex-Chrome), 2013-06-11 05:15:25]

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp
File gm/gradient_matrix.cpp (right):

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:17: static SkShader* SimpleLinearGradient(const SkPoint
pts[2]) {
MakeLinearGradient

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:22: static SkShader* SimpleRadialGradient(const SkPoint
pts[2], const GradData& data,
MakeRadialGRadient

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:23: SkShader::TileMode tm, SkUnitMapper* mapper) {
line up previous line (first const).

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:29: gPosAll, 2, SkShader::kClamp_TileMode, NULL);
nit: gPosAll, 2, on previous line.

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:47: virtual void onDraw(SkCanvas* canvas) {
This method is rather repetitive.  Can you pack the test values into an array
and use or a loop or something along those lines?

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:118: virtual void drawRadialGradient(SkCanvas* canvas,
float x1, float y1, float x2, float y2) {
This method is basically a duplicate of the next one.  Pass in SkShader instead
of pts.

[reed1, 2013-06-11 13:54:16]

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp
File gm/gradient_matrix.cpp (right):

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:28: return SkGradientShader::CreateRadial(center, radius,
gColors,
Another common way to pass these...

..., gColors, NULL, SK_ARRAY_COUNT(gColors), ...

If you're going to pass [ 0, 1 ] for pos, its common to pass null instead. NULL
means "evenly spread out the positions of all the colors".

SK_ARRAY_COUNT macro ensures that if you later add colors, the count will always
be correct.

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:45: virtual SkISize onISize() { return make_isize(800,
800); }
make_isize() is a hold-over from before we had good Make methods. Try

SkISize::Make()

[ducky, 2013-06-11 19:51:01]

Also addresses comments in https://codereview.chromium.org/16094020/

How do I update the previous issue above? Is there a way to merge that one into
this one?

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp
File gm/gradient_matrix.cpp (right):

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:17: static SkShader* SimpleLinearGradient(const SkPoint
pts[2]) {
On 2013/06/11 05:15:25, vandebo wrote:
> MakeLinearGradient

Done.

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:22: static SkShader* SimpleRadialGradient(const SkPoint
pts[2], const GradData& data,
On 2013/06/11 05:15:25, vandebo wrote:
> MakeRadialGRadient

Done.

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:23: SkShader::TileMode tm, SkUnitMapper* mapper) {
On 2013/06/11 05:15:25, vandebo wrote:
> line up previous line (first const).

Done.

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:28: return SkGradientShader::CreateRadial(center, radius,
gColors,
On 2013/06/11 13:54:16, reed1 wrote:
> Another common way to pass these...
> 
> ..., gColors, NULL, SK_ARRAY_COUNT(gColors), ...
> 
> If you're going to pass [ 0, 1 ] for pos, its common to pass null instead.
NULL
> means "evenly spread out the positions of all the colors".
> 
> SK_ARRAY_COUNT macro ensures that if you later add colors, the count will
always
> be correct.

Done.

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:29: gPosAll, 2, SkShader::kClamp_TileMode, NULL);
On 2013/06/11 05:15:25, vandebo wrote:
> nit: gPosAll, 2, on previous line.

Done.

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:45: virtual SkISize onISize() { return make_isize(800,
800); }
On 2013/06/11 13:54:16, reed1 wrote:
> make_isize() is a hold-over from before we had good Make methods. Try
> 
> SkISize::Make()

Done.

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:47: virtual void onDraw(SkCanvas* canvas) {
On 2013/06/11 05:15:25, vandebo wrote:
> This method is rather repetitive.  Can you pack the test values into an array
> and use or a loop or something along those lines?

Done.

https://codereview.chromium.org/16467002/diff/2001/gm/gradient_matrix.cpp#new...
gm/gradient_matrix.cpp:118: virtual void drawRadialGradient(SkCanvas* canvas,
float x1, float y1, float x2, float y2) {
On 2013/06/11 05:15:25, vandebo wrote:
> This method is basically a duplicate of the next one.  Pass in SkShader
instead
> of pts.

Done.

[vandebo (ex-Chrome), 2013-06-11 22:12:00]

Rebase this CL so it only shows the radial gradient changes.

[ducky, 2013-06-11 23:21:20]

Rebased to track only changes after gradient matrix fix.

[vandebo (ex-Chrome), 2013-06-12 16:28:02]

https://codereview.chromium.org/16467002/diff/19001/gm/gradient_matrix.cpp
File gm/gradient_matrix.cpp (left):

https://codereview.chromium.org/16467002/diff/19001/gm/gradient_matrix.cpp#ol...
gm/gradient_matrix.cpp:49: canvas->save();
These can stay here.

[ducky, 2013-06-12 18:24:06]

Fixed code review comments.

https://codereview.chromium.org/16467002/diff/19001/gm/gradient_matrix.cpp
File gm/gradient_matrix.cpp (left):

https://codereview.chromium.org/16467002/diff/19001/gm/gradient_matrix.cpp#ol...
gm/gradient_matrix.cpp:49: canvas->save();
On 2013/06/12 16:28:03, vandebo wrote:
> These can stay here.

Done.

[ducky, 2013-06-12 19:02:32]

Generates shader matrix from rectangle.

[vandebo (ex-Chrome), 2013-06-12 20:02:32]

LGTM

[commit-bot: I haz the power, 2013-06-12 21:37:37]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/richardlin@chromium.org/16467002/33001

[commit-bot: I haz the power, 2013-06-12 21:47:41]

Change committed as 9554

[ducky, 2013-06-12 22:32:05]

Attempt to fix Mac10.6 builds by doing explicit SkIntToScalar and
SkFloatToScalar