Enabling Perlin Noise on Android

*** Perlin noise GM needs to be rebaselined ***

Enabling Perlin Noise on Android

I enabled the Perlin Noise shader on Android after doing some minor modifications to the shader, specifically for Android (and #ifdefed for Android, to make sure none of this affects other platforms).

For Tegra devices (Nexus 7, Xoom), a precision issue related to the color values read from textures caused the noise to read the wrong indices and produce bad noise. I fixed this by adding a founding of the values read by simply doing the equivalent of "colorValue = floor(colorValue * 255.0) / 255.0" to make sure we retrieve the colors that were written in the texture originally.

For non-Tegra devices (Nexus 10), dealing with values in the order of 4096.0 was problematic without using the "highp" precision setting. To solve this, a few variables were given the high precision setting.

Since both fixes don't seem to do considerable harm to the platforms that are not being targetted, I left both fixes on all android devices for now.

I also reduced the Perlin noise gm so that it takes less time to test it on the Xoom (Original time was about 20 seconds, this shold take less than 10, hopefully)

BUG=

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

[sugoi, 2013-06-13 15:34:15]

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
File src/effects/SkPerlinNoiseShader.cpp (right):

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
src/effects/SkPerlinNoiseShader.cpp:1079: noiseCode.appendf("\n\t%s = floor(%s *
vec2(255.0) + vec2(0.5)) * vec2(0.003921569);",
1 / 255 = 0.003921569

[Stephen White, 2013-06-13 16:05:15]

Shader LGTM with comment fix; will leave for Brian for the GrGLShaderVar change.

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
File src/effects/SkPerlinNoiseShader.cpp (right):

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
src/effects/SkPerlinNoiseShader.cpp:1074: noiseCode.append(".r;");
As mentioned, If you want to move to Ganesh's single-channel texture support at
some point, you'll need to change this swizzle to a function call, rather than
hard-coded. NBD for now, though.

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
src/effects/SkPerlinNoiseShader.cpp:1079: noiseCode.appendf("\n\t%s = floor(%s *
vec2(255.0) + vec2(0.5)) * vec2(0.003921569);",
On 2013/06/13 15:34:15, sugoi wrote:
> 1 / 255 = 0.003921569

This ^^ should probably be in a comment in the code. Also, a clearer explanation
of the Tegra problem would be helpful.

[bsalomon, 2013-06-13 16:11:30]

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
File src/effects/SkPerlinNoiseShader.cpp (right):

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
src/effects/SkPerlinNoiseShader.cpp:1012: #if defined(SK_BUILD_FOR_ANDROID)
Do we really need this to be an android-only flag?  Can we do it all the time,
or if on ES2?

[sugoi, 2013-06-13 16:57:13]

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
File src/effects/SkPerlinNoiseShader.cpp (right):

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
src/effects/SkPerlinNoiseShader.cpp:1012: #if defined(SK_BUILD_FOR_ANDROID)
On 2013/06/13 16:11:31, bsalomon wrote:
> Do we really need this to be an android-only flag?  Can we do it all the time,
or if on ES2?

I can remove it if you want. Note that GrGLShaderVar::PrecisionString() will
only return a non empty precision string if the binding is ES2. Is Angle also
using ES2 binding ? If so, I'll have to double check that Angle handles these
keywords well.

[bsalomon, 2013-06-13 17:46:13]

On 2013/06/13 16:57:13, sugoi wrote:
>
https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
> File src/effects/SkPerlinNoiseShader.cpp (right):
> 
>
https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
> src/effects/SkPerlinNoiseShader.cpp:1012: #if defined(SK_BUILD_FOR_ANDROID)
> On 2013/06/13 16:11:31, bsalomon wrote:
> > Do we really need this to be an android-only flag?  Can we do it all the
time,
> or if on ES2?
> 
> I can remove it if you want. Note that GrGLShaderVar::PrecisionString() will
> only return a non empty precision string if the binding is ES2. Is Angle also
> using ES2 binding ? If so, I'll have to double check that Angle handles these
> keywords well.

Let's just enable it unconditionally. ANGLE is ES2, but that should already be
well tested.

[sugoi, 2013-06-13 18:04:42]

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
File src/effects/SkPerlinNoiseShader.cpp (right):

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
src/effects/SkPerlinNoiseShader.cpp:1012: #if defined(SK_BUILD_FOR_ANDROID)
On 2013/06/13 16:57:13, sugoi wrote:
> On 2013/06/13 16:11:31, bsalomon wrote:
> > Do we really need this to be an android-only flag?  Can we do it all the
time,
> or if on ES2?
> 
> I can remove it if you want. Note that GrGLShaderVar::PrecisionString() will
> only return a non empty precision string if the binding is ES2. Is Angle also
> using ES2 binding ? If so, I'll have to double check that Angle handles these
> keywords well.

Done.

https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
src/effects/SkPerlinNoiseShader.cpp:1079: noiseCode.appendf("\n\t%s = floor(%s *
vec2(255.0) + vec2(0.5)) * vec2(0.003921569);",
On 2013/06/13 16:05:15, Stephen White wrote:
> On 2013/06/13 15:34:15, sugoi wrote:
> > 1 / 255 = 0.003921569
> 
> This ^^ should probably be in a comment in the code. Also, a clearer
explanation
> of the Tegra problem would be helpful.

Done.

[bsalomon, 2013-06-13 19:21:19]

On 2013/06/13 18:04:42, sugoi wrote:
>
https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
> File src/effects/SkPerlinNoiseShader.cpp (right):
> 
>
https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
> src/effects/SkPerlinNoiseShader.cpp:1012: #if defined(SK_BUILD_FOR_ANDROID)
> On 2013/06/13 16:57:13, sugoi wrote:
> > On 2013/06/13 16:11:31, bsalomon wrote:
> > > Do we really need this to be an android-only flag?  Can we do it all the
> time,
> > or if on ES2?
> > 
> > I can remove it if you want. Note that GrGLShaderVar::PrecisionString() will
> > only return a non empty precision string if the binding is ES2. Is Angle
also
> > using ES2 binding ? If so, I'll have to double check that Angle handles
these
> > keywords well.
> 
> Done.
> 
>
https://codereview.chromium.org/16818013/diff/1/src/effects/SkPerlinNoiseShad...
> src/effects/SkPerlinNoiseShader.cpp:1079: noiseCode.appendf("\n\t%s = floor(%s
*
> vec2(255.0) + vec2(0.5)) * vec2(0.003921569);",
> On 2013/06/13 16:05:15, Stephen White wrote:
> > On 2013/06/13 15:34:15, sugoi wrote:
> > > 1 / 255 = 0.003921569
> > 
> > This ^^ should probably be in a comment in the code. Also, a clearer
> explanation
> > of the Tegra problem would be helpful.
> 
> Done.

lgtm

[commit-bot: I haz the power, 2013-06-17 14:09:34]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/sugoi@chromium.org/16818013/19001

[commit-bot: I haz the power, 2013-06-17 14:19:03]

Change committed as 9637