Faster hashing for IndexedTriangle to improve MeshShape::Sanitize (#1368

)

Also replaced HashCombine with a 64-bit version. Note that this version is slower but it's not used in any perf critical code.

Jolt/Core/HashCombine.h

@@ -17,8 +17,8 @@ inline uint64 HashBytes(const void *inData, uint inSize, uint64 inSeed = 0xcbf29
 	uint64 hash = inSeed;
 	for (const uint8 *data = reinterpret_cast<const uint8 *>(inData); data < reinterpret_cast<const uint8 *>(inData) + inSize; ++data)
 	{
-		hash = hash ^ uint64(*data);
-		hash = hash * 0x100000001b3UL;
+		hash ^= uint64(*data);
+		hash *= 0x100000001b3UL;
 	}
 	return hash;
 }
@@ -31,7 +31,7 @@ constexpr uint64 HashString(const char *inString, uint64 inSeed = 0xcbf29ce48422
 	for (const char *c = inString; *c != 0; ++c)
 	{
 		hash ^= uint64(*c);
-		hash = hash * 0x100000001b3UL;
+		hash *= 0x100000001b3UL;
 	}
 	return hash;
 }
@@ -142,12 +142,33 @@ JPH_DEFINE_TRIVIAL_HASH(int)
 JPH_DEFINE_TRIVIAL_HASH(uint32)
 JPH_DEFINE_TRIVIAL_HASH(uint64)
 
-/// @brief Helper function that hashes a single value into ioSeed
-/// Taken from: https://stackoverflow.com/questions/2590677/how-do-i-combine-hash-values-in-c0x
+/// Helper function that hashes a single value into ioSeed
+/// Based on https://github.com/jonmaiga/mx3 by Jon Maiga
 template <typename T>
 inline void HashCombine(uint64 &ioSeed, const T &inValue)
 {
-	ioSeed ^= Hash<T> { } (inValue) + 0x9e3779b9 + (ioSeed << 6) + (ioSeed >> 2);
+	constexpr uint64 c = 0xbea225f9eb34556dUL;
+
+	uint64 h = ioSeed;
+	uint64 x = Hash<T> { } (inValue);
+
+	// See: https://github.com/jonmaiga/mx3/blob/master/mx3.h
+	// mix_stream(h, x)
+	x *= c;
+	x ^= x >> 39;
+	h += x * c;
+	h *= c;
+
+	// mix(h)
+	h ^= h >> 32;
+	h *= c;
+	h ^= h >> 29;
+	h *= c;
+	h ^= h >> 32;
+	h *= c;
+	h ^= h >> 29;
+
+	ioSeed = h;
 }
 
 /// Hash combiner to use a custom struct in an unordered map or set
@@ -174,11 +195,6 @@ inline uint64 HashCombineArgs(const FirstValue &inFirstValue, Values... inValues
 	return seed;
 }
 
-JPH_NAMESPACE_END
-
-JPH_SUPPRESS_WARNING_PUSH
-JPH_CLANG_SUPPRESS_WARNING("-Wc++98-compat-pedantic")
-
 #define JPH_MAKE_HASH_STRUCT(type, name, ...)				\
 	struct [[nodiscard]] name								\
 	{														\
@@ -188,25 +204,31 @@ JPH_CLANG_SUPPRESS_WARNING("-Wc++98-compat-pedantic")
 		}													\
 	};
 
-#define JPH_MAKE_HASHABLE(type, ...)						\
+#define JPH_MAKE_STD_HASH(type)								\
 	JPH_SUPPRESS_WARNING_PUSH								\
 	JPH_SUPPRESS_WARNINGS									\
-	namespace JPH											\
-	{														\
-		template<>											\
-		JPH_MAKE_HASH_STRUCT(type, Hash<type>, __VA_ARGS__) \
-	}														\
 	namespace std											\
 	{														\
 		template<>											\
 		struct [[nodiscard]] hash<type>						\
 		{													\
-			std::size_t operator()(const type &t) const		\
+			size_t operator()(const type &t) const			\
 			{												\
-				return std::size_t(::JPH::Hash<type>{ }(t));\
+				return size_t(::JPH::Hash<type>{ }(t));		\
 			}												\
 		};													\
 	}														\
 	JPH_SUPPRESS_WARNING_POP
 
-JPH_SUPPRESS_WARNING_POP
+#define JPH_MAKE_HASHABLE(type, ...)						\
+	JPH_SUPPRESS_WARNING_PUSH								\
+	JPH_SUPPRESS_WARNINGS									\
+	namespace JPH											\
+	{														\
+		template<>											\
+		JPH_MAKE_HASH_STRUCT(type, Hash<type>, __VA_ARGS__) \
+	}														\
+	JPH_SUPPRESS_WARNING_POP								\
+	JPH_MAKE_STD_HASH(type)
+
+JPH_NAMESPACE_END

Jolt/Geometry/IndexedTriangle.h

@@ -65,6 +65,13 @@ class IndexedTriangleNoMaterial
 		return (Vec3(inVertices[mIdx[0]]) + Vec3(inVertices[mIdx[1]]) + Vec3(inVertices[mIdx[2]])) / 3.0f;
 	}
 
+	/// Get the hash value of this structure
+	uint64			GetHash() const
+	{
+		static_assert(sizeof(IndexedTriangleNoMaterial) == 3 * sizeof(uint32), "Class should have no padding");
+		return HashBytes(this, sizeof(IndexedTriangleNoMaterial));
+	}
+
 	uint32			mIdx[3];
 };
 
@@ -102,6 +109,13 @@ class IndexedTriangle : public IndexedTriangleNoMaterial
 		}
 	}
 
+	/// Get the hash value of this structure
+	uint64			GetHash() const
+	{
+		static_assert(sizeof(IndexedTriangle) == 5 * sizeof(uint32), "Class should have no padding");
+		return HashBytes(this, sizeof(IndexedTriangle));
+	}
+
 	uint32			mMaterialIndex = 0;
 	uint32			mUserData = 0;				///< User data that can be used for anything by the application, e.g. for tracking the original index of the triangle
 };
@@ -111,6 +125,6 @@ using IndexedTriangleList = Array<IndexedTriangle>;
 
 JPH_NAMESPACE_END
 
-// Create a std::hash/JPH::Hash for IndexedTriangleNoMaterial and IndexedTriangle
-JPH_MAKE_HASHABLE(JPH::IndexedTriangleNoMaterial, t.mIdx[0], t.mIdx[1], t.mIdx[2])
-JPH_MAKE_HASHABLE(JPH::IndexedTriangle, t.mIdx[0], t.mIdx[1], t.mIdx[2], t.mMaterialIndex, t.mUserData)
+// Create a std::hash for IndexedTriangleNoMaterial and IndexedTriangle
+JPH_MAKE_STD_HASH(JPH::IndexedTriangleNoMaterial)
+JPH_MAKE_STD_HASH(JPH::IndexedTriangle)

Jolt/Physics/Collision/Shape/SubShapeIDPair.h

@@ -62,19 +62,4 @@ static_assert(alignof(SubShapeIDPair) == 4, "Assuming 4 byte aligned");
 
 JPH_NAMESPACE_END
 
-JPH_SUPPRESS_WARNINGS_STD_BEGIN
-
-namespace std
-{
-	/// Declare std::hash for SubShapeIDPair
-	template <>
-	struct hash<JPH::SubShapeIDPair>
-	{
-		inline size_t operator () (const JPH::SubShapeIDPair &inRHS) const
-		{
-			return static_cast<size_t>(inRHS.GetHash());
-		}
-	};
-}
-
-JPH_SUPPRESS_WARNINGS_STD_END
+JPH_MAKE_STD_HASH(JPH::SubShapeIDPair)

UnitTests/Core/HashCombineTest.cpp

@@ -28,4 +28,24 @@ TEST_SUITE("HashCombineTest")
 		String str_test = "This is a test";
 		CHECK(Hash<String> { } (str_test) == 2733878766136413408UL);
 	}
+
+	TEST_CASE("TestHashCombine")
+	{
+		int val1 = 0;
+		uint64 val1_hash = Hash<int> { } (val1);
+		int val2 = 1;
+		uint64 val2_hash = Hash<int> { } (val2);
+
+		// Check non-commutative
+		uint64 seed1 = val1_hash;
+		HashCombine(seed1, val2);
+		uint64 seed2 = val2_hash;
+		HashCombine(seed2, val1);
+		CHECK(seed1 != seed2);
+
+		// Check that adding a 0 changes the hash
+		uint64 seed3 = val1_hash;
+		HashCombine(seed3, val1);
+		CHECK(seed3 != val1_hash);
+	}
 }