[Cogl] [PATCH v2 1/5] Adds initial GLES2 integration support

[Neil Roberts]

Robert Bragg <robert at sixbynine.org> writes:

(in cogl-gles2-context.c:)

> + #include "winsys/cogl-winsys-egl-private.h"

When compiling for WebOS you get GLES2 support without EGL. That means
it's not safe to assume that EGL is enabled when GLES2 is. However it
doesn't look like that file actually uses anything from this header so
it seems to work just to remove the #include.

Actually it seems to break the build in the same way if you --enable-gl
without --enable-gles2.

> +  gboolean have_last_offscreen_allocate_flags;
> +  CoglOffscreenAllocateFlags last_offscreen_allocate_flags;
> +
> +  CoglGLES2Context *current_gles2_context;
> +  GQueue *gles2_context_stack;

It still might be nice to allocate this GQueue struct inline and use
g_queue_init instead of g_queue_new. I know this is just massively
nitpicking though so I'll stop saying it now :)

> +  gles2_ctx->vtable->glBindFramebuffer = gl_bind_framebuffer_wrapper;
> +  gles2_ctx->vtable->glReadPixels = gl_read_pixels_wrapper;

This still needs to wrap glCopyTexImage2D and glCopyTexSubImage2D too.

> +  /* So we avoid building up an ever growing collection of ancillary
> +   * buffers for wrapped framebuffers, we make sure that the wrappers
> +   * get freed when the original offscreen framebuffer is freed. */
> +  cogl_object_set_user_data (COGL_OBJECT (framebuffer),
> +                             &offscreen_wrapper_key,
> +                             gles2_offscreen,
> +                             (CoglUserDataDestroyCallback)
> +                                _cogl_gles2_offscreen_free);

The weak reference destroy function can't just call
_cogl_gles2_offscreen_free because it also needs to remove itself from
the linked list. Maybe this would be a good excuse to use an embedded
list so that you can get a pointer back to the list node.

> +CoglBool
> +cogl_gles2_texture_get_handle (CoglTexture *texture,
> +                               unsigned int *handle,
> +                               unsigned int *target)

It might be nice to document the lifespan of the handle. Ie, we could
say after the texture is destroyed the handle can no longer be used from
GLES2.

It's kind of awkward that once you pull in a texture ID to the GLES2
context like this you could totally change it from the GL side. Ie, you
could change the size and add another target etc. Maybe we should just
say ‘don't do that’.

> diff --git a/cogl/cogl-gles2-types.h b/cogl/cogl-gles2-types.h
> new file mode 100644
> index 0000000..9f59bff
> --- /dev/null
> +++ b/cogl/cogl-gles2-types.h
> @@ -0,0 +1,472 @@
> +/*-------------------------------------------------------------------------
> + * Data type definitions
> + *-----------------------------------------------------------------------*/
> +
> +typedef void             GLvoid;
> +typedef char             GLchar;
> +typedef unsigned int     GLenum;
> +typedef unsigned char    GLboolean;
> +typedef unsigned int     GLbitfield;
> +typedef khronos_int8_t   GLbyte;

I think these 'khronos_blah_t' types depend on including an EGL header to
get EGL/eglplatform.h included. This doesn't compile when building with
the GLES2 support of SDL because there is no EGL. Can we just modify the
header to use the types from stdint.h instead?

Also there are a few gbooleans in the public API.

Regards,
- Neil