Small improvements to SkBitSet:

src/utils/SkBitSet.h

 
 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #ifndef SkBitSet_DEFINED
@@ skipping 12 matching lines @@
     SkBitSet& operator=(const SkBitSet& rhs);
     bool operator==(const SkBitSet& rhs);
     bool operator!=(const SkBitSet& rhs);
 
     /** Clear all data.
      */
     void clearAll();
 
     /** Set the value of the index-th bit.
      */
-    void setBit(int index, bool value);
+    void setBit(int index, bool value) {
+        uint32_t mask = 1 << (index % 32);

[reed1, 2014/10/09 15:53:43]

nit: in skia using % 32 is far more rare than (index & 0x1F)

its also possible, since index is signed, that the two are not equivalent (in
terms of perf).

[mtklein, 2014/10/09 16:12:59]

Done.

Generated code looks fine to me both ways, but might as well use a mask. 
Probably was inlining helping here, effectively letting setBit know index is
actually unsigned.

+        if (value) {
+            *(this->internalGet(index)) |= mask;

[reed1, 2014/10/09 15:53:43]

stylistic:

uint32_t mask = ...
uint32_t* chunk = this->internalGet(index);
if (value) {
    *chunk |= mask;
} else {
    *chunk &= ~mask;
}

I just get the hebejebees seeing the same function called twice, even when I
assume that the compiler will likely only call it once...

[mtklein, 2014/10/09 16:12:59]

Done.

For what it's worth, it had better only call it once.  They're on opposite sides
of an if-statement.  (And now that it's inlined, if we call this with a
compile-time constant value, the whole if-statement compiles away.)

+        } else {
+            *(this->internalGet(index)) &= ~mask;
+        }
+    }
 
     /** Test if bit index is set.
      */
-    bool isBitSet(int index) const;
+    bool isBitSet(int index) const {
+        uint32_t mask = 1 << (index % 32);
+        return 0 != (*this->internalGet(index) & mask);

[reed1, 2014/10/09 15:53:43]

trivial nit: use SkToBool(...) instead?

[mtklein, 2014/10/09 16:12:59]

Done.

+    }
 
     /** Or bits from source.  false is returned if this doesn't have the same
      *  bit count as source.
      */
     bool orBits(const SkBitSet& source);
 
     /** Export indices of set bits to T array.
      */
     template<typename T>
     void exportTo(SkTDArray<T>* array) const {
@@ skipping 21 matching lines @@
     uint32_t* internalGet(int index) const {
         SkASSERT((size_t)index < fBitCount);
         size_t internalIndex = index / 32;
         SkASSERT(internalIndex < fDwordCount);
         return reinterpret_cast<uint32_t*>(fBitData.get()) + internalIndex;
     }
 };
 
 
 #endif