cogl-context: Store winsys features in an array of unsigned ints

Previously the mask of available winsys features was stored in a
CoglBitmask. That isn't the ideal type to use for this because it is
intended for a growable array of bits so it can allocate extra memory
if there are more than 31 flags set. For the winsys feature flags the
highest used bit is known at compile time so it makes sense to
allocate a fixed array instead. This is conceptually similar to the
CoglDebugFlags which are stored in an array of integers with macros to
test a bit in the array. This moves the macros used for CoglDebugFlags
to cogl-flags.h and makes them more generic so they can be shared with
CoglContext.

clutter/cogl/cogl/Makefile.am

@@ -297,6 +297,7 @@ cogl_sources_c = \
 	$(srcdir)/cogl-matrix-mesa.c 			\
 	$(srcdir)/cogl-profile.h 			\
 	$(srcdir)/cogl-profile.c 			\
+	$(srcdir)/cogl-flags.h				\
 	$(srcdir)/cogl-bitmask.h                        \
 	$(srcdir)/cogl-bitmask.c                        \
 	$(srcdir)/cogl-shader-boilerplate.h		\

clutter/cogl/cogl/cogl-bitmask.h

@@ -44,6 +44,10 @@ G_BEGIN_DECLS
  * unsigned ints. This relies on the fact the g_malloc will return a
  * pointer aligned to at least two bytes (so that the least
  * significant bit of the address is always 0)
+ *
+ * If the maximum possible bit number in the set is known at compile
+ * time, it may make more sense to use the macros in cogl-flags.h
+ * instead of this type.
  */
 
 typedef struct _CoglBitmaskImaginaryType *CoglBitmask;

clutter/cogl/cogl/cogl-context-private.h

@@ -27,6 +27,7 @@
 #include "cogl-internal.h"
 #include "cogl-context.h"
 #include "cogl-winsys-private.h"
+#include "cogl-flags.h"
 
 #ifdef COGL_HAS_XLIB_SUPPORT
 #include "cogl-xlib-private.h"
@@ -258,7 +259,8 @@ struct _CoglContext
 
   CoglContextDriver drv;
 
-  CoglBitmask winsys_features;
+  unsigned int winsys_features
+    [COGL_FLAGS_N_INTS_FOR_SIZE (COGL_WINSYS_FEATURE_N_FEATURES)];
   void *winsys;
   gboolean stub_winsys;
 };

clutter/cogl/cogl/cogl-context.c

@@ -137,7 +137,7 @@ cogl_context_new (CoglDisplay *display,
 
   context->rectangle_state = COGL_WINSYS_RECTANGLE_STATE_UNKNOWN;
 
-  _cogl_bitmask_init (&context->winsys_features);
+  memset (context->winsys_features, 0, sizeof (context->winsys_features));
 
   if (!display)
     display = cogl_display_new (NULL, NULL);
@@ -441,8 +441,6 @@ _cogl_context_free (CoglContext *context)
 
   g_byte_array_free (context->buffer_map_fallback_array, TRUE);
 
-  _cogl_bitmask_destroy (&context->winsys_features);
-
   cogl_object_unref (context->display);
 
   g_free (context);

clutter/cogl/cogl/cogl-debug.c

@@ -103,11 +103,11 @@ _cogl_parse_debug_string_for_keys (const char *value,
       int nkeys_for_int = 0;
 
       for (key_num = 0; key_num < nkeys; key_num++)
-        if (COGL_DEBUG_GET_FLAG_INDEX (keys[key_num].value) == int_num)
+        if (COGL_FLAGS_GET_INDEX (keys[key_num].value) == int_num)
           {
             keys_for_int[nkeys_for_int] = keys[key_num];
             keys_for_int[nkeys_for_int].value =
-              COGL_DEBUG_GET_FLAG_MASK (keys[key_num].value);
+              COGL_FLAGS_GET_MASK (keys[key_num].value);
             nkeys_for_int++;
           }
 

clutter/cogl/cogl/cogl-debug.h

@@ -25,6 +25,7 @@
 #define __COGL_DEBUG_H__
 
 #include "cogl-profile.h"
+#include "cogl-flags.h"
 
 #include <glib.h>
 
@@ -70,32 +71,18 @@ typedef enum {
 
 #ifdef COGL_ENABLE_DEBUG
 
-#define COGL_DEBUG_N_INTS ((COGL_DEBUG_N_FLAGS + \
-                            (sizeof (unsigned int) * 8 - 1)) \
-                           / (sizeof (unsigned int) * 8))
+#define COGL_DEBUG_N_INTS COGL_FLAGS_N_INTS_FOR_SIZE (COGL_DEBUG_N_FLAGS)
 
-/* It would probably make sense to use unsigned long here instead
-   because then on 64-bit systems where it can handle 64-bits just as
-   easily and it can test more bits. However GDebugKey uses a guint
-   for the mask and we need to fit the masks into this */
 extern unsigned int _cogl_debug_flags[COGL_DEBUG_N_INTS];
 
-#define COGL_DEBUG_GET_FLAG_INDEX(flag) \
-  ((flag) / (sizeof (unsigned int) * 8))
-#define COGL_DEBUG_GET_FLAG_MASK(flag) \
-  (1U << ((unsigned int) (flag) & (sizeof (unsigned int) * 8 - 1)))
-
 #define COGL_DEBUG_ENABLED(flag) \
-  (!!(_cogl_debug_flags[COGL_DEBUG_GET_FLAG_INDEX (flag)] & \
-      COGL_DEBUG_GET_FLAG_MASK (flag)))
+  COGL_FLAGS_GET (_cogl_debug_flags, flag)
 
 #define COGL_DEBUG_SET_FLAG(flag) \
-  (_cogl_debug_flags[COGL_DEBUG_GET_FLAG_INDEX (flag)] |= \
-   COGL_DEBUG_GET_FLAG_MASK (flag))
+  COGL_FLAGS_SET (_cogl_debug_flags, flag, TRUE)
 
 #define COGL_DEBUG_CLEAR_FLAG(flag) \
-  (_cogl_debug_flags[COGL_DEBUG_GET_FLAG_INDEX (flag)] &= \
-   ~COGL_DEBUG_GET_FLAG_MASK (flag))
+  COGL_FLAGS_SET (_cogl_debug_flags, flag, FALSE)
 
 #ifdef __GNUC__
 #define COGL_NOTE(type,x,a...)                      G_STMT_START {            \

clutter/cogl/cogl/cogl-flags.h

@@ -0,0 +1,89 @@
+/*
+ * Cogl
+ *
+ * An object oriented GL/GLES Abstraction/Utility Layer
+ *
+ * Copyright (C) 2011 Intel Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ *
+ * Authors:
+ *   Neil Roberts <neil@linux.intel.com>
+ */
+
+#ifndef __COGL_FLAGS_H
+#define __COGL_FLAGS_H
+
+#include <glib.h>
+
+G_BEGIN_DECLS
+
+/* These are macros used to implement a fixed-size array of bits. This
+   should be used instead of CoglBitmask when the maximum bit number
+   that will be set is known at compile time, for example when setting
+   for recording a set of known available features */
+
+/* The bits are stored in an array of unsigned ints. It would probably
+   make sense to use unsigned long instead because then on 64-bit
+   systems where it can handle 64-bits just as easily and it can test
+   more bits. However GDebugKey uses a guint for the mask and we need
+   to fit the masks into this */
+
+/* To use these macros, you would typically have an enum defining the
+   available bits with an extra last enum to define the maximum
+   value. Then to store the flags you would declare an array of
+   unsigned ints sized using COGL_FLAGS_N_INTS_FOR_SIZE, eg:
+
+   typedef enum { FEATURE_A, FEATURE_B, FEATURE_C, N_FEATURES } Features;
+
+   unsigned int feature_flags[COGL_FLAGS_N_INTS_FOR_SIZE (N_FEATURES)];
+*/
+
+#define COGL_FLAGS_N_INTS_FOR_SIZE(size)        \
+  (((size) +                                    \
+    (sizeof (unsigned int) * 8 - 1))            \
+   / (sizeof (unsigned int) * 8))
+
+/* @flag is expected to be constant so these should result in a
+   constant expression. This means that setting a flag is equivalent
+   to just setting in a bit in a global variable at a known
+   location */
+#define COGL_FLAGS_GET_INDEX(flag)              \
+  ((flag) / (sizeof (unsigned int) * 8))
+#define COGL_FLAGS_GET_MASK(flag)               \
+  (1U << ((unsigned int) (flag) &               \
+          (sizeof (unsigned int) * 8 - 1)))
+
+#define COGL_FLAGS_GET(array, flag)             \
+  (!!((array)[COGL_FLAGS_GET_INDEX (flag)] &    \
+      COGL_FLAGS_GET_MASK (flag)))
+
+/* The expectation here is that @value will be constant so the if
+   statement will be optimised out */
+#define COGL_FLAGS_SET(array, flag, value)      \
+  G_STMT_START {                                \
+    if (value)                                  \
+      ((array)[COGL_FLAGS_GET_INDEX (flag)] |=  \
+       COGL_FLAGS_GET_MASK (flag));             \
+    else                                        \
+      ((array)[COGL_FLAGS_GET_INDEX (flag)] &=  \
+       ~COGL_FLAGS_GET_MASK (flag));            \
+  } G_STMT_END
+
+G_END_DECLS
+
+#endif /* __COGL_FLAGS_H */
+

clutter/cogl/cogl/cogl-types.h

@@ -612,8 +612,6 @@ typedef enum _CoglFilterReturn { /*< prefix=COGL_FILTER >*/
 
 typedef enum _CoglWinsysFeature
 {
-  COGL_WINSYS_FEATURE_NONE,
-
   /* Available if the window system can support multiple onscreen
    * framebuffers at the same time. */
   COGL_WINSYS_FEATURE_MULTIPLE_ONSCREEN,
@@ -644,7 +642,9 @@ typedef enum _CoglWinsysFeature
 
   /* Available if swap_region requests can be automatically throttled
    * to the vblank frequency. */
-  COGL_WINSYS_FEATURE_SWAP_REGION_THROTTLE
+  COGL_WINSYS_FEATURE_SWAP_REGION_THROTTLE,
+
+  COGL_WINSYS_FEATURE_N_FEATURES
 } CoglWinsysFeature;
 
 /* XXX: Note these enum types are only referenced by experimental API

clutter/cogl/cogl/winsys/cogl-winsys-glx.c

@@ -332,7 +332,7 @@ update_winsys_features (CoglContext *context)
 
   _cogl_gl_update_features (context);
 
-  _cogl_bitmask_init (&context->winsys_features);
+  memset (context->winsys_features, 0, sizeof (context->winsys_features));
 
   glx_extensions =
     glXQueryExtensionsString (xlib_renderer->xdpy,
@@ -341,7 +341,7 @@ update_winsys_features (CoglContext *context)
   COGL_NOTE (WINSYS, "  GLX Extensions: %s", glx_extensions);
 
   context->feature_flags |= COGL_FEATURE_ONSCREEN_MULTIPLE;
-  _cogl_bitmask_set (&context->winsys_features,
+  COGL_FLAGS_SET (context->winsys_features,
                   COGL_WINSYS_FEATURE_MULTIPLE_ONSCREEN,
                   TRUE);
 
@@ -354,7 +354,7 @@ update_winsys_features (CoglContext *context)
       {
         context->feature_flags |= winsys_feature_data[i].feature_flags;
         if (winsys_feature_data[i].winsys_feature)
-          _cogl_bitmask_set (&context->winsys_features,
+          COGL_FLAGS_SET (context->winsys_features,
                           winsys_feature_data[i].winsys_feature,
                           TRUE);
       }
@@ -368,7 +368,7 @@ update_winsys_features (CoglContext *context)
     }
 
   if (glx_renderer->pf_glXWaitVideoSync)
-    _cogl_bitmask_set (&context->winsys_features,
+    COGL_FLAGS_SET (context->winsys_features,
                     COGL_WINSYS_FEATURE_VBLANK_WAIT,
                     TRUE);
 
@@ -380,14 +380,14 @@ update_winsys_features (CoglContext *context)
       if (glx_renderer->dri_fd < 0)
         glx_renderer->dri_fd = open("/dev/dri/card0", O_RDWR);
       if (glx_renderer->dri_fd >= 0)
-        _cogl_bitmask_set (&context->winsys_features,
+        COGL_FLAGS_SET (context->winsys_features,
                         COGL_WINSYS_FEATURE_VBLANK_WAIT,
                         TRUE);
     }
 #endif
 
   if (glx_renderer->pf_glXCopySubBuffer || context->drv.pf_glBlitFramebuffer)
-    _cogl_bitmask_set (&context->winsys_features,
+    COGL_FLAGS_SET (context->winsys_features,
                     COGL_WINSYS_FEATURE_SWAP_REGION, TRUE);
 
   /* Note: glXCopySubBuffer and glBlitFramebuffer won't be throttled
@@ -395,7 +395,7 @@ update_winsys_features (CoglContext *context)
    * manually... */
   if (_cogl_winsys_has_feature (COGL_WINSYS_FEATURE_SWAP_REGION) &&
       _cogl_winsys_has_feature (COGL_WINSYS_FEATURE_VBLANK_WAIT))
-    _cogl_bitmask_set (&context->winsys_features,
+    COGL_FLAGS_SET (context->winsys_features,
                     COGL_WINSYS_FEATURE_SWAP_REGION_THROTTLE, TRUE);
 }
 
@@ -1314,7 +1314,7 @@ _cogl_winsys_has_feature (CoglWinsysFeature feature)
 {
   _COGL_GET_CONTEXT (ctx, FALSE);
 
-  return _cogl_bitmask_get (&ctx->winsys_features, feature);
+  return COGL_FLAGS_GET (ctx->winsys_features, feature);
 }
 
 /* XXX: This is a particularly hacky _cogl_winsys interface... */