[Cogl] [PATCH/RFC] Adds initial GLES2 integration support
Neil Roberts
neil at linux.intel.com
Thu Mar 15 15:12:53 PDT 2012
Here's some minor feedback:
> + context->gles2_context_stack = g_queue_new ();
> +
GQueue is documented as being OK to allocate on the stack so I guess you
could put it directly inline in the context and avoid this separate
allocation.
> + (ctx->have_last_offscreen_allocate_flags &&
> + try_creating_fbo (ctx,
> + offscreen->texture,
> + offscreen->texture_level,
> + offscreen->texture_level_width,
> + offscreen->texture_level_height,
> + &fb->config,
> + ctx->last_offscreen_allocate_flags,
> + gl_framebuffer)) ||
This needs to set flags = ctx->last_offscreen_allocate_flags, otherwise
flags will be left as 0 and later it will reset
lsat_offscreen_allocate_flags to 0 too.
> +/* When drawing to a CoglFramebuffer from separate context we have
> + * to be able to allocate ancillary buffers for that context...
> + */
> +static CoglGLES2Framebuffer *
> +_cogl_gles2_framebuffer_allocate (CoglFramebuffer *framebuffer,
> + CoglGLES2Context *gles2_context,
> + GError **error)
> +{
...
> + gles2_framebuffer->original = cogl_object_ref (framebuffer);
It doesn't seem ideal that the context permanently keeps alive any
framebuffers that is has ever been used with. Could it instead register
a weak reference with cogl_object_set_user_data and remove the
framebuffer from the list when it is destroyed?
> +/**
> + * cogl_push_gles2_context:
We should maybe also document here that it's invalid to use regular Cogl
calls while a GLES2 context is pushed.
> +/* We wrap glReadPixels so when framebuffer 0 is bound then we can
> + * read from the read_framebuffer passed to cogl_push_gles2_context().
> + */
> +static void
> +gl_read_pixels_wrapper (GLint x,
We probably would also need to do this for glCopyTexImage2D and
glCopyTexSubImage2D, right?
I guess it would be nice to have a conformance test at some point.
Otherwise it looks really good to me.
Regards,
- Neil
More information about the Cogl
mailing list