Bug 123399 – [EFL][GLES] OpenGL should be optional

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Ryuan Choi]

Now, EFL port depends on OpenGL with GLX.
But we need to support GLES with EGL for TIZEN.

Almost code is already in trunk, but OpenGL is mandatory in build script.

[Ryuan Choi]

Created attachment 215283 [details]
Patch

[Ryuan Choi]

Created attachment 215284 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 215284 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215284&action=review

> Source/WebCore/platform/graphics/surfaces/GraphicsSurfaceToken.h:68
> +        return frontBufferHandle != rhs.frontBufferHandle;

It looks you need to get LGTM from graphics folks.

> Source/cmake/OptionsEfl.cmake:223
> +if (ENABLE_WEBGL AND OPENGLX_FOUND)

Can't we move this condition to above condition ? ENABLE_WEBGL is already checked previous block.

[Ryuan Choi]

Comment on attachment 215284 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215284&action=review

>> Source/WebCore/platform/graphics/surfaces/GraphicsSurfaceToken.h:68
>> +        return frontBufferHandle != rhs.frontBufferHandle;
> 
> It looks you need to get LGTM from graphics folks.

Well, stupid diff looks make confusion.
I just move the code in #if USE(GLX) block to #else block of `#if OS(DARWIN) || OS(WINDOWS)`,
because it should be valid not only GLX but also EGL.

>> Source/cmake/OptionsEfl.cmake:223
>> +if (ENABLE_WEBGL AND OPENGLX_FOUND)
> 
> Can't we move this condition to above condition ? ENABLE_WEBGL is already checked previous block.

Above condition can be true when ENABLE_WEBGL is off and ENABLE_TILED_BACKINGSTORE is On.
So, ENABLE_WEBGL should be checked although we move this condition to above block.

If you preferred nested condition nevertheless, I will move it.

[Gyuyoung Kim]

Comment on attachment 215284 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215284&action=review

Looks ok on CMake side. But, it would be good if coordinated graphics folk reviews codes related to coordinated files.

>>> Source/cmake/OptionsEfl.cmake:223
>>> +if (ENABLE_WEBGL AND OPENGLX_FOUND)
>> 
>> Can't we move this condition to above condition ? ENABLE_WEBGL is already checked previous block.
> 
> Above condition can be true when ENABLE_WEBGL is off and ENABLE_TILED_BACKINGSTORE is On.
> So, ENABLE_WEBGL should be checked although we move this condition to above block.
> 
> If you preferred nested condition nevertheless, I will move it.

ok, I see.

[Noam Rosenthal]

Comment on attachment 215284 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215284&action=review

OK, but please fix English.

> Source/WebCore/ChangeLog:3
> +        [EFL][GLES] OpenGL should be an optional

an optional -> optional

> Source/WebCore/ChangeLog:10
> +        Move OpenGL macro to OptionsEfl.cmake and Make Xcomposite and XRender as optional.

Make -> make

[Ryuan Choi]

Committed r158351: <http://trac.webkit.org/changeset/158351>

[Ryuan Choi]

Comment on attachment 215284 [details]
Patch

Thank you,
landed after fixed typos.

And cleared flags.