Added native EGL types for pepper platform.

Added native EGL types for pepper platform.

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

[apatrick, 2010-12-15 18:36:46]

lgtm

[piman, 2010-12-15 18:49:35]

http://codereview.chromium.org/5865002/diff/1/ppapi/lib/gl/include/EGL/eglpla...
File ppapi/lib/gl/include/EGL/eglplatform.h (right):

http://codereview.chromium.org/5865002/diff/1/ppapi/lib/gl/include/EGL/eglpla...
ppapi/lib/gl/include/EGL/eglplatform.h:73: typedef PP_Module
EGLNativeDisplayType;
Shouldn't EGLNativeDisplayType be a PP_Instance, and EGLNativeWindowType a
PP_Resource representing a Surface3D ?

[alokp, 2010-12-15 18:54:53]

http://codereview.chromium.org/5865002/diff/1/ppapi/lib/gl/include/EGL/eglpla...
File ppapi/lib/gl/include/EGL/eglplatform.h (right):

http://codereview.chromium.org/5865002/diff/1/ppapi/lib/gl/include/EGL/eglpla...
ppapi/lib/gl/include/EGL/eglplatform.h:73: typedef PP_Module
EGLNativeDisplayType;
On 2010/12/15 18:49:35, piman wrote:
> Shouldn't EGLNativeDisplayType be a PP_Instance, and EGLNativeWindowType a
> PP_Resource representing a Surface3D ?

Not sure about EGLNativeDisplayType at this time. It may change to PP_Instance.

I think using PP_Instance for EGLNativeWindowType is correct. Surface3D is the
internal equivalent to EGLSurface which is bound to PP_Instance.

[piman, 2010-12-15 19:04:41]

On Wed, Dec 15, 2010 at 10:54 AM, <alokp@chromium.org> wrote:

>
>
>
http://codereview.chromium.org/5865002/diff/1/ppapi/lib/gl/include/EGL/eglpla...
> File ppapi/lib/gl/include/EGL/eglplatform.h (right):
>
>
>
http://codereview.chromium.org/5865002/diff/1/ppapi/lib/gl/include/EGL/eglpla...
> ppapi/lib/gl/include/EGL/eglplatform.h:73: typedef PP_Module
> EGLNativeDisplayType;
> On 2010/12/15 18:49:35, piman wrote:
>
>> Shouldn't EGLNativeDisplayType be a PP_Instance, and
>>
> EGLNativeWindowType a
>
>> PP_Resource representing a Surface3D ?
>>
>
> Not sure about EGLNativeDisplayType at this time. It may change to
> PP_Instance.
>

> I think using PP_Instance for EGLNativeWindowType is correct. Surface3D
> is the internal equivalent to EGLSurface which is bound to PP_Instance.


Given the semantics we talked about:
- you'll need the PP_Instance when creating the context, and
eglCreateContext takes the EGLDisplay not the window. So the EGLDisplay has
to wrap the PP_Instance, so eglGetDisplay has to take the PP_Instance in
parameter, so EGLNativeDisplayType has to be the PP_Instance.
- you'll need to bind the context to the surface when calling
eglMakeCurrent. So EGLSurface has to wrap a surface. eglCreateWindowSurface
is what creates the EGLSurface, and only takes the EGLNativeWindowType to
represent the surface, so EGLNativeWindowType has to be the PP_Resource for
the surface.



>
> http://codereview.chromium.org/5865002/
>

[alokp, 2010-12-15 19:27:31]

> Given the semantics we talked about:
> - you'll need the PP_Instance when creating the context, and
> eglCreateContext takes the EGLDisplay not the window. So the EGLDisplay has
> to wrap the PP_Instance, so eglGetDisplay has to take the PP_Instance in
> parameter, so EGLNativeDisplayType has to be the PP_Instance.

Was the decision to use PP_Instance final? I thought we decided to use one of
the two - PP_Module or PP_Instance. The current snapshot of the CL takes
PP_Module.
http://codereview.chromium.org/5567004/diff/55001/ppapi/c/dev/ppb_context_3d_...
I can change it to PP_Instance but it will be inconsistent with Graphics2D.

> - you'll need to bind the context to the surface when calling
> eglMakeCurrent. So EGLSurface has to wrap a surface. eglCreateWindowSurface
> is what creates the EGLSurface, and only takes the EGLNativeWindowType to
> represent the surface, so EGLNativeWindowType has to be the PP_Resource for
> the surface.

An application using the egl library does not directly handle Surface3D, but
EGLSurface which wraps Surface3D. I do not understand the sequence of operations
you are proposing. How would an application create Surface3D? And if a Surface3D
is already created what is eglCreateWindowSurface supposed to do?

I am thinking applications would do something like this:
EGLDisplay display = eglGetDisplay(module/instance);
EGLContext context = eglCreateContext(display);
EGLSurface surface = eglCreateWindowSurface(display, instance);
eglMakeCurrent(display, surface, surface, context);

eglCreateWindowSurface internally creates a Surface3D; pastes it onto the
instance location on the webpage; wraps it in a EGLSurface and returns.

[piman, 2010-12-15 19:45:30]

On Wed, Dec 15, 2010 at 11:27 AM, <alokp@chromium.org> wrote:

>  Given the semantics we talked about:
>> - you'll need the PP_Instance when creating the context, and
>> eglCreateContext takes the EGLDisplay not the window. So the EGLDisplay
>> has
>> to wrap the PP_Instance, so eglGetDisplay has to take the PP_Instance in
>> parameter, so EGLNativeDisplayType has to be the PP_Instance.
>>
>
> Was the decision to use PP_Instance final? I thought we decided to use one
> of
> the two - PP_Module or PP_Instance. The current snapshot of the CL takes
> PP_Module.
>
>
http://codereview.chromium.org/5567004/diff/55001/ppapi/c/dev/ppb_context_3d_...
> I can change it to PP_Instance but it will be inconsistent with Graphics2D.
>
>
I thought we decided that Graphics2D would also take an instance. Note that
no matter what you will need the instance when implementing it: you need the
instance to be able to create the command buffer context.


>
>  - you'll need to bind the context to the surface when calling
>> eglMakeCurrent. So EGLSurface has to wrap a surface.
>> eglCreateWindowSurface
>> is what creates the EGLSurface, and only takes the EGLNativeWindowType to
>> represent the surface, so EGLNativeWindowType has to be the PP_Resource
>> for
>> the surface.
>>
>
> An application using the egl library does not directly handle Surface3D,
> but
> EGLSurface which wraps Surface3D. I do not understand the sequence of
> operations
> you are proposing. How would an application create Surface3D? And if a
> Surface3D
> is already created what is eglCreateWindowSurface supposed to do?
>
> I am thinking applications would do something like this:
> EGLDisplay display = eglGetDisplay(module/instance);
> EGLContext context = eglCreateContext(display);
> EGLSurface surface = eglCreateWindowSurface(display, instance);
> eglMakeCurrent(display, surface, surface, context);
>
> eglCreateWindowSurface internally creates a Surface3D; pastes it onto the
> instance location on the webpage; wraps it in a EGLSurface and returns.


That's inconsistent with other platforms, and makes the mapping of EGL
native types to actual native types very confusing.
eglGetDisplay and eglCreateWindowSurface just wrap the native resources. For
example with X, you call XOpenDisplay and pass the returned Display to
eglGetDisplay, you call XCreateWindow and pass the returned Window to
eglCreateWindowSurface.
It should be the same with Pepper. You create an instance using pepper APIs
(well, it's created for you), and you pass that to eglDisplay. You create a
Surface using pepper APIs and you pass that to eglCreateWindowSurface.


>
>
>
> http://codereview.chromium.org/5865002/
>

[alokp, 2010-12-15 20:03:51]

> I thought we decided that Graphics2D would also take an instance. Note that
> no matter what you will need the instance when implementing it: you need the
> instance to be able to create the command buffer context.

Thanks for the heads up. I will change it to PP_Instance.

> That's inconsistent with other platforms, and makes the mapping of EGL
> native types to actual native types very confusing.
> eglGetDisplay and eglCreateWindowSurface just wrap the native resources. For
> example with X, you call XOpenDisplay and pass the returned Display to
> eglGetDisplay, you call XCreateWindow and pass the returned Window to
> eglCreateWindowSurface.
> It should be the same with Pepper. You create an instance using pepper APIs
> (well, it's created for you), and you pass that to eglDisplay.

I totally agree with you on this one, and the current API does exactly this
except it takes module, which I will change to instance.

You create a
> Surface using pepper APIs and you pass that to eglCreateWindowSurface.

I think the inconsistency is coming from the fact that there is no equivalent of
XCreateWindow. Here are the things I do not like in your proposal:
1. We are asking apps to use both pepper 3d api and egl, which supposed to wrap
pepper 3d.
2. Once you have created a surface3d, it is confusing to call
eglCreateWindowSurface again.

We can iterate over this once I make some more progress on EGL implementation.

[piman, 2010-12-16 01:44:59]

On Wed, Dec 15, 2010 at 12:03 PM, <alokp@chromium.org> wrote:

>  I thought we decided that Graphics2D would also take an instance. Note
>> that
>> no matter what you will need the instance when implementing it: you need
>> the
>> instance to be able to create the command buffer context.
>>
>
> Thanks for the heads up. I will change it to PP_Instance.
>
>
>  That's inconsistent with other platforms, and makes the mapping of EGL
>> native types to actual native types very confusing.
>> eglGetDisplay and eglCreateWindowSurface just wrap the native resources.
>> For
>> example with X, you call XOpenDisplay and pass the returned Display to
>> eglGetDisplay, you call XCreateWindow and pass the returned Window to
>> eglCreateWindowSurface.
>> It should be the same with Pepper. You create an instance using pepper
>> APIs
>> (well, it's created for you), and you pass that to eglDisplay.
>>
>
> I totally agree with you on this one, and the current API does exactly this
> except it takes module, which I will change to instance.
>
>
> You create a
>
>> Surface using pepper APIs and you pass that to eglCreateWindowSurface.
>>
>
> I think the inconsistency is coming from the fact that there is no
> equivalent of
> XCreateWindow.


It does, it's the ppapi call to create a surface3d.


> Here are the things I do not like in your proposal:
> 1. We are asking apps to use both pepper 3d api and egl, which supposed to
> wrap
> pepper 3d.
>

It will never completely wrap it and in any case that's expected on other
platforms like I mentioned.


> 2. Once you have created a surface3d, it is confusing to call
> eglCreateWindowSurface again.
>

No, that's the API. Besides, you don't have enough information in the
eglCreateWindowSurface parameters to be able to create the surface3d, e.g.
the dimensions or the pixel format.


> We can iterate over this once I make some more progress on EGL
> implementation.
>
>
>
> http://codereview.chromium.org/5865002/
>