From f2f66d764ffbd93ee3f5f26251cc3a90546ccd85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 31 Mar 2020 19:10:06 +0200 Subject: [PATCH] Math: make TypeTraits::epsilon() consistent with Corrade. The precision stays the same, but the long double variant is now exposed on Emscripten as well, following a similar change in Corrade. Additionally, the alien-looking _EQUALITY_PRECISION macros are now unused and deprecated. For some reason these weren't ever prefixed with MAGNUM_, and the ability to override those is an extremely rare use case that would break half of the assumptions everywhere, so better not allow that at all. The TypeTraits test is further extended to compare directly the epsilons between Magnum and Corrade, in addition to verifying that TestSuite and TypeTraits have the same comparison results. --- doc/changelog.dox | 7 ++++ src/Magnum/Math/Test/TypeTraitsTest.cpp | 36 ++++++++++++----- src/Magnum/Math/TypeTraits.h | 52 ++++++++++++++++++------- 3 files changed, 72 insertions(+), 23 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 466ac0a52..dac2dfc14 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -549,6 +549,13 @@ See also: deprecated on the 3D variant, use separate @ref Shaders::MeshVisualizer3D::setTransformationMatrix() and @ref Shaders::MeshVisualizer3D::setProjectionMatrix() instead +- The unprefixed and alien-looking @cpp FLOAT_EQUALITY_PRECISION @ce, + @cpp DOUBLE_EQUALITY_PRECISION @ce and @cpp LONG_DOUBLE_EQUALITY_PRECISION @ce + macros are no longer used by any code and thus deprecated in favor of using + @ref Math::TypeTraits::epsilon() instead, which is also consistent with how + This also means it's no longer + possible to override equality comparison epsilons at compile time, but that + was a rarely (if ever) used feature. @subsection changelog-latest-compatibility Potential compatibility breakages, removed APIs diff --git a/src/Magnum/Math/Test/TypeTraitsTest.cpp b/src/Magnum/Math/Test/TypeTraitsTest.cpp index fbc843e7d..c485eb6d2 100644 --- a/src/Magnum/Math/Test/TypeTraitsTest.cpp +++ b/src/Magnum/Math/Test/TypeTraitsTest.cpp @@ -48,6 +48,8 @@ struct TypeTraitsTest: Corrade::TestSuite::Tester { void underlyingTypeOf(); + template void epsilonConsistentWithCorrade(); + template void equalsIntegral(); void equalsHalf(); template void equalsFloatingPoint0(); @@ -116,6 +118,10 @@ TypeTraitsTest::TypeTraitsTest() { &TypeTraitsTest::underlyingTypeOf, + &TypeTraitsTest::epsilonConsistentWithCorrade, + &TypeTraitsTest::epsilonConsistentWithCorrade, + &TypeTraitsTest::epsilonConsistentWithCorrade, + &TypeTraitsTest::equalsIntegral, &TypeTraitsTest::equalsIntegral, &TypeTraitsTest::equalsIntegral, @@ -131,19 +137,13 @@ TypeTraitsTest::TypeTraitsTest() { &TypeTraitsTest::equalsFloatingPoint0, &TypeTraitsTest::equalsFloatingPoint0, - #ifndef CORRADE_TARGET_EMSCRIPTEN &TypeTraitsTest::equalsFloatingPoint0, - #endif &TypeTraitsTest::equalsFloatingPoint1, &TypeTraitsTest::equalsFloatingPoint1, - #ifndef CORRADE_TARGET_EMSCRIPTEN &TypeTraitsTest::equalsFloatingPoint1, - #endif &TypeTraitsTest::equalsFloatingPointLarge, &TypeTraitsTest::equalsFloatingPointLarge, - #ifndef CORRADE_TARGET_EMSCRIPTEN &TypeTraitsTest::equalsFloatingPointLarge, - #endif &TypeTraitsTest::equalsFloatingPointInfinity, &TypeTraitsTest::equalsFloatingPointInfinity, &TypeTraitsTest::equalsFloatingPointNaN, @@ -268,6 +268,14 @@ void TypeTraitsTest::underlyingTypeOf() { CORRADE_VERIFY((std::is_same>, Float>::value)); } +template void TypeTraitsTest::epsilonConsistentWithCorrade() { + setTestCaseTemplateName(TypeTraits::name()); + + /* Using VERIFY because we *don't* want fuzzy comparison in this case. The + equals*() tests below do further checks against TestSuite. */ + CORRADE_VERIFY(TypeTraits::epsilon() == Corrade::Utility::Implementation::FloatPrecision::epsilon()); +} + template void TypeTraitsTest::equalsIntegral() { setTestCaseTemplateName(TypeTraits::name()); @@ -289,7 +297,9 @@ template void TypeTraitsTest::equalsFloatingPoint0() { CORRADE_VERIFY(TypeTraits::equals(T(0)+TypeTraits::epsilon()/T(2), T(0))); CORRADE_VERIFY(!TypeTraits::equals(T(0)+TypeTraits::epsilon()*T(2), T(0))); - /* Ensure we have the same behavior as TestSuite */ + /* Ensure we have the same behavior as TestSuite. Done in addition to the + epsilonConsistentWithCorrade() test above, since that one alone might + give a false sense of security. */ CORRADE_COMPARE(Corrade::TestSuite::Implementation::FloatComparator{}( T(0)+TypeTraits::epsilon()/T(2), T(0)), Corrade::TestSuite::ComparisonStatusFlags{}); @@ -304,7 +314,9 @@ template void TypeTraitsTest::equalsFloatingPoint1() { CORRADE_VERIFY(TypeTraits::equals(T(1)+TypeTraits::epsilon()/T(2), T(1))); CORRADE_VERIFY(!TypeTraits::equals(T(1)+TypeTraits::epsilon()*T(3), T(1))); - /* Ensure we have the same behavior as TestSuite */ + /* Ensure we have the same behavior as TestSuite. Done in addition to the + epsilonConsistentWithCorrade() test above, since that one alone might + give a false sense of security. */ CORRADE_COMPARE(Corrade::TestSuite::Implementation::FloatComparator{}( T(1)+TypeTraits::epsilon()/T(2), T(1)), Corrade::TestSuite::ComparisonStatusFlags{}); @@ -319,7 +331,9 @@ template void TypeTraitsTest::equalsFloatingPointLarge() { CORRADE_VERIFY(TypeTraits::equals(T(25)+TypeTraits::epsilon()*T(2), T(25))); CORRADE_VERIFY(!TypeTraits::equals(T(25)+TypeTraits::epsilon()*T(75), T(25))); - /* Ensure we have the same behavior as TestSuite */ + /* Ensure we have the same behavior as TestSuite. Done in addition to the + epsilonConsistentWithCorrade() test above, since that one alone might + give a false sense of security. */ CORRADE_COMPARE(Corrade::TestSuite::Implementation::FloatComparator{}( T(25)+TypeTraits::epsilon()*T(2), T(25)), Corrade::TestSuite::ComparisonStatusFlags{}); @@ -336,7 +350,9 @@ template void TypeTraitsTest::equalsFloatingPointInfinity() { CORRADE_VERIFY(!TypeTraits::equals(Constants::inf(), -Constants::inf())); - /* Ensure we have the same behavior as TestSuite */ + /* Ensure we have the same behavior as TestSuite. Done in addition to the + epsilonConsistentWithCorrade() test above, since that one alone might + give a false sense of security. */ CORRADE_COMPARE(Corrade::TestSuite::Implementation::FloatComparator{}( Constants::inf(), Constants::inf()), Corrade::TestSuite::ComparisonStatusFlags{}); diff --git a/src/Magnum/Math/TypeTraits.h b/src/Magnum/Math/TypeTraits.h index b9412a2ba..1905ac5af 100644 --- a/src/Magnum/Math/TypeTraits.h +++ b/src/Magnum/Math/TypeTraits.h @@ -34,29 +34,42 @@ #include "Magnum/Math/Math.h" #include "Magnum/Types.h" +#ifdef MAGNUM_BUILD_DEPRECATED +#include +#endif + +#ifdef MAGNUM_BUILD_DEPRECATED /** @brief Precision when testing floats for equality +@m_deprecated_since_latest Use @ref Magnum::Math::TypeTraits::epsilon() + instead. They have "at least" 6 significant digits of precision, taking one digit less for more headroom. */ #ifndef FLOAT_EQUALITY_PRECISION -#define FLOAT_EQUALITY_PRECISION 1.0e-5f +#define FLOAT_EQUALITY_PRECISION \ + CORRADE_DEPRECATED_MACRO(FLOAT_EQUALITY_PRECISION, "use Math::TypeTraits instead") 1.0e-5f #endif /** @brief Precision when testing doubles for equality +@m_deprecated_since_latest Use @ref Magnum::Math::TypeTraits::epsilon() + instead. They have "at least" 15 significant digits of precision, taking one digit less for more headroom. */ #ifndef DOUBLE_EQUALITY_PRECISION -#define DOUBLE_EQUALITY_PRECISION 1.0e-14 +#define DOUBLE_EQUALITY_PRECISION \ + CORRADE_DEPRECATED_MACRO(DOUBLE_EQUALITY_PRECISION, "use Math::TypeTraits instead") 1.0e-14 #endif #ifndef CORRADE_TARGET_EMSCRIPTEN /** @brief Precision when testing long doubles for equality +@m_deprecated_since_latest Use @ref Magnum::Math::TypeTraits::epsilon() + instead. They have "at least" 18 significant digits of precision, taking one digit less for more headroom. @@ -71,9 +84,12 @@ for more headroom. */ #ifndef LONG_DOUBLE_EQUALITY_PRECISION #if !defined(_MSC_VER) && (!defined(CORRADE_TARGET_ANDROID) || __LP64__) -#define LONG_DOUBLE_EQUALITY_PRECISION 1.0e-17l +#define LONG_DOUBLE_EQUALITY_PRECISION \ + CORRADE_DEPRECATED_MACRO(LONG_DOUBLE_EQUALITY_PRECISION, "use Math::TypeTraits instead") 1.0e-17l #else -#define LONG_DOUBLE_EQUALITY_PRECISION 1.0e-14 +#define LONG_DOUBLE_EQUALITY_PRECISION \ + CORRADE_DEPRECATED_MACRO(LONG_DOUBLE_EQUALITY_PRECISION, "use Math::TypeTraits instead") 1.0e-14l +#endif #endif #endif #endif @@ -338,8 +354,18 @@ template struct TypeTraits: Implementation::TypeTraitsDefault { * @brief Epsilon value for fuzzy compare * * Returns minimal difference between numbers to be considered - * inequal. Returns 1 for integer types and reasonably small value for - * floating-point types. Not implemented for arbitrary types. + * inequal. Not implemented for arbitrary types. Returns @cpp 1 @ce for + * integer types and + * + * - @cpp 1.0e-5f @ce for @cpp float @ce, + * - @cpp 1.0e-15 @ce for @cpp double @ce, + * - @cpp 1.0e-17l @ce for @cpp long double @ce on platforms where it is + * 80-bit, and @cpp 1.0e-14l @ce on platforms + * @ref CORRADE_LONG_DOUBLE_SAME_AS_DOUBLE "where it is 64-bit". + * + * This matches fuzzy comparison precision in @ref Corrade::TestSuite and + * is always one digit less than how @ref Corrade::Utility::Debug or + * @ref Corrade::Utility::format() prints given type. */ constexpr static T epsilon(); @@ -414,9 +440,7 @@ namespace Implementation { _c(Float) _c(Half) _c(Double) - #ifndef CORRADE_TARGET_EMSCRIPTEN _c(long double) - #endif #undef _c #endif @@ -499,7 +523,7 @@ template bool TypeTraitsFloatingPoint::equalsZero(const T a, const T template<> struct TypeTraits: Implementation::TypeTraitsFloatingPoint { typedef Float FloatingPointType; - constexpr static Float epsilon() { return FLOAT_EQUALITY_PRECISION; } + constexpr static Float epsilon() { return 1.0e-5f; } }; /* A bit special -- using integer comparison for equality but presenting itself as a floating-point type so Color's fullChannel() works correctly */ @@ -509,15 +533,17 @@ template<> struct TypeTraits: Implementation::TypeTraitsName, Implem template<> struct TypeTraits: Implementation::TypeTraitsFloatingPoint { typedef Double FloatingPointType; - constexpr static Double epsilon() { return DOUBLE_EQUALITY_PRECISION; } + constexpr static Double epsilon() { return 1.0e-14; } }; -#ifndef CORRADE_TARGET_EMSCRIPTEN template<> struct TypeTraits: Implementation::TypeTraitsFloatingPoint { typedef long double FloatingPointType; - constexpr static long double epsilon() { return LONG_DOUBLE_EQUALITY_PRECISION; } + #ifndef CORRADE_LONG_DOUBLE_SAME_AS_DOUBLE + constexpr static long double epsilon() { return 1.0e-17l; } + #else + constexpr static long double epsilon() { return 1.0e-14l; } + #endif }; -#endif namespace Implementation {