Use the NDK's cpu-features library when building skia for Chromium/Android.

Use the NDK's cpu-features library when building skia for Chromium/Android.

This patch ensures that when Skia is built for Chromium, it will
always use the Android NDK's cpu-features helper library to detect
NEON at runtime.

This is needed because sandboxed Chromium renderer processes cannot
access /proc, and the probing performed in SkUtilsArm.cpp will never
work. As such, the NEON code paths will never be used even when the
device supports them.

Chromium has special code that ensures that the browser process
passes the CPU features flags to every renderer process, but
Skia needs to use android_getCpuFeatures() to get them.

See http://crbug.com/164154 for full details.

[digit1, 2013-01-14 13:55:37]

Hello, this is a small patch to enable NEON code-paths for Skia when run in
Chromium/Android renderer processes.

[caryclark1, 2013-01-14 14:14:22]

lgtm

[DerekS, 2013-01-14 14:20:52]

https://codereview.appspot.com/7102045/diff/1/src/core/SkUtilsArm.cpp
File src/core/SkUtilsArm.cpp (right):

https://codereview.appspot.com/7102045/diff/1/src/core/SkUtilsArm.cpp#newcode81
src/core/SkUtilsArm.cpp:81: result = (android_getCpuFeatures() &
ANDROID_CPU_ARM_FEATURE_NEON) != 0;
what is the downside to doing this in all cases?  If the impact is minimal then
I would lean toward doing this for all SK_BUILD_FOR_ANDROID_NDK builds.

[digit1, 2013-01-14 14:37:45]

https://codereview.appspot.com/7102045/diff/1/src/core/SkUtilsArm.cpp
File src/core/SkUtilsArm.cpp (right):

https://codereview.appspot.com/7102045/diff/1/src/core/SkUtilsArm.cpp#newcode81
src/core/SkUtilsArm.cpp:81: result = (android_getCpuFeatures() &
ANDROID_CPU_ARM_FEATURE_NEON) != 0;
The downside is that this adds a dependency on the NDK's cpu-features library.
It's a small static library that must be linked against whatever program uses
its features.

I'm not sure it's completely ok to expose a new dependency for other Skia
builds, which is why I made this optional.

[DerekS, 2013-01-14 14:50:13]

https://codereview.appspot.com/7102045/diff/1/src/core/SkUtilsArm.cpp
File src/core/SkUtilsArm.cpp (right):

https://codereview.appspot.com/7102045/diff/1/src/core/SkUtilsArm.cpp#newcode81
src/core/SkUtilsArm.cpp:81: result = (android_getCpuFeatures() &
ANDROID_CPU_ARM_FEATURE_NEON) != 0;
Ok.  If the lib is indeed small then I would prefer to pay the price for that
static lib and include it in our standard build.

On 2013/01/14 14:37:45, digit1 wrote:
> The downside is that this adds a dependency on the NDK's cpu-features library.
> It's a small static library that must be linked against whatever program uses
> its features.
> 
> I'm not sure it's completely ok to expose a new dependency for other Skia
> builds, which is why I made this optional.

[digit1, 2013-01-14 15:03:52]

https://codereview.appspot.com/7102045/diff/1/src/core/SkUtilsArm.cpp
File src/core/SkUtilsArm.cpp (right):

https://codereview.appspot.com/7102045/diff/1/src/core/SkUtilsArm.cpp#newcode81
src/core/SkUtilsArm.cpp:81: result = (android_getCpuFeatures() &
ANDROID_CPU_ARM_FEATURE_NEON) != 0;
I'll prepare another patch on top of this one to do it then. I really need this
patch as-is submitted asap though, since it must go into the next M25 release.

[DerekS, 2013-01-14 15:05:58]

lgtm

https://codereview.appspot.com/7102045/diff/1/src/core/SkUtilsArm.cpp
File src/core/SkUtilsArm.cpp (right):

https://codereview.appspot.com/7102045/diff/1/src/core/SkUtilsArm.cpp#newcode81
src/core/SkUtilsArm.cpp:81: result = (android_getCpuFeatures() &
ANDROID_CPU_ARM_FEATURE_NEON) != 0;
Understood.

On 2013/01/14 15:03:53, digit1 wrote:
> I'll prepare another patch on top of this one to do it then. I really need
this
> patch as-is submitted asap though, since it must go into the next M25 release.
>

[TomH, 2013-01-14 16:58:26]

Were we not already getting NEON? Aargh! Glad you caught this, at least.

[digit1, 2013-01-24 10:11:21]

Closing this, as it's been submitted in 7149. Not sure why this doesn't appear
here.