Skip to content

Commit

Permalink
Refactors Manifold to remove two member variables & store that state …
Browse files Browse the repository at this point in the history
…more compactly
  • Loading branch information
louis-langholtz committed Jan 16, 2024
1 parent 388a4e1 commit 946e288
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 29 deletions.
30 changes: 26 additions & 4 deletions Library/include/playrho/ContactFeature.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
#include <cstdint> // for std::uint8_t
#include <ostream>

#include <playrho/Settings.hpp> // for InvalidVertex

namespace playrho {

/// @brief The contact feature.
Expand All @@ -43,15 +45,35 @@ struct ContactFeature
using Index = std::uint8_t; ///< Index type.

/// @brief Type of the associated index value.
enum Type : std::uint8_t
enum Type: std::uint16_t
{
e_vertex = 0,
e_face = 1
e_face = 1,
};

Type typeA; ///< The feature type on shape A
/// @brief Default constructor.
/// @post <code>typeA == e_vertex</code>, <code>typeB == e_vertex</code>,
/// <code>other == 0</code>, <code>indexA == InvalidVertex</code>,
/// <code>indexB == InvalidVertex</code>.
constexpr ContactFeature() noexcept
: typeA{e_vertex}, typeB{e_vertex}, other{}, indexA{InvalidVertex}, indexB{InvalidVertex}
{
// Intentionally empty.
}

/// @brief Initializing constructor.
/// @post <code>typeA == tA</code>, <code>typeB == tB</code>, <code>other == 0</code>,
/// <code>indexA == iA</code>, <code>indexB == iB</code>.
constexpr ContactFeature(Type tA, Index iA, Type tB, Index iB) noexcept
: typeA{tA}, typeB{tB}, other{}, indexA{iA}, indexB{iB}
{
// Intentionally empty.
}

Type typeA: 1; ///< The feature type on shape A
Type typeB: 1; ///< The feature type on shape B
Type other: 14; ///< Private for internal use!
Index indexA; ///< Feature index on shape A
Type typeB; ///< The feature type on shape B
Index indexB; ///< Feature index on shape B
};

Expand Down
65 changes: 42 additions & 23 deletions Library/include/playrho/d2/Manifold.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class Manifold
/// @param iB Index of vertex from shape B representing the local center of "circle" B.
static Manifold GetForCircles(const Length2& vA, CfIndex iA, const Length2& vB, CfIndex iB) noexcept
{
return {e_circles, UnitVec(), vA, 1, {{Point{vB, GetVertexVertexContactFeature(iA, iB)}}}};
return {e_circles, UnitVec(), vA, {{Point{vB, GetVertexVertexContactFeature(iA, iB)}}}};
}

// For Face A type manifolds...
Expand All @@ -177,7 +177,7 @@ class Manifold
/// @param faceA Any point in local coordinates on the face whose normal was provided.
static Manifold GetForFaceA(const UnitVec& normalA, const Length2& faceA) noexcept
{
return {e_faceA, normalA, faceA, 0, {{}}};
return {e_faceA, normalA, faceA, {{}}};
}

/// Gets a face A typed manifold.
Expand All @@ -188,7 +188,7 @@ class Manifold
{
// assert(mp1.contactFeature.typeA == ContactFeature::e_face || mp1.contactFeature.typeB ==
// ContactFeature::e_face);
return {e_faceA, ln, lp, 1, {{mp1}}};
return {e_faceA, ln, lp, {{mp1}}};
}

/// Gets a face A typed manifold.
Expand All @@ -203,7 +203,7 @@ class Manifold
// ContactFeature::e_face); assert(mp2.contactFeature.typeA == ContactFeature::e_face ||
// mp2.contactFeature.typeB == ContactFeature::e_face); assert(mp1.contactFeature !=
// mp2.contactFeature);
return {e_faceA, ln, lp, 2, {{mp1, mp2}}};
return {e_faceA, ln, lp, {{mp1, mp2}}};
}

// For Face B...
Expand All @@ -213,7 +213,7 @@ class Manifold
/// @param lp Center of face B.
static Manifold GetForFaceB(const UnitVec& ln, const Length2& lp) noexcept
{
return {e_faceB, ln, lp, 0, {{}}};
return {e_faceB, ln, lp, {{}}};
}

/// Gets a face B typed manifold.
Expand All @@ -224,22 +224,22 @@ class Manifold
{
// assert(mp1.contactFeature.typeA == ContactFeature::e_face || mp1.contactFeature.typeB ==
// ContactFeature::e_face);
return {e_faceB, ln, lp, 1, {{mp1}}};
return {e_faceB, ln, lp, {{mp1}}};
}

/// Gets a face B typed manifold.
/// @param ln Normal on polygon B.
/// @param lp Center of face B.
/// @param mp1 Manifold point 1 (of 2).
/// @param mp2 Manifold point 2 (of 2).
static Manifold GetForFaceB(const UnitVec& ln, const Length2& lp, const Point& mp1,
const Point& mp2) noexcept
static Manifold GetForFaceB(const UnitVec& ln, const Length2& lp, // newline
const Point& mp1, const Point& mp2) noexcept
{
// assert(mp1.contactFeature.typeA == ContactFeature::e_face || mp1.contactFeature.typeB ==
// ContactFeature::e_face); assert(mp2.contactFeature.typeA == ContactFeature::e_face ||
// mp2.contactFeature.typeB == ContactFeature::e_face); assert(mp1.contactFeature !=
// mp2.contactFeature);
return {e_faceB, ln, lp, 2, {{mp1, mp2}}};
return {e_faceB, ln, lp, {{mp1, mp2}}};
}

