Add alternative ambient shadow method to Android shadow sample

Add alternative ambient shadow method to Android shadow sample

TBR=bsalomon@google.com
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2249973003

Committed: https://skia.googlesource.com/skia/+/6c177a1a49fcfe8bfd5f3ffda3ee50bbe2679463

[jvanverth1, 2016-08-16 16:52:05]

Description was changed from

==========
Add alternative ambient shadow method to Android shadow sample

BUG=skia:
==========

to

==========
Add alternative ambient shadow method to Android shadow sample

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2249973003
==========

[jvanverth1, 2016-08-16 17:16:54]

jvanverth@google.com changed reviewers:
+ bsalomon@google.com, djsollen@google.com, robertphillips@google.com

[robertphillips, 2016-08-16 17:56:07]

https://codereview.chromium.org/2249973003/diff/60001/include/effects/SkGauss...
File include/effects/SkGaussianEdgeShader.h (right):

https://codereview.chromium.org/2249973003/diff/60001/include/effects/SkGauss...
include/effects/SkGaussianEdgeShader.h:15: /** Returns a shader that applies a
Gaussian blur depending on distance to the edge
* Currently this is only useable by XXX with the GPU backend ?

https://codereview.chromium.org/2249973003/diff/60001/samplecode/SampleAndroi...
File samplecode/SampleAndroidShadows.cpp (right):

https://codereview.chromium.org/2249973003/diff/60001/samplecode/SampleAndroi...
samplecode/SampleAndroidShadows.cpp:172: sk_sp<SkShader> gaussShader =
SkGaussianEdgeShader::Make();
std::move ?

https://codereview.chromium.org/2249973003/diff/60001/samplecode/SampleAndroi...
samplecode/SampleAndroidShadows.cpp:268: if (fShowAmbient) {
Do we want to toggle between the two options ?

https://codereview.chromium.org/2249973003/diff/60001/src/effects/SkGaussianE...
File src/effects/SkGaussianEdgeShader.cpp (right):

https://codereview.chromium.org/2249973003/diff/60001/src/effects/SkGaussianE...
src/effects/SkGaussianEdgeShader.cpp:16: public:
Ditch comment ?

https://codereview.chromium.org/2249973003/diff/60001/src/effects/SkGaussianE...
src/effects/SkGaussianEdgeShader.cpp:59: 
delete ?

https://codereview.chromium.org/2249973003/diff/60001/src/effects/SkGaussianE...
src/effects/SkGaussianEdgeShader.cpp:88: GrProcessorKeyBuilder* b) {
kill dead code ?

https://codereview.chromium.org/2249973003/diff/60001/src/effects/SkGaussianE...
src/effects/SkGaussianEdgeShader.cpp:121: 
Does raster crash ?

https://codereview.chromium.org/2249973003/diff/60001/src/gpu/GrOvalRenderer.cpp
File src/gpu/GrOvalRenderer.cpp (right):

https://codereview.chromium.org/2249973003/diff/60001/src/gpu/GrOvalRenderer....
src/gpu/GrOvalRenderer.cpp:71: *             innerRad is the innerRadius in
normalized space (ignored if not stroking).
Should we be documenting the distance vector output here ?

https://codereview.chromium.org/2249973003/diff/60001/src/gpu/glsl/GrGLSLProg...
File src/gpu/glsl/GrGLSLProgramBuilder.cpp (right):

https://codereview.chromium.org/2249973003/diff/60001/src/gpu/glsl/GrGLSLProg...
src/gpu/glsl/GrGLSLProgramBuilder.cpp:94: fFS.codeAppend( "// Distance to the
edge encoded in the z-component\n");
Does any one else needs to be updated for this change?

[jvanverth1, 2016-08-16 19:14:11]

https://codereview.chromium.org/2249973003/diff/60001/include/effects/SkGauss...
File include/effects/SkGaussianEdgeShader.h (right):

https://codereview.chromium.org/2249973003/diff/60001/include/effects/SkGauss...
include/effects/SkGaussianEdgeShader.h:15: /** Returns a shader that applies a
Gaussian blur depending on distance to the edge
On 2016/08/16 17:56:07, robertphillips wrote:
> * Currently this is only useable by XXX with the GPU backend ?

Done.

https://codereview.chromium.org/2249973003/diff/60001/samplecode/SampleAndroi...
File samplecode/SampleAndroidShadows.cpp (right):

https://codereview.chromium.org/2249973003/diff/60001/samplecode/SampleAndroi...
samplecode/SampleAndroidShadows.cpp:172: sk_sp<SkShader> gaussShader =
SkGaussianEdgeShader::Make();
On 2016/08/16 17:56:07, robertphillips wrote:
> std::move ?

Done.

https://codereview.chromium.org/2249973003/diff/60001/samplecode/SampleAndroi...
samplecode/SampleAndroidShadows.cpp:268: if (fShowAmbient) {
On 2016/08/16 17:56:07, robertphillips wrote:
> Do we want to toggle between the two options ?

Done.

https://codereview.chromium.org/2249973003/diff/60001/src/effects/SkGaussianE...
File src/effects/SkGaussianEdgeShader.cpp (right):

https://codereview.chromium.org/2249973003/diff/60001/src/effects/SkGaussianE...
src/effects/SkGaussianEdgeShader.cpp:16: public:
On 2016/08/16 17:56:07, robertphillips wrote:
> Ditch comment ?

Done.

https://codereview.chromium.org/2249973003/diff/60001/src/effects/SkGaussianE...
src/effects/SkGaussianEdgeShader.cpp:59: 
On 2016/08/16 17:56:07, robertphillips wrote:
> delete ?

No, it's needed.

https://codereview.chromium.org/2249973003/diff/60001/src/effects/SkGaussianE...
src/effects/SkGaussianEdgeShader.cpp:88: GrProcessorKeyBuilder* b) {
On 2016/08/16 17:56:07, robertphillips wrote:
> kill dead code ?

Done.

https://codereview.chromium.org/2249973003/diff/60001/src/effects/SkGaussianE...
src/effects/SkGaussianEdgeShader.cpp:121: 
On 2016/08/16 17:56:07, robertphillips wrote:
> Does raster crash ?

Nope, just draws nothing. Added to comment.

[jvanverth1, 2016-08-16 19:24:54]

Missed a couple:

https://codereview.chromium.org/2249973003/diff/60001/src/gpu/GrOvalRenderer.cpp
File src/gpu/GrOvalRenderer.cpp (right):

https://codereview.chromium.org/2249973003/diff/60001/src/gpu/GrOvalRenderer....
src/gpu/GrOvalRenderer.cpp:71: *             innerRad is the innerRadius in
normalized space (ignored if not stroking).
On 2016/08/16 17:56:07, robertphillips wrote:
> Should we be documenting the distance vector output here ?

Done.

https://codereview.chromium.org/2249973003/diff/60001/src/gpu/glsl/GrGLSLProg...
File src/gpu/glsl/GrGLSLProgramBuilder.cpp (right):

https://codereview.chromium.org/2249973003/diff/60001/src/gpu/glsl/GrGLSLProg...
src/gpu/glsl/GrGLSLProgramBuilder.cpp:94: fFS.codeAppend( "// Distance to the
edge encoded in the z-component\n");
On 2016/08/16 17:56:07, robertphillips wrote:
> Does any one else needs to be updated for this change?

No, this is the only place.

[jvanverth1, 2016-08-16 20:15:01]

Fixed up the other distance vector uses. PTAL.

[robertphillips, 2016-08-16 21:25:42]

https://codereview.chromium.org/2249973003/diff/120001/samplecode/SampleAndro...
File samplecode/SampleAndroidShadows.cpp (right):

https://codereview.chromium.org/2249973003/diff/120001/samplecode/SampleAndro...
samplecode/SampleAndroidShadows.cpp:176: 
do we need std::move here ?

Can't we just say "paint.setShader(SkGaussianEdgeshader::Make());" ?

https://codereview.chromium.org/2249973003/diff/120001/src/effects/SkGaussian...
File src/effects/SkGaussianEdgeShader.cpp (right):

https://codereview.chromium.org/2249973003/diff/120001/src/effects/SkGaussian...
src/effects/SkGaussianEdgeShader.cpp:25: SK_TO_STRING_OVERRIDE()
Is this indent correct/meaningful ?

[robertphillips, 2016-08-16 21:30:54]

lgtm

[jvanverth1, 2016-08-16 21:31:20]

https://codereview.chromium.org/2249973003/diff/120001/samplecode/SampleAndro...
File samplecode/SampleAndroidShadows.cpp (right):

https://codereview.chromium.org/2249973003/diff/120001/samplecode/SampleAndro...
samplecode/SampleAndroidShadows.cpp:176: 
On 2016/08/16 21:25:42, robertphillips wrote:
> do we need std::move here ?
> 
> Can't we just say "paint.setShader(SkGaussianEdgeshader::Make());" ?

Done.

https://codereview.chromium.org/2249973003/diff/120001/src/effects/SkGaussian...
File src/effects/SkGaussianEdgeShader.cpp (right):

https://codereview.chromium.org/2249973003/diff/120001/src/effects/SkGaussian...
src/effects/SkGaussianEdgeShader.cpp:25: SK_TO_STRING_OVERRIDE()
On 2016/08/16 21:25:42, robertphillips wrote:
> Is this indent correct/meaningful ?

Done.

[jvanverth1, 2016-08-16 21:31:26]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2016-08-16 21:31:35]

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, 2016-08-16 21:33:05]

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

[commit-bot: I haz the power, 2016-08-16 21:33:06]

Try jobs failed on following builders:
  skia_presubmit-Trybot on master.client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[jvanverth1, 2016-08-17 13:26:32]

On 2016/08/16 21:33:06, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   skia_presubmit-Trybot on master.client.skia.fyi (JOB_FAILED,
>
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

Brian, can you take a look, please?

[robertphillips, 2016-08-17 14:58:27]

Description was changed from

==========
Add alternative ambient shadow method to Android shadow sample

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2249973003
==========

to

==========
Add alternative ambient shadow method to Android shadow sample

TBR=bsalomon@google.com
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2249973003
==========

[robertphillips, 2016-08-17 14:58:34]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2016-08-17 14:58:43]

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, 2016-08-17 14:59:43]

Description was changed from

==========
Add alternative ambient shadow method to Android shadow sample

TBR=bsalomon@google.com
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2249973003
==========

to

==========
Add alternative ambient shadow method to Android shadow sample

TBR=bsalomon@google.com
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2249973003

Committed:
https://skia.googlesource.com/skia/+/6c177a1a49fcfe8bfd5f3ffda3ee50bbe2679463
==========

[commit-bot: I haz the power, 2016-08-17 14:59:44]

Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/6c177a1a49fcfe8bfd5f3ffda3ee50bbe2679463