From f3fbc8b3cafa9656e0f12ac03b4ad82ea60b7f00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Fri, 25 Apr 2025 22:42:30 +0200 Subject: [PATCH] Math: make Vector::isZero() equivalent to comparison to a zero vector. The original behavior was extremely imprecise. I remember hitting this in the UI library, where it was happily telling me that a vector is zero, and I spent ages debugging only to replace it with an equality comparison that behaved correctly. Let's fix that properly. --- doc/changelog.dox | 4 ++++ src/Magnum/Math/Test/VectorTest.cpp | 13 +++++++++++-- src/Magnum/Math/Vector.h | 26 +++++++------------------- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index cd146abc8..abd11a7bb 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -1252,6 +1252,10 @@ See also: @cpp transformation.rotationScaling().inverted().transposed() @ce every time, it's just corrected to return an inverse transpose instead of a @ref Math::Matrix::cofactor() of the upper 3x3 matrix. +- Changed @ref Math::Vector::isZero() to have exactly the same behavior as + comparing to a zero vector, as those operations should be interchangeable. + Before the function returned @cpp true @ce for values up to @cpp 0.003f @ce + in case of 32-bit floats which was extremely imprecise. - Fixed an assertion when using @ref MeshTools::removeDuplicates() on an interleaved @ref Trade::MeshData that included padding at the beginning or end of each vertex diff --git a/src/Magnum/Math/Test/VectorTest.cpp b/src/Magnum/Math/Test/VectorTest.cpp index b325eba59..21970501b 100644 --- a/src/Magnum/Math/Test/VectorTest.cpp +++ b/src/Magnum/Math/Test/VectorTest.cpp @@ -415,9 +415,18 @@ void VectorTest::convert() { } void VectorTest::isZeroFloat() { - CORRADE_VERIFY(!Vector3(0.01f, 0.0f, 0.0f).isZero()); - CORRADE_VERIFY(Vector3(0.0f, Math::TypeTraits::epsilon()/2.0f, 0.0f).isZero()); + /* Zero vector is zero */ CORRADE_VERIFY(Vector3(0.0f, 0.0f, 0.0f).isZero()); + + /* Small vector is not */ + CORRADE_VERIFY(!Vector3(0.01f, 0.0f, 0.0f).isZero()); + + /* Should behave the same as comparison to a zero Vector3 */ + CORRADE_VERIFY(Vector3(0.0f, Math::TypeTraits::epsilon()*0.9f, 0.0f).isZero()); + CORRADE_VERIFY(Vector3(0.0f, Math::TypeTraits::epsilon()*0.9f, 0.0f) == Vector3{}); + + CORRADE_VERIFY(!Vector3(0.0f, Math::TypeTraits::epsilon(), 0.0f).isZero()); + CORRADE_VERIFY(Vector3(0.0f, Math::TypeTraits::epsilon(), 0.0f) != Vector3{}); } void VectorTest::isZeroInteger() { diff --git a/src/Magnum/Math/Vector.h b/src/Magnum/Math/Vector.h index 400acf612..299804f99 100644 --- a/src/Magnum/Math/Vector.h +++ b/src/Magnum/Math/Vector.h @@ -81,20 +81,6 @@ namespace Implementation { return T((U(1) - t)*a + t*b); } - template struct IsZero; - template<> struct IsZero { - template bool operator()(const Vector& vec) const { - /* Proper comparison should be with epsilon^2, but the value is not - representable in given precision. Comparing to epsilon instead. */ - return std::abs(vec.dot()) < TypeTraits::epsilon(); - } - }; - template<> struct IsZero { - template bool operator()(const Vector& vec) const { - return vec == Vector{}; - } - }; - /* Used to make friends to speed up debug builds */ template struct MatrixDeterminant; /* To make gather() / scatter() faster */ @@ -360,13 +346,15 @@ template class Vector { /** * @brief Whether the vector is zero * - * @f[ - * |\boldsymbol a \cdot \boldsymbol a - 0| < \epsilon^2 \cong \epsilon - * @f] - * @see @ref dot(), @ref normalized() + * Equivalent to @ref operator==() with a default-constructed vector. + * Done using @ref TypeTraits::equals(), i.e. with fuzzy compare for + * floating-point types. */ bool isZero() const { - return Implementation::IsZero::value>{}(*this); + for(std::size_t i = 0; i != size; ++i) + if(!TypeTraits::equals(_data[i], T())) + return false; + return true; } /**