/// Default constructor.
Expand All @@ -256,7 +256,7 @@ class Manifold
///
constexpr Type GetType() const noexcept
{
return m_type;
return m_points.GetType();
}

/// Gets the manifold point count.
Expand All @@ -268,7 +268,7 @@ class Manifold
/// @see MaxManifoldPoints, AddPoint(), GetPoint().
constexpr size_type GetPointCount() const noexcept
{
return m_pointCount;
return m_points.size();
}

/// @brief Gets the contact feature for the given index.
Expand Down Expand Up @@ -363,6 +363,25 @@ class Manifold
struct PointArray {
Point elements[MaxManifoldPoints]; ///< Elements.

/// @brief Retrieves the type value.
constexpr auto GetType() const noexcept -> Type
{
return static_cast<Type>(elements[0].contactFeature.other);
}

/// @brief Stores the given type in this.
constexpr void SetType(Type t) noexcept
{
elements[0].contactFeature.other = static_cast<ContactFeature::Type>(t);
}

/// @brief Size in number of elements.
constexpr auto size() const noexcept -> size_type
{
return ((elements[0].contactFeature.indexA != InvalidVertex)? 1u: 0u)
+ ((elements[1].contactFeature.indexA != InvalidVertex)? 1u: 0u);
}

/// @brief Array indexing operator.
constexpr Point& operator[](std::size_t i)
{
Expand All @@ -382,12 +401,8 @@ class Manifold
/// @param t Manifold type.
/// @param ln Local normal.
/// @param lp Local point.
/// @param n number of points defined in array.
/// @param mpa Manifold point array.
constexpr Manifold(Type t, const UnitVec& ln, const Length2& lp, size_type n, const PointArray& mpa) noexcept;

Type m_type = e_unset; ///< Type of collision this manifold is associated with.
size_type m_pointCount{}; ///< Number of defined manifold points.
constexpr Manifold(Type t, const UnitVec& ln, const Length2& lp, const PointArray& mpa) noexcept;

/// Local normal.
/// @details Exact usage depends on manifold type.
Expand Down Expand Up @@ -430,31 +445,35 @@ constexpr Manifold::Conf GetDefaultManifoldConf() noexcept
/// @relatedalso Manifold::Conf
Manifold::Conf GetManifoldConf(const StepConf& conf) noexcept;

constexpr Manifold::Manifold(Type t, const UnitVec& ln, const Length2& lp, size_type n,
const PointArray& mpa) noexcept
: m_type{t}, m_pointCount{n}, m_localNormal{ln}, m_localPoint{lp}, m_points{mpa}
constexpr Manifold::Manifold(Type t, const UnitVec& ln, const Length2& lp, const PointArray& mpa) noexcept
: m_localNormal{ln}, m_localPoint{lp}, m_points{mpa}
{
assert(t != e_unset || n == 0);
assert(t != e_unset || mpa.size() == 0);
assert(t == e_unset || IsValid(lp));
assert((t == e_unset) || (t == e_circles) || IsValid(ln));
assert((t != e_circles) || (n == 1 && !IsValid(ln)));
assert((t != e_circles) || ((mpa.size() == 1) && !IsValid(ln)));
// assert((t != e_circles) || (n == 1 && !IsValid(ln) && mpa[0].contactFeature.typeA ==
// ContactFeature::e_vertex && mpa[0].contactFeature.typeB == ContactFeature::e_vertex));
m_points.SetType(t);
}

inline void Manifold::AddPoint(const Point& mp) noexcept
{
assert(GetType() != e_unset);
assert(GetType() != e_circles || GetPointCount() == 0);
assert(GetPointCount() < MaxManifoldPoints);
assert(mp.contactFeature.other == 0u);
assert(mp.contactFeature.indexA != InvalidVertex);
assert(mp.contactFeature.indexB != InvalidVertex);
// assert((GetPointCount() == 0) || (mp.contactFeature != m_points[0].contactFeature));
// assert((GetType() != e_circles) || (mp.contactFeature.typeA == ContactFeature::e_vertex ||
// mp.contactFeature.typeB == ContactFeature::e_vertex)); assert((GetType() != e_faceA) ||
// ((mp.contactFeature.typeA == ContactFeature::e_face) && (GetPointCount() == 0 ||
// mp.contactFeature.indexA == m_points[0].contactFeature.indexA))); assert((GetType() != e_faceB)
// || (mp.contactFeature.typeB == ContactFeature::e_face));
m_points[m_pointCount] = mp;
++m_pointCount;
auto cf = mp.contactFeature;
cf.other = static_cast<decltype(cf.other)>(GetType());
m_points[GetPointCount()] = {mp.localPoint, cf, mp.normalImpulse, mp.tangentImpulse};
}

// Free functions...
Expand Down
4 changes: 2 additions & 2 deletions UnitTests/PositionConstraint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ TEST(PositionConstraint, ByteSize)
// builds and to report actual size rather than just reporting that expected size is wrong.
switch (sizeof(Real))
{
case 4: EXPECT_EQ(sizeof(PositionConstraint), std::size_t(68)); break;
case 8: EXPECT_EQ(sizeof(PositionConstraint), std::size_t(136)); break;
case 4: EXPECT_EQ(sizeof(PositionConstraint), std::size_t(64)); break;
case 8: EXPECT_EQ(sizeof(PositionConstraint), std::size_t(128)); break;
case 16: EXPECT_EQ(sizeof(PositionConstraint), std::size_t(272)); break;
default: FAIL(); break;
}
Expand Down

0 comments on commit 946e288

Please sign in to comment.