From 904a63f33bf673a12a54059b987959ace12c2941 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 28 May 2019 14:39:24 +0200 Subject: [PATCH] MeshTools: make generateSmoothNormals() working with NaNs. --- src/Magnum/MeshTools/GenerateNormals.cpp | 20 ++++++-- src/Magnum/MeshTools/GenerateNormals.h | 2 + .../MeshTools/Test/GenerateNormalsTest.cpp | 51 +++++++++++++++++++ 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/Magnum/MeshTools/GenerateNormals.cpp b/src/Magnum/MeshTools/GenerateNormals.cpp index fceb11a41..e14e6408a 100644 --- a/src/Magnum/MeshTools/GenerateNormals.cpp +++ b/src/Magnum/MeshTools/GenerateNormals.cpp @@ -28,6 +28,7 @@ #include #include +#include "Magnum/Math/Functions.h" #include "Magnum/Math/Vector3.h" #ifdef MAGNUM_BUILD_DEPRECATED @@ -143,13 +144,24 @@ template void generateSmoothNormalsInto(const Containers::StridedArrayV /* Cross product */ crossAngles[i].first = Math::cross(v2 - v1, v0 - v1); + /* If any of the vectors is zero, the normalization would result in a + NaN and the angle calculation will assert. This happens also when + any of the original positions is NaN. If that's the case, skip the + rest. Given triangle will then contribute with a zero total angle, + effectively getting ignored for normal calculation. */ + const Vector3 v10n = (v1 - v0).normalized(); + const Vector3 v20n = (v2 - v0).normalized(); + const Vector3 v21n = (v2 - v1).normalized(); + if(Math::isNan(v10n) || Math::isNan(v20n) || Math::isNan(v21n)) { + crossAngles[i].second = Math::Vector3{Math::ZeroInit}; + continue; + } + /* Inner angle at each vertex of the triangle. The last one can be calculated as a remainder to 180°. */ using namespace Math::Literals; - crossAngles[i].second[0] = Math::angle( - (v1 - v0).normalized(), (v2 - v0).normalized()); - crossAngles[i].second[1] = Math::angle( - (v0 - v1).normalized(), (v2 - v1).normalized()); + crossAngles[i].second[0] = Math::angle(v10n, v20n); + crossAngles[i].second[1] = Math::angle(-v10n, v21n); crossAngles[i].second[2] = Rad(180.0_degf) - crossAngles[i].second[0] - crossAngles[i].second[1]; } diff --git a/src/Magnum/MeshTools/GenerateNormals.h b/src/Magnum/MeshTools/GenerateNormals.h index 7d45d660f..a370d4b1f 100644 --- a/src/Magnum/MeshTools/GenerateNormals.h +++ b/src/Magnum/MeshTools/GenerateNormals.h @@ -99,6 +99,8 @@ Uses the @p indices array to discover adjacent triangles and then for each vertex position calculates a normal averaged from all triangles that share it. The normal is weighted according to adjacent triangle area and angle at given vertex; hard edges are preserved where adjacent triangles don't share vertices. +Triangles with zero area or triangles containing invalid positions (NaNs) don't +contribute to calculated vertex normals. Implementation is based on the article [Weighted Vertex Normals](http://www.bytehazard.com/articles/vertnorm.html) by diff --git a/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp b/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp index 5fed3ecce..915da8a93 100644 --- a/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp +++ b/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp @@ -55,6 +55,8 @@ struct GenerateNormalsTest: TestSuite::Tester { void smoothCube(); void smoothBeveledCube(); void smoothCylinder(); + void smoothZeroAreaTriangle(); + void smoothNanPosition(); void smoothWrongCount(); void smoothIntoWrongSize(); @@ -74,6 +76,8 @@ GenerateNormalsTest::GenerateNormalsTest() { &GenerateNormalsTest::smoothCube, &GenerateNormalsTest::smoothBeveledCube, &GenerateNormalsTest::smoothCylinder, + &GenerateNormalsTest::smoothZeroAreaTriangle, + &GenerateNormalsTest::smoothNanPosition, &GenerateNormalsTest::smoothWrongCount, &GenerateNormalsTest::smoothIntoWrongSize}); @@ -324,6 +328,53 @@ void GenerateNormalsTest::smoothCylinder() { Containers::arrayView(data.normals(0)), TestSuite::Compare::Container); } +void GenerateNormalsTest::smoothZeroAreaTriangle() { + constexpr Vector3 positions[] { + {-1.0f, 0.0f, 0.0f}, + { 1.0f, 0.0f, 0.0f}, + { 0.0f, 1.0f, 0.0f}, + }; + + /* Second triangle is just an edge, so it shouldn't contribute to the first + triangle normal */ + constexpr UnsignedInt indices[] { + 0, 1, 2, 1, 2, 1 + }; + + CORRADE_COMPARE_AS(generateSmoothNormals( + Containers::stridedArrayView(indices), positions), + (Containers::Array{Containers::InPlaceInit, { + Vector3::zAxis(), + Vector3::zAxis(), + Vector3::zAxis() + }}), TestSuite::Compare::Container); +} + +void GenerateNormalsTest::smoothNanPosition() { + constexpr Vector3 positions[] { + {-1.0f, 0.0f, 0.0f}, + { 1.0f, 0.0f, 0.0f}, + { 0.0f, 1.0f, 0.0f}, + { 0.0f, Constants::nan(), 0.0f}, + }; + + /* Second triangle is just an edge, so it shouldn't contribute to the first + triangle normal */ + constexpr UnsignedInt indices[] { + 0, 1, 2, 1, 2, 1 + }; + + Containers::Array generated = generateSmoothNormals( + Containers::stridedArrayView(indices), positions); + CORRADE_COMPARE_AS(generated.prefix(3), + (Containers::Array{Containers::InPlaceInit, { + Vector3::zAxis(), + Vector3::zAxis(), + Vector3::zAxis() + }}), TestSuite::Compare::Container>); + CORRADE_COMPARE(Math::isNan(generated[3]), BoolVector3{0x7}); +} + void GenerateNormalsTest::smoothWrongCount() { std::stringstream out; Error redirectError{&out};