Clening up GrBinHashKey. Removing unnecessary streaming capability
BUG=http://code.google.com/p/skia/issues/detail?id=278
REVIEW=http://codereview.appspot.com/4910045/
git-svn-id: http://skia.googlecode.com/svn/trunk@2136 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gpu/src/GrBinHashKey.h b/gpu/src/GrBinHashKey.h
index bae9ed4..936eab0 100644
--- a/gpu/src/GrBinHashKey.h
+++ b/gpu/src/GrBinHashKey.h
@@ -13,213 +13,77 @@
#include "GrTypes.h"
/**
- * Abstract base class that presents the building interface of GrBinHashKey.
- * This base class allows builder methods to not know the exact template
- * parameters of GrBinHashKey
- */
-class GrBinHashKeyBuilder {
-public:
- GrBinHashKeyBuilder() {}
- virtual ~GrBinHashKeyBuilder() {}
- virtual void keyData(const uint32_t *dataToAdd, size_t len) = 0;
-};
-
-/**
- * Hash function class than can take a data stream of indeterminate length.
- * It also has the ability to recieve data in several chunks (steamed). The
- * hash function used is the One-at-a-Time Hash
+ * Hash function class that can take a data chunk of any predetermined
+ * length. The hash function used is the One-at-a-Time Hash
* (http://burtleburtle.net/bob/hash/doobs.html).
- *
- * Keys are built in two passes the first pass builds the key until the
- * allocated storage for the key runs out, raw data accumulation stops, but
- * the calculation of the 32-bit hash value and total key length continue.
- * The second pass is only necessary if storage ran-out during the first pass.
- * If that is the case, the heap storage portion of the key will be
- * re-allocated so that the entire key can be stored in the second pass.
- *
- * Code for building a key:
- *
- * GrBinHashKey<MyEntryStruct, MyStackSize> MyKey;
- * while( MyKey->doPass() )
- * {
- * MyObject->buildKey(&MyKey); //invoke a builder method
- * }
- *
- * All the builder method needs to do is make calls to the keyData method to
- * append binary data to the key.
*/
-template<typename Entry, size_t StackSize>
-class GrBinHashKey : public GrBinHashKeyBuilder {
+template<typename Entry, size_t KeySize>
+class GrBinHashKey {
public:
GrBinHashKey()
- : fA(0)
- , fLength(0)
- , fHeapData(NULL)
- , fPhysicalSize(StackSize)
- , fUseHeap(false)
- , fPass(0)
+ : fHash(0)
#if GR_DEBUG
- , fIsValid(true)
+ , fIsValid(false)
#endif
{}
-private:
- // Illegal: must choose explicitly between copyAndTakeOwnership
- // and deepCopyFrom.
- // Not inheriting GrNoncopyable, because it causes very obscure compiler
- // errors with template classes, which are hard to trace back to the use
- // of assignment.
- GrBinHashKey(const GrBinHashKey<Entry, StackSize>&) {}
- GrBinHashKey<Entry, StackSize>& operator=(const GrBinHashKey<Entry,
- StackSize>&) {
- return this;
+ GrBinHashKey(const GrBinHashKey<Entry, KeySize>& other) {
+ *this = other;
}
-
-public:
- void copyAndTakeOwnership(GrBinHashKey<Entry, StackSize>& key) {
- GrAssert(key.fIsValid);
- copyFields(key);
- if (fUseHeap) {
- key.fHeapData = NULL; // ownership transfer
- }
- // Consistency Checking
- // To avoid the overhead of copying or ref-counting the dynamically
- // allocated portion of the key, we use ownership transfer
- // Key usability is only tracked in debug builds.
- GR_DEBUGCODE(key.fIsValid = false;)
- }
-
- void deepCopyFrom(const GrBinHashKey<Entry, StackSize>& key) {
- GrAssert(key.fIsValid);
- copyFields(key);
- if (fUseHeap) {
- fHeapData = reinterpret_cast<uint8_t*>(
- GrMalloc(sizeof(uint8_t) * fPhysicalSize));
- memcpy(fHeapData, key.fHeapData, fLength);
- }
+ GrBinHashKey<Entry, KeySize>& operator=(const GrBinHashKey<Entry,
+ KeySize>& other) {
+ memcpy(this, &other, sizeof(*this));
+ return *this;
}
virtual ~GrBinHashKey() {
- if (fUseHeap) {
- GrFree(fHeapData);
- }
}
- bool doPass() {
- GrAssert(fIsValid);
- if (0 == fPass) {
- fPass++;
- return true;
+ void setKeyData(const uint32_t *data) {
+ GrAssert(GrIsALIGN4(KeySize));
+ memcpy(&fData, data, KeySize);
+
+ fHash = 0;
+ size_t len = KeySize;
+ while (len >= 4) {
+ fHash += *data++;
+ fHash += (fHash << 10);
+ fHash ^= (fHash >> 6);
+ len -= 4;
}
- if (1 == fPass) {
- bool passNeeded = false;
- if (fLength > fPhysicalSize) {
- // If the first pass ran out of space the we need to
- // re-allocate and perform a second pass
- GrFree(fHeapData);
- fHeapData = reinterpret_cast<uint8_t*>(
- GrMalloc(sizeof(uint8_t) * fLength));
- fPhysicalSize = fLength;
- fUseHeap = true;
- passNeeded = true;
- fLength = 0;
- }
- fPass++;
- return passNeeded;
- }
- return false;
+ fHash += (fHash << 3);
+ fHash ^= (fHash >> 11);
+ fHash += (fHash << 15);
+#if GR_DEBUG
+ fIsValid = true;
+#endif
}
- void keyData(const uint32_t *dataToAdd, size_t len) {
- GrAssert(fIsValid);
- GrAssert(fPass);
- GrAssert(GrIsALIGN4(len));
- if (fUseHeap) {
- GrAssert(fHeapData);
- GrAssert(fLength + len <= fPhysicalSize);
- memcpy(&fHeapData[fLength], dataToAdd, len );
- } else {
- if (fLength + len <= StackSize) {
- memcpy(&fStackData[fLength], dataToAdd, len);
- } else {
- GrAssert(1 == fPass);
- }
- }
-
- fLength += len;
-
- if (1 == fPass) {
- uint32_t a = fA;
- while (len >= 4) {
- a += *dataToAdd++;
- a += (a << 10);
- a ^= (a >> 6);
- len -= 4;
- }
- a += (a << 3);
- a ^= (a >> 11);
- a += (a << 15);
-
- fA = a;
- }
- }
-
- int compare(const GrBinHashKey<Entry, StackSize>& key) const {
- GrAssert(fIsValid);
- if (fLength == key.fLength) {
- GrAssert(fUseHeap == key.fUseHeap);
- if(fUseHeap) {
- return memcmp(fHeapData, key.fHeapData, fLength);
- } else {
- return memcmp(fStackData, key.fStackData, fLength);
- }
- }
-
- return (fLength - key.fLength);
+ int compare(const GrBinHashKey<Entry, KeySize>& key) const {
+ GrAssert(fIsValid && key.fIsValid);
+ return memcmp(fData, key.fData, KeySize);
}
static bool
- EQ(const Entry& entry, const GrBinHashKey<Entry, StackSize>& key) {
+ EQ(const Entry& entry, const GrBinHashKey<Entry, KeySize>& key) {
GrAssert(key.fIsValid);
return 0 == entry.compare(key);
}
static bool
- LT(const Entry& entry, const GrBinHashKey<Entry, StackSize>& key) {
+ LT(const Entry& entry, const GrBinHashKey<Entry, KeySize>& key) {
GrAssert(key.fIsValid);
return entry.compare(key) < 0;
}
uint32_t getHash() const {
GrAssert(fIsValid);
- return fA;
+ return fHash;
}
private:
- void copyFields(const GrBinHashKey<Entry, StackSize>& src) {
- if (fUseHeap) {
- GrFree(fHeapData);
- }
- // We do a field-by-field copy because this is a non-POD
- // class, and therefore memcpy would be bad
- fA = src.fA;
- fLength = src.fLength;
- memcpy(fStackData, src.fStackData, StackSize);
- fHeapData = src.fHeapData;
- fPhysicalSize = src.fPhysicalSize;
- fUseHeap = src.fUseHeap;
- fPass = src.fPass;
- }
-
- uint32_t fA;
-
- // For accumulating the variable length binary key
- size_t fLength; // length of data accumulated so far
- uint8_t fStackData[StackSize]; //Buffer for key storage
- uint8_t* fHeapData; //Dynamically allocated extended key storage
- size_t fPhysicalSize; //Total size available for key storage
- bool fUseHeap; //Using a dynamically allocated key storage
- int fPass; //Key generation pass counter
+ uint32_t fHash;
+ uint8_t fData[KeySize]; //Buffer for key storage
#if GR_DEBUG
public:
diff --git a/gpu/src/GrGLProgram.cpp b/gpu/src/GrGLProgram.cpp
index 4699851..4cdbe7c 100644
--- a/gpu/src/GrGLProgram.cpp
+++ b/gpu/src/GrGLProgram.cpp
@@ -9,7 +9,6 @@
#include "GrGLProgram.h"
-#include "GrBinHashKey.h"
#include "GrGLConfig.h"
#include "SkTrace.h"
@@ -165,11 +164,6 @@
}
}
-void GrGLProgram::buildKey(GrBinHashKeyBuilder& key) const {
- // Add stage configuration to the key
- key.keyData(reinterpret_cast<const uint32_t*>(&fProgramDesc), sizeof(ProgramDesc));
-}
-
// assigns modulation of two vars to an output var
// vars can be vec4s or floats (or one of each)
// result is always vec4
diff --git a/gpu/src/GrGLProgram.h b/gpu/src/GrGLProgram.h
index 026f68b..ffe9f00 100644
--- a/gpu/src/GrGLProgram.h
+++ b/gpu/src/GrGLProgram.h
@@ -47,14 +47,6 @@
~GrGLProgram();
/**
- * Streams data that can uniquely identifies the generated
- * gpu program into a key, for cache indexing purposes.
- *
- * @param key The key object to receive the key data
- */
- void buildKey(GrBinHashKeyBuilder& key) const;
-
- /**
* This is the heavy initilization routine for building a GLProgram.
* The result of heavy init is not stored in datamembers of GrGLProgam,
* but in a separate cacheable container.
@@ -265,6 +257,15 @@
}; // CachedData
+ enum Constants {
+ kProgramKeySize = sizeof(ProgramDesc)
+ };
+
+ // Provide an opaque ProgramDesc
+ const uint32_t* keyData() const{
+ return reinterpret_cast<const uint32_t*>(&fProgramDesc);
+ }
+
private:
enum {
kUseUniform = 2000
diff --git a/gpu/src/GrGpuGLShaders.cpp b/gpu/src/GrGpuGLShaders.cpp
index 5afd77e..96b03da 100644
--- a/gpu/src/GrGpuGLShaders.cpp
+++ b/gpu/src/GrGpuGLShaders.cpp
@@ -24,18 +24,14 @@
private:
class Entry;
-#if GR_DEBUG
- typedef GrBinHashKey<Entry, 4> ProgramHashKey; // Flex the dynamic allocation muscle in debug
-#else
- typedef GrBinHashKey<Entry, 64> ProgramHashKey;
-#endif
+ typedef GrBinHashKey<Entry, GrGLProgram::kProgramKeySize> ProgramHashKey;
class Entry : public ::GrNoncopyable {
public:
Entry() {}
void copyAndTakeOwnership(Entry& entry) {
fProgramData.copyAndTakeOwnership(entry.fProgramData);
- fKey.copyAndTakeOwnership(entry.fKey); // ownership transfer
+ fKey = entry.fKey; // ownership transfer
fLRUStamp = entry.fLRUStamp;
}
@@ -84,9 +80,8 @@
GrGLProgram::CachedData* getProgramData(const GrGLProgram& desc) {
Entry newEntry;
- while (newEntry.fKey.doPass()) {
- desc.buildKey(newEntry.fKey);
- }
+ newEntry.fKey.setKeyData(desc.keyData());
+
Entry* entry = fHashCache.find(newEntry.fKey);
if (NULL == entry) {
if (!desc.genProgram(&newEntry.fProgramData)) {
diff --git a/gpu/src/gr_unittests.cpp b/gpu/src/gr_unittests.cpp
index de69edc..1761421 100644
--- a/gpu/src/gr_unittests.cpp
+++ b/gpu/src/gr_unittests.cpp
@@ -77,72 +77,31 @@
static void test_binHashKey()
{
- const char* testStringA = "abcdABCD";
- const char* testStringB = "abcdBBCD";
+ const char* testStringA_ = "abcdABCD";
+ const char* testStringB_ = "abcdBBCD";
+ const uint32_t* testStringA = reinterpret_cast<const uint32_t*>(testStringA_);
+ const uint32_t* testStringB = reinterpret_cast<const uint32_t*>(testStringB_);
enum {
kDataLenUsedForKey = 8
};
- typedef GrBinHashKey<BogusEntry, kDataLenUsedForKey> KeyType;
+ class Entry {};
- KeyType keyA;
- int passCnt = 0;
- while (keyA.doPass()) {
- ++passCnt;
- keyA.keyData(reinterpret_cast<const uint32_t*>(testStringA), kDataLenUsedForKey);
- }
- GrAssert(passCnt == 1); //We expect the static allocation to suffice
- GrBinHashKey<BogusEntry, kDataLenUsedForKey-1> keyBust;
- passCnt = 0;
- while (keyBust.doPass()) {
- ++passCnt;
- // Exceed static storage by 1
- keyBust.keyData(reinterpret_cast<const uint32_t*>(testStringA), kDataLenUsedForKey);
- }
- GrAssert(passCnt == 2); //We expect dynamic allocation to be necessary
- GrAssert(keyA.getHash() == keyBust.getHash());
-
- // Test that adding keyData in chunks gives
- // the same hash as with one chunk
- KeyType keyA2;
- while (keyA2.doPass()) {
- keyA2.keyData(reinterpret_cast<const uint32_t*>(testStringA), 4);
- keyA2.keyData(&reinterpret_cast<const uint32_t*>(testStringA)[4], kDataLenUsedForKey-4);
- }
+ GrBinHashKey<Entry, kDataLenUsedForKey> keyA;
+ keyA.setKeyData(testStringA);
+ // test copy constructor and comparison
+ GrBinHashKey<Entry, kDataLenUsedForKey> keyA2(keyA);
+ GrAssert(keyA.compare(keyA2) == 0);
GrAssert(keyA.getHash() == keyA2.getHash());
-
- KeyType keyB;
- while (keyB.doPass()){
- keyB.keyData(reinterpret_cast<const uint32_t*>(testStringB), kDataLenUsedForKey);
- }
+ // test re-init
+ keyA2.setKeyData(testStringA);
+ GrAssert(keyA.compare(keyA2) == 0);
+ GrAssert(keyA.getHash() == keyA2.getHash());
+ // test sorting
+ GrBinHashKey<Entry, kDataLenUsedForKey> keyB;
+ keyB.setKeyData(testStringB);
GrAssert(keyA.compare(keyB) < 0);
- GrAssert(keyA.compare(keyA2) == 0);
-
- //Test ownership tranfer and copying
- keyB.copyAndTakeOwnership(keyA);
- GrAssert(keyA.fIsValid == false);
- GrAssert(keyB.fIsValid);
- GrAssert(keyB.getHash() == keyA2.getHash());
- GrAssert(keyB.compare(keyA2) == 0);
- keyA.deepCopyFrom(keyB);
- GrAssert(keyA.fIsValid);
- GrAssert(keyB.fIsValid);
- GrAssert(keyA.getHash() == keyA2.getHash());
- GrAssert(keyA.compare(keyA2) == 0);
-
- //Test ownership tranfer and copying with key on heap
- GrBinHashKey<BogusEntry, kDataLenUsedForKey-1> keyBust2;
- keyBust2.deepCopyFrom(keyBust);
- GrAssert(keyBust.fIsValid);
- GrAssert(keyBust2.fIsValid);
- GrAssert(keyBust.getHash() == keyBust2.getHash());
- GrAssert(keyBust.compare(keyBust2) == 0);
- GrBinHashKey<BogusEntry, kDataLenUsedForKey-1> keyBust3;
- keyBust3.deepCopyFrom(keyBust);
- GrAssert(keyBust.fIsValid == false);
- GrAssert(keyBust3.fIsValid);
- GrAssert(keyBust3.getHash() == keyBust2.getHash());
- GrAssert(keyBust3.compare(keyBust2) == 0);
+ GrAssert(keyA.getHash() != keyB.getHash());
}
static void test_convex() {