Kaynağa Gözat

Fixed issue where -zero was not deterministic (#1100)

- Due to a difference between the used instructions in NEON and SSE -Vec3::sZero() returned different binary results on ARM vs x86. When JPH_CROSS_PLATFORM_DETERMINISTIC is defined, we ensure that the calculation is the same now.
- Added unit tests to determinism check (there's an ifdef in there now that runs an additional test)
- Upgraded to latest gradle on Android

Fixes #1098
Jorrit Rouwe 1 yıl önce
ebeveyn
işleme
56adf54375

+ 17 - 5
.github/workflows/determinism_check.yml

@@ -25,9 +25,12 @@ jobs:
     - name: Checkout Code
       uses: actions/checkout@v4
     - name: Configure CMake
-      run: cmake -B ${{github.workspace}}/Build/Linux_Distribution -DCMAKE_BUILD_TYPE=Distribution -DCMAKE_CXX_COMPILER=clang++ Build -DCROSS_PLATFORM_DETERMINISTIC=ON -DTARGET_VIEWER=OFF -DTARGET_SAMPLES=OFF -DTARGET_HELLO_WORLD=OFF -DTARGET_UNIT_TESTS=OFF
+      run: cmake -B ${{github.workspace}}/Build/Linux_Distribution -DCMAKE_BUILD_TYPE=Distribution -DCMAKE_CXX_COMPILER=clang++ Build -DCROSS_PLATFORM_DETERMINISTIC=ON -DTARGET_VIEWER=OFF -DTARGET_SAMPLES=OFF -DTARGET_HELLO_WORLD=OFF -DTARGET_UNIT_TESTS=ON
     - name: Build
       run: cmake --build ${{github.workspace}}/Build/Linux_Distribution
+    - name: Unit Tests
+      working-directory: ${{github.workspace}}/Build/Linux_Distribution
+      run: ctest --output-on-failure --verbose
     - name: Test ConvexVsMesh
       working-directory: ${{github.workspace}}/Build/Linux_Distribution
       run: ./PerformanceTest -q=LinearCast -t=2 -s=ConvexVsMesh -validate_hash=${CONVEX_VS_MESH_HASH}
@@ -45,9 +48,12 @@ jobs:
     - name: Add msbuild to PATH
       uses: microsoft/setup-msbuild@v2
     - name: Configure CMake
-      run: cmake -B ${{github.workspace}}/Build/VS2022_CL -G "Visual Studio 17 2022" -A x64 Build -DCROSS_PLATFORM_DETERMINISTIC=ON -DTARGET_VIEWER=OFF -DTARGET_SAMPLES=OFF -DTARGET_HELLO_WORLD=OFF -DTARGET_UNIT_TESTS=OFF
+      run: cmake -B ${{github.workspace}}/Build/VS2022_CL -G "Visual Studio 17 2022" -A x64 Build -DCROSS_PLATFORM_DETERMINISTIC=ON -DTARGET_VIEWER=OFF -DTARGET_SAMPLES=OFF -DTARGET_HELLO_WORLD=OFF -DTARGET_UNIT_TESTS=ON
     - name: Build
       run: msbuild Build\VS2022_CL\JoltPhysics.sln /property:Configuration=Distribution
+    - name: Unit Tests
+      working-directory: ${{github.workspace}}/Build/VS2022_CL/Distribution
+      run: ./UnitTests.exe
     - name: Test ConvexVsMesh
       working-directory: ${{github.workspace}}/Build/VS2022_CL/Distribution
       run: ./PerformanceTest -q=LinearCast -t=2 -s=ConvexVsMesh "-validate_hash=$env:CONVEX_VS_MESH_HASH"
@@ -66,9 +72,12 @@ jobs:
       uses: microsoft/setup-msbuild@v2
     - name: Configure CMake
       working-directory: ${{github.workspace}}/Build
-      run: ./cmake_vs2022_cl_32bit.bat -DCROSS_PLATFORM_DETERMINISTIC=ON -DTARGET_VIEWER=OFF -DTARGET_SAMPLES=OFF -DTARGET_HELLO_WORLD=OFF -DTARGET_UNIT_TESTS=OFF
+      run: ./cmake_vs2022_cl_32bit.bat -DCROSS_PLATFORM_DETERMINISTIC=ON -DTARGET_VIEWER=OFF -DTARGET_SAMPLES=OFF -DTARGET_HELLO_WORLD=OFF -DTARGET_UNIT_TESTS=ON
     - name: Build
       run: msbuild Build\VS2022_CL_32BIT\JoltPhysics.sln /property:Configuration=Distribution
+    - name: Unit Tests
+      working-directory: ${{github.workspace}}/Build/VS2022_CL_32BIT/Distribution
+      run: ./UnitTests.exe
     - name: Test ConvexVsMesh
       working-directory: ${{github.workspace}}/Build/VS2022_CL_32BIT/Distribution
       run: ./PerformanceTest -q=LinearCast -t=2 -s=ConvexVsMesh "-validate_hash=$env:CONVEX_VS_MESH_HASH"
@@ -84,9 +93,12 @@ jobs:
     - name: Checkout Code
       uses: actions/checkout@v4
     - name: Configure CMake
-      run: cmake -B ${{github.workspace}}/Build/Linux_Distribution -DCMAKE_BUILD_TYPE=Distribution Build -DCROSS_PLATFORM_DETERMINISTIC=ON -DTARGET_VIEWER=OFF -DTARGET_SAMPLES=OFF -DTARGET_HELLO_WORLD=OFF -DTARGET_UNIT_TESTS=OFF -DUSE_AVX2=OFF -DUSE_AVX512=OFF -DUSE_LZCNT=OFF -DUSE_TZCNT=OFF
+      run: cmake -B ${{github.workspace}}/Build/Linux_Distribution -DCMAKE_BUILD_TYPE=Distribution Build -DCROSS_PLATFORM_DETERMINISTIC=ON -DTARGET_VIEWER=OFF -DTARGET_SAMPLES=OFF -DTARGET_HELLO_WORLD=OFF -DTARGET_UNIT_TESTS=ON -DUSE_AVX2=OFF -DUSE_AVX512=OFF -DUSE_LZCNT=OFF -DUSE_TZCNT=OFF
     - name: Build
       run: cmake --build ${{github.workspace}}/Build/Linux_Distribution
+    - name: Unit Tests
+      working-directory: ${{github.workspace}}/Build/Linux_Distribution
+      run: ctest --output-on-failure --verbose
     - name: Test ConvexVsMesh
       working-directory: ${{github.workspace}}/Build/Linux_Distribution
       run: ./PerformanceTest -q=LinearCast -t=2 -s=ConvexVsMesh -validate_hash=${CONVEX_VS_MESH_HASH}
@@ -108,7 +120,7 @@ jobs:
       run: cmake -B ${{github.workspace}}/Build/Linux_Distribution -DCMAKE_BUILD_TYPE=Distribution -DCMAKE_CXX_COMPILER=clang++ Build -DCROSS_PLATFORM_DETERMINISTIC=ON -DCROSS_COMPILE_ARM=ON -DTARGET_VIEWER=OFF -DTARGET_SAMPLES=OFF -DTARGET_HELLO_WORLD=OFF -DTARGET_UNIT_TESTS=ON
     - name: Build
       run: cmake --build ${{github.workspace}}/Build/Linux_Distribution
-    - name: Test
+    - name: Unit Tests
       working-directory: ${{github.workspace}}/Build/Linux_Distribution
       run: qemu-aarch64 -L /usr/aarch64-linux-gnu/ ./UnitTests
     - name: Test ConvexVsMesh

+ 1 - 1
Build/Android/build.gradle

@@ -5,7 +5,7 @@ buildscript {
         mavenCentral()
     }
     dependencies {
-        classpath 'com.android.tools.build:gradle:8.1.2'
+        classpath 'com.android.tools.build:gradle:8.4.0'
 
         // NOTE: Do not place your application dependencies here; they belong
         // in the individual module build.gradle files

+ 0 - 1
Build/Android/gradle.properties

@@ -17,6 +17,5 @@ org.gradle.jvmargs=-Xmx2048m -Dfile.encoding=UTF-8
 android.useAndroidX=true
 # Automatically convert third-party libraries to use AndroidX
 android.enableJetifier=true
-android.defaults.buildfeatures.buildconfig=true
 android.nonTransitiveRClass=false
 android.nonFinalResIds=false

+ 1 - 1
Build/Android/gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.0-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.6-bin.zip
 distributionPath=wrapper/dists
 zipStorePath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME

+ 1 - 0
Docs/ReleaseNotes.md

@@ -44,6 +44,7 @@ For breaking API changes see [this document](https://github.com/jrouwe/JoltPhysi
 * Fixed crash in Ragdoll::DriveToPoseUsingMotors when using constraints other than SwingTwistConstraint.
 * Fixed -Wunused-parameter warning on GCC when building in Release mode with -Wextra.
 * Fixed tolerance in assert in GetPenetrationDepthStepEPA.
+* Due to a difference between the used instructions in NEON and SSE -Vec3::sZero() returned different binary results on ARM vs x86. When JPH_CROSS_PLATFORM_DETERMINISTIC is defined, we ensure that the calculation is the same now.
 
 ## v5.0.0
 

+ 10 - 2
Jolt/Math/Vec3.inl

@@ -473,9 +473,17 @@ Vec3 Vec3::operator - () const
 #if defined(JPH_USE_SSE)
 	return _mm_sub_ps(_mm_setzero_ps(), mValue);
 #elif defined(JPH_USE_NEON)
-	return vnegq_f32(mValue);
+	#ifdef JPH_CROSS_PLATFORM_DETERMINISTIC
+		return vsubq_f32(vdupq_n_f32(0), mValue);
+	#else
+		return vnegq_f32(mValue);
+	#endif
 #else
-	return Vec3(-mF32[0], -mF32[1], -mF32[2]);
+	#ifdef JPH_CROSS_PLATFORM_DETERMINISTIC
+		return Vec3(0.0f - mF32[0], 0.0f - mF32[1], 0.0f - mF32[2]);
+	#else
+		return Vec3(-mF32[0], -mF32[1], -mF32[2]);
+	#endif
 #endif
 }
 

+ 10 - 2
Jolt/Math/Vec4.inl

@@ -497,9 +497,17 @@ Vec4 Vec4::operator - () const
 #if defined(JPH_USE_SSE)
 	return _mm_sub_ps(_mm_setzero_ps(), mValue);
 #elif defined(JPH_USE_NEON)
-	return vnegq_f32(mValue);
+	#ifdef JPH_CROSS_PLATFORM_DETERMINISTIC
+		return vsubq_f32(vdupq_n_f32(0), mValue);
+	#else
+		return vnegq_f32(mValue);
+	#endif
 #else
-	return Vec4(-mF32[0], -mF32[1], -mF32[2], -mF32[3]);
+	#ifdef JPH_CROSS_PLATFORM_DETERMINISTIC
+		return Vec4(0.0f - mF32[0], 0.0f - mF32[1], 0.0f - mF32[2], 0.0f - mF32[3]);
+	#else
+		return Vec4(-mF32[0], -mF32[1], -mF32[2], -mF32[3]);
+	#endif
 #endif
 }
 

+ 11 - 0
UnitTests/Math/Vec3Tests.cpp

@@ -155,6 +155,17 @@ TEST_SUITE("Vec3Tests")
 	{
 		CHECK(-Vec3(1, 2, 3) == Vec3(-1, -2, -3));
 
+		Vec3 neg_zero = -Vec3::sZero();
+		CHECK(neg_zero == Vec3::sZero());
+
+	#ifdef JPH_CROSS_PLATFORM_DETERMINISTIC
+		// When cross platform deterministic, we want to make sure that -0 is represented as 0
+		UVec4 neg_zero_bin = neg_zero.ReinterpretAsInt();
+		CHECK(neg_zero_bin.GetX() == 0);
+		CHECK(neg_zero_bin.GetY() == 0);
+		CHECK(neg_zero_bin.GetZ() == 0);
+	#endif // JPH_CROSS_PLATFORM_DETERMINISTIC
+
 		CHECK(Vec3(1, 2, 3) + Vec3(4, 5, 6) == Vec3(5, 7, 9));
 		CHECK(Vec3(1, 2, 3) - Vec3(6, 5, 4) == Vec3(-5, -3, -1));
 

+ 12 - 0
UnitTests/Math/Vec4Tests.cpp

@@ -143,6 +143,18 @@ TEST_SUITE("Vec4Tests")
 	{
 		CHECK(-Vec4(1, 2, 3, 4) == Vec4(-1, -2, -3, -4));
 
+		Vec4 neg_zero = -Vec4::sZero();
+		CHECK(neg_zero == Vec4::sZero());
+
+	#ifdef JPH_CROSS_PLATFORM_DETERMINISTIC
+		// When cross platform deterministic, we want to make sure that -0 is represented as 0
+		UVec4 neg_zero_bin = neg_zero.ReinterpretAsInt();
+		CHECK(neg_zero_bin.GetX() == 0);
+		CHECK(neg_zero_bin.GetY() == 0);
+		CHECK(neg_zero_bin.GetZ() == 0);
+		CHECK(neg_zero_bin.GetW() == 0);
+	#endif // JPH_CROSS_PLATFORM_DETERMINISTIC
+
 		CHECK(Vec4(1, 2, 3, 4) + Vec4(5, 6, 7, 8) == Vec4(6, 8, 10, 12));
 		CHECK(Vec4(1, 2, 3, 4) - Vec4(8, 7, 6, 5) == Vec4(-7, -5, -3, -1));