Added Pyramid test to determinism checks (#1351)

* Implemented own version of std::heap_push and std::heap_pop. These functions behave differently on different platforms when elements are equal causing differences in the Pyramid test.
* Removed the use of std::priority_queue. This may suffer from the same problem (not verified).
* Removed use of std::queue
* Removed unused fstream includes

Fixes #1336

.github/workflows/determinism_check.yml

@@ -3,6 +3,7 @@ name: Determinism Check
 env:
   CONVEX_VS_MESH_HASH: '0x16ca5bf7f9da5f74'
   RAGDOLL_HASH: '0xa50ce2dc5684626d'
+  PYRAMID_HASH: '0x198b8eeaee57e29a'
   EMSCRIPTEN_VERSION: 3.1.64
   UBUNTU_CLANG_VERSION: clang++-15
   UBUNTU_GCC_VERSION: g++-12
@@ -42,6 +43,9 @@ jobs:
     - name: Test Ragdoll
       working-directory: ${{github.workspace}}/Build/Linux_Distribution
       run: ./PerformanceTest -q=LinearCast -t=max -s=Ragdoll -validate_hash=${RAGDOLL_HASH}
+    - name: Test Pyramid
+      working-directory: ${{github.workspace}}/Build/Linux_Distribution
+      run: ./PerformanceTest -q=LinearCast -t=max -s=Pyramid -validate_hash=${PYRAMID_HASH}
 
   linux_gcc:
     runs-on: ubuntu-latest
@@ -64,6 +68,9 @@ jobs:
     - name: Test Ragdoll
       working-directory: ${{github.workspace}}/Build/Linux_Distribution
       run: ./PerformanceTest -q=LinearCast -t=max -s=Ragdoll -validate_hash=${RAGDOLL_HASH}
+    - name: Test Pyramid
+      working-directory: ${{github.workspace}}/Build/Linux_Distribution
+      run: ./PerformanceTest -q=LinearCast -t=max -s=Pyramid -validate_hash=${PYRAMID_HASH}
 
   msvc_cl:
     runs-on: windows-latest
@@ -88,6 +95,9 @@ jobs:
     - name: Test Ragdoll
       working-directory: ${{github.workspace}}/Build/VS2022_CL/Distribution
       run: ./PerformanceTest -q=LinearCast -t=max -s=Ragdoll "-validate_hash=$env:RAGDOLL_HASH"
+    - name: Test Pyramid
+      working-directory: ${{github.workspace}}/Build/VS2022_CL/Distribution
+      run: ./PerformanceTest -q=LinearCast -t=max -s=Pyramid "-validate_hash=$env:PYRAMID_HASH"
 
   msvc_cl_32:
     runs-on: windows-latest
@@ -112,6 +122,9 @@ jobs:
     - name: Test Ragdoll
       working-directory: ${{github.workspace}}/Build/VS2022_CL_32BIT/Distribution
       run: ./PerformanceTest -q=LinearCast -t=max -s=Ragdoll "-validate_hash=$env:RAGDOLL_HASH"
+    - name: Test Pyramid
+      working-directory: ${{github.workspace}}/Build/VS2022_CL_32BIT/Distribution
+      run: ./PerformanceTest -q=LinearCast -t=max -s=Pyramid "-validate_hash=$env:PYRAMID_HASH"
 
   macos:
     runs-on: macos-latest
@@ -134,6 +147,9 @@ jobs:
     - name: Test Ragdoll
       working-directory: ${{github.workspace}}/Build/Linux_Distribution
       run: ./PerformanceTest -q=LinearCast -t=max -s=Ragdoll -validate_hash=${RAGDOLL_HASH}
+    - name: Test Pyramid
+      working-directory: ${{github.workspace}}/Build/Linux_Distribution
+      run: ./PerformanceTest -q=LinearCast -t=max -s=Pyramid -validate_hash=${PYRAMID_HASH}
 
   arm_clang:
     runs-on: ubuntu-latest
@@ -159,6 +175,9 @@ jobs:
     - name: Test Ragdoll
       working-directory: ${{github.workspace}}/Build/Linux_Distribution
       run: qemu-aarch64 -L /usr/aarch64-linux-gnu/ ./PerformanceTest -q=LinearCast -t=max -s=Ragdoll -validate_hash=${RAGDOLL_HASH}
+    - name: Test Pyramid
+      working-directory: ${{github.workspace}}/Build/Linux_Distribution
+      run: qemu-aarch64 -L /usr/aarch64-linux-gnu/ ./PerformanceTest -q=LinearCast -t=max -s=Pyramid -validate_hash=${PYRAMID_HASH}
 
   arm_gcc:
     runs-on: ubuntu-latest
@@ -184,6 +203,9 @@ jobs:
     - name: Test Ragdoll
       working-directory: ${{github.workspace}}/Build/Linux_Distribution
       run: qemu-aarch64 -L /usr/aarch64-linux-gnu/ ./PerformanceTest -q=LinearCast -t=max -s=Ragdoll -validate_hash=${RAGDOLL_HASH}
+    - name: Test Pyramid
+      working-directory: ${{github.workspace}}/Build/Linux_Distribution
+      run: qemu-aarch64 -L /usr/aarch64-linux-gnu/ ./PerformanceTest -q=LinearCast -t=max -s=Pyramid -validate_hash=${PYRAMID_HASH}
 
   emscripten:
     runs-on: ubuntu-latest
@@ -216,3 +238,6 @@ jobs:
     - name: Test Ragdoll
       working-directory: ${{github.workspace}}/Build/WASM_Distribution
       run: node PerformanceTest.js -q=LinearCast -t=max -s=Ragdoll -validate_hash=${RAGDOLL_HASH}
+    - name: Test Pyramid
+      working-directory: ${{github.workspace}}/Build/WASM_Distribution
+      run: node PerformanceTest.js -q=LinearCast -t=max -s=Pyramid -validate_hash=${PYRAMID_HASH}

Docs/ReleaseNotes.md

@@ -14,6 +14,7 @@ For breaking API changes see [this document](https://github.com/jrouwe/JoltPhysi
 ### Bug fixes
 
 * Fixed bodies gaining more energy than intended due to restitution. E.g. A restitution of 1 could lead to bodies bouncing ever higher.
+* std::push_heap/pop_heap behave differently on macOS vs Windows/Linux when elements compare equal, this made the cross platform deterministic build not deterministic in some cases.
 
 ## v5.2.0
 

Jolt/Core/BinaryHeap.h

@@ -0,0 +1,96 @@
+// Jolt Physics Library (https://github.com/jrouwe/JoltPhysics)
+// SPDX-FileCopyrightText: 2024 Jorrit Rouwe
+// SPDX-License-Identifier: MIT
+
+#pragma once
+
+JPH_NAMESPACE_BEGIN
+
+/// Push a new element into a binary max-heap.
+/// [inBegin, inEnd - 1) must be a a valid heap. Element inEnd - 1 will be inserted into the heap. The heap will be [inBegin, inEnd) after this call.
+/// inPred is a function that returns true if the first element is less or equal than the second element.
+/// See: https://en.wikipedia.org/wiki/Binary_heap
+template <typename Iterator, typename Pred>
+void BinaryHeapPush(Iterator inBegin, Iterator inEnd, Pred inPred)
+{
+	using diff_t = typename std::iterator_traits<Iterator>::difference_type;
+	using elem_t = typename std::iterator_traits<Iterator>::value_type;
+
+	// New heap size
+	diff_t count = std::distance(inBegin, inEnd);
+
+	// Start from the last element
+	diff_t current = count - 1;
+	while (current > 0)
+	{
+		// Get current element
+		elem_t &current_elem = *(inBegin + current);
+
+		// Get parent element
+		diff_t parent = (current - 1) >> 1;
+		elem_t &parent_elem = *(inBegin + parent);
+
+		// Sort them so that the parent is larger than the child
+		if (inPred(parent_elem, current_elem))
+		{
+			std::swap(parent_elem, current_elem);
+			current = parent;
+		}
+		else
+		{
+			// When there's no change, we're done
+			break;
+		}
+	}
+}
+
+/// Pop an element from a binary max-heap.
+/// [inBegin, inEnd) must be a valid heap. The largest element will be removed from the heap. The heap will be [inBegin, inEnd - 1) after this call.
+/// inPred is a function that returns true if the first element is less or equal than the second element.
+/// See: https://en.wikipedia.org/wiki/Binary_heap
+template <typename Iterator, typename Pred>
+void BinaryHeapPop(Iterator inBegin, Iterator inEnd, Pred inPred)
+{
+	using diff_t = typename std::iterator_traits<Iterator>::difference_type;
+
+	// Begin by moving the highest element to the end, this is the popped element
+	std::swap(*(inEnd - 1), *inBegin);
+
+	// New heap size
+	diff_t count = std::distance(inBegin, inEnd) - 1;
+
+	// Start from the root
+	diff_t largest = 0;
+	for (;;)
+	{
+		// Get first child
+		diff_t child = (largest << 1) + 1;
+
+		// Check if we're beyond the end of the heap, if so the 2nd child is also beyond the end
+		if (child >= count)
+			break;
+
+		// Remember the largest element from the previous iteration
+		diff_t prev_largest = largest;
+
+		// Check if first child is bigger, if so select it
+		if (inPred(*(inBegin + largest), *(inBegin + child)))
+			largest = child;
+
+		// Switch to the second child
+		++child;
+
+		// Check if second child is bigger, if so select it
+		if (child < count && inPred(*(inBegin + largest), *(inBegin + child)))
+			largest = child;
+
+		// If there was no change, we're done
+		if (prev_largest == largest)
+			break;
+
+		// Swap element
+		std::swap(*(inBegin + prev_largest), *(inBegin + largest));
+	}
+}
+
+JPH_NAMESPACE_END

Jolt/Core/IssueReporting.cpp

@@ -4,10 +4,6 @@
 
 #include <Jolt/Jolt.h>
 
-JPH_SUPPRESS_WARNINGS_STD_BEGIN
-#include <fstream>
-JPH_SUPPRESS_WARNINGS_STD_END
-
 JPH_NAMESPACE_BEGIN
 
 static void DummyTrace([[maybe_unused]] const char *inFMT, ...)

Jolt/Geometry/ConvexHullBuilder.cpp

@@ -10,9 +10,11 @@
 #include <Jolt/Core/StringTools.h>
 #include <Jolt/Core/UnorderedSet.h>
 
+#ifdef JPH_CONVEX_BUILDER_DUMP_SHAPE
 JPH_SUPPRESS_WARNINGS_STD_BEGIN
 #include <fstream>
 JPH_SUPPRESS_WARNINGS_STD_END
+#endif // JPH_CONVEX_BUILDER_DUMP_SHAPE
 
 #ifdef JPH_CONVEX_BUILDER_DEBUG
 	#include <Jolt/Renderer/DebugRenderer.h>

Jolt/Geometry/EPAConvexHullBuilder.h

@@ -11,6 +11,7 @@
 //#define JPH_EPA_CONVEX_BUILDER_DRAW
 
 #include <Jolt/Core/NonCopyable.h>
+#include <Jolt/Core/BinaryHeap.h>
 
 #ifdef JPH_EPA_CONVEX_BUILDER_DRAW
 	#include <Jolt/Renderer/DebugRenderer.h>
@@ -197,7 +198,7 @@ public:
 			inT->mInQueue = true;
 
 			// Resort heap
-			std::push_heap(begin(), end(), sTriangleSorter);
+			BinaryHeapPush(begin(), end(), sTriangleSorter);
 		}
 
 		/// Peek the next closest triangle without removing it
@@ -210,7 +211,7 @@ public:
 		Triangle *		PopClosest()
 		{
 			// Move closest to end
-			std::pop_heap(begin(), end(), sTriangleSorter);
+			BinaryHeapPop(begin(), end(), sTriangleSorter);
 
 			// Remove last triangle
 			Triangle *t = back();

Jolt/Jolt.cmake

@@ -17,6 +17,7 @@ set(JOLT_PHYSICS_SRC_FILES
 	${JOLT_PHYSICS_ROOT}/Core/ARMNeon.h
 	${JOLT_PHYSICS_ROOT}/Core/Array.h
 	${JOLT_PHYSICS_ROOT}/Core/Atomics.h
+	${JOLT_PHYSICS_ROOT}/Core/BinaryHeap.h
 	${JOLT_PHYSICS_ROOT}/Core/ByteBuffer.h
 	${JOLT_PHYSICS_ROOT}/Core/Color.cpp
 	${JOLT_PHYSICS_ROOT}/Core/Color.h

Jolt/ObjectStream/ObjectStreamOut.cpp

@@ -40,12 +40,11 @@ bool ObjectStreamOut::Write(const void *inObject, const RTTI *inRTTI)
 	WriteObject(inObject);
 
 	// Write all linked objects
-	while (!mObjectQueue.empty() && !mStream.fail())
-	{
-		const void *linked_object = mObjectQueue.front();
-		WriteObject(linked_object);
-		mObjectQueue.pop();
-	}
+	ObjectQueue::size_type cur = 0;
+	for (; cur < mObjectQueue.size() && !mStream.fail(); ++cur)
+		WriteObject(mObjectQueue[cur]);
+	mObjectQueue.erase(mObjectQueue.begin(), mObjectQueue.begin() + cur);
+
 	return !mStream.fail();
 }
 
@@ -57,11 +56,10 @@ void ObjectStreamOut::WriteObject(const void *inObject)
 
 	// Write class description and associated descriptions
 	QueueRTTI(i->second.mRTTI);
-	while (!mClassQueue.empty() && !mStream.fail())
-	{
-		WriteRTTI(mClassQueue.front());
-		mClassQueue.pop();
-	}
+	ClassQueue::size_type cur = 0;
+	for (; cur < mClassQueue.size() && !mStream.fail(); ++cur)
+		WriteRTTI(mClassQueue[cur]);
+	mClassQueue.erase(mClassQueue.begin(), mClassQueue.begin() + cur);
 
 	HintNextItem();
 	HintNextItem();
@@ -81,7 +79,7 @@ void ObjectStreamOut::QueueRTTI(const RTTI *inRTTI)
 	if (i == mClassSet.end())
 	{
 		mClassSet.insert(inRTTI);
-		mClassQueue.push(inRTTI);
+		mClassQueue.push_back(inRTTI);
 	}
 }
 
@@ -149,7 +147,7 @@ void ObjectStreamOut::WritePointerData(const RTTI *inRTTI, const void *inPointer
 			// Assign a new identifier to this object and queue it for serialization
 			identifier = mNextIdentifier++;
 			mIdentifierMap.try_emplace(inPointer, identifier, inRTTI);
-			mObjectQueue.push(inPointer);
+			mObjectQueue.push_back(inPointer);
 		}
 	}
 	else

Jolt/ObjectStream/ObjectStreamOut.h

@@ -10,7 +10,6 @@
 #include <Jolt/Core/UnorderedSet.h>
 
 JPH_SUPPRESS_WARNINGS_STD_BEGIN
-#include <queue>
 #include <fstream>
 JPH_SUPPRESS_WARNINGS_STD_END
 
@@ -18,8 +17,6 @@ JPH_SUPPRESS_WARNINGS_STD_END
 
 JPH_NAMESPACE_BEGIN
 
-template <class T> using Queue = std::queue<T, std::deque<T, STLAllocator<T>>>;
-
 /// ObjectStreamOut contains all logic for writing an object to disk. It is the base
 /// class for the text and binary output streams (ObjectStreamTextOut and ObjectStreamBinaryOut).
 class JPH_EXPORT ObjectStreamOut : public IObjectStreamOut
@@ -89,8 +86,8 @@ private:
 
 	using IdentifierMap = UnorderedMap<const void *, ObjectInfo>;
 	using ClassSet = UnorderedSet<const RTTI *>;
-	using ObjectQueue = Queue<const void *>;
-	using ClassQueue = Queue<const RTTI *>;
+	using ObjectQueue = Array<const void *>;
+	using ClassQueue = Array<const RTTI *>;
 
 	Identifier					mNextIdentifier = sNullIdentifier + 1;						///< Next free identifier for this stream
 	IdentifierMap				mIdentifierMap;												///< Links object pointer to an identifier

Jolt/Physics/SoftBody/SoftBodySharedSettings.cpp

@@ -12,15 +12,10 @@
 #include <Jolt/Core/QuickSort.h>
 #include <Jolt/Core/UnorderedMap.h>
 #include <Jolt/Core/UnorderedSet.h>
-
-JPH_SUPPRESS_WARNINGS_STD_BEGIN
-#include <queue>
-JPH_SUPPRESS_WARNINGS_STD_END
+#include <Jolt/Core/BinaryHeap.h>
 
 JPH_NAMESPACE_BEGIN
 
-template<class T, class Container = Array<T>, class Compare = std::less<typename Container::value_type>> using PriorityQueue = std::priority_queue<T, Container, Compare>;
-
 JPH_IMPLEMENT_SERIALIZABLE_NON_VIRTUAL(SoftBodySharedSettings::Vertex)
 {
 	JPH_ADD_ATTRIBUTE(SoftBodySharedSettings::Vertex, mPosition)
@@ -131,21 +126,26 @@ void SoftBodySharedSettings::CalculateClosestKinematic()
 	};
 
 	// Start with all kinematic elements
-	PriorityQueue<Open> to_visit;
+	Array<Open> to_visit;
 	for (uint32 v = 0; v < mVertices.size(); ++v)
 		if (mVertices[v].mInvMass == 0.0f)
 		{
 			mClosestKinematic[v].mVertex = v;
 			mClosestKinematic[v].mDistance = 0.0f;
-			to_visit.push({ v, 0.0f });
+			to_visit.push_back({ v, 0.0f });
+			BinaryHeapPush(to_visit.begin(), to_visit.end(), std::less<Open> { });
 		}
 
 	// Visit all vertices remembering the closest kinematic vertex and its distance
+	JPH_IF_ENABLE_ASSERTS(float last_closest = 0.0f;)
 	while (!to_visit.empty())
 	{
 		// Pop element from the open list
-		Open current = to_visit.top();
-		to_visit.pop();
+		BinaryHeapPop(to_visit.begin(), to_visit.end(), std::less<Open> { });
+		Open current = to_visit.back();
+		to_visit.pop_back();
+		JPH_ASSERT(current.mDistance >= last_closest);
+		JPH_IF_ENABLE_ASSERTS(last_closest = current.mDistance;)
 
 		// Loop through all of its connected vertices
 		for (uint32 v : connectivity[current.mVertex])
@@ -157,7 +157,8 @@ void SoftBodySharedSettings::CalculateClosestKinematic()
 				// Remember new closest vertex
 				mClosestKinematic[v].mVertex = mClosestKinematic[current.mVertex].mVertex;
 				mClosestKinematic[v].mDistance = new_distance;
-				to_visit.push({ v, new_distance });
+				to_visit.push_back({ v, new_distance });
+				BinaryHeapPush(to_visit.begin(), to_visit.end(), std::less<Open> { });
 			}
 		}
 	}

UnitTests/Core/BinaryHeapTest.cpp

@@ -0,0 +1,47 @@
+// Jolt Physics Library (https://github.com/jrouwe/JoltPhysics)
+// SPDX-FileCopyrightText: 2024 Jorrit Rouwe
+// SPDX-License-Identifier: MIT
+
+#include "UnitTestFramework.h"
+#include <Jolt/Core/BinaryHeap.h>
+
+TEST_SUITE("BinaryHeapTest")
+{
+	TEST_CASE("TestBinaryHeap")
+	{
+		// Add some numbers
+		Array<int> array;
+		array.reserve(1100);
+		for (int i = 0; i < 1000; ++i)
+			array.push_back(i);
+
+		// Ensure we have some duplicates
+		for (int i = 0; i < 1000; i += 10)
+			array.push_back(i);
+
+		// Shuffle the array
+		UnitTestRandom random(123);
+		std::shuffle(array.begin(), array.end(), random);
+
+		// Add the numbers to the heap
+		Array<int> heap;
+		for (int i : array)
+		{
+			heap.push_back(i);
+			BinaryHeapPush(heap.begin(), heap.end(), std::less<int> { });
+		}
+
+		// Check that the heap is sorted
+		int last = INT_MAX;
+		int seen[1000] { 0 };
+		while (!heap.empty())
+		{
+			BinaryHeapPop(heap.begin(), heap.end(), std::less<int> { });
+			int current = heap.back();
+			CHECK(++seen[current] <= (current % 10 == 0? 2 : 1));
+			heap.pop_back();
+			CHECK(current <= last);
+			last = current;
+		}
+	}
+}

UnitTests/UnitTests.cmake

@@ -4,6 +4,7 @@ set(UNIT_TESTS_ROOT ${PHYSICS_REPO_ROOT}/UnitTests)
 # Source files
 set(UNIT_TESTS_SRC_FILES
 	${UNIT_TESTS_ROOT}/Core/ArrayTest.cpp
+	${UNIT_TESTS_ROOT}/Core/BinaryHeapTest.cpp
 	${UNIT_TESTS_ROOT}/Core/FPFlushDenormalsTest.cpp
 	${UNIT_TESTS_ROOT}/Core/HashCombineTest.cpp
 	${UNIT_TESTS_ROOT}/Core/InsertionSortTest.cpp