[cogl] cogl-bitmask: Use longs instead of ints

[Neil Roberts <nroberts src gnome org>]

commit 93321d9b8961ac9a4ecdb616486cf4cedd47c4fb
Author: Neil Roberts <neil linux intel com>
Date:   Fri Oct 28 14:58:02 2011 +0100

    cogl-bitmask: Use longs instead of ints
    
    Instead of storing only 31 bits in the pointer for a CoglBitmask, it
    now assumes it can store a whole unsigned long minus the one bit used
    to mark whether it has been converted to a GArray or not. This works
    on the assumption that we can cast an unsigned long to a pointer and
    back without losing information which I think should be true for any
    platforms that Cogl is interested in. This has the advantage that on
    64-bit architectures we can store 63 bits before we have to resort to
    using a GArray at no extra cost. The values in the GArray are now
    stored as unsigned longs as well on the assumption that it is more
    efficient to load and store data in chunks of longs rather than ints.
    
    Reviewed-by: Robert Bragg <robert linux intel com>

 cogl/cogl-bitmask.c |   90 +++++++++++++++++++++++++++++---------------------
 cogl/cogl-bitmask.h |   45 ++++++++++++++++---------
 2 files changed, 80 insertions(+), 55 deletions(-)
---
diff --git a/cogl/cogl-bitmask.c b/cogl/cogl-bitmask.c
index 568d549..0e47d4f 100644
--- a/cogl/cogl-bitmask.c
+++ b/cogl/cogl-bitmask.c
@@ -33,6 +33,17 @@
 
 #include "cogl-bitmask.h"
 
+/* This code assumes that we can cast an unsigned long to a pointer
+   and back without losing any data */
+G_STATIC_ASSERT (sizeof (unsigned long) <= sizeof (void *));
+
+#define ARRAY_INDEX(bit_num) \
+  ((bit_num) / (sizeof (unsigned long) * 8))
+#define BIT_INDEX(bit_num) \
+  ((bit_num) & (sizeof (unsigned long) * 8 - 1))
+#define BIT_MASK(bit_num) \
+  (1UL << BIT_INDEX (bit_num))
+
 gboolean
 _cogl_bitmask_get_from_array (const CoglBitmask *bitmask,
                               unsigned int bit_num)
