From b7eb367dde197270f4593fc669a1b624595f7505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 28 Nov 2016 17:27:31 +0100 Subject: [PATCH] Math: cleanup, code compression. --- src/Magnum/Math/Frustum.h | 36 +++-------- src/Magnum/Math/Geometry/Distance.h | 39 ++++++------ src/Magnum/Math/Geometry/Intersection.h | 63 ++++++++----------- .../Math/Geometry/Test/DistanceTest.cpp | 4 +- .../Math/Geometry/Test/IntersectionTest.cpp | 14 +++-- src/Magnum/Math/Test/FrustumTest.cpp | 51 ++++++++------- 6 files changed, 92 insertions(+), 115 deletions(-) diff --git a/src/Magnum/Math/Frustum.h b/src/Magnum/Math/Frustum.h index 6396d51a0..27ad2f29e 100644 --- a/src/Magnum/Math/Frustum.h +++ b/src/Magnum/Math/Frustum.h @@ -42,27 +42,22 @@ namespace Magnum { namespace Math { /** @brief Camera frustum +Stores camera frustum planes in order left (index `0`), right (index `1`), +bottom (index `2`), top (index `3`), near (index `4`) and far (index `5`). */ template class Frustum { public: - - /** - * @brief Create a frustum from projection matrix - */ + /** @brief Create a frustum from projection matrix */ static Frustum fromMatrix(const Matrix4& m) { - return Frustum{ - m.row(3) + m.row(0), + return {m.row(3) + m.row(0), m.row(3) - m.row(0), m.row(3) + m.row(1), m.row(3) - m.row(1), m.row(3) + m.row(2), - m.row(3) - m.row(2) - }; + m.row(3) - m.row(2)}; } - /** - * @brief Construct frustum from frustum planes - */ + /** @brief Constructor */ constexpr /*implicit*/ Frustum(const Vector4& left, const Vector4& right, const Vector4& bottom, const Vector4& top, const Vector4& near, const Vector4& far): _data{left, right, bottom, top, near, far} {} /** @@ -70,26 +65,15 @@ template class Frustum { * @return One-dimensional array of length `24`. */ T* data() { return _data[0].data(); } + constexpr const T* data() const { return _data[0].data(); } /**< @overload */ - /** @overload */ - constexpr const T* data() const { return _data[0].data(); } - - /** - * @brief The frustum planes - * - * In order left (index `0`), right (index `1`), bottom (index `1`), - * top (index `3`), near (index `4`), far (index `5`). - */ + /** @brief Frustum planes */ constexpr Corrade::Containers::StaticArrayView<6, const Vector4> planes() const { + /* GCC 4.7 needs explicit construction */ return Corrade::Containers::StaticArrayView<6, const Vector4>{_data}; } - /** - * @brief Plane at given index - * - * In order left (index `0`), right (index `1`), bottom (index `1`), - * top (index `3`), near (index `4`), far (index `5`). - */ + /** @brief Plane at given index */ constexpr Vector4 operator[](std::size_t i) const { return _data[i]; } private: diff --git a/src/Magnum/Math/Geometry/Distance.h b/src/Magnum/Math/Geometry/Distance.h index 5caa47d44..634a0c69a 100644 --- a/src/Magnum/Math/Geometry/Distance.h +++ b/src/Magnum/Math/Geometry/Distance.h @@ -168,17 +168,16 @@ class Distance { /** * @brief Distance of point from plane * - * The distance **d** is computed from point **p** and plane with normal - * **n** and **w** using: @f[ + * The distance **d** is computed from point **p** and plane with + * normal **n** and **w** using: @f[ * d = \frac{\sum_i^3 (p \cdot n) + w}{\left| n \right|} * @f] * The distance is negative if the point lies behind the plane. * - * In cases where the planes normal is a unit vector, @ref pointPlaneUnnormalized() - * is more efficient. - * - * If merely the sign of the distance is of interest, @ref pointPlaneScaled() - * is more efficient. + * In cases where the planes normal is a unit vector, + * @ref pointPlaneUnnormalized() is more efficient. If merely the sign + * of the distance is of interest, @ref pointPlaneScaled() is more + * efficient. */ template static T pointPlane(const Vector3& point, const Vector4& plane) { return pointPlaneScaled(point, plane)/plane.xyz().length(); @@ -187,15 +186,16 @@ class Distance { /** * @brief Distance of point from plane, scaled by the length of the planes normal * - * The distance **d** is computed from point **p** and plane with normal - * **n** and **w** using: @f[ + * The distance **d** is computed from point **p** and plane with + * normal **n** and **w** using: @f[ * d = \sum_i^3 (p \cdot n) + w * @f] * The distance is negative if the point lies behind the plane. * - * More efficient than @ref pointPlane() when merely the sign of the distance is - * of interest, for example when testing on which half space of the plane the - * point lies. + * More efficient than @ref pointPlane() when merely the sign of the + * distance is of interest, for example when testing on which half + * space of the plane the point lies. + * @see @ref pointPlaneNormalized() */ template static T pointPlaneScaled(const Vector3& point, const Vector4& plane) { return (plane.xyz()*point).sum() + plane.w(); @@ -204,21 +204,22 @@ class Distance { /** * @brief Distance of point from plane with normalized normal * - * The distance **d** is computed from point **p** and plane with normal - * **n** and **w** using: @f[ + * The distance **d** is computed from point **p** and plane with + * normal **n** and **w** using: @f[ * d = \sum_i^3 (p \cdot n) + w * @f] - * The distance is negative if the point lies behind the plane. + * The distance is negative if the point lies behind the plane. Expects + * that @p plane normal is normalized. * - * More efficient than @ref pointPlane() in cases where the planes normal is - * normalized. + * More efficient than @ref pointPlane() in cases where the planes + * normal is normalized. Equivalent to @ref pointPlaneScaled() but with + * assertion added on top. */ template static T pointPlaneNormalized(const Vector3& point, const Vector4& plane) { CORRADE_ASSERT(plane.xyz().isNormalized(), - "Math::Geometry::Distance::pointPlaneNormalized(): the planes normal is not a unit vector", {}); + "Math::Geometry::Distance::pointPlaneNormalized(): plane normal is not an unit vector", {}); return pointPlaneScaled(point, plane); } - }; template T Distance::lineSegmentPoint(const Vector2& a, const Vector2& b, const Vector2& point) { diff --git a/src/Magnum/Math/Geometry/Intersection.h b/src/Magnum/Math/Geometry/Intersection.h index 7c16f536a..06d557c4e 100644 --- a/src/Magnum/Math/Geometry/Intersection.h +++ b/src/Magnum/Math/Geometry/Intersection.h @@ -134,72 +134,61 @@ class Intersection { * @param frustum Frustum planes with normals pointing outwards * @return `true` if the point is on or inside the frustum. * - * Checks for each plane of the frustum whether the point is behind the plane - * (the points distance from the plane is negative) using + * Checks for each plane of the frustum whether the point is behind the + * plane (the points distance from the plane is negative) using * @ref Distance::pointPlaneScaled(). */ template static bool pointFrustum(const Vector3& point, const Frustum& frustum); /** * @brief Intersection of a range and a camera frustum - * @return `true` if the box intersects with the camera frustum. + * @return `true` if the box intersects with the camera frustum * - * Counts for each plane of the frustum how many points of the box lie in - * front of the plane (outside of the frustum). If none, the box must lie - * entirely outside of the frustum and there is no intersection. - * Else, the box is considered as intersecting, even if it is merely corners - * of the box overlapping with corners of the frustum, since checking the - * corners is less efficient. + * Counts for each plane of the frustum how many points of the box lie + * in front of the plane (outside of the frustum). If none, the box + * must lie entirely outside of the frustum and there is no + * intersection. Else, the box is considered as intersecting, even if + * it is merely corners of the box overlapping with corners of the + * frustum, since checking the corners is less efficient. */ template static bool boxFrustum(const Range3D& box, const Frustum& frustum); - }; template bool Intersection::pointFrustum(const Vector3& point, const Frustum& frustum) { - for(const Vector4& f : frustum.planes()) { - if(Distance::pointPlaneScaled(point, f) < T(0)) { - /* the point is in front of one of the frustum planes (normals point outwards) */ + for(const Vector4& plane: frustum.planes()) { + /* The point is in front of one of the frustum planes (normals point + outwards) */ + if(Distance::pointPlaneScaled(point, plane) < T(0)) return false; - } } return true; } template bool Intersection::boxFrustum(const Range3D& box, const Frustum& frustum) { - /* - * Create the 8 vertices of the box from the 2 given vertices min and max - * Check for each corner of an octant whether it is inside the frustum. - * If only some of the corners are inside, the octant requires further checks. - */ - int planes = 0; + /* Create the 8 vertices of the box from the 2 given vertices min and max + Check for each corner of an octant whether it is inside the frustum. If + only some of the corners are inside, the octant requires further checks. */ + Int planes = 0; - for(const Vector4& plane : frustum.planes()) { - int corners = 0; + for(const Vector4& plane: frustum.planes()) { + Int corners = 0; - for(UnsignedByte c = 0; c < 8; ++c) { + for(UnsignedByte c = 0; c != 8; ++c) { const Vector3 corner = Math::lerp(box.min(), box.max(), Math::BoolVector<3>{c}); - if(Distance::pointPlaneScaled(corner, plane) >= T(0)) { + if(Distance::pointPlaneScaled(corner, plane) >= T(0)) ++corners; - } } - if(corners == 0) { - /* all corners are outside this plane */ - return false; - } - - if(corners == 8) { - ++planes; - } - } + /* All corners are outside this plane */ + if(corners == 0) return false; - if(planes == 6) { - return true; + if(corners == 8) ++planes; } - // potentially check corners here to avoid false positives! + /** @todo potentially check corners here to avoid false positives */ + /* if(planes == 6) return true; */ return true; } diff --git a/src/Magnum/Math/Geometry/Test/DistanceTest.cpp b/src/Magnum/Math/Geometry/Test/DistanceTest.cpp index ddebf186c..a66bf5202 100644 --- a/src/Magnum/Math/Geometry/Test/DistanceTest.cpp +++ b/src/Magnum/Math/Geometry/Test/DistanceTest.cpp @@ -39,6 +39,7 @@ struct DistanceTest: Corrade::TestSuite::Tester { void linePoint3D(); void lineSegmentPoint2D(); void lineSegmentPoint3D(); + void pointPlane(); void pointPlaneScaled(); void pointPlaneNormalized(); @@ -54,6 +55,7 @@ DistanceTest::DistanceTest() { &DistanceTest::linePoint3D, &DistanceTest::lineSegmentPoint2D, &DistanceTest::lineSegmentPoint3D, + &DistanceTest::pointPlane, &DistanceTest::pointPlaneScaled, &DistanceTest::pointPlaneNormalized}); @@ -189,7 +191,7 @@ void DistanceTest::pointPlaneNormalized() { std::ostringstream o; Error redirectError{&o}; Distance::pointPlaneNormalized(point, invalidPlane); - CORRADE_COMPARE(o.str(), "Math::Geometry::Distance::pointPlaneNormalized(): the planes normal is not a unit vector\n"); + CORRADE_COMPARE(o.str(), "Math::Geometry::Distance::pointPlaneNormalized(): plane normal is not an unit vector\n"); } }}}} diff --git a/src/Magnum/Math/Geometry/Test/IntersectionTest.cpp b/src/Magnum/Math/Geometry/Test/IntersectionTest.cpp index f4882ba90..42145c11a 100644 --- a/src/Magnum/Math/Geometry/Test/IntersectionTest.cpp +++ b/src/Magnum/Math/Geometry/Test/IntersectionTest.cpp @@ -35,6 +35,7 @@ struct IntersectionTest: Corrade::TestSuite::Tester { void planeLine(); void lineLine(); + void pointFrustum(); void boxFrustum(); }; @@ -49,6 +50,7 @@ typedef Math::Range3D Range3D; IntersectionTest::IntersectionTest() { addTests({&IntersectionTest::planeLine, &IntersectionTest::lineLine, + &IntersectionTest::pointFrustum, &IntersectionTest::boxFrustum}); } @@ -115,11 +117,11 @@ void IntersectionTest::pointFrustum() { {0.0f, 0.0f, -1.0f, 10.0f}}; /* Point on edge */ - CORRADE_VERIFY(Intersection::pointFrustum(Vector3{}, frustum)); + CORRADE_VERIFY(Intersection::pointFrustum({}, frustum)); /* Point inside */ - CORRADE_VERIFY(Intersection::pointFrustum(Vector3{5.0f, 5.0f, 5.0f}, frustum)); + CORRADE_VERIFY(Intersection::pointFrustum({5.0f, 5.0f, 5.0f}, frustum)); /* Point outside */ - CORRADE_VERIFY(!Intersection::pointFrustum(Vector3{0.0f, 0.0f, 100.0f}, frustum)); + CORRADE_VERIFY(!Intersection::pointFrustum({0.0f, 0.0f, 100.0f}, frustum)); } void IntersectionTest::boxFrustum() { @@ -131,11 +133,11 @@ void IntersectionTest::boxFrustum() { {0.0f, 0.0f, 1.0f, 0.0f}, {0.0f, 0.0f, -1.0f, 10.0f}}; - CORRADE_VERIFY(Intersection::boxFrustum(Range3D{Vector3{1.0f}, Vector3{2.0f}}, frustum)); + CORRADE_VERIFY(Intersection::boxFrustum({Vector3{1.0f}, Vector3{2.0f}}, frustum)); /* Bigger than frustum, but still intersects */ - CORRADE_VERIFY(Intersection::boxFrustum(Range3D{Vector3{-100.0f}, Vector3{100.0f}}, frustum)); + CORRADE_VERIFY(Intersection::boxFrustum(Range3D{Vector3{-100.0f}, Vector3{100.0f}}, frustum)); /* Outside of frustum */ - CORRADE_VERIFY(!Intersection::boxFrustum(Range3D{Vector3{-10.0f}, Vector3{-5.0f}}, frustum)); + CORRADE_VERIFY(!Intersection::boxFrustum(Range3D{Vector3{-10.0f}, Vector3{-5.0f}}, frustum)); } }}}} diff --git a/src/Magnum/Math/Test/FrustumTest.cpp b/src/Magnum/Math/Test/FrustumTest.cpp index be75700b5..417cee211 100644 --- a/src/Magnum/Math/Test/FrustumTest.cpp +++ b/src/Magnum/Math/Test/FrustumTest.cpp @@ -24,27 +24,23 @@ DEALINGS IN THE SOFTWARE. */ -#include #include #include #include "Magnum/Math/Frustum.h" -using namespace Corrade; - namespace Magnum { namespace Math { namespace Test { -struct FrustumTest: TestSuite::Tester { +struct FrustumTest: Corrade::TestSuite::Tester { explicit FrustumTest(); void construct(); void constructFromMatrix(); }; -typedef Vector4 Vector4; -typedef Matrix4 Matrix4; -typedef Frustum Frustum; -typedef Deg Degf; +typedef Math::Vector4 Vector4; +typedef Math::Matrix4 Matrix4; +typedef Math::Frustum Frustum; FrustumTest::FrustumTest() { addTests({&FrustumTest::construct, @@ -53,35 +49,38 @@ FrustumTest::FrustumTest() { void FrustumTest::construct() { Vector4 planes[6]{ - {-1.0f, 0.0f, 0.0f, 1.0f}, - { 1.0f, 0.0f, 0.0f, 1.0f}, - { 0.0f,-1.0f, 0.0f, 1.0f}, - { 0.0f, 1.0f, 0.0f, 1.0f}, - { 0.0f, 0.0f,-1.0f, 1.0f}, - { 0.0f, 0.0f, 1.0f, 1.0f}}; + {-1.0f, 0.0f, 0.0f, 1.0f}, + { 1.0f, 0.0f, 0.0f, 1.0f}, + { 0.0f, -1.0f, 0.0f, 1.0f}, + { 0.0f, 1.0f, 0.0f, 1.0f}, + { 0.0f, 0.0f, -1.0f, 1.0f}, + { 0.0f, 0.0f, 1.0f, 1.0f}}; Frustum frustum{ planes[0], planes[1], planes[2], planes[3], - planes[4], planes[5], - }; + planes[4], planes[5]}; - CORRADE_COMPARE_AS(frustum.planes(), Containers::ArrayView(planes), TestSuite::Compare::Container); + CORRADE_COMPARE_AS(frustum.planes(), Corrade::Containers::ArrayView(planes), + Corrade::TestSuite::Compare::Container); } void FrustumTest::constructFromMatrix() { - Vector4 planes[6]{ - { 1.0f, 0.0f,-1.0f, 0.0f}, - {-1.0f, 0.0f,-1.0f, 0.0f}, - { 0.0f, 1.0f,-1.0f, 0.0f}, - { 0.0f,-1.0f,-1.0f, 0.0f}, - { 0.0f, 0.0f,-2.22222f,-2.22222f}, - { 0.0f, 0.0f, 0.22222f, 2.22222f}}; + using namespace Magnum::Math::Literals; + + Frustum expected{ + { 1.0f, 0.0f, -1.0f, 0.0f}, + {-1.0f, 0.0f, -1.0f, 0.0f}, + { 0.0f, 1.0f, -1.0f, 0.0f}, + { 0.0f, -1.0f, -1.0f, 0.0f}, + { 0.0f, 0.0f, -2.22222f, -2.22222f}, + { 0.0f, 0.0f, 0.22222f, 2.22222f}}; const Frustum frustum = Frustum::fromMatrix( - Matrix4::perspectiveProjection(Degf(90.0f), 1.0f, 1.0f, 10.0f)); + Matrix4::perspectiveProjection(90.0_degf, 1.0f, 1.0f, 10.0f)); - CORRADE_COMPARE_AS(frustum.planes(), Containers::ArrayView(planes), TestSuite::Compare::Container); + CORRADE_COMPARE_AS(frustum.planes(), expected.planes(), + TestSuite::Compare::Container); } }}}