Add support for EGL on linux

Add support for EGL on linux

Allow setting skia_egl=1 to build skia against EGL instead of GLX on unix

Committed: https://skia.googlesource.com/skia/+/29dda80c0178371feea3aa81d72e1f8ff3b17bb6

[derekf, 2014-09-25 17:34:44]

Patchset #1 (id:1) has been deleted

[derekf, 2014-09-25 17:34:50]

Patchset #1 (id:20001) has been deleted

[derekf, 2014-09-25 18:13:13]

derekf@osg.samsung.com changed reviewers:
+ borenet@google.com, bsalomon@google.com, djsollen@google.com

[derekf, 2014-09-25 18:13:13]

Continuation of previous attempt at: https://codereview.chromium.org/543363004/


The first attempt removed some #if defined(BUILD_FOR_ANDROID) and BUILD_FOR_NACL
from SkNativeGLContext.h and replaced them with SK_EGL.

The problem with that is that SK_EGL was only set for building the skia
libraries, but not for the other programs that include SkNativeGLContext.h
(Sample app, gm)

So, the apps thought the class was a different size than it actually was,
segfaults ensued.

I could make the gyp stuff set SK_EGL for the entire build, but external users
of skia for android or nacl would likely be surprised by the new need to build
with -DSK_EGL (at least when using SkGLContextHelper).

This version of the patch keeps the SK_BUILD_FOR_ANDROID and SK_BUILD_FOR_NACL
conditions in the header file so only users of the new functionality
(-Dskia_egl=1 when skia_os=linux) need to set -DSK_EGL in their builds.

I don't know if this is an acceptable solution, please let me know if there's
something better I can do.

Thanks

[bsalomon, 2014-09-27 13:59:49]

On 2014/09/25 18:13:13, derekf wrote:
> Continuation of previous attempt at:
https://codereview.chromium.org/543363004/
> 
> 
> The first attempt removed some #if defined(BUILD_FOR_ANDROID) and
BUILD_FOR_NACL
> from SkNativeGLContext.h and replaced them with SK_EGL.
> 
> The problem with that is that SK_EGL was only set for building the skia
> libraries, but not for the other programs that include SkNativeGLContext.h
> (Sample app, gm)
> 
> So, the apps thought the class was a different size than it actually was,
> segfaults ensued.
> 
> I could make the gyp stuff set SK_EGL for the entire build, but external users
> of skia for android or nacl would likely be surprised by the new need to build
> with -DSK_EGL (at least when using SkGLContextHelper).



So there are two ways of setting defaults for SK_ macros:

1) Based on GYP vars like you've done here

2) In the SkPostConfig.h file you can have something like:

We could decide that SK_EGL is always defined to either 0 or 1 (and check its
value rather than existence). Then in SkPostConfig you could have:

#ifndef SK_EGL
    #if defined(SK_BUILD_FOR_ANDROID) || defined(...
        #define SK_EGL 0
    #else
        #define SK_EGL 1
    #endif
#endif

Then people using a build system other than our gyp files would get the right
default values.


Seems like we should also rename the glx-related files from _unix to _glx as
well.

[bsalomon, 2014-09-27 14:00:54]

https://codereview.chromium.org/604853003/diff/40001/gyp/gpu.gyp
File gyp/gpu.gyp (right):

https://codereview.chromium.org/604853003/diff/40001/gyp/gpu.gyp#newcode15
gyp/gpu.gyp:15: ['exclude', 'GrGLCreateNativeInterface_egl.cpp'],
Can this be excluded based on whether skia_egl is defined or not?

[derekf, 2014-09-29 20:36:10]

Patchset #2 (id:60001) has been deleted

[derekf, 2014-09-29 20:36:23]

Patchset #2 (id:80001) has been deleted

[derekf, 2014-09-29 22:10:24]

Patchset #2 (id:100001) has been deleted

[derekf, 2014-09-29 22:18:37]

renames _unix -> _glx

uses SkPostConfig.h to ensure SK_EGL is set when needed

moves the exclude for GrGLCreateNativeInterface_egl.cpp to skia_os == "android" 
(it can't be based exclusively on skia_egl because android has a special version
of the egl interface)

bsalomon: Do you still see the need for Define SK_EGL 0 or 1 and not testing for
existence?

[bsalomon, 2014-09-30 13:43:58]

https://codereview.chromium.org/604853003/diff/120001/include/core/SkPostConf...
File include/core/SkPostConfig.h (right):

https://codereview.chromium.org/604853003/diff/120001/include/core/SkPostConf...
include/core/SkPostConfig.h:416: #ifndef SK_EGL
I think the problem here is that there is no way for someone to say "I don't
want EGL". By using 0 and 1 they can define SK_EGL to 0 and avoid this default
because SK_EGL will be defined.

[derekf, 2014-10-01 15:56:20]

PTAL

I've made SK_EGL=1 or 0 and now test value instead of presence

[bsalomon, 2014-10-01 16:00:18]

I'd remove the extraneous "== 1"s but otherwise lgtm

https://codereview.chromium.org/604853003/diff/140001/include/gpu/gl/SkNative...
File include/gpu/gl/SkNativeGLContext.h (right):

https://codereview.chromium.org/604853003/diff/140001/include/gpu/gl/SkNative...
include/gpu/gl/SkNativeGLContext.h:38: #elif SK_EGL == 1
This can just be:

#if SK_EGL

[derekf, 2014-10-01 17:42:27]

The CQ bit was checked by derekf@osg.samsung.com

[commit-bot: I haz the power, 2014-10-01 17:43:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/604853003/160001

[commit-bot: I haz the power, 2014-10-01 17:52:57]

Committed patchset #4 (id:160001) as 29dda80c0178371feea3aa81d72e1f8ff3b17bb6