@@ -41,22 +52,23 @@ _cogl_bitmask_get_from_array (const CoglBitmask *bitmask,
 
   /* If the index is off the end of the array then assume the bit is
      not set */
-  if (bit_num >= sizeof (unsigned int) * 8 * array->len)
+  if (bit_num >= sizeof (unsigned long) * 8 * array->len)
     return FALSE;
   else
-    return !!(g_array_index (array, unsigned int,
-                             bit_num / (sizeof (unsigned int) * 8)) &
-              (1 << (bit_num % (sizeof (unsigned int) * 8))));
+    return !!(g_array_index (array, unsigned long, ARRAY_INDEX (bit_num)) &
+              BIT_MASK (bit_num));
 }
 
 static void
 _cogl_bitmask_convert_to_array (CoglBitmask *bitmask)
 {
   GArray *array;
-  /* Fetch the old values, ignoring the least significant bit */
-  unsigned int old_values = GPOINTER_TO_UINT (*bitmask) >> 1;
+  /* Fetch the old values */
+  unsigned long old_values = _cogl_bitmask_to_bits (bitmask);
 
-  array = g_array_new (FALSE, TRUE, sizeof (unsigned int));
+  array = g_array_new (FALSE, /* not zero-terminated */
+                       TRUE, /* do clear new entries */
+                       sizeof (unsigned long));
   /* Copy the old values back in */
   g_array_append_val (array, old_values);
 
@@ -69,7 +81,8 @@ _cogl_bitmask_set_in_array (CoglBitmask *bitmask,
                             gboolean value)
 {
   GArray *array;
-  unsigned int array_index, new_value_mask;
+  unsigned int array_index;
+  unsigned long new_value_mask;
 
   /* If the bitmask is not already an array then we need to allocate one */
   if (!_cogl_bitmask_has_array (bitmask))
@@ -77,17 +90,17 @@ _cogl_bitmask_set_in_array (CoglBitmask *bitmask,
 
   array = (GArray *) *bitmask;
 
-  array_index = bit_num / (sizeof (unsigned int) * 8);
+  array_index = ARRAY_INDEX (bit_num);
   /* Grow the array if necessary. This will clear the new data */
   if (array_index >= array->len)
     g_array_set_size (array, array_index + 1);
 
-  new_value_mask = 1 << (bit_num % (sizeof (unsigned int) * 8));
+  new_value_mask = BIT_MASK (bit_num);
 
   if (value)
-    g_array_index (array, unsigned int, array_index) |= new_value_mask;
+    g_array_index (array, unsigned long, array_index) |= new_value_mask;
   else
-    g_array_index (array, unsigned int, array_index) &= ~new_value_mask;
+    g_array_index (array, unsigned long, array_index) &= ~new_value_mask;
 }
 
 void
@@ -109,8 +122,8 @@ _cogl_bitmask_set_bits (CoglBitmask *dst,
         g_array_set_size (dst_array, src_array->len);
 
       for (i = 0; i < src_array->len; i++)
-        g_array_index (dst_array, unsigned int, i) |=
-          g_array_index (src_array, unsigned int, i);
+        g_array_index (dst_array, unsigned long, i) |=
+          g_array_index (src_array, unsigned long, i);
     }
   else if (_cogl_bitmask_has_array (dst))
     {
@@ -118,12 +131,12 @@ _cogl_bitmask_set_bits (CoglBitmask *dst,
 
       dst_array = (GArray *) *dst;
 
-      g_array_index (dst_array, unsigned int, 0) |=
-        (GPOINTER_TO_UINT (*src) >> 1);
+      g_array_index (dst_array, unsigned long, 0) |=
+        _cogl_bitmask_to_bits (src);
     }
   else
-    *dst = GUINT_TO_POINTER (GPOINTER_TO_UINT (*dst) |
-                             GPOINTER_TO_UINT (*src));
+    *dst = _cogl_bitmask_from_bits (_cogl_bitmask_to_bits (dst) |
+                                    _cogl_bitmask_to_bits (src));
 }
 
 void
@@ -144,9 +157,9 @@ _cogl_bitmask_set_range_in_array (CoglBitmask *bitmask,
   array = (GArray *) *bitmask;
 
   /* Get the array index of the top most value that will be touched */
-  array_index = (n_bits - 1) / (sizeof (unsigned int) * 8);
+  array_index = ARRAY_INDEX (n_bits - 1);
   /* Get the bit index of the top most value */
-  bit_index = (n_bits - 1) % (sizeof (unsigned int) * 8);
+  bit_index = BIT_INDEX (n_bits - 1);
   /* Grow the array if necessary. This will clear the new data */
   if (array_index >= array->len)
     g_array_set_size (array, array_index + 1);
@@ -154,20 +167,19 @@ _cogl_bitmask_set_range_in_array (CoglBitmask *bitmask,
   if (value)
     {
       /* Set the bits that are touching this index */
-      g_array_index (array, unsigned int, array_index) |=
-        ~(unsigned int) 0 >> (sizeof (unsigned int) * 8 - 1 - bit_index);
+      g_array_index (array, unsigned long, array_index) |=
+        ~0UL >> (sizeof (unsigned long) * 8 - 1 - bit_index);
 
       /* Set all of the bits in any lesser indices */
-      memset (array->data, 0xff, sizeof (unsigned int) * array_index);
+      memset (array->data, 0xff, sizeof (unsigned long) * array_index);
     }
   else
     {
       /* Clear the bits that are touching this index */
-      g_array_index (array, unsigned int, array_index) &=
-        ~(unsigned int) 1 << bit_index;
+      g_array_index (array, unsigned long, array_index) &= ~1UL << bit_index;
 
       /* Clear all of the bits in any lesser indices */
-      memset (array->data, 0x00, sizeof (unsigned int) * array_index);
+      memset (array->data, 0x00, sizeof (unsigned long) * array_index);
     }
 }
 
@@ -190,8 +202,8 @@ _cogl_bitmask_xor_bits (CoglBitmask *dst,
         g_array_set_size (dst_array, src_array->len);
 
       for (i = 0; i < src_array->len; i++)
-        g_array_index (dst_array, unsigned int, i) ^=
-          g_array_index (src_array, unsigned int, i);
+        g_array_index (dst_array, unsigned long, i) ^=
+          g_array_index (src_array, unsigned long, i);
     }
   else if (_cogl_bitmask_has_array (dst))
     {
@@ -199,12 +211,12 @@ _cogl_bitmask_xor_bits (CoglBitmask *dst,
 
       dst_array = (GArray *) *dst;
 
-      g_array_index (dst_array, unsigned int, 0) ^=
-        (GPOINTER_TO_UINT (*src) >> 1);
+      g_array_index (dst_array, unsigned long, 0) ^=
+        _cogl_bitmask_to_bits (src);
     }
   else
-    *dst = GUINT_TO_POINTER ((GPOINTER_TO_UINT (*dst) ^
-                              GPOINTER_TO_UINT (*src)) | 1);
+    *dst = _cogl_bitmask_from_bits (_cogl_bitmask_to_bits (dst) ^
+                                    _cogl_bitmask_to_bits (src));
 }
 
 void
@@ -212,7 +224,7 @@ _cogl_bitmask_clear_all_in_array (CoglBitmask *bitmask)
 {
   GArray *array = (GArray *) *bitmask;
 
-  memset (array->data, 0, sizeof (unsigned int) * array->len);
+  memset (array->data, 0, sizeof (unsigned long) * array->len);
 }
 
 void
@@ -227,13 +239,15 @@ _cogl_bitmask_foreach (const CoglBitmask *bitmask,
 
       for (array_index = 0; array_index < array->len; array_index++)
         {
-          unsigned int mask = g_array_index (array, unsigned int, array_index);
+          unsigned long mask =
+            g_array_index (array, unsigned long, array_index);
           int bit = 0;
 
           while (mask)
             {
-              if (mask & 1)
-                func (array_index * sizeof (unsigned int) * 8 + bit, user_data);
+              if (mask & 1UL)
+                func (array_index * sizeof (unsigned long) * 8 + bit,
+                      user_data);
 
               bit++;
               mask >>= 1;
@@ -242,12 +256,12 @@ _cogl_bitmask_foreach (const CoglBitmask *bitmask,
     }
   else
     {
-      unsigned int mask = GPOINTER_TO_UINT (*bitmask) >> 1;
+      unsigned long mask = _cogl_bitmask_to_bits (bitmask);
       int bit = 0;
 
       while (mask)
         {
-          if (mask & 1)
+          if (mask & 1UL)
             func (bit, user_data);
 
           bit++;
diff --git a/cogl/cogl-bitmask.h b/cogl/cogl-bitmask.h
index d3c36a4..42ba356 100644
--- a/cogl/cogl-bitmask.h
+++ b/cogl/cogl-bitmask.h
@@ -36,14 +36,17 @@ G_BEGIN_DECLS
  * be allocated on the stack but it must be initialised with
  * _cogl_bitmask_init() before use and then destroyed with
  * _cogl_bitmask_destroy(). A CoglBitmask will try to avoid allocating
- * any memory unless more than 31 bits are needed.
+ * any memory unless more than the number of bits in a long - 1 bits
+ * are needed.
  *
  * Internally a CoglBitmask is a pointer. If the least significant bit
  * of the pointer is 1 then the rest of the bits are directly used as
  * part of the bitmask, otherwise it is a pointer to a GArray of
  * 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)
+ * significant bit of the address is always 0). It also assumes that
+ * the size of a pointer is always greater than or equal to the size
+ * of a long (although there is a compile time assert to verify this).
  *
  * 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
@@ -52,13 +55,23 @@ G_BEGIN_DECLS
 
 typedef struct _CoglBitmaskImaginaryType *CoglBitmask;
 
+/* These are internal helper macros */
+#define _cogl_bitmask_to_number(bitmask) \
+  ((unsigned long) (*bitmask))
+#define _cogl_bitmask_to_bits(bitmask) \
+  (_cogl_bitmask_to_number (bitmask) >> 1UL)
+/* The least significant bit is set to mark that no array has been
+   allocated yet */
+#define _cogl_bitmask_from_bits(bits) \
+  ((void *) ((((unsigned long) (bits)) << 1UL) | 1UL))
+
 /* Internal helper macro to determine whether this bitmask has a
    GArray allocated or whether the pointer is just used directly */
 #define _cogl_bitmask_has_array(bitmask) \
-  (!(GPOINTER_TO_UINT (*bitmask) & 1))
+  (!(_cogl_bitmask_to_number (bitmask) & 1UL))
 
 /* Number of bits we can use before needing to allocate an array */
-#define COGL_BITMASK_MAX_DIRECT_BITS (sizeof (unsigned int) * 8 - 1)
+#define COGL_BITMASK_MAX_DIRECT_BITS (sizeof (unsigned long) * 8 - 1)
 
 /*
  * _cogl_bitmask_init:
@@ -68,10 +81,8 @@ typedef struct _CoglBitmaskImaginaryType *CoglBitmask;
  * bitmask functions are called. Initially all of the values are
  * zero
  */
-/* Set the last significant bit to mark that no array has been
-   allocated yet */
 #define _cogl_bitmask_init(bitmask) \
-  G_STMT_START { *(bitmask) = GUINT_TO_POINTER (1); } G_STMT_END
+  G_STMT_START { *(bitmask) = _cogl_bitmask_from_bits (0); } G_STMT_END
 
 gboolean
 _cogl_bitmask_get_from_array (const CoglBitmask *bitmask,
@@ -144,7 +155,7 @@ _cogl_bitmask_get (const CoglBitmask *bitmask, unsigned int bit_num)
   else if (bit_num >= COGL_BITMASK_MAX_DIRECT_BITS)
     return FALSE;
   else
-    return !!(GPOINTER_TO_UINT (*bitmask) & (1 << (bit_num + 1)));
+    return !!(_cogl_bitmask_to_bits (bitmask) & (1UL << bit_num));
 }
 
 /*
@@ -162,11 +173,11 @@ _cogl_bitmask_set (CoglBitmask *bitmask, unsigned int bit_num, gboolean value)
       bit_num >= COGL_BITMASK_MAX_DIRECT_BITS)
     _cogl_bitmask_set_in_array (bitmask, bit_num, value);
   else if (value)
-    *bitmask = GUINT_TO_POINTER (GPOINTER_TO_UINT (*bitmask) |
-                                 (1 << (bit_num + 1)));
+    *bitmask = _cogl_bitmask_from_bits (_cogl_bitmask_to_bits (bitmask) |
+                                        (1UL << bit_num));
   else
-    *bitmask = GUINT_TO_POINTER (GPOINTER_TO_UINT (*bitmask) &
-                                 ~(1 << (bit_num + 1)));
+    *bitmask = _cogl_bitmask_from_bits (_cogl_bitmask_to_bits (bitmask) &
+                                        ~(1UL << bit_num));
 }
 
 /*
@@ -186,11 +197,11 @@ _cogl_bitmask_set_range (CoglBitmask *bitmask,
       n_bits > COGL_BITMASK_MAX_DIRECT_BITS)
     _cogl_bitmask_set_range_in_array (bitmask, n_bits, value);
   else if (value)
-    *bitmask = GUINT_TO_POINTER (GPOINTER_TO_UINT (*bitmask) |
-                                 ~(~(unsigned int) 1 << n_bits));
+    *bitmask = _cogl_bitmask_from_bits (_cogl_bitmask_to_bits (bitmask) |
+                                        ~(~0UL << n_bits));
   else
-    *bitmask = GUINT_TO_POINTER (GPOINTER_TO_UINT (*bitmask) &
-                                 ((~(unsigned int) 1 << n_bits) | 1));
+    *bitmask = _cogl_bitmask_from_bits (_cogl_bitmask_to_bits (bitmask) &
+                                        (~0UL << n_bits));
 }
 
 /*
@@ -218,7 +229,7 @@ _cogl_bitmask_clear_all (CoglBitmask *bitmask)
   if (_cogl_bitmask_has_array (bitmask))
     _cogl_bitmask_clear_all_in_array (bitmask);
   else
-    *bitmask = GUINT_TO_POINTER (1);
+    *bitmask = _cogl_bitmask_from_bits (0);
 }
 
 G_END_DECLS