From 45e571dd02cfed91b8a4c7e954b7693fd1b11617 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Thu, 26 Oct 2017 08:16:41 -0700
Subject: [PATCH 1/4] cleanup ShapeInfo::getHash()
---
libraries/physics/src/ShapeManager.cpp | 8 +-
libraries/physics/src/ShapeManager.h | 10 +-
libraries/shared/src/HashKey.cpp | 67 +++++++++++++
libraries/shared/src/HashKey.h | 52 ++++++++++
libraries/shared/src/ShapeInfo.cpp | 119 +++++------------------
libraries/shared/src/ShapeInfo.h | 6 +-
tests/physics/src/ShapeInfoTests.cpp | 129 +++++++++----------------
tests/physics/src/ShapeInfoTests.h | 4 -
8 files changed, 202 insertions(+), 193 deletions(-)
create mode 100644 libraries/shared/src/HashKey.cpp
create mode 100644 libraries/shared/src/HashKey.h
diff --git a/libraries/physics/src/ShapeManager.cpp b/libraries/physics/src/ShapeManager.cpp
index 77716f671b..97b9e5dab1 100644
--- a/libraries/physics/src/ShapeManager.cpp
+++ b/libraries/physics/src/ShapeManager.cpp
@@ -32,7 +32,7 @@ const btCollisionShape* ShapeManager::getShape(const ShapeInfo& info) {
if (info.getType() == SHAPE_TYPE_NONE) {
return nullptr;
}
- DoubleHashKey key = info.getHash();
+ HashKey key = info.getHash();
ShapeReference* shapeRef = _shapeMap.find(key);
if (shapeRef) {
shapeRef->refCount++;
@@ -50,7 +50,7 @@ const btCollisionShape* ShapeManager::getShape(const ShapeInfo& info) {
}
// private helper method
-bool ShapeManager::releaseShapeByKey(const DoubleHashKey& key) {
+bool ShapeManager::releaseShapeByKey(const HashKey& key) {
ShapeReference* shapeRef = _shapeMap.find(key);
if (shapeRef) {
if (shapeRef->refCount > 0) {
@@ -88,7 +88,7 @@ bool ShapeManager::releaseShape(const btCollisionShape* shape) {
void ShapeManager::collectGarbage() {
int numShapes = _pendingGarbage.size();
for (int i = 0; i < numShapes; ++i) {
- DoubleHashKey& key = _pendingGarbage[i];
+ HashKey& key = _pendingGarbage[i];
ShapeReference* shapeRef = _shapeMap.find(key);
if (shapeRef && shapeRef->refCount == 0) {
ShapeFactory::deleteShape(shapeRef->shape);
@@ -99,7 +99,7 @@ void ShapeManager::collectGarbage() {
}
int ShapeManager::getNumReferences(const ShapeInfo& info) const {
- DoubleHashKey key = info.getHash();
+ HashKey key = info.getHash();
const ShapeReference* shapeRef = _shapeMap.find(key);
if (shapeRef) {
return shapeRef->refCount;
diff --git a/libraries/physics/src/ShapeManager.h b/libraries/physics/src/ShapeManager.h
index ed81b5e8f8..6ec3573b53 100644
--- a/libraries/physics/src/ShapeManager.h
+++ b/libraries/physics/src/ShapeManager.h
@@ -17,7 +17,7 @@
#include <ShapeInfo.h>
-#include "DoubleHashKey.h"
+#include "HashKey.h"
class ShapeManager {
public:
@@ -41,18 +41,18 @@ public:
bool hasShape(const btCollisionShape* shape) const;
private:
- bool releaseShapeByKey(const DoubleHashKey& key);
+ bool releaseShapeByKey(const HashKey& key);
class ShapeReference {
public:
int refCount;
const btCollisionShape* shape;
- DoubleHashKey key;
+ HashKey key;
ShapeReference() : refCount(0), shape(nullptr) {}
};
- btHashMap<DoubleHashKey, ShapeReference> _shapeMap;
- btAlignedObjectArray<DoubleHashKey> _pendingGarbage;
+ btHashMap<HashKey, ShapeReference> _shapeMap;
+ btAlignedObjectArray<HashKey> _pendingGarbage;
};
#endif // hifi_ShapeManager_h
diff --git a/libraries/shared/src/HashKey.cpp b/libraries/shared/src/HashKey.cpp
new file mode 100644
index 0000000000..488eccb1bf
--- /dev/null
+++ b/libraries/shared/src/HashKey.cpp
@@ -0,0 +1,67 @@
+//
+// HashKey.cpp
+// libraries/shared/src
+//
+// Created by Andrew Meadows 2017.10.25
+// Copyright 2017 High Fidelity, Inc.
+//
+// Distributed under the Apache License, Version 2.0.
+// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#include "HashKey.h"
+
+#include "NumericalConstants.h"
+
+
+const uint8_t NUM_PRIMES = 64;
+const uint64_t PRIMES[] = {
+ 4194301UL, 4194287UL, 4194277UL, 4194271UL, 4194247UL, 4194217UL, 4194199UL, 4194191UL,
+ 4194187UL, 4194181UL, 4194173UL, 4194167UL, 4194143UL, 4194137UL, 4194131UL, 4194107UL,
+ 4194103UL, 4194023UL, 4194011UL, 4194007UL, 4193977UL, 4193971UL, 4193963UL, 4193957UL,
+ 4193939UL, 4193929UL, 4193909UL, 4193869UL, 4193807UL, 4193803UL, 4193801UL, 4193789UL,
+ 4193759UL, 4193753UL, 4193743UL, 4193701UL, 4193663UL, 4193633UL, 4193573UL, 4193569UL,
+ 4193551UL, 4193549UL, 4193531UL, 4193513UL, 4193507UL, 4193459UL, 4193447UL, 4193443UL,
+ 4193417UL, 4193411UL, 4193393UL, 4193389UL, 4193381UL, 4193377UL, 4193369UL, 4193359UL,
+ 4193353UL, 4193327UL, 4193309UL, 4193303UL, 4193297UL, 4193279UL, 4193269UL, 4193263UL
+};
+
+
+// this hash function inspired by Squirrel Eiserloh's GDC2017 talk: "Noise-Based RNG"
+uint64_t squirrel3_64(uint64_t data, uint8_t primeIndex) {
+ constexpr uint64_t BIT_NOISE1 = 2760725261486592643UL;
+ constexpr uint64_t BIT_NOISE2 = 6774464464027632833UL;
+ constexpr uint64_t BIT_NOISE3 = 5545331650366059883UL;
+
+ // blend prime numbers into the hash to prevent dupes
+ // when hashing the same set of numbers in a different order
+ uint64_t hash = PRIMES[primeIndex % NUM_PRIMES] * data;
+ hash *= BIT_NOISE1;
+ hash ^= (hash >> 16);
+ hash += BIT_NOISE2;
+ hash ^= (hash << 16);
+ hash *= BIT_NOISE3;
+ return hash ^ (hash >> 16);
+}
+
+constexpr float QUANTIZED_VALUES_PER_METER = 250.0f;
+
+// static
+float HashKey::getNumQuantizedValuesPerMeter() {
+ return QUANTIZED_VALUES_PER_METER;
+}
+
+void HashKey::hashUint64(uint64_t data) {
+ _hash += squirrel3_64(data, ++_hashCount);
+}
+
+void HashKey::hashFloat(float data) {
+ _hash += squirrel3_64((uint64_t)((int64_t)(data * QUANTIZED_VALUES_PER_METER)), ++_hashCount);
+}
+
+void HashKey::hashVec3(const glm::vec3& data) {
+ _hash += squirrel3_64((uint64_t)((int64_t)(data[0] * QUANTIZED_VALUES_PER_METER)), ++_hashCount);
+ _hash *= squirrel3_64((uint64_t)((int64_t)(data[1] * QUANTIZED_VALUES_PER_METER)), ++_hashCount);
+ _hash ^= squirrel3_64((uint64_t)((int64_t)(data[2] * QUANTIZED_VALUES_PER_METER)), ++_hashCount);
+}
+
diff --git a/libraries/shared/src/HashKey.h b/libraries/shared/src/HashKey.h
new file mode 100644
index 0000000000..5fce182084
--- /dev/null
+++ b/libraries/shared/src/HashKey.h
@@ -0,0 +1,52 @@
+//
+// HashKey.h
+// libraries/shared/src
+//
+// Created by Andrew Meadows 2017.10.25
+// Copyright 2017 High Fidelity, Inc.
+//
+// Distributed under the Apache License, Version 2.0.
+// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#ifndef hifi_HashKey_h
+#define hifi_HashKey_h
+
+#include <cstddef>
+#include <stdint.h>
+
+#include <glm/glm.hpp>
+
+
+// HashKey for use with btHashMap which requires a particular API for its keys. In particular it
+// requires its Key to implement these methods:
+//
+// bool Key::equals()
+// int32_t Key::getHash()
+//
+// The important thing about the HashKey implementation is that while getHash() returns 32-bits,
+// internally HashKey stores a 64-bit hash which is used for the equals() comparison. This allows
+// btHashMap to insert "dupe" 32-bit keys to different "values".
+
+class HashKey {
+public:
+ static float getNumQuantizedValuesPerMeter();
+
+ // These two methods are required by btHashMap.
+ bool equals(const HashKey& other) const { return _hash == other._hash; }
+ int32_t getHash() const { return (int32_t)((uint32_t)_hash); }
+
+ void clear() { _hash = _hashCount = 0; }
+ bool isNull() const { return _hash == 0 && _hashCount == 0; }
+ void hashUint64(uint64_t data);
+ void hashFloat(float data);
+ void hashVec3(const glm::vec3& data);
+
+ uint64_t getHash64() const { return _hash; } // for debug/test purposes
+
+private:
+ uint64_t _hash { 0 };
+ uint8_t _hashCount { 0 };
+};
+
+#endif // hifi_HashKey_h
diff --git a/libraries/shared/src/ShapeInfo.cpp b/libraries/shared/src/ShapeInfo.cpp
index 36ce38335a..8cdc4bcf14 100644
--- a/libraries/shared/src/ShapeInfo.cpp
+++ b/libraries/shared/src/ShapeInfo.cpp
@@ -53,7 +53,7 @@ void ShapeInfo::clear() {
_triangleIndices.clear();
_halfExtents = glm::vec3(0.0f);
_offset = glm::vec3(0.0f);
- _doubleHashKey.clear();
+ _hashKey.clear();
_type = SHAPE_TYPE_NONE;
}
@@ -87,14 +87,14 @@ void ShapeInfo::setParams(ShapeType type, const glm::vec3& halfExtents, QString
default:
break;
}
- _doubleHashKey.clear();
+ _hashKey.clear();
}
void ShapeInfo::setBox(const glm::vec3& halfExtents) {
_url = "";
_type = SHAPE_TYPE_BOX;
setHalfExtents(halfExtents);
- _doubleHashKey.clear();
+ _hashKey.clear();
}
void ShapeInfo::setSphere(float radius) {
@@ -102,12 +102,12 @@ void ShapeInfo::setSphere(float radius) {
_type = SHAPE_TYPE_SPHERE;
radius = glm::max(radius, MIN_HALF_EXTENT);
_halfExtents = glm::vec3(radius);
- _doubleHashKey.clear();
+ _hashKey.clear();
}
void ShapeInfo::setPointCollection(const ShapeInfo::PointCollection& pointCollection) {
_pointCollection = pointCollection;
- _doubleHashKey.clear();
+ _hashKey.clear();
}
void ShapeInfo::setCapsuleY(float radius, float halfHeight) {
@@ -116,12 +116,12 @@ void ShapeInfo::setCapsuleY(float radius, float halfHeight) {
radius = glm::max(radius, MIN_HALF_EXTENT);
halfHeight = glm::max(halfHeight, 0.0f);
_halfExtents = glm::vec3(radius, halfHeight, radius);
- _doubleHashKey.clear();
+ _hashKey.clear();
}
void ShapeInfo::setOffset(const glm::vec3& offset) {
_offset = offset;
- _doubleHashKey.clear();
+ _hashKey.clear();
}
uint32_t ShapeInfo::getNumSubShapes() const {
@@ -256,119 +256,46 @@ bool ShapeInfo::contains(const glm::vec3& point) const {
}
}
-const DoubleHashKey& ShapeInfo::getHash() const {
+const HashKey& ShapeInfo::getHash() const {
// NOTE: we cache the key so we only ever need to compute it once for any valid ShapeInfo instance.
- if (_doubleHashKey.isNull() && _type != SHAPE_TYPE_NONE) {
- bool useOffset = glm::length2(_offset) > MIN_SHAPE_OFFSET * MIN_SHAPE_OFFSET;
+ if (_hashKey.isNull() && _type != SHAPE_TYPE_NONE) {
// The key is not yet cached therefore we must compute it.
- // compute hash1
- // TODO?: provide lookup table for hash/hash2 of _type rather than recompute?
- uint32_t primeIndex = 0;
- _doubleHashKey.computeHash((uint32_t)_type, primeIndex++);
-
+ _hashKey.hashUint64((uint64_t)_type);
if (_type != SHAPE_TYPE_SIMPLE_HULL) {
- // compute hash1
- uint32_t hash = _doubleHashKey.getHash();
- for (int j = 0; j < 3; ++j) {
- // NOTE: 0.49f is used to bump the float up almost half a millimeter
- // so the cast to int produces a round() effect rather than a floor()
- hash ^= DoubleHashKey::hashFunction(
- (uint32_t)(_halfExtents[j] * MILLIMETERS_PER_METER + copysignf(1.0f, _halfExtents[j]) * 0.49f),
- primeIndex++);
- if (useOffset) {
- hash ^= DoubleHashKey::hashFunction(
- (uint32_t)(_offset[j] * MILLIMETERS_PER_METER + copysignf(1.0f, _offset[j]) * 0.49f),
- primeIndex++);
- }
- }
- _doubleHashKey.setHash(hash);
-
- // compute hash2
- hash = _doubleHashKey.getHash2();
- for (int j = 0; j < 3; ++j) {
- // NOTE: 0.49f is used to bump the float up almost half a millimeter
- // so the cast to int produces a round() effect rather than a floor()
- uint32_t floatHash = DoubleHashKey::hashFunction2(
- (uint32_t)(_halfExtents[j] * MILLIMETERS_PER_METER + copysignf(1.0f, _halfExtents[j]) * 0.49f));
- if (useOffset) {
- floatHash ^= DoubleHashKey::hashFunction2(
- (uint32_t)(_offset[j] * MILLIMETERS_PER_METER + copysignf(1.0f, _offset[j]) * 0.49f));
- }
- hash += ~(floatHash << 17);
- hash ^= (floatHash >> 11);
- hash += (floatHash << 4);
- hash ^= (floatHash >> 7);
- hash += ~(floatHash << 10);
- hash = (hash << 16) | (hash >> 16);
- }
- _doubleHashKey.setHash2(hash);
+ _hashKey.hashVec3(_halfExtents);
+ _hashKey.hashVec3(_offset);
} else {
-
+ // TODO: we could avoid hashing all of these points if we were to supply the ShapeInfo with a unique
+ // descriptive string. Shapes that are uniquely described by their type and URL could just put their
+ // url in the description.
assert(_pointCollection.size() == (size_t)1);
const PointList & points = _pointCollection.back();
const int numPoints = (int)points.size();
- uint32_t hash = _doubleHashKey.getHash();
- uint32_t hash2 = _doubleHashKey.getHash2();
- for (int pointIndex = 0; pointIndex < numPoints; ++pointIndex) {
- // compute hash1 & 2
- const glm::vec3 &curPoint = points[pointIndex];
- for (int vecCompIndex = 0; vecCompIndex < 3; ++vecCompIndex) {
-
- // NOTE: 0.49f is used to bump the float up almost half a millimeter
- // so the cast to int produces a round() effect rather than a floor()
- uint32_t valueToHash = (uint32_t)(curPoint[vecCompIndex] * MILLIMETERS_PER_METER + copysignf(1.0f, curPoint[vecCompIndex]) * 0.49f);
-
- hash ^= DoubleHashKey::hashFunction(valueToHash, primeIndex++);
- uint32_t floatHash = DoubleHashKey::hashFunction2(valueToHash);
-
- if (useOffset) {
-
- const uint32_t offsetValToHash = (uint32_t)(_offset[vecCompIndex] * MILLIMETERS_PER_METER + copysignf(1.0f, _offset[vecCompIndex])* 0.49f);
-
- hash ^= DoubleHashKey::hashFunction(offsetValToHash, primeIndex++);
- floatHash ^= DoubleHashKey::hashFunction2(offsetValToHash);
- }
-
- hash2 += ~(floatHash << 17);
- hash2 ^= (floatHash >> 11);
- hash2 += (floatHash << 4);
- hash2 ^= (floatHash >> 7);
- hash2 += ~(floatHash << 10);
- hash2 = (hash2 << 16) | (hash2 >> 16);
- }
+ for (int i = 0; i < numPoints; ++i) {
+ _hashKey.hashVec3(points[i]);
}
-
- _doubleHashKey.setHash(hash);
- _doubleHashKey.setHash2(hash2);
}
QString url = _url.toString();
if (!url.isEmpty()) {
- // fold the urlHash into both parts
QByteArray baUrl = url.toLocal8Bit();
uint32_t urlHash = qChecksum(baUrl.data(), baUrl.size());
- _doubleHashKey.setHash(_doubleHashKey.getHash() ^ urlHash);
- _doubleHashKey.setHash2(_doubleHashKey.getHash2() ^ urlHash);
+ _hashKey.hashUint64((uint64_t)urlHash);
}
- uint32_t numHulls = 0;
if (_type == SHAPE_TYPE_COMPOUND || _type == SHAPE_TYPE_SIMPLE_COMPOUND) {
- numHulls = (uint32_t)_pointCollection.size();
+ uint64_t numHulls = (uint64_t)_pointCollection.size();
+ _hashKey.hashUint64(numHulls);
} else if (_type == SHAPE_TYPE_SIMPLE_HULL) {
- numHulls = 1;
- }
- if (numHulls > 0) {
- uint32_t hash = DoubleHashKey::hashFunction(numHulls, primeIndex++);
- _doubleHashKey.setHash(_doubleHashKey.getHash() ^ hash);
- hash = DoubleHashKey::hashFunction2(numHulls);
- _doubleHashKey.setHash2(_doubleHashKey.getHash2() ^ hash);
+ _hashKey.hashUint64(1);
}
}
- return _doubleHashKey;
+ return _hashKey;
}
void ShapeInfo::setHalfExtents(const glm::vec3& halfExtents) {
_halfExtents = glm::max(halfExtents, glm::vec3(MIN_HALF_EXTENT));
+ _hashKey.clear();
}
diff --git a/libraries/shared/src/ShapeInfo.h b/libraries/shared/src/ShapeInfo.h
index d658b936a3..069241e29d 100644
--- a/libraries/shared/src/ShapeInfo.h
+++ b/libraries/shared/src/ShapeInfo.h
@@ -18,7 +18,7 @@
#include <glm/glm.hpp>
#include <glm/gtx/norm.hpp>
-#include "DoubleHashKey.h"
+#include "HashKey.h"
const float MIN_SHAPE_OFFSET = 0.001f; // offsets less than 1mm will be ignored
@@ -89,7 +89,7 @@ public:
/// For compound shapes it will only return whether it is inside the bounding box
bool contains(const glm::vec3& point) const;
- const DoubleHashKey& getHash() const;
+ const HashKey& getHash() const;
protected:
void setHalfExtents(const glm::vec3& halfExtents);
@@ -99,7 +99,7 @@ protected:
TriangleIndices _triangleIndices;
glm::vec3 _halfExtents = glm::vec3(0.0f);
glm::vec3 _offset = glm::vec3(0.0f);
- mutable DoubleHashKey _doubleHashKey;
+ mutable HashKey _hashKey;
ShapeType _type = SHAPE_TYPE_NONE;
};
diff --git a/tests/physics/src/ShapeInfoTests.cpp b/tests/physics/src/ShapeInfoTests.cpp
index c6a19084a2..79d0092dc3 100644
--- a/tests/physics/src/ShapeInfoTests.cpp
+++ b/tests/physics/src/ShapeInfoTests.cpp
@@ -14,7 +14,7 @@
#include <btBulletDynamicsCommon.h>
#include <LinearMath/btHashMap.h>
-#include <DoubleHashKey.h>
+#include <HashKey.h>
#include <ShapeInfo.h>
#include <ShapeFactory.h>
#include <StreamUtils.h>
@@ -23,108 +23,78 @@
QTEST_MAIN(ShapeInfoTests)
+// Enable this to manually run testHashCollisions
+// (NOT a regular unit test; takes ~40 secs to run on an i7)
+//#define MANUAL_TEST true
+
void ShapeInfoTests::testHashFunctions() {
#if MANUAL_TEST
int maxTests = 10000000;
ShapeInfo info;
- btHashMap<btHashInt, uint32_t> hashes;
+ btHashMap<HashKey, int32_t> hashes;
- uint32_t bits[32];
- uint32_t masks[32];
- for (int i = 0; i < 32; ++i) {
+ const int32_t NUM_HASH_BITS = 32;
+ uint32_t bits[NUM_HASH_BITS];
+ uint32_t masks[NUM_HASH_BITS];
+ for (int i = 0; i < NUM_HASH_BITS; ++i) {
bits[i] = 0;
- masks[i] = 1U << i;
+ masks[i] = 1UL << i;
}
- float deltaLength = 0.002f;
- float endLength = 100.0f;
+ float deltaLength = 1.0f / (HashKey::getNumQuantizedValuesPerMeter() - 3.0f);
+ float endLength = 2000.0f * deltaLength;
int numSteps = (int)(endLength / deltaLength);
int testCount = 0;
int numCollisions = 0;
btClock timer;
- for (int x = 1; x < numSteps && testCount < maxTests; ++x) {
- float radiusX = (float)x * deltaLength;
+ for (int i = 1; i < numSteps && testCount < maxTests; ++i) {
+ float radiusX = (float)i * deltaLength;
+ int32_t* hashPtr;
// test sphere
info.setSphere(radiusX);
++testCount;
- DoubleHashKey key = info.getHash();
- uint32_t* hashPtr = hashes.find(key.getHash());
- if (hashPtr && *hashPtr == key.getHash2()) {
- std::cout << testCount << " hash collision radiusX = " << radiusX
- << " h1 = 0x" << std::hex << key.getHash()
- << " h2 = 0x" << std::hex << key.getHash2()
- << std::endl;
+ HashKey key = info.getHash();
+ hashPtr = hashes.find(key);
+ if (hashPtr) {
+ std::cout << testCount << " hash collision sphere radius = " << radiusX
+ << " h = 0x" << std::hex << key.getHash() << " : 0x" << *hashPtr
+ << std::dec << std::endl;
++numCollisions;
assert(false);
} else {
- hashes.insert(key.getHash(), key.getHash2());
+ hashes.insert(key, key.getHash());
}
- for (int k = 0; k < 32; ++k) {
- if (masks[k] & key.getHash2()) {
- ++bits[k];
+ // track bit distribution counts to evaluate hash function randomness
+ for (int j = 0; j < NUM_HASH_BITS; ++j) {
+ if (masks[j] & key.getHash()) {
+ ++bits[j];
}
}
for (int y = 1; y < numSteps && testCount < maxTests; ++y) {
float radiusY = (float)y * deltaLength;
- /* TODO: reimplement Cylinder and Capsule shapes
- // test cylinder and capsule
- int types[] = { CYLINDER_SHAPE_PROXYTYPE, CAPSULE_SHAPE_PROXYTYPE };
- for (int i = 0; i < 2; ++i) {
- switch(types[i]) {
- case CYLINDER_SHAPE_PROXYTYPE: {
- info.setCylinder(radiusX, radiusY);
- break;
- }
- case CAPSULE_SHAPE_PROXYTYPE: {
- info.setCapsuleY(radiusX, radiusY);
- break;
- }
- }
-
- ++testCount;
- key = info.getHash();
- hashPtr = hashes.find(key.getHash());
- if (hashPtr && *hashPtr == key.getHash2()) {
- std::cout << testCount << " hash collision radiusX = " << radiusX << " radiusY = " << radiusY
- << " h1 = 0x" << std::hex << key.getHash()
- << " h2 = 0x" << std::hex << key.getHash2()
- << std::endl;
- ++numCollisions;
- assert(false);
- } else {
- hashes.insert(key.getHash(), key.getHash2());
- }
- for (int k = 0; k < 32; ++k) {
- if (masks[k] & key.getHash2()) {
- ++bits[k];
- }
- }
- }
- */
-
for (int z = 1; z < numSteps && testCount < maxTests; ++z) {
float radiusZ = (float)z * deltaLength;
// test box
info.setBox(glm::vec3(radiusX, radiusY, radiusZ));
++testCount;
- DoubleHashKey key = info.getHash();
- hashPtr = hashes.find(key.getHash());
- if (hashPtr && *hashPtr == key.getHash2()) {
- std::cout << testCount << " hash collision radiusX = " << radiusX
- << " radiusY = " << radiusY << " radiusZ = " << radiusZ
- << " h1 = 0x" << std::hex << key.getHash()
- << " h2 = 0x" << std::hex << key.getHash2()
- << std::endl;
+ HashKey key = info.getHash();
+ hashPtr = hashes.find(key);
+ if (hashPtr) {
+ std::cout << testCount << " hash collision box dimensions = < " << radiusX
+ << ", " << radiusY << ", " << radiusZ << " >"
+ << " h = 0x" << std::hex << key.getHash() << " : 0x" << *hashPtr << " : 0x" << key.getHash64()
+ << std::dec << std::endl;
++numCollisions;
assert(false);
} else {
- hashes.insert(key.getHash(), key.getHash2());
+ hashes.insert(key, key.getHash());
}
- for (int k = 0; k < 32; ++k) {
- if (masks[k] & key.getHash2()) {
+ // track bit distribution counts to evaluate hash function randomness
+ for (int k = 0; k < NUM_HASH_BITS; ++k) {
+ if (masks[k] & key.getHash()) {
++bits[k];
}
}
@@ -135,7 +105,8 @@ void ShapeInfoTests::testHashFunctions() {
std::cout << msec << " msec with " << numCollisions << " collisions out of " << testCount << " hashes" << std::endl;
// print out distribution of bits
- for (int i = 0; i < 32; ++i) {
+ // ideally the numbers in each bin will be about the same
+ for (int i = 0; i < NUM_HASH_BITS; ++i) {
std::cout << "bit 0x" << std::hex << masks[i] << std::dec << " = " << bits[i] << std::endl;
}
QCOMPARE(numCollisions, 0);
@@ -146,15 +117,14 @@ void ShapeInfoTests::testBoxShape() {
ShapeInfo info;
glm::vec3 halfExtents(1.23f, 4.56f, 7.89f);
info.setBox(halfExtents);
- DoubleHashKey key = info.getHash();
+ HashKey key = info.getHash();
const btCollisionShape* shape = ShapeFactory::createShapeFromInfo(info);
QCOMPARE(shape != nullptr, true);
ShapeInfo otherInfo = info;
- DoubleHashKey otherKey = otherInfo.getHash();
+ HashKey otherKey = otherInfo.getHash();
QCOMPARE(key.getHash(), otherKey.getHash());
- QCOMPARE(key.getHash2(), otherKey.getHash2());
delete shape;
}
@@ -163,15 +133,14 @@ void ShapeInfoTests::testSphereShape() {
ShapeInfo info;
float radius = 1.23f;
info.setSphere(radius);
- DoubleHashKey key = info.getHash();
+ HashKey key = info.getHash();
const btCollisionShape* shape = ShapeFactory::createShapeFromInfo(info);
QCOMPARE(shape != nullptr, true);
ShapeInfo otherInfo = info;
- DoubleHashKey otherKey = otherInfo.getHash();
+ HashKey otherKey = otherInfo.getHash();
QCOMPARE(key.getHash(), otherKey.getHash());
- QCOMPARE(key.getHash2(), otherKey.getHash2());
delete shape;
}
@@ -182,15 +151,14 @@ void ShapeInfoTests::testCylinderShape() {
float radius = 1.23f;
float height = 4.56f;
info.setCylinder(radius, height);
- DoubleHashKey key = info.getHash();
+ HashKey key = info.getHash();
btCollisionShape* shape = ShapeFactory::createShapeFromInfo(info);
QCOMPARE(shape != nullptr, true);
ShapeInfo otherInfo = info;
- DoubleHashKey otherKey = otherInfo.getHash();
+ HashKey otherKey = otherInfo.getHash();
QCOMPARE(key.getHash(), otherKey.getHash());
- QCOMPARE(key.getHash2(), otherKey.getHash2());
delete shape;
*/
@@ -202,15 +170,14 @@ void ShapeInfoTests::testCapsuleShape() {
float radius = 1.23f;
float height = 4.56f;
info.setCapsule(radius, height);
- DoubleHashKey key = info.getHash();
+ HashKey key = info.getHash();
btCollisionShape* shape = ShapeFactory::createShapeFromInfo(info);
QCOMPARE(shape != nullptr, true);
ShapeInfo otherInfo = info;
- DoubleHashKey otherKey = otherInfo.getHash();
+ HashKey otherKey = otherInfo.getHash();
QCOMPARE(key.getHash(), otherKey.getHash());
- QCOMPARE(key.getHash2(), otherKey.getHash2());
delete shape;
*/
diff --git a/tests/physics/src/ShapeInfoTests.h b/tests/physics/src/ShapeInfoTests.h
index fbd89a13a8..1f6054dd1a 100644
--- a/tests/physics/src/ShapeInfoTests.h
+++ b/tests/physics/src/ShapeInfoTests.h
@@ -18,10 +18,6 @@
//#include "BulletTestUtils.h"
//#include "../QTestExtensions.h"
-// Enable this to manually run testHashCollisions
-// (NOT a regular unit test; takes ~17 secs to run on an i7)
-#define MANUAL_TEST false
-
class ShapeInfoTests : public QObject {
Q_OBJECT
private slots:
From b00d15834255f9ad1f8d1e6359e104efbfb72698 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Thu, 26 Oct 2017 09:00:42 -0700
Subject: [PATCH 2/4] remove DoubleHashKey: it is no longer used
---
libraries/shared/src/DoubleHashKey.cpp | 48 -------------------------
libraries/shared/src/DoubleHashKey.h | 49 --------------------------
2 files changed, 97 deletions(-)
delete mode 100644 libraries/shared/src/DoubleHashKey.cpp
delete mode 100644 libraries/shared/src/DoubleHashKey.h
diff --git a/libraries/shared/src/DoubleHashKey.cpp b/libraries/shared/src/DoubleHashKey.cpp
deleted file mode 100644
index ded2f073eb..0000000000
--- a/libraries/shared/src/DoubleHashKey.cpp
+++ /dev/null
@@ -1,48 +0,0 @@
-//
-// DoubleHashKey.cpp
-// libraries/shared/src
-//
-// Created by Andrew Meadows 2014.11.02
-// Copyright 2014 High Fidelity, Inc.
-//
-// Distributed under the Apache License, Version 2.0.
-// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
-//
-
-#include "DoubleHashKey.h"
-
-const uint32_t NUM_PRIMES = 64;
-const uint32_t PRIMES[] = {
- 4194301U, 4194287U, 4194277U, 4194271U, 4194247U, 4194217U, 4194199U, 4194191U,
- 4194187U, 4194181U, 4194173U, 4194167U, 4194143U, 4194137U, 4194131U, 4194107U,
- 4194103U, 4194023U, 4194011U, 4194007U, 4193977U, 4193971U, 4193963U, 4193957U,
- 4193939U, 4193929U, 4193909U, 4193869U, 4193807U, 4193803U, 4193801U, 4193789U,
- 4193759U, 4193753U, 4193743U, 4193701U, 4193663U, 4193633U, 4193573U, 4193569U,
- 4193551U, 4193549U, 4193531U, 4193513U, 4193507U, 4193459U, 4193447U, 4193443U,
- 4193417U, 4193411U, 4193393U, 4193389U, 4193381U, 4193377U, 4193369U, 4193359U,
- 4193353U, 4193327U, 4193309U, 4193303U, 4193297U, 4193279U, 4193269U, 4193263U
-};
-
-uint32_t DoubleHashKey::hashFunction(uint32_t value, uint32_t primeIndex) {
- uint32_t hash = PRIMES[primeIndex % NUM_PRIMES] * (value + 1U);
- hash += ~(hash << 15);
- hash ^= (hash >> 10);
- hash += (hash << 3);
- hash ^= (hash >> 6);
- hash += ~(hash << 11);
- return hash ^ (hash >> 16);
-}
-
-uint32_t DoubleHashKey::hashFunction2(uint32_t value) {
- uint32_t hash = 0x811c9dc5U;
- for (uint32_t i = 0; i < 4; i++ ) {
- uint32_t byte = (value << (i * 8)) >> (24 - i * 8);
- hash = ( hash ^ byte ) * 0x01000193U;
- }
- return hash;
-}
-
-void DoubleHashKey::computeHash(uint32_t value, uint32_t primeIndex) {
- _hash = DoubleHashKey::hashFunction(value, primeIndex);
- _hash2 = DoubleHashKey::hashFunction2(value);
-}
diff --git a/libraries/shared/src/DoubleHashKey.h b/libraries/shared/src/DoubleHashKey.h
deleted file mode 100644
index ca92a7197f..0000000000
--- a/libraries/shared/src/DoubleHashKey.h
+++ /dev/null
@@ -1,49 +0,0 @@
-//
-// DoubleHashKey.h
-// libraries/shared/src
-//
-// Created by Andrew Meadows 2014.11.02
-// Copyright 2014 High Fidelity, Inc.
-//
-// Distributed under the Apache License, Version 2.0.
-// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
-//
-
-#ifndef hifi_DoubleHashKey_h
-#define hifi_DoubleHashKey_h
-
-#include <stdint.h>
-
-// DoubleHashKey for use with btHashMap
-class DoubleHashKey {
-public:
- static uint32_t hashFunction(uint32_t value, uint32_t primeIndex);
- static uint32_t hashFunction2(uint32_t value);
-
- DoubleHashKey() : _hash(0), _hash2(0) { }
-
- DoubleHashKey(uint32_t value, uint32_t primeIndex = 0) :
- _hash(hashFunction(value, primeIndex)),
- _hash2(hashFunction2(value)) {
- }
-
- void clear() { _hash = 0; _hash2 = 0; }
- bool isNull() const { return _hash == 0 && _hash2 == 0; }
-
- bool equals(const DoubleHashKey& other) const {
- return _hash == other._hash && _hash2 == other._hash2;
- }
-
- void computeHash(uint32_t value, uint32_t primeIndex = 0);
- uint32_t getHash() const { return _hash; }
- uint32_t getHash2() const { return _hash2; }
-
- void setHash(uint32_t hash) { _hash = hash; }
- void setHash2(uint32_t hash2) { _hash2 = hash2; }
-
-private:
- uint32_t _hash;
- uint32_t _hash2;
-};
-
-#endif // hifi_DoubleHashKey_h
From d45febf1db706a7f8855004818816d9220a9f823 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 27 Oct 2017 08:55:41 -0700
Subject: [PATCH 3/4] add description of ShapeManager in comments
---
libraries/physics/src/ShapeManager.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/libraries/physics/src/ShapeManager.h b/libraries/physics/src/ShapeManager.h
index 6ec3573b53..d75bb1dc4a 100644
--- a/libraries/physics/src/ShapeManager.h
+++ b/libraries/physics/src/ShapeManager.h
@@ -19,6 +19,28 @@
#include "HashKey.h"
+// The ShapeManager handles the ref-counting on shared shapes:
+//
+// Each object added to the physics simulation gets a corresponding btRigidBody.
+// The body has a btCollisionShape that represents the contours of its collision
+// surface. Multiple bodies may have the same shape. Rather than create a unique
+// btCollisionShape instance for every body with a particular shape we can instead
+// use a single shape instance for all of the bodies. This is called "shape
+// sharing".
+//
+// When body needs a new shape a description of ths shape (ShapeInfo) is assembled
+// and a request is sent to the ShapeManager for a corresponding btCollisionShape
+// pointer. The ShapeManager will compute a hash of the ShapeInfo's data and use
+// that to find the shape in its map. If it finds one it increments the ref-count
+// and returns the pointer. If not it asks the ShapeFactory to create it, adds an
+// entry in the map with a ref-count of 1, and returns the pointer.
+//
+// When a body stops using a shape the ShapeManager must be informed so it can
+// decrement its ref-count. When a ref-count drops to zero the ShapeManager
+// doesn't delete it right away. Instead it puts the shape's key on a list delete
+// later. When that list grows big enough the ShapeManager will remove any matching
+// entries that still have zero ref-count.
+
class ShapeManager {
public:
@@ -51,6 +73,7 @@ private:
ShapeReference() : refCount(0), shape(nullptr) {}
};
+ // btHashMap is required because it supports memory alignment of the btCollisionShapes
btHashMap<HashKey, ShapeReference> _shapeMap;
btAlignedObjectArray<HashKey> _pendingGarbage;
};
From 137fccbd91b46c20fddc891e52915824bd440f72 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 27 Oct 2017 08:56:26 -0700
Subject: [PATCH 4/4] cleanup ShapeFactory implementation
---
libraries/physics/src/ShapeFactory.cpp | 63 ++++++++++++++------------
libraries/physics/src/ShapeFactory.h | 16 +------
2 files changed, 35 insertions(+), 44 deletions(-)
diff --git a/libraries/physics/src/ShapeFactory.cpp b/libraries/physics/src/ShapeFactory.cpp
index cd0fba848a..f484f32fdf 100644
--- a/libraries/physics/src/ShapeFactory.cpp
+++ b/libraries/physics/src/ShapeFactory.cpp
@@ -16,6 +16,40 @@
#include "ShapeFactory.h"
#include "BulletUtil.h"
+
+class StaticMeshShape : public btBvhTriangleMeshShape {
+public:
+ StaticMeshShape() = delete;
+
+ StaticMeshShape(btTriangleIndexVertexArray* dataArray)
+ : btBvhTriangleMeshShape(dataArray, true), _dataArray(dataArray) {
+ assert(_dataArray);
+ }
+
+ ~StaticMeshShape() {
+ assert(_dataArray);
+ IndexedMeshArray& meshes = _dataArray->getIndexedMeshArray();
+ for (int32_t i = 0; i < meshes.size(); ++i) {
+ btIndexedMesh mesh = meshes[i];
+ mesh.m_numTriangles = 0;
+ delete [] mesh.m_triangleIndexBase;
+ mesh.m_triangleIndexBase = nullptr;
+ mesh.m_numVertices = 0;
+ delete [] mesh.m_vertexBase;
+ mesh.m_vertexBase = nullptr;
+ }
+ meshes.clear();
+ delete _dataArray;
+ _dataArray = nullptr;
+ }
+
+private:
+ // the StaticMeshShape owns its vertex/index data
+ btTriangleIndexVertexArray* _dataArray;
+};
+
+// the dataArray must be created before we create the StaticMeshShape
+
// These are the same normalized directions used by the btShapeHull class.
// 12 points for the face centers of a dodecahedron plus another 30 points
// for the midpoints the edges, for a total of 42.
@@ -230,23 +264,6 @@ btTriangleIndexVertexArray* createStaticMeshArray(const ShapeInfo& info) {
return dataArray;
}
-// util method
-void deleteStaticMeshArray(btTriangleIndexVertexArray* dataArray) {
- assert(dataArray);
- IndexedMeshArray& meshes = dataArray->getIndexedMeshArray();
- for (int32_t i = 0; i < meshes.size(); ++i) {
- btIndexedMesh mesh = meshes[i];
- mesh.m_numTriangles = 0;
- delete [] mesh.m_triangleIndexBase;
- mesh.m_triangleIndexBase = nullptr;
- mesh.m_numVertices = 0;
- delete [] mesh.m_vertexBase;
- mesh.m_vertexBase = nullptr;
- }
- meshes.clear();
- delete dataArray;
-}
-
const btCollisionShape* ShapeFactory::createShapeFromInfo(const ShapeInfo& info) {
btCollisionShape* shape = NULL;
int type = info.getType();
@@ -431,7 +448,6 @@ void ShapeFactory::deleteShape(const btCollisionShape* shape) {
assert(shape);
// ShapeFactory is responsible for deleting all shapes, even the const ones that are stored
// in the ShapeManager, so we must cast to non-const here when deleting.
- // so we cast to non-const here when deleting memory.
btCollisionShape* nonConstShape = const_cast<btCollisionShape*>(shape);
if (nonConstShape->getShapeType() == (int)COMPOUND_SHAPE_PROXYTYPE) {
btCompoundShape* compoundShape = static_cast<btCompoundShape*>(nonConstShape);
@@ -448,14 +464,3 @@ void ShapeFactory::deleteShape(const btCollisionShape* shape) {
}
delete nonConstShape;
}
-
-// the dataArray must be created before we create the StaticMeshShape
-ShapeFactory::StaticMeshShape::StaticMeshShape(btTriangleIndexVertexArray* dataArray)
-: btBvhTriangleMeshShape(dataArray, true), _dataArray(dataArray) {
- assert(dataArray);
-}
-
-ShapeFactory::StaticMeshShape::~StaticMeshShape() {
- deleteStaticMeshArray(_dataArray);
- _dataArray = nullptr;
-}
diff --git a/libraries/physics/src/ShapeFactory.h b/libraries/physics/src/ShapeFactory.h
index 2bf79f390c..704a7804b3 100644
--- a/libraries/physics/src/ShapeFactory.h
+++ b/libraries/physics/src/ShapeFactory.h
@@ -17,25 +17,11 @@
#include <ShapeInfo.h>
-// translates between ShapeInfo and btShape
+// The ShapeFactory assembles and correctly disassembles btCollisionShapes.
namespace ShapeFactory {
const btCollisionShape* createShapeFromInfo(const ShapeInfo& info);
void deleteShape(const btCollisionShape* shape);
-
- //btTriangleIndexVertexArray* createStaticMeshArray(const ShapeInfo& info);
- //void deleteStaticMeshArray(btTriangleIndexVertexArray* dataArray);
-
- class StaticMeshShape : public btBvhTriangleMeshShape {
- public:
- StaticMeshShape() = delete;
- StaticMeshShape(btTriangleIndexVertexArray* dataArray);
- ~StaticMeshShape();
-
- private:
- // the StaticMeshShape owns its vertex/index data
- btTriangleIndexVertexArray* _dataArray;
- };
};
#endif // hifi_ShapeFactory_h