Bug 123399 - [EFL][GLES] OpenGL should be optional
Summary: [EFL][GLES] OpenGL should be optional
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks: 105286
  Show dependency treegraph
 
Reported: 2013-10-28 00:18 PDT by Ryuan Choi
Modified: 2013-10-31 00:11 PDT (History)
9 users (show)

See Also:


Attachments
Patch (9.70 KB, patch)
2013-10-28 00:29 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (9.67 KB, patch)
2013-10-28 00:31 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2013-10-28 00:18:57 PDT
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.
Comment 1 Ryuan Choi 2013-10-28 00:29:39 PDT
Created attachment 215283 [details]
Patch
Comment 2 Ryuan Choi 2013-10-28 00:31:14 PDT
Created attachment 215284 [details]
Patch
Comment 3 Gyuyoung Kim 2013-10-28 04:15:11 PDT
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.
Comment 4 Ryuan Choi 2013-10-28 05:04:04 PDT
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.
Comment 5 Gyuyoung Kim 2013-10-30 22:16:34 PDT
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.
Comment 6 Noam Rosenthal 2013-10-30 23:51:25 PDT
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
Comment 7 Ryuan Choi 2013-10-31 00:09:34 PDT
Committed r158351: <http://trac.webkit.org/changeset/158351>
Comment 8 Ryuan Choi 2013-10-31 00:11:07 PDT
Comment on attachment 215284 [details]
Patch

Thank you,
landed after fixed typos.

And cleared flags.