Pārlūkot izejas kodu

-mfma automatically turns on float contaction on clang which is causing precision issues (#382)

This led to an assert JPH_ASSERT(last->mClosestPointInterior) in the TaperedCapsuleShape test on the MSVC clang build because GetClosestPointOnTriangle reported a distance of 0 while the actual distance was approximately 0.96. The offending triangle is now checked in a unit test.
Jorrit Rouwe 2 gadi atpakaļ
vecāks
revīzija
4f76bb9ec7

+ 8 - 2
Build/CMakeLists.txt

@@ -178,8 +178,14 @@ elseif ("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux" OR "${CMAKE_SYSTEM_NAME}" STREQU
 		# Also turn off automatic fused multiply add contractions, there doesn't seem to be a way to do this selectively through the macro JPH_PRECISE_MATH_OFF
 		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-comment -Wno-stringop-overflow -ffp-contract=off")
 	else()
-		# Do not use -ffast-math or -ffp-contract=on since it cannot be turned off in a single compilation unit under clang, see Core.h
-		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ffp-model=precise -ffp-contract=off")
+		# Do not use -ffast-math since it cannot be turned off in a single compilation unit under clang, see Core.h
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ffp-model=precise")
+
+		# On clang 14 and later we can turn off float contraction through a pragma, older versions need it off always, see Core.h
+		# Note that this does not seem to work on ARM so we turn fp contract off there always
+		if (CMAKE_CXX_COMPILER_VERSION LESS 14 OR CROSS_COMPILE_ARM OR CMAKE_OSX_ARCHITECTURES MATCHES "arm64" OR "${CMAKE_SYSTEM_PROCESSOR}" STREQUAL "aarch64")
+			set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ffp-contract=off")
+		endif()
 	endif()
 
 	# Platform specific compiler flags

+ 14 - 5
Jolt/Core/Core.h

@@ -378,13 +378,22 @@ static_assert(sizeof(void *) == (JPH_CPU_ADDRESS_BITS == 64? 8 : 4), "Invalid si
 #define JPH_UNUSED(x)			(void)x
 
 // Macro to enable floating point precise mode and to disable fused multiply add instructions
-#if defined(JPH_COMPILER_CLANG) || defined(JPH_COMPILER_GCC) || defined(JPH_CROSS_PLATFORM_DETERMINISTIC)
-	// In clang it appears you cannot turn off -ffast-math and -ffp-contract=fast for a code block
-	// There is #pragma clang fp contract (off) but that doesn't seem to work under clang 9 & 10 when -ffast-math is specified on the commandline (you override it to turn it on, but not off)
-	// There is #pragma float_control(precise, on) but that doesn't work under clang 9.
-	// So for now we compile clang without -ffast-math so the macros are empty
+#if defined(JPH_COMPILER_GCC) || defined(JPH_CROSS_PLATFORM_DETERMINISTIC)
+	// We compile without -ffast-math and -ffp-contract=fast, so we don't need to disable anything
 	#define JPH_PRECISE_MATH_ON
 	#define JPH_PRECISE_MATH_OFF
+#elif defined(JPH_COMPILER_CLANG)
+	// We compile without -ffast-math because it cannot be turned off for a single compilation unit
+	// On clang 14 and later we can turn off float contraction through a pragma, so if FMA is on we can disable it through this macro
+	#if __clang_major__ >= 14 && defined(JPH_USE_FMADD)
+		#define JPH_PRECISE_MATH_ON					\
+			_Pragma("clang fp contract(off)")
+		#define JPH_PRECISE_MATH_OFF				\
+			_Pragma("clang fp contract(on)")
+	#else
+		#define JPH_PRECISE_MATH_ON
+		#define JPH_PRECISE_MATH_OFF
+	#endif
 #elif defined(JPH_COMPILER_MSVC)
 	// Unfortunately there is no way to push the state of fp_contract, so we have to assume it was turned on before JPH_PRECISE_MATH_ON
 	#define JPH_PRECISE_MATH_ON						\

+ 22 - 0
UnitTests/Geometry/ClosestPointTests.cpp

@@ -0,0 +1,22 @@
+// SPDX-FileCopyrightText: 2023 Jorrit Rouwe
+// SPDX-License-Identifier: MIT
+
+#include "UnitTestFramework.h"
+#include <Jolt/Geometry/ClosestPoint.h>
+
+TEST_SUITE("ClosestPointTests")
+{
+	TEST_CASE("TestNearDegenerateTriangle")
+	{
+		// A very long triangle that is nearly colinear
+		Vec3 a(99.9999847f, 0.946687222f, 99.9999847f);
+		Vec3 b(-100.010002f, 0.977360725f, -100.010002f);
+		Vec3 c(-100.000137f, 0.977310658f, -100.000137f);
+
+		uint32 set;
+		Vec3 p = ClosestPoint::GetClosestPointOnTriangle(a, b, c, set);
+
+		CHECK(set == 0x3);
+		CHECK_APPROX_EQUAL(p, Vec3(7.62939453e-05f, 0.962023199f, 7.62939453e-05f));
+	}
+}

+ 1 - 0
UnitTests/UnitTests.cmake

@@ -10,6 +10,7 @@ set(UNIT_TESTS_SRC_FILES
 	${UNIT_TESTS_ROOT}/Core/StringToolsTest.cpp
 	${UNIT_TESTS_ROOT}/Core/QuickSortTest.cpp
 	${UNIT_TESTS_ROOT}/doctest.h
+	${UNIT_TESTS_ROOT}/Geometry/ClosestPointTests.cpp
 	${UNIT_TESTS_ROOT}/Geometry/ConvexHullBuilderTest.cpp
 	${UNIT_TESTS_ROOT}/Geometry/EllipseTest.cpp
 	${UNIT_TESTS_ROOT}/Geometry/EPATests.cpp