Moved the logic of maintaining the current context to gles2 helper library.

Moved the logic of maintaining the current context to gles2 helper library.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=69838

[apatrick_chromium, 2010-12-16 22:46:00]

LGTM

http://codereview.chromium.org/5927002/diff/1/gpu/demos/framework/pepper.cc
File gpu/demos/framework/pepper.cc (right):

http://codereview.chromium.org/5927002/diff/1/gpu/demos/framework/pepper.cc#n...
gpu/demos/framework/pepper.cc:35:
glSetCurrentContextPPAPI(graphics_.pp_resource());
Unrelated to this patch and an aside but this naming convention makes these
functions look like they are part of a GL extension. The function to set the GL
context is not normally implemented as what Khronos calls a "native platform
graphics interface" like EGL.

http://codereview.chromium.org/5927002/diff/1/ppapi/lib/gl/gles2/gl2ext_ppapi.c
File ppapi/lib/gl/gles2/gl2ext_ppapi.c (right):

http://codereview.chromium.org/5927002/diff/1/ppapi/lib/gl/gles2/gl2ext_ppapi...
ppapi/lib/gl/gles2/gl2ext_ppapi.c:22: #define PP_TLS __declspec(thread)
This probably isn't an issue for NaCl but perhaps it is for trusted plugins.
_declspec(thread) does not work for runtime loaded DLLs on Windows XP. It was
fixed with Vista. I ended up having to use TlsGetValue and friends on Windows.

[alokp, 2010-12-20 17:24:40]

http://codereview.chromium.org/5927002/diff/1/gpu/demos/framework/pepper.cc
File gpu/demos/framework/pepper.cc (right):

http://codereview.chromium.org/5927002/diff/1/gpu/demos/framework/pepper.cc#n...
gpu/demos/framework/pepper.cc:35:
glSetCurrentContextPPAPI(graphics_.pp_resource());
On 2010/12/16 22:46:00, apatrick_chromium wrote:
> Unrelated to this patch and an aside but this naming convention makes these
> functions look like they are part of a GL extension. The function to set the
GL
> context is not normally implemented as what Khronos calls a "native platform
> graphics interface" like EGL.

If we have to keep GLES2 lib independent of EGL lib then I need a hook to set
the current context to be used by the GLES2 lib. Modeling these extra functions
as extensions seemed like a good thing but may be confusing. Fortunately the
clients will never have to use these functions if they are using EGL library.

http://codereview.chromium.org/5927002/diff/1/ppapi/lib/gl/gles2/gl2ext_ppapi.c
File ppapi/lib/gl/gles2/gl2ext_ppapi.c (right):

http://codereview.chromium.org/5927002/diff/1/ppapi/lib/gl/gles2/gl2ext_ppapi...
ppapi/lib/gl/gles2/gl2ext_ppapi.c:22: #define PP_TLS __declspec(thread)
I guess I will have to do the same then. There is a TLS implementation in base,
but I do not want to add a dependency on base. Another option would be to use
the TLS implementation in EGL lib.

[alokp, 2010-12-20 19:10:12]

Gregg: Could you please look at build_gles2_cmd_buffer.py changes.

Neb: For some reason ppapi_cpp target does not work for me. In particular
linking a shared_library target (gpu ppapi demos) with a static library
(ppapi_cpp) does not export the ppapi entry-points defined in ppapi_cpp. So I am
compiling the entry-points file directly in the shared-library via a variable
defined in ppapi.gypi. Since ppapi_cpp does not do what it was intended to do,
do you want to get rid of ppapi_cpp target? perhaps rename ppapi_cpp_objects to
ppapi_cpp?

[neb at google, 2010-12-20 19:38:54]

We should figure out why are the entry points not exported, because that
worked fine in the gpu ppapi demos before. As for why there is ppapi_cpp and
ppapi_cpp_objects, that is weird to me. Git blame says ask brettw?

N

On Mon, Dec 20, 2010 at 11:10 AM, <alokp@chromium.org> wrote:

> Gregg: Could you please look at build_gles2_cmd_buffer.py changes.
>
> Neb: For some reason ppapi_cpp target does not work for me. In particular
> linking a shared_library target (gpu ppapi demos) with a static library
> (ppapi_cpp) does not export the ppapi entry-points defined in ppapi_cpp. So
> I am
> compiling the entry-points file directly in the shared-library via a
> variable
> defined in ppapi.gypi. Since ppapi_cpp does not do what it was intended to
> do,
> do you want to get rid of ppapi_cpp target? perhaps rename
> ppapi_cpp_objects to
> ppapi_cpp?
>
> http://codereview.chromium.org/5927002/
>

[greggman, 2010-12-20 19:56:21]

the build_gles2_cmd_buffer.py changes look fine. I don't like the hardcoded
check for "swapbuffers" though. I'd prefer you either mark it as an extension or
add new flag.

[alokp, 2010-12-20 21:32:54]

On 2010/12/20 19:56:21, greggman wrote:
> the build_gles2_cmd_buffer.py changes look fine. I don't like the hardcoded
> check for "swapbuffers" though. I'd prefer you either mark it as an extension
or
> add new flag.

Added extension flag. Thanks.