src/utils/SkBitSet: simplify

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2265623002

Review-Url: https://codereview.chromium.org/2265623002

diff --git a/gyp/utils.gypi b/gyp/utils.gypi
index bc7001a..bf7f797 100644 (file)
--- a/gyp/utils.gypi
+++ b/gyp/utils.gypi
@@ -34,7 +34,6 @@
         '<(skia_src_path)/utils/SkBase64.h',
         '<(skia_src_path)/utils/SkBitmapSourceDeserializer.cpp',
         '<(skia_src_path)/utils/SkBitmapSourceDeserializer.h',
-        '<(skia_src_path)/utils/SkBitSet.cpp',
         '<(skia_src_path)/utils/SkBitSet.h',
         '<(skia_src_path)/utils/SkBoundaryPatch.cpp',
         '<(skia_src_path)/utils/SkFrontBufferedStream.cpp',

diff --git a/src/pdf/SkPDFMakeCIDGlyphWidthsArray.cpp b/src/pdf/SkPDFMakeCIDGlyphWidthsArray.cpp
index e1bb0d2..988961e 100644 (file)
--- a/src/pdf/SkPDFMakeCIDGlyphWidthsArray.cpp
+++ b/src/pdf/SkPDFMakeCIDGlyphWidthsArray.cpp
@@ -171,7 +171,7 @@ sk_sp<SkPDFArray> SkPDFMakeCIDGlyphWidthsArray(SkGlyphCache* cache,
     // Limit the loop count to glyph id ranges provided.
     int lastIndex = num_glyphs;
     if (subset) {
-        while (!subset->isBitSet(lastIndex - 1) && lastIndex > 0) {
+        while (!subset->has(lastIndex - 1) && lastIndex > 0) {
             --lastIndex;
         }
     }
@@ -180,7 +180,7 @@ sk_sp<SkPDFArray> SkPDFMakeCIDGlyphWidthsArray(SkGlyphCache* cache,
     for (int gId = 0; gId <= lastIndex; gId++) {
         int16_t advance = kInvalidAdvance;
         if (gId < lastIndex) {
-            if (!subset || 0 == gId || subset->isBitSet(gId)) {
+            if (!subset || 0 == gId || subset->has(gId)) {
                 advance = (int16_t)cache->getGlyphIDAdvance(gId).fAdvanceX;
             } else {
                 advance = kDontCareAdvance;

diff --git a/src/utils/SkBitSet.cpp b/src/utils/SkBitSet.cpp
deleted file mode 100755 (executable)
index 4323ffb..0000000
--- a/src/utils/SkBitSet.cpp
+++ /dev/null
@@ -1,70 +0,0 @@
-/*
- * Copyright 2011 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-
-#include "SkBitSet.h"
-
-SkBitSet::SkBitSet(int numberOfBits)
-    : fBitData(nullptr), fDwordCount(0), fBitCount(numberOfBits) {
-    SkASSERT(numberOfBits > 0);
-    // Round up size to 32-bit boundary.
-    fDwordCount = (numberOfBits + 31) / 32;
-    fBitData.set(sk_calloc_throw(fDwordCount * sizeof(uint32_t)));
-}
-
-SkBitSet::SkBitSet(SkBitSet&& source)
-    : fBitData(source.fBitData.release())
-    , fDwordCount(source.fDwordCount)
-    , fBitCount(source.fBitCount) {
-    source.fDwordCount = 0;
-    source.fBitCount = 0;
-}
-
-SkBitSet& SkBitSet::operator=(SkBitSet&& rhs) {
-    if (this != &rhs) {
-        fBitCount = rhs.fBitCount;
-        fDwordCount = rhs.fDwordCount;
-        fBitData.reset();  // Free old pointer.
-        fBitData.set(rhs.fBitData.release());
-        rhs.fBitCount = 0;
-        rhs.fDwordCount = 0;
-    }
-    return *this;
-}
-
-bool SkBitSet::operator==(const SkBitSet& rhs) {
-    if (fBitCount == rhs.fBitCount) {
-        if (fBitData.get() != nullptr) {
-            return (memcmp(fBitData.get(), rhs.fBitData.get(),
-                           fDwordCount * sizeof(uint32_t)) == 0);
-        }
-        return true;
-    }
-    return false;
-}
-
-bool SkBitSet::operator!=(const SkBitSet& rhs) {
-    return !(*this == rhs);
-}
-
-void SkBitSet::clearAll() {
-    if (fBitData.get() != nullptr) {
-        sk_bzero(fBitData.get(), fDwordCount * sizeof(uint32_t));
-    }
-}
-
-bool SkBitSet::orBits(const SkBitSet& source) {
-    if (fBitCount != source.fBitCount) {
-        return false;
-    }
-    uint32_t* targetBitmap = this->internalGet(0);
-    uint32_t* sourceBitmap = source.internalGet(0);
-    for (size_t i = 0; i < fDwordCount; ++i) {
-        targetBitmap[i] |= sourceBitmap[i];
-    }
-    return true;
-}

diff --git a/src/utils/SkBitSet.h b/src/utils/SkBitSet.h
index 1e35c91..a8585a2 100644 (file)
--- a/src/utils/SkBitSet.h
+++ b/src/utils/SkBitSet.h
@@ -5,42 +5,30 @@
  * found in the LICENSE file.
  */
 
-
 #ifndef SkBitSet_DEFINED
 #define SkBitSet_DEFINED
 
-#include "SkTypes.h"
 #include "SkTDArray.h"
+#include "SkTemplates.h"
 
 class SkBitSet {
 public:
-    /** NumberOfBits must be greater than zero.
-     */
-    explicit SkBitSet(int numberOfBits);
+    explicit SkBitSet(int numberOfBits) {
+        SkASSERT(numberOfBits > 0);
+        fDwordCount = (numberOfBits + 31) / 32;  // Round up size to 32-bit boundary.
+        fBitData.reset((uint32_t*)sk_calloc_throw(fDwordCount * sizeof(uint32_t)));
+    }
+
     SkBitSet(const SkBitSet&) = delete;
-    SkBitSet(SkBitSet&&);
     SkBitSet& operator=(const SkBitSet&) = delete;
-    SkBitSet& operator=(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) {
+    /** Set the value of the index-th bit to true.  */
+    void set(int index) {
         uint32_t mask = 1 << (index & 31);
         uint32_t* chunk = this->internalGet(index);
-        if (value) {
-            *chunk |= mask;
-        } else {
-            *chunk &= ~mask;
-        }
+        SkASSERT(chunk);
+        *chunk |= mask;
     }
-    void set(int index) { this->setBit(index, true); }
 
     template<typename T>
     void setAll(T* array, int len) {
@@ -50,21 +38,13 @@ public:
         }
     }
 
-    /** Test if bit index is set.
-     */
-    bool isBitSet(int index) const {
+    bool has(int index) const {
+        const uint32_t* chunk = this->internalGet(index);
         uint32_t mask = 1 << (index & 31);
-        return SkToBool(*this->internalGet(index) & mask);
+        return chunk && SkToBool(*chunk & mask);
     }
-    bool has(int index) const { return this->isBitSet(index); }
 
-    /** 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.
-     */
+    /** Export indices of set bits to T array. */
     template<typename T>
     void exportTo(SkTDArray<T>* array) const {
         static_assert(std::is_integral<T>::value, "T is integral");
@@ -84,16 +64,15 @@ public:
     }
 
 private:
-    SkAutoFree fBitData;
-    // Dword (32-bit) count of the bitset.
-    size_t fDwordCount;
-    size_t fBitCount;
+    std::unique_ptr<uint32_t, SkFunctionWrapper<void, void, sk_free>> fBitData;
+    size_t fDwordCount;  // Dword (32-bit) count of the bitset.
 
     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;
+        if (internalIndex >= fDwordCount) {
+            return nullptr;
+        }
+        return fBitData.get() + internalIndex;
     }
 };
 

diff --git a/src/xps/SkXPSDevice.cpp b/src/xps/SkXPSDevice.cpp
index ca0ee04..db15284 100644 (file)
--- a/src/xps/SkXPSDevice.cpp
+++ b/src/xps/SkXPSDevice.cpp
@@ -2095,7 +2095,7 @@ public:
 
         XPS_GLYPH_INDEX* xpsGlyph = fXpsGlyphs->append();
         uint16_t glyphID = glyph.getGlyphID();
-        fGlyphUse->setBit(glyphID, true);
+        fGlyphUse->set(glyphID);
         xpsGlyph->index = glyphID;
         if (1 == fXpsGlyphs->count()) {
             xpsGlyph->advanceWidth = 0.0f;

diff --git a/tests/BitSetTest.cpp b/tests/BitSetTest.cpp
index 716f414..6fc8351 100644 (file)
--- a/tests/BitSetTest.cpp
+++ b/tests/BitSetTest.cpp
@@ -10,64 +10,30 @@
 
 DEF_TEST(BitSet, reporter) {
     SkBitSet set0(65536);
-    REPORTER_ASSERT(reporter, set0.isBitSet(0) == false);
-    REPORTER_ASSERT(reporter, set0.isBitSet(32767) == false);
-    REPORTER_ASSERT(reporter, set0.isBitSet(65535) == false);
-
-    SkBitSet set1(65536);
-    REPORTER_ASSERT(reporter, set0 == set1);
-
-    set0.setBit(22, true);
-    REPORTER_ASSERT(reporter, set0.isBitSet(22) == true);
-    set0.setBit(24, true);
-    REPORTER_ASSERT(reporter, set0.isBitSet(24) == true);
-    set0.setBit(35, true);  // on a different DWORD
-    REPORTER_ASSERT(reporter, set0.isBitSet(35) == true);
-    set0.setBit(22, false);
-    REPORTER_ASSERT(reporter, set0.isBitSet(22) == false);
-    REPORTER_ASSERT(reporter, set0.isBitSet(24) == true);
-    REPORTER_ASSERT(reporter, set0.isBitSet(35) == true);
+    REPORTER_ASSERT(reporter, set0.has(0) == false);
+    REPORTER_ASSERT(reporter, set0.has(32767) == false);
+    REPORTER_ASSERT(reporter, set0.has(65535) == false);
+
+    set0.set(22);
+    REPORTER_ASSERT(reporter, set0.has(22) == true);
+    set0.set(24);
+    REPORTER_ASSERT(reporter, set0.has(24) == true);
+    set0.set(35);  // on a different DWORD
+    REPORTER_ASSERT(reporter, set0.has(35) == true);
+    REPORTER_ASSERT(reporter, set0.has(24) == true);
+    REPORTER_ASSERT(reporter, set0.has(35) == true);
 
     SkTDArray<unsigned int> data;
     set0.exportTo(&data);
-    REPORTER_ASSERT(reporter, data.count() == 2);
-    REPORTER_ASSERT(reporter, data[0] == 24);
-    REPORTER_ASSERT(reporter, data[1] == 35);
-
-    set1.setBit(12345, true);
-    set1.orBits(set0);
-    REPORTER_ASSERT(reporter, set0.isBitSet(12345) == false);
-    REPORTER_ASSERT(reporter, set1.isBitSet(12345) == true);
-    REPORTER_ASSERT(reporter, set1.isBitSet(22) == false);
-    REPORTER_ASSERT(reporter, set1.isBitSet(24) == true);
-    REPORTER_ASSERT(reporter, set0.isBitSet(35) == true);
-    REPORTER_ASSERT(reporter, set1 != set0);
-
-    set1.clearAll();
-    REPORTER_ASSERT(reporter, set0.isBitSet(12345) == false);
-    REPORTER_ASSERT(reporter, set1.isBitSet(12345) == false);
-    REPORTER_ASSERT(reporter, set1.isBitSet(22) == false);
-    REPORTER_ASSERT(reporter, set1.isBitSet(24) == false);
-    REPORTER_ASSERT(reporter, set1.isBitSet(35) == false);
+    REPORTER_ASSERT(reporter, data.count() == 3);
+    REPORTER_ASSERT(reporter, data[0] == 22);
+    REPORTER_ASSERT(reporter, data[1] == 24);
+    REPORTER_ASSERT(reporter, data[2] == 35);
 
-    set1.orBits(set0);
-    REPORTER_ASSERT(reporter, set1 == set0);
-
-    SkBitSet set2(1);
-    SkBitSet set3(1);
-    SkBitSet set4(4);
-    SkBitSet set5(33);
-
-    REPORTER_ASSERT(reporter, set2 == set3);
-    REPORTER_ASSERT(reporter, set2 != set4);
-    REPORTER_ASSERT(reporter, set2 != set5);
-
-    set2.setBit(0, true);
-    REPORTER_ASSERT(reporter, set2 != set5);
-    set5.setBit(0, true);
-    REPORTER_ASSERT(reporter, set2 != set5);
-    REPORTER_ASSERT(reporter, set2 != set3);
-    set3.setBit(0, true);
-    REPORTER_ASSERT(reporter, set2 == set3);
-    set3.clearAll();
+    SkBitSet set1(65536);
+    set1.set(12345);
+    REPORTER_ASSERT(reporter, set0.has(12345) == false);
+    REPORTER_ASSERT(reporter, set1.has(12345) == true);
+    REPORTER_ASSERT(reporter, set1.has(22) == false);
+    REPORTER_ASSERT(reporter, set0.has(35) == true);
 }