Proposed changes to Pepper 3D API.

ppapi/c/dev/ppb_graphics_3d_dev.h

Index: ppapi/c/dev/ppb_graphics_3d_dev.h
===================================================================
--- ppapi/c/dev/ppb_graphics_3d_dev.h	(revision 68085)
+++ ppapi/c/dev/ppb_graphics_3d_dev.h	(working copy)
@@ -31,61 +31,76 @@
 enum {
   PP_GRAPHICS_3D_ERROR_SUCCESS         = 0x3000,
   PP_GRAPHICS_3D_ERROR_NOT_INITIALIZED = 0x3001,
-  PP_GRAOHICS_3D_ERROR_BAD_CONTEXT     = 0x3006,
+  PP_GRAPHICS_3D_ERROR_BAD_CONTEXT     = 0x3006,
   PP_GRAPHICS_3D_ERROR_BAD_PARAMETER   = 0x300C,
   PP_GRAPHICS_3D_ERROR_CONTEXT_LOST    = 0x300E
 };
 
 // QueryString targets, matching EGL ones.
 enum {
-  EGL_VENDOR      = 0x3053,
-  EGL_VERSION     = 0x3054,
-  EGL_EXTENSIONS  = 0x3055,
-  EGL_CLIENT_APIS = 0x308D
+  PP_GRAPHICS_3D_VENDOR      = 0x3053,
+  PP_GRAPHICS_3D_VERSION     = 0x3054,
+  PP_GRAPHICS_3D_EXTENSIONS  = 0x3055,
+  PP_GRAPHICS_3D_CLIENT_APIS = 0x308D
 };
 
 struct PPB_Graphics3D_Dev {
-  PP_Bool (*IsGraphics3D)(PP_Resource resource);
+  uint32_t (*IsGraphics3D)(PP_Resource resource, PP_Bool* value);

[apatrick, 2010/12/03 18:36:36]

I think there is a convention that PPAPI interfaces have an IsX method that
returns PP_Bool and takes only the resource as argument. This is perhaps the
only method that should not return an integer error code.

[alokp, 2010/12/03 19:06:37]

Done.

 
   // EGL-like configuration ----------------------------------------------------
-  PP_Bool (*GetConfigs)(int32_t* configs,
-                        int32_t config_size,
-                        int32_t* num_config);
+  uint32_t (*GetConfigs)(int32_t* configs,
+                         int32_t config_size,
+                         int32_t* num_config);
 
-  PP_Bool (*ChooseConfig)(const int32_t* attrib_list,
-                          int32_t* configs,
-                          int32_t config_size,
-                          int32_t* num_config);
+  uint32_t (*ChooseConfig)(const int32_t* attrib_list,

[apatrick, 2010/12/03 18:36:36]

Can you add a comment indicating that this method is redundant and could
potentially be moved into an EGL-style library?

[alokp, 2010/12/03 19:06:37]

Done.

+                           int32_t* configs,
+                           int32_t config_size,
+                           int32_t* num_config);
 
   // TODO(apatrick): What to do if the browser window is moved to

[apatrick, 2010/12/03 18:36:36]

Can you remove this comment? We decided last meeting that since Chrome can move
from one display to another and straddle displays that the we are pretty much
forced to virtualize multiple displays as a single display.

[alokp, 2010/12/03 19:06:37]

Done.

   // another display? Do the configs potentially change?
-  PP_Bool (*GetConfigAttrib)(int32_t config, int32_t attribute, int32_t* value);
+  uint32_t (*GetConfigAttrib)(int32_t config,
+                              int32_t attribute,
+                              int32_t* value);
 
-  const char* (*QueryString)(int32_t name);
+  // TODO(alokp): GetString may be a better name
+  int32_t (*QueryString)(int32_t name, char* str, int32_t* str_size);

[apatrick, 2010/12/03 18:36:36]

How about:
uint32_t (*QueryString)(int32_t name, char** str);

The strings are constants as far as I know so no neeed to worry about length.

[alokp, 2010/12/03 19:06:37]

Done.

   // ---------------------------------------------------------------------------
 
+  // Render surface
+  // Decouple context and buffer.
+  // Context is only a state machine. Surface contains buffers.
+  // TODO(alokp): What about BindGraphics()? Move to PPB_Graphics3D.
+  // TODO(alokp): Do we need off-screen surface? No - use FBO.
+  uint32_t (*CreateSurface)(int32_t config,

[apatrick, 2010/12/03 18:36:36]

These surface related functions could go in a separate interface?

[alokp, 2010/12/03 19:06:37]

Yes they could go into PPB_Surface3D.

+                            PP_Instance instance,

[apatrick, 2010/12/03 18:36:36]

Do you want to delete the instance argument here as well?

[alokp, 2010/12/03 19:06:37]

Actually instance argument is required here. We are creating a surface for this
particular instance.

+                            const int32_t* attrib_list,
+                            PP_Resource* surface);
+  // TODO(alokp): SetSurfaceAttrib may be a better name
+  uint32_t (*SurfaceAttrib)(PP_Resource surface,
+                            int32_t attribute,
+                            int32_t value);
+  // TODO(alokp): GetSurfaceAttrib may be a better name
+  uint32_t (*QuerySurface)(PP_Resource surface,
+                           int32_t attribute,
+                           int32_t* value);
 
   // Create a reference counted 3D context. Releasing a context while it is
   // current automatically sets the current context to NULL. This is only true
   // for the releasing thread. Releasing a context while it is current on
   // another thread leads to undefined behavior.
-  PP_Resource (*CreateContext)(PP_Instance instance,
-                               int32_t config,
-                               int32_t share_context,
-                               const int32_t* attrib_list);
+  uint32_t (*CreateContext)(int32_t config,
+                            PP_Resource share_context,
+                            const int32_t* attrib_list,
+                            PP_Resource* context);
 
-  // Get the address of any GL functions, whether core or part of an extension.
-  // Any thread.
-  void* (*GetProcAddress)(const char* name);
+  // This is only necessary if we need to draw into the web page.
+  // Off-screen rendering can be done by creating and binding an FBO.
+  // If neither is present all gl commands will vanish into a black hole.
+  uint32_t (*BindContext)(PP_Resource draw,

[apatrick, 2010/12/03 18:36:36]

I don't understand how this works but I'm about to get off a bus...

[alokp, 2010/12/03 19:06:37]

I have a couple of options that we can discuss.

+                          PP_Resource read,
+                          PP_Resource context);
 
-  // Make a particular context current of the calling thread.  Returns PP_TRUE
-  // on success, PP_FALSE on failure.
-  PP_Bool (*MakeCurent)(PP_Resource context);
-
-  // Returns the calling thread's current context or NULL if no context is
-  // current.
-  PP_Resource (*GetCurrentContext)();
-
   // Snapshots the rendered frame and makes it available for composition with
   // the rest of the page. The alpha channel is used for translucency effects.
   // One means fully opaque. Zero means fully transparent. Any thread.
@@ -93,12 +108,7 @@
   // better for correct alpha blending effect. Most existing OpenGL code assumes
   // linear. I could convert from linear to premultiplied during the copy from
   // back-buffer to offscreen "front-buffer".
-  PP_Bool (*SwapBuffers)(PP_Resource context);
-
-  // Returns the current error for this thread. This is not associated with a
-  // particular context. It is distinct from the GL error returned by
-  // glGetError.
-  uint32_t (*GetError)();
+  uint32_t (*SwapBuffers)(PP_Resource surface);
 };
 
 #endif  // PPAPI_C_DEV_PPB_GRAPHICS_3D_DEV_H_