From 0b01c91fc52ad779b4b6dff85cd6fcd3490ccf5b Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 13 Dec 2012 15:39:55 +0000 Subject: [PATCH] framebuffer: Bind the framebuffer before querying the bits The GL framebuffer driver now makes sure to bind the framebuffer before counting the number of bits. Previously it would just query the number of bits for whatever framebuffer happened to be used last. In addition the virtual for querying the framebuffer bits has been modified to take a pointer to a structure instead of a separate pointer to each component. This should make it slightly more efficient and easier to maintain. Reviewed-by: Robert Bragg (cherry picked from commit e9c58b2ba23a7cebcd4e633ea7c3191f02056fb5) --- cogl/cogl-driver.h | 5 +- cogl/cogl-framebuffer-private.h | 13 ++- cogl/cogl-framebuffer.c | 29 +++--- cogl/driver/gl/cogl-framebuffer-gl-private.h | 5 +- cogl/driver/gl/cogl-framebuffer-gl.c | 88 +++++++++---------- .../driver/nop/cogl-framebuffer-nop-private.h | 5 +- cogl/driver/nop/cogl-framebuffer-nop.c | 12 +-- tests/conform/test-conform-main.c | 4 +- 8 files changed, 70 insertions(+), 91 deletions(-) diff --git a/cogl/cogl-driver.h b/cogl/cogl-driver.h index 0c42e716c..196f5a19f 100644 --- a/cogl/cogl-driver.h +++ b/cogl/cogl-driver.h @@ -75,10 +75,7 @@ struct _CoglDriverVtable void (* framebuffer_query_bits) (CoglFramebuffer *framebuffer, - int *red, - int *green, - int *blue, - int *alpha); + CoglFramebufferBits *bits); void (* framebuffer_finish) (CoglFramebuffer *framebuffer); diff --git a/cogl/cogl-framebuffer-private.h b/cogl/cogl-framebuffer-private.h index a88c59181..996bc0ae2 100644 --- a/cogl/cogl-framebuffer-private.h +++ b/cogl/cogl-framebuffer-private.h @@ -105,6 +105,14 @@ typedef enum COGL_READ_PIXELS_NO_FLIP = 1L << 30 } CoglPrivateReadPixelsFlags; +typedef struct +{ + int red; + int blue; + int green; + int alpha; +} CoglFramebufferBits; + struct _CoglFramebuffer { CoglObject _parent; @@ -162,10 +170,7 @@ struct _CoglFramebuffer /* driver specific */ CoglBool dirty_bitmasks; - int red_bits; - int blue_bits; - int green_bits; - int alpha_bits; + CoglFramebufferBits bits; int samples_per_pixel; }; diff --git a/cogl/cogl-framebuffer.c b/cogl/cogl-framebuffer.c index 8b95796f3..8edf6564d 100644 --- a/cogl/cogl-framebuffer.c +++ b/cogl/cogl-framebuffer.c @@ -1093,47 +1093,44 @@ int cogl_framebuffer_get_red_bits (CoglFramebuffer *framebuffer) { CoglContext *ctx = framebuffer->context; - int red, green, blue, alpha; + CoglFramebufferBits bits; - ctx->driver_vtable->framebuffer_query_bits (framebuffer, - &red, &green, &blue, &alpha); + ctx->driver_vtable->framebuffer_query_bits (framebuffer, &bits); - return red; + return bits.red; } + int cogl_framebuffer_get_green_bits (CoglFramebuffer *framebuffer) { CoglContext *ctx = framebuffer->context; - int red, green, blue, alpha; + CoglFramebufferBits bits; - ctx->driver_vtable->framebuffer_query_bits (framebuffer, - &red, &green, &blue, &alpha); + ctx->driver_vtable->framebuffer_query_bits (framebuffer, &bits); - return green; + return bits.green; } int cogl_framebuffer_get_blue_bits (CoglFramebuffer *framebuffer) { CoglContext *ctx = framebuffer->context; - int red, green, blue, alpha; + CoglFramebufferBits bits; - ctx->driver_vtable->framebuffer_query_bits (framebuffer, - &red, &green, &blue, &alpha); + ctx->driver_vtable->framebuffer_query_bits (framebuffer, &bits); - return blue; + return bits.blue; } int cogl_framebuffer_get_alpha_bits (CoglFramebuffer *framebuffer) { CoglContext *ctx = framebuffer->context; - int red, green, blue, alpha; + CoglFramebufferBits bits; - ctx->driver_vtable->framebuffer_query_bits (framebuffer, - &red, &green, &blue, &alpha); + ctx->driver_vtable->framebuffer_query_bits (framebuffer, &bits); - return alpha; + return bits.alpha; } CoglColorMask diff --git a/cogl/driver/gl/cogl-framebuffer-gl-private.h b/cogl/driver/gl/cogl-framebuffer-gl-private.h index 43588eaa3..ab1a71be2 100644 --- a/cogl/driver/gl/cogl-framebuffer-gl-private.h +++ b/cogl/driver/gl/cogl-framebuffer-gl-private.h @@ -50,10 +50,7 @@ _cogl_framebuffer_gl_clear (CoglFramebuffer *framebuffer, void _cogl_framebuffer_gl_query_bits (CoglFramebuffer *framebuffer, - int *red, - int *green, - int *blue, - int *alpha); + CoglFramebufferBits *bits); void _cogl_framebuffer_gl_finish (CoglFramebuffer *framebuffer); diff --git a/cogl/driver/gl/cogl-framebuffer-gl.c b/cogl/driver/gl/cogl-framebuffer-gl.c index 043d047ec..5888f72da 100644 --- a/cogl/driver/gl/cogl-framebuffer-gl.c +++ b/cogl/driver/gl/cogl-framebuffer-gl.c @@ -892,54 +892,54 @@ _cogl_framebuffer_init_bits (CoglFramebuffer *framebuffer) { CoglContext *ctx = framebuffer->context; - cogl_framebuffer_allocate (framebuffer, NULL); - if (G_LIKELY (!framebuffer->dirty_bitmasks)) return; + cogl_framebuffer_allocate (framebuffer, NULL); + + _cogl_framebuffer_flush_state (framebuffer, + framebuffer, + COGL_FRAMEBUFFER_STATE_BIND); + #ifdef HAVE_COGL_GL if ((ctx->private_feature_flags & COGL_PRIVATE_FEATURE_QUERY_FRAMEBUFFER_BITS) && framebuffer->type == COGL_FRAMEBUFFER_TYPE_OFFSCREEN) { - GLenum attachment, pname; + static const struct + { + GLenum attachment, pname; + size_t offset; + } params[] = + { + { GL_COLOR_ATTACHMENT0, GL_FRAMEBUFFER_ATTACHMENT_RED_SIZE, + offsetof (CoglFramebufferBits, red) }, + { GL_COLOR_ATTACHMENT0, GL_FRAMEBUFFER_ATTACHMENT_GREEN_SIZE, + offsetof (CoglFramebufferBits, green) }, + { GL_COLOR_ATTACHMENT0, GL_FRAMEBUFFER_ATTACHMENT_BLUE_SIZE, + offsetof (CoglFramebufferBits, blue) }, + { GL_COLOR_ATTACHMENT0, GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE, + offsetof (CoglFramebufferBits, alpha) }, + }; + int i; - attachment = GL_COLOR_ATTACHMENT0; - - pname = GL_FRAMEBUFFER_ATTACHMENT_RED_SIZE; - GE( ctx, glGetFramebufferAttachmentParameteriv (GL_FRAMEBUFFER, - attachment, - pname, - &framebuffer->red_bits) ); - - pname = GL_FRAMEBUFFER_ATTACHMENT_GREEN_SIZE; - GE( ctx, glGetFramebufferAttachmentParameteriv (GL_FRAMEBUFFER, - attachment, - pname, - &framebuffer->green_bits) - ); - - pname = GL_FRAMEBUFFER_ATTACHMENT_BLUE_SIZE; - GE( ctx, glGetFramebufferAttachmentParameteriv (GL_FRAMEBUFFER, - attachment, - pname, - &framebuffer->blue_bits) - ); - - pname = GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE; - GE( ctx, glGetFramebufferAttachmentParameteriv (GL_FRAMEBUFFER, - attachment, - pname, - &framebuffer->alpha_bits) - ); + for (i = 0; i < G_N_ELEMENTS (params); i++) + { + int *value = + (int *) ((uint8_t *) &framebuffer->bits + params[i].offset); + GE( ctx, glGetFramebufferAttachmentParameteriv (GL_FRAMEBUFFER, + params[i].attachment, + params[i].pname, + value) ); + } } else #endif /* HAVE_COGL_GL */ { - GE( ctx, glGetIntegerv (GL_RED_BITS, &framebuffer->red_bits) ); - GE( ctx, glGetIntegerv (GL_GREEN_BITS, &framebuffer->green_bits) ); - GE( ctx, glGetIntegerv (GL_BLUE_BITS, &framebuffer->blue_bits) ); - GE( ctx, glGetIntegerv (GL_ALPHA_BITS, &framebuffer->alpha_bits) ); + GE( ctx, glGetIntegerv (GL_RED_BITS, &framebuffer->bits.red) ); + GE( ctx, glGetIntegerv (GL_GREEN_BITS, &framebuffer->bits.green) ); + GE( ctx, glGetIntegerv (GL_BLUE_BITS, &framebuffer->bits.blue) ); + GE( ctx, glGetIntegerv (GL_ALPHA_BITS, &framebuffer->bits.alpha) ); } @@ -949,29 +949,23 @@ _cogl_framebuffer_init_bits (CoglFramebuffer *framebuffer) framebuffer->type == COGL_FRAMEBUFFER_TYPE_OFFSCREEN ? "offscreen" : "onscreen", - framebuffer->red_bits, - framebuffer->blue_bits, - framebuffer->green_bits, - framebuffer->alpha_bits); + framebuffer->bits.red, + framebuffer->bits.blue, + framebuffer->bits.green, + framebuffer->bits.alpha); framebuffer->dirty_bitmasks = FALSE; } void _cogl_framebuffer_gl_query_bits (CoglFramebuffer *framebuffer, - int *red, - int *green, - int *blue, - int *alpha) + CoglFramebufferBits *bits) { _cogl_framebuffer_init_bits (framebuffer); /* TODO: cache these in some driver specific location not * directly as part of CoglFramebuffer. */ - *red = framebuffer->red_bits; - *green = framebuffer->green_bits; - *blue = framebuffer->blue_bits; - *alpha = framebuffer->alpha_bits; + *bits = framebuffer->bits; } void diff --git a/cogl/driver/nop/cogl-framebuffer-nop-private.h b/cogl/driver/nop/cogl-framebuffer-nop-private.h index 0016ec860..40d5574a9 100644 --- a/cogl/driver/nop/cogl-framebuffer-nop-private.h +++ b/cogl/driver/nop/cogl-framebuffer-nop-private.h @@ -53,10 +53,7 @@ _cogl_framebuffer_nop_clear (CoglFramebuffer *framebuffer, void _cogl_framebuffer_nop_query_bits (CoglFramebuffer *framebuffer, - int *red, - int *green, - int *blue, - int *alpha); + CoglFramebufferBits *bits); void _cogl_framebuffer_nop_finish (CoglFramebuffer *framebuffer); diff --git a/cogl/driver/nop/cogl-framebuffer-nop.c b/cogl/driver/nop/cogl-framebuffer-nop.c index cd20c6ecb..540974ef6 100644 --- a/cogl/driver/nop/cogl-framebuffer-nop.c +++ b/cogl/driver/nop/cogl-framebuffer-nop.c @@ -29,7 +29,7 @@ #include "cogl-framebuffer-nop-private.h" #include - +#include void _cogl_framebuffer_nop_flush_state (CoglFramebuffer *draw_buffer, @@ -62,15 +62,9 @@ _cogl_framebuffer_nop_clear (CoglFramebuffer *framebuffer, void _cogl_framebuffer_nop_query_bits (CoglFramebuffer *framebuffer, - int *red, - int *green, - int *blue, - int *alpha) + CoglFramebufferBits *bits) { - *red = 0; - *green = 0; - *blue = 0; - *alpha = 0; + memset (bits, 0, sizeof (CoglFramebufferBits)); } void diff --git a/tests/conform/test-conform-main.c b/tests/conform/test-conform-main.c index 21df85125..f2a23e0d0 100644 --- a/tests/conform/test-conform-main.c +++ b/tests/conform/test-conform-main.c @@ -98,9 +98,7 @@ main (int argc, char **argv) ADD_TEST (test_bitmask, 0, 0); ADD_TEST (test_offscreen, 0, 0); - ADD_TEST (test_framebuffer_get_bits, - TEST_REQUIREMENT_OFFSCREEN, - TEST_KNOWN_FAILURE); + ADD_TEST (test_framebuffer_get_bits, TEST_REQUIREMENT_OFFSCREEN, 0); ADD_TEST (test_point_size, 0, 0); ADD_TEST (test_point_sprite,