[cogl/wip/cogl-1.14: 169/177] framebuffer: Bind the framebuffer before querying the bits

[Robert Bragg <rbragg src gnome org>]

commit 460242cee187c2851f80e80890ab702c2380c653
Author: Neil Roberts <neil linux intel com>
Date:   Thu Dec 13 15:39:55 2012 +0000

    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 <robert linux intel com>
    
    (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           |   90 +++++++++++-------------
 cogl/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, 71 insertions(+), 92 deletions(-)
---
diff --git a/cogl/cogl-driver.h b/cogl/cogl-driver.h
index 0c42e71..196f5a1 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 a88c591..996bc0a 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 8b95796..8edf656 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 43588ea..ab1a71b 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 043d047..5888f72 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;
-
-      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)
-          );
+      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;
+
+      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 0016ec8..40d5574 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 cd20c6e..540974e 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 <glib.h>
-
+#include <string.h>
 
 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 21df851..f2a23e0 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,