From 55afd472fe112df95e1e7ea951e15f9162b5e69d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 20 Aug 2013 22:25:03 +0200 Subject: [PATCH] Shapes: expect only uniform scaling for some primitives. Capsule, Sphere and Plane would behave weirdly if non-uniform scale would be applied to them. This restriction allows to use more performant code (i.e. less normalization and matrix multiplication). Updated the tests to verify that uniform scaling is handled well, for Capsule and Sphere there is now also no need to test both 2D and 3D transformation. --- src/Shapes/Capsule.cpp | 15 +------------ src/Shapes/Capsule.h | 6 ++--- src/Shapes/Plane.cpp | 5 ++++- src/Shapes/Plane.h | 4 ++-- src/Shapes/Sphere.cpp | 15 +------------ src/Shapes/Sphere.h | 6 ++--- src/Shapes/Test/CapsuleTest.cpp | 33 ++++++--------------------- src/Shapes/Test/PlaneTest.cpp | 10 +++------ src/Shapes/Test/SphereTest.cpp | 40 +++++---------------------------- 9 files changed, 28 insertions(+), 106 deletions(-) diff --git a/src/Shapes/Capsule.cpp b/src/Shapes/Capsule.cpp index 7fada2aee..d0df89afa 100644 --- a/src/Shapes/Capsule.cpp +++ b/src/Shapes/Capsule.cpp @@ -36,21 +36,8 @@ using namespace Magnum::Math::Geometry; namespace Magnum { namespace Shapes { -namespace { - template static typename DimensionTraits::VectorType unitVector(); - - template<> inline Vector2 unitVector<2>() { - return Vector2(1/Constants::sqrt2()); - } - - template<> inline Vector3 unitVector<3>() { - return Vector3(1/Constants::sqrt3()); - } -} - template Capsule Capsule::transformed(const typename DimensionTraits::MatrixType& matrix) const { - return Capsule(matrix.transformPoint(_a), matrix.transformPoint(_b), - (matrix.rotationScaling()*unitVector()).length()*_radius); + return Capsule(matrix.transformPoint(_a), matrix.transformPoint(_b), matrix.uniformScaling()*_radius); } template bool Capsule::operator%(const Point& other) const { diff --git a/src/Shapes/Capsule.h b/src/Shapes/Capsule.h index fb0de064c..f0a654cbc 100644 --- a/src/Shapes/Capsule.h +++ b/src/Shapes/Capsule.h @@ -38,11 +38,9 @@ namespace Magnum { namespace Shapes { /** @brief %Capsule defined by cylinder start and end point and radius -Unlike other elements the capsule doesn't support asymmetric scaling. When -applying transformation, the scale factor is averaged from all axes. See -@ref shapes for brief introduction. +Unlike other elements the capsule expects uniform scaling. See @ref shapes for +brief introduction. @see Capsule2D, Capsule3D -@todo Assert for asymmetric scaling to avoid costly sqrt? */ template class MAGNUM_SHAPES_EXPORT Capsule { public: diff --git a/src/Shapes/Plane.cpp b/src/Shapes/Plane.cpp index e62d5a15f..7c2ef4fc0 100644 --- a/src/Shapes/Plane.cpp +++ b/src/Shapes/Plane.cpp @@ -35,8 +35,11 @@ using namespace Magnum::Math::Geometry; namespace Magnum { namespace Shapes { Plane Plane::transformed(const Matrix4& matrix) const { + /* Using matrix.rotation() would result in two more normalizations (slow), + using .normalized() instead of matrix.uniformScaling() would not check + uniform scaling */ return Plane(matrix.transformPoint(_position), - matrix.rotation()*_normal); + matrix.rotationScaling()*_normal/matrix.uniformScaling()); } bool Plane::operator%(const Line3D& other) const { diff --git a/src/Shapes/Plane.h b/src/Shapes/Plane.h index 50058fc68..81f1608e5 100644 --- a/src/Shapes/Plane.h +++ b/src/Shapes/Plane.h @@ -38,8 +38,8 @@ namespace Magnum { namespace Shapes { /** @brief Infinite plane, defined by position and normal (3D only) -See @ref shapes for brief introduction. -@todo Assert for uniform scaling to avoid costly normalization? +Unlike other elements the plane expects uniform scaling. See @ref shapes for +brief introduction. */ class MAGNUM_SHAPES_EXPORT Plane { public: diff --git a/src/Shapes/Sphere.cpp b/src/Shapes/Sphere.cpp index f1af70317..295a9ab0a 100644 --- a/src/Shapes/Sphere.cpp +++ b/src/Shapes/Sphere.cpp @@ -36,21 +36,8 @@ using namespace Magnum::Math::Geometry; namespace Magnum { namespace Shapes { -namespace { - template static typename DimensionTraits::VectorType unitVector(); - - template<> inline Vector2 unitVector<2>() { - return Vector2(1/Constants::sqrt2()); - } - - template<> inline Vector3 unitVector<3>() { - return Vector3(1/Constants::sqrt3()); - } -} - template Sphere Sphere::transformed(const typename DimensionTraits::MatrixType& matrix) const { - return Sphere(matrix.transformPoint(_position), - (matrix.rotationScaling()*unitVector()).length()*_radius); + return Sphere(matrix.transformPoint(_position), matrix.uniformScaling()*_radius); } template bool Sphere::operator%(const Point& other) const { diff --git a/src/Shapes/Sphere.h b/src/Shapes/Sphere.h index dc4c5e2f0..44f7faa03 100644 --- a/src/Shapes/Sphere.h +++ b/src/Shapes/Sphere.h @@ -38,11 +38,9 @@ namespace Magnum { namespace Shapes { /** @brief %Sphere defined by position and radius -Unlike other elements the sphere doesn't support asymmetric scaling. When -applying transformation, the scale factor is averaged from all axes. See -@ref shapes for brief introduction. +Unlike other elements the sphere expects uniform scaling. See @ref shapes for +brief introduction. @see Sphere2D, Sphere3D -@todo Assert for asymmetric scaling to avoid costly sqrt? */ template class MAGNUM_SHAPES_EXPORT Sphere { public: diff --git a/src/Shapes/Test/CapsuleTest.cpp b/src/Shapes/Test/CapsuleTest.cpp index 03bd09848..ebbb78241 100644 --- a/src/Shapes/Test/CapsuleTest.cpp +++ b/src/Shapes/Test/CapsuleTest.cpp @@ -37,44 +37,25 @@ class CapsuleTest: public TestSuite::Tester { public: CapsuleTest(); - void transformed2D(); - void transformed3D(); + void transformed(); void transformedAverageScaling(); void collisionPoint(); void collisionSphere(); }; CapsuleTest::CapsuleTest() { - addTests({&CapsuleTest::transformed2D, - &CapsuleTest::transformed3D, + addTests({&CapsuleTest::transformed, &CapsuleTest::collisionPoint, &CapsuleTest::collisionSphere}); } -void CapsuleTest::transformed2D() { - const Shapes::Capsule2D capsule({1.0f, 2.0f}, {-1.0f, -2.0f}, 7.0f); - - const auto transformed = capsule.transformed(Matrix3::rotation(Deg(90.0f))); - CORRADE_COMPARE(transformed.a(), Vector2(-2.0f, 1.0f)); - CORRADE_COMPARE(transformed.b(), Vector2(2.0f, -1.0f)); - CORRADE_COMPARE(transformed.radius(), 7.0f); - - /* Apply average scaling to radius */ - const auto scaled = capsule.transformed(Matrix3::scaling({-Constants::sqrt2(), 2.0f})); - CORRADE_COMPARE(scaled.radius(), Constants::sqrt3()*7.0f); -} - -void CapsuleTest::transformed3D() { +void CapsuleTest::transformed() { const Shapes::Capsule3D capsule({1.0f, 2.0f, 3.0f}, {-1.0f, -2.0f, -3.0f}, 7.0f); - const auto transformed = capsule.transformed(Matrix4::rotation(Deg(90.0f), Vector3::zAxis())); - CORRADE_COMPARE(transformed.a(), Vector3(-2.0f, 1.0f, 3.0f)); - CORRADE_COMPARE(transformed.b(), Vector3(2.0f, -1.0f, -3.0f)); - CORRADE_COMPARE(transformed.radius(), 7.0f); - - /* Apply average scaling to radius */ - const auto scaled = capsule.transformed(Matrix4::scaling({Constants::sqrt3(), -Constants::sqrt2(), 2.0f})); - CORRADE_COMPARE(scaled.radius(), Constants::sqrt3()*7.0f); + const auto transformed = capsule.transformed(Matrix4::scaling(Vector3(2.0f))*Matrix4::rotation(Deg(90.0f), Vector3::zAxis())); + CORRADE_COMPARE(transformed.a(), Vector3(-4.0f, 2.0f, 6.0f)); + CORRADE_COMPARE(transformed.b(), Vector3(4.0f, -2.0f, -6.0f)); + CORRADE_COMPARE(transformed.radius(), 14.0f); } void CapsuleTest::collisionPoint() { diff --git a/src/Shapes/Test/PlaneTest.cpp b/src/Shapes/Test/PlaneTest.cpp index c86a6386b..5041aeb85 100644 --- a/src/Shapes/Test/PlaneTest.cpp +++ b/src/Shapes/Test/PlaneTest.cpp @@ -49,14 +49,10 @@ PlaneTest::PlaneTest() { void PlaneTest::transformed() { const Shapes::Plane plane({1.0f, 2.0f, 3.0f}, {Constants::sqrt2(), -Constants::sqrt2(), 0}); - const auto transformed = plane.transformed(Matrix4::rotation(Deg(90.0f), Vector3::xAxis())); - CORRADE_COMPARE(transformed.position(), Vector3(1.0f, -3.0f, 2.0f)); + /* The normal should stay normalized after scaling */ + const auto transformed = plane.transformed(Matrix4::scaling(Vector3(2.0f))*Matrix4::rotation(Deg(90.0f), Vector3::xAxis())); + CORRADE_COMPARE(transformed.position(), Vector3(2.0f, -6.0f, 4.0f)); CORRADE_COMPARE(transformed.normal(), Vector3(Constants::sqrt2(), 0, -Constants::sqrt2())); - - /* The normal should stay normalized */ - const auto scaled = plane.transformed(Matrix4::scaling({1.5f, 2.0f, 3.0f})); - CORRADE_COMPARE(scaled.position(), Vector3(1.5f, 4.0f, 9.0f)); - CORRADE_COMPARE(scaled.normal(), Vector3(Constants::sqrt2(), -Constants::sqrt2(), 0)); } void PlaneTest::collisionLine() { diff --git a/src/Shapes/Test/SphereTest.cpp b/src/Shapes/Test/SphereTest.cpp index 538a5b282..b66e86e50 100644 --- a/src/Shapes/Test/SphereTest.cpp +++ b/src/Shapes/Test/SphereTest.cpp @@ -37,8 +37,7 @@ class SphereTest: public TestSuite::Tester { public: SphereTest(); - void transformed2D(); - void transformed3D(); + void transformed(); void collisionPoint(); void collisionLine(); void collisionLineSegment(); @@ -46,46 +45,19 @@ class SphereTest: public TestSuite::Tester { }; SphereTest::SphereTest() { - addTests({&SphereTest::transformed2D, - &SphereTest::transformed3D, + addTests({&SphereTest::transformed, &SphereTest::collisionPoint, &SphereTest::collisionLine, &SphereTest::collisionLineSegment, &SphereTest::collisionSphere}); } -void SphereTest::transformed2D() { - const Shapes::Sphere2D sphere({1.0f, 2.0f}, 7.0f); - - const auto transformed = sphere.transformed(Matrix3::rotation(Deg(90.0f))); - CORRADE_COMPARE(transformed.position(), Vector2(-2.0f, 1.0f)); - CORRADE_COMPARE(transformed.radius(), 7.0f); - - /* Symmetric scaling */ - const auto scaled = sphere.transformed(Matrix3::scaling(Vector2(2.0f))); - CORRADE_COMPARE(scaled.position(), Vector2(2.0f, 4.0f)); - CORRADE_COMPARE(scaled.radius(), 14.0f); - - /* Apply average scaling to radius */ - const auto nonEven = sphere.transformed(Matrix3::scaling({-Constants::sqrt2(), 2.0f})); - CORRADE_COMPARE(nonEven.radius(), Constants::sqrt3()*7.0f); -} - -void SphereTest::transformed3D() { +void SphereTest::transformed() { const Shapes::Sphere3D sphere({1.0f, 2.0f, 3.0f}, 7.0f); - const auto transformed = sphere.transformed(Matrix4::rotation(Deg(90.0f), Vector3::yAxis())); - CORRADE_COMPARE(transformed.position(), Vector3(3.0f, 2.0f, -1.0f)); - CORRADE_COMPARE(transformed.radius(), 7.0f); - - /* Symmetric scaling */ - const auto scaled = sphere.transformed(Matrix4::scaling(Vector3(2.0f))); - CORRADE_COMPARE(scaled.position(), Vector3(2.0f, 4.0f, 6.0f)); - CORRADE_COMPARE(scaled.radius(), 14.0f); - - /* Apply average scaling to radius */ - const auto nonEven = sphere.transformed(Matrix4::scaling({Constants::sqrt3(), -Constants::sqrt2(), 2.0f})); - CORRADE_COMPARE(nonEven.radius(), Constants::sqrt3()*7.0f); + const auto transformed = sphere.transformed(Matrix4::scaling(Vector3(2.0f))*Matrix4::rotation(Deg(90.0f), Vector3::yAxis())); + CORRADE_COMPARE(transformed.position(), Vector3(6.0f, 4.0f, -2.0f)); + CORRADE_COMPARE(transformed.radius(), 14.0f); } void SphereTest::collisionPoint() {