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,77 @@
 enum {
   PP_GRAPHICS_3D_ERROR_SUCCESS         = 0x3000,

[brettw, 2010/12/04 00:27:09]

We should move these to pp_errors.h and do the EGL mapping in the wrapper
library.

   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 {

[brettw, 2010/12/04 00:27:09]

This should be a named 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 {

[brettw, 2010/12/04 00:27:09]

We should have two different interfaces for Surface and Context. We should
defined separate enums for the configs for each of these things.

   PP_Bool (*IsGraphics3D)(PP_Resource resource);
 
   // EGL-like configuration ----------------------------------------------------
-  PP_Bool (*GetConfigs)(int32_t* configs,
-                        int32_t config_size,
-                        int32_t* num_config);
+  // Not that config functions do not take display argument.
+  // Browser window can move from one display to another and straddle displays.
+  // So we are forced to virtualize multiple displays as a single display.
+  uint32_t (*GetConfigs)(int32_t* configs,

[brettw, 2010/12/04 00:27:09]

When these return a "real" PP error, these should all be int32_t.

+                         int32_t config_size,
+                         int32_t* num_config);
+  // This can potentially be implemented in EGL helper library.
+  // TODO(alokp): Should we remove it from here?
+  uint32_t (*ChooseConfig)(const int32_t* attrib_list,

[brettw, 2010/12/04 00:27:09]

We're thinking there will be one function to get all configs that will return
all the configs and attributes at once, so we can avoid IPC traffic and the
extra complexity.

+                           int32_t* configs,
+                           int32_t config_size,
+                           int32_t* num_config);
+  uint32_t (*GetConfigAttrib)(int32_t config,
+                              int32_t attribute,
+                              int32_t* value);
 
-  PP_Bool (*ChooseConfig)(const int32_t* attrib_list,
-                          int32_t* configs,
-                          int32_t config_size,
-                          int32_t* num_config);
-
-  // TODO(apatrick): What to do if the browser window is moved to
-  // another display? Do the configs potentially change?
-  PP_Bool (*GetConfigAttrib)(int32_t config, int32_t attribute, int32_t* value);
-
-  const char* (*QueryString)(int32_t name);
+  // TODO(alokp): GetString may be a better name

[neb, 2010/12/03 19:27:21]

Isn't QueryString the EGL name?

[alokp, 2010/12/03 19:39:51]

Yes and thats why I am using it here but I think it is inconsistent. EGL
inconsistently uses Get and Query.

+  int32_t (*QueryString)(int32_t name, const char** str);

[brettw, 2010/12/04 00:27:09]

As discussed, this will be:
  PP_Var (*QueryString)(enum SomeEnumName name);
and return a VARTYPE_UNDEFINED on an invalid name.

   // ---------------------------------------------------------------------------
 
+  // Render surface
+  // Decouple context and buffer.
+  // Context is only a state machine. Surface contains buffers.
+  // TODO(alokp): What about BindGraphics()? Move to PPB_Graphics2D.

[neb, 2010/12/03 19:27:21]

I don't understand this comment.

[alokp, 2010/12/03 19:39:51]

This renders PPB_Instance::BindGraphics redundant. What should we do about that?

+  // TODO(alokp): Do we need off-screen surface? No - use FBO.
+  uint32_t (*CreateSurface)(int32_t config,
+                            PP_Instance instance,
+                            const int32_t* attrib_list,

[brettw, 2010/12/04 00:27:09]

I'd rather be explicit about the size and add a PP_Size argument.

+                            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,

[brettw, 2010/12/04 00:27:09]

This should probably be called SetSurfaces

+                          PP_Resource read,
+                          PP_Resource context);
 

[brettw, 2010/12/04 00:27:09]

There was talk about adding a fence callback.

-  // 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 +109,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);

[brettw, 2010/12/04 00:27:09]

This should take a completion callback to optionally have a blocking call or an
asynchronous call (from the main thread).

 };
 
 #endif  // PPAPI_C_DEV_PPB_GRAPHICS_3D_DEV_H_