From 782f14caa15bcdd803655ff743647075ff7ef231 Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Sun, 28 Feb 2021 15:15:07 +0000 Subject: [PATCH 1/5] heightfield processAllTriangles: Copy 1 less vertex The 2 triangles of a heightfield quad share 2 vertices. Co-authored-by: Andrew Shulaev --- .../btHeightfieldTerrainShape.cpp | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp b/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp index cab6980b6..8aafc0505 100644 --- a/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp +++ b/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp @@ -349,27 +349,32 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback if (m_flipQuadEdges || (m_useDiamondSubdivision && !((j + x) & 1)) || (m_useZigzagSubdivision && !(j & 1))) { - //first triangle getVertex(x, j, vertices[indices[0]]); getVertex(x, j + 1, vertices[indices[1]]); getVertex(x + 1, j + 1, vertices[indices[2]]); callback->processTriangle(vertices, 2 * x, j); - //second triangle - // getVertex(x,j,vertices[0]);//already got this vertex before, thanks to Danny Chapman - getVertex(x + 1, j + 1, vertices[indices[1]]); + + // already set: getVertex(x, j, vertices[indices[0]]) + + // equivalent to: getVertex(x + 1, j + 1, vertices[indices[1]]); + vertices[indices[1]] = vertices[indices[2]]; + getVertex(x + 1, j, vertices[indices[2]]); + callback->processTriangle(vertices, 2 * x+1, j); } else { - //first triangle getVertex(x, j, vertices[indices[0]]); getVertex(x, j + 1, vertices[indices[1]]); getVertex(x + 1, j, vertices[indices[2]]); callback->processTriangle(vertices, 2 * x, j); - //second triangle - getVertex(x + 1, j, vertices[indices[0]]); - //getVertex(x,j+1,vertices[1]); + + // already set: getVertex(x, j + 1, vertices[indices[1]]); + + // equivalent to: getVertex(x + 1, j, vertices[indices[0]]); + vertices[indices[0]] = vertices[indices[2]]; + getVertex(x + 1, j + 1, vertices[indices[2]]); callback->processTriangle(vertices, 2 * x+1, j); } @@ -846,4 +851,4 @@ void btHeightfieldTerrainShape::buildAccelerator(int chunkSize) void btHeightfieldTerrainShape::clearAccelerator() { m_vboundsGrid.clear(); -} \ No newline at end of file +} From e7e28bebf8eb639702ea4315b3aeb730a7356e9f Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Sun, 28 Feb 2021 15:39:36 +0000 Subject: [PATCH 2/5] heightfield processAllTriangles: Skip triangle processing if the triangle is out-of-AABB Skips triangle processing if the triangle is out-of-AABB on the up axis. This massively improves OpenMW performance on my test machine. Co-authored-by: Andrew Shulaev --- .../btHeightfieldTerrainShape.cpp | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp b/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp index 8aafc0505..3acfa79b7 100644 --- a/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp +++ b/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp @@ -352,7 +352,13 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback getVertex(x, j, vertices[indices[0]]); getVertex(x, j + 1, vertices[indices[1]]); getVertex(x + 1, j + 1, vertices[indices[2]]); - callback->processTriangle(vertices, 2 * x, j); + + // Skip triangle processing if the triangle is out-of-AABB. + btScalar minUp = btMin(vertices[0][m_upAxis], btMin(vertices[1][m_upAxis], vertices[2][m_upAxis])); + btScalar maxUp = btMax(vertices[0][m_upAxis], btMax(vertices[1][m_upAxis], vertices[2][m_upAxis])); + + if (!(minUp > aabbMax[m_upAxis] || maxUp < aabbMin[m_upAxis])) + callback->processTriangle(vertices, 2 * x, j); // already set: getVertex(x, j, vertices[indices[0]]) @@ -360,15 +366,24 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback vertices[indices[1]] = vertices[indices[2]]; getVertex(x + 1, j, vertices[indices[2]]); - - callback->processTriangle(vertices, 2 * x+1, j); + minUp = btMin(minUp, vertices[indices[2]][m_upAxis]); + maxUp = btMax(maxUp, vertices[indices[2]][m_upAxis]); + + if (!(minUp > aabbMax[m_upAxis] || maxUp < aabbMin[m_upAxis])) + callback->processTriangle(vertices, 2 * x+1, j); } else { getVertex(x, j, vertices[indices[0]]); getVertex(x, j + 1, vertices[indices[1]]); getVertex(x + 1, j, vertices[indices[2]]); - callback->processTriangle(vertices, 2 * x, j); + + // Skip triangle processing if the triangle is out-of-AABB. + btScalar minUp = btMin(vertices[0][m_upAxis], btMin(vertices[1][m_upAxis], vertices[2][m_upAxis])); + btScalar maxUp = btMax(vertices[0][m_upAxis], btMax(vertices[1][m_upAxis], vertices[2][m_upAxis])); + + if (!(minUp > aabbMax[m_upAxis] || maxUp < aabbMin[m_upAxis])) + callback->processTriangle(vertices, 2 * x, j); // already set: getVertex(x, j + 1, vertices[indices[1]]); @@ -376,7 +391,11 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback vertices[indices[0]] = vertices[indices[2]]; getVertex(x + 1, j + 1, vertices[indices[2]]); - callback->processTriangle(vertices, 2 * x+1, j); + minUp = btMin(minUp, vertices[indices[2]][m_upAxis]); + maxUp = btMax(maxUp, vertices[indices[2]][m_upAxis]); + + if (!(minUp > aabbMax[m_upAxis] || maxUp < aabbMin[m_upAxis])) + callback->processTriangle(vertices, 2 * x+1, j); } } } From bb838dbec08cbf1b7c1460899dd1f8d4af606ddc Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Mon, 1 Mar 2021 09:57:08 +0000 Subject: [PATCH 3/5] heightfield up axis: Use Range Reads a bit better than using individual min/max scalars. --- .../btHeightfieldTerrainShape.cpp | 66 ++++++++++++++----- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp b/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp index 3acfa79b7..74a2a327b 100644 --- a/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp +++ b/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp @@ -232,6 +232,43 @@ getQuantized( return (int)(x + 0.5); } +static btHeightfieldTerrainShape::Range makeRange(btScalar min, btScalar max) +{ + btHeightfieldTerrainShape::Range result; + result.min = min; + result.max = max; + return result; +} + +static bool rangesHaveOverlap(const btHeightfieldTerrainShape::Range& a, const btHeightfieldTerrainShape::Range& b) +{ + return !(a.min > b.max || a.max < b.min); +} + +// Equivalent to std::minmax({a, b, c}). +// Performs at most 3 comparisons. +static btHeightfieldTerrainShape::Range minmaxRange(btScalar a, btScalar b, btScalar c) +{ + if (a > b) + { + if (b > c) + return makeRange(c, a); + else if (a > c) + return makeRange(b, a); + else + return makeRange(b, c); + } + else + { + if (a > c) + return makeRange(c, b); + else if (b > c) + return makeRange(a, b); + else + return makeRange(a, c); + } +} + /// given input vector, return quantized version /** This routine is basically determining the gridpoint indices for a given @@ -334,7 +371,8 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback } // TODO If m_vboundsGrid is available, use it to determine if we really need to process this area - + + const Range aabbUpRange = makeRange(aabbMin[m_upAxis], aabbMax[m_upAxis]); for (int j = startJ; j < endJ; j++) { for (int x = startX; x < endX; x++) @@ -354,10 +392,9 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback getVertex(x + 1, j + 1, vertices[indices[2]]); // Skip triangle processing if the triangle is out-of-AABB. - btScalar minUp = btMin(vertices[0][m_upAxis], btMin(vertices[1][m_upAxis], vertices[2][m_upAxis])); - btScalar maxUp = btMax(vertices[0][m_upAxis], btMax(vertices[1][m_upAxis], vertices[2][m_upAxis])); + Range upRange = minmaxRange(vertices[0][m_upAxis], vertices[1][m_upAxis], vertices[2][m_upAxis]); - if (!(minUp > aabbMax[m_upAxis] || maxUp < aabbMin[m_upAxis])) + if (rangesHaveOverlap(upRange, aabbUpRange)) callback->processTriangle(vertices, 2 * x, j); // already set: getVertex(x, j, vertices[indices[0]]) @@ -366,11 +403,11 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback vertices[indices[1]] = vertices[indices[2]]; getVertex(x + 1, j, vertices[indices[2]]); - minUp = btMin(minUp, vertices[indices[2]][m_upAxis]); - maxUp = btMax(maxUp, vertices[indices[2]][m_upAxis]); + upRange.min = btMin(upRange.min, vertices[indices[2]][m_upAxis]); + upRange.max = btMax(upRange.max, vertices[indices[2]][m_upAxis]); - if (!(minUp > aabbMax[m_upAxis] || maxUp < aabbMin[m_upAxis])) - callback->processTriangle(vertices, 2 * x+1, j); + if (rangesHaveOverlap(upRange, aabbUpRange)) + callback->processTriangle(vertices, 2 * x + 1, j); } else { @@ -379,10 +416,9 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback getVertex(x + 1, j, vertices[indices[2]]); // Skip triangle processing if the triangle is out-of-AABB. - btScalar minUp = btMin(vertices[0][m_upAxis], btMin(vertices[1][m_upAxis], vertices[2][m_upAxis])); - btScalar maxUp = btMax(vertices[0][m_upAxis], btMax(vertices[1][m_upAxis], vertices[2][m_upAxis])); + Range upRange = minmaxRange(vertices[0][m_upAxis], vertices[1][m_upAxis], vertices[2][m_upAxis]); - if (!(minUp > aabbMax[m_upAxis] || maxUp < aabbMin[m_upAxis])) + if (rangesHaveOverlap(upRange, aabbUpRange)) callback->processTriangle(vertices, 2 * x, j); // already set: getVertex(x, j + 1, vertices[indices[1]]); @@ -391,11 +427,11 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback vertices[indices[0]] = vertices[indices[2]]; getVertex(x + 1, j + 1, vertices[indices[2]]); - minUp = btMin(minUp, vertices[indices[2]][m_upAxis]); - maxUp = btMax(maxUp, vertices[indices[2]][m_upAxis]); + upRange.min = btMin(upRange.min, vertices[indices[2]][m_upAxis]); + upRange.max = btMax(upRange.max, vertices[indices[2]][m_upAxis]); - if (!(minUp > aabbMax[m_upAxis] || maxUp < aabbMin[m_upAxis])) - callback->processTriangle(vertices, 2 * x+1, j); + if (rangesHaveOverlap(upRange, aabbUpRange)) + callback->processTriangle(vertices, 2 * x + 1, j); } } } From 8f5a00dd1e3b6ad4815eee2bf5c2987ce7a0b1d6 Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Wed, 3 Mar 2021 00:07:51 +0000 Subject: [PATCH 4/5] heightfield: Add range constructor and `overlaps` --- .../btHeightfieldTerrainShape.cpp | 35 ++++++------------- .../btHeightfieldTerrainShape.h | 10 +++++- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp b/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp index 74a2a327b..4ab9795d8 100644 --- a/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp +++ b/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp @@ -232,19 +232,6 @@ getQuantized( return (int)(x + 0.5); } -static btHeightfieldTerrainShape::Range makeRange(btScalar min, btScalar max) -{ - btHeightfieldTerrainShape::Range result; - result.min = min; - result.max = max; - return result; -} - -static bool rangesHaveOverlap(const btHeightfieldTerrainShape::Range& a, const btHeightfieldTerrainShape::Range& b) -{ - return !(a.min > b.max || a.max < b.min); -} - // Equivalent to std::minmax({a, b, c}). // Performs at most 3 comparisons. static btHeightfieldTerrainShape::Range minmaxRange(btScalar a, btScalar b, btScalar c) @@ -252,20 +239,20 @@ static btHeightfieldTerrainShape::Range minmaxRange(btScalar a, btScalar b, btSc if (a > b) { if (b > c) - return makeRange(c, a); + return btHeightfieldTerrainShape::Range(c, a); else if (a > c) - return makeRange(b, a); + return btHeightfieldTerrainShape::Range(b, a); else - return makeRange(b, c); + return btHeightfieldTerrainShape::Range(b, c); } else { if (a > c) - return makeRange(c, b); + return btHeightfieldTerrainShape::Range(c, b); else if (b > c) - return makeRange(a, b); + return btHeightfieldTerrainShape::Range(a, b); else - return makeRange(a, c); + return btHeightfieldTerrainShape::Range(a, c); } } @@ -372,7 +359,7 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback // TODO If m_vboundsGrid is available, use it to determine if we really need to process this area - const Range aabbUpRange = makeRange(aabbMin[m_upAxis], aabbMax[m_upAxis]); + const Range aabbUpRange(aabbMin[m_upAxis], aabbMax[m_upAxis]); for (int j = startJ; j < endJ; j++) { for (int x = startX; x < endX; x++) @@ -394,7 +381,7 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback // Skip triangle processing if the triangle is out-of-AABB. Range upRange = minmaxRange(vertices[0][m_upAxis], vertices[1][m_upAxis], vertices[2][m_upAxis]); - if (rangesHaveOverlap(upRange, aabbUpRange)) + if (upRange.overlaps(aabbUpRange)) callback->processTriangle(vertices, 2 * x, j); // already set: getVertex(x, j, vertices[indices[0]]) @@ -406,7 +393,7 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback upRange.min = btMin(upRange.min, vertices[indices[2]][m_upAxis]); upRange.max = btMax(upRange.max, vertices[indices[2]][m_upAxis]); - if (rangesHaveOverlap(upRange, aabbUpRange)) + if (upRange.overlaps(aabbUpRange)) callback->processTriangle(vertices, 2 * x + 1, j); } else @@ -418,7 +405,7 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback // Skip triangle processing if the triangle is out-of-AABB. Range upRange = minmaxRange(vertices[0][m_upAxis], vertices[1][m_upAxis], vertices[2][m_upAxis]); - if (rangesHaveOverlap(upRange, aabbUpRange)) + if (upRange.overlaps(aabbUpRange)) callback->processTriangle(vertices, 2 * x, j); // already set: getVertex(x, j + 1, vertices[indices[1]]); @@ -430,7 +417,7 @@ void btHeightfieldTerrainShape::processAllTriangles(btTriangleCallback* callback upRange.min = btMin(upRange.min, vertices[indices[2]][m_upAxis]); upRange.max = btMax(upRange.max, vertices[indices[2]][m_upAxis]); - if (rangesHaveOverlap(upRange, aabbUpRange)) + if (upRange.overlaps(aabbUpRange)) callback->processTriangle(vertices, 2 * x + 1, j); } } diff --git a/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.h b/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.h index 2cf3c0072..987ea324f 100644 --- a/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.h +++ b/src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.h @@ -75,6 +75,14 @@ btHeightfieldTerrainShape : public btConcaveShape public: struct Range { + Range() {} + Range(btScalar min, btScalar max) : min(min), max(max) {} + + bool overlaps(const Range& other) const + { + return !(min > other.max || max < other.min); + } + btScalar min; btScalar max; }; @@ -218,4 +226,4 @@ public: } }; -#endif //BT_HEIGHTFIELD_TERRAIN_SHAPE_H \ No newline at end of file +#endif //BT_HEIGHTFIELD_TERRAIN_SHAPE_H From 40a9584ccbe36b61e7ec7a4809e6f9ef5707ce51 Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Wed, 3 Mar 2021 00:38:21 +0000 Subject: [PATCH 5/5] heightfield: Add a test for processAllTriangles up-axis filtering --- test/collision/CMakeLists.txt | 3 +++ test/collision/main.cpp | 47 +++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/test/collision/CMakeLists.txt b/test/collision/CMakeLists.txt index b36c151be..ba6c783b4 100644 --- a/test/collision/CMakeLists.txt +++ b/test/collision/CMakeLists.txt @@ -24,10 +24,13 @@ ENDIF() ../../src/BulletCollision/CollisionShapes/btSphereShape.cpp ../../src/BulletCollision/CollisionShapes/btMultiSphereShape.cpp ../../src/BulletCollision/CollisionShapes/btPolyhedralConvexShape.cpp + ../../src/BulletCollision/CollisionShapes/btConcaveShape.cpp ../../src/BulletCollision/CollisionShapes/btConvexShape.cpp ../../src/BulletCollision/CollisionShapes/btConvexInternalShape.cpp ../../src/BulletCollision/CollisionShapes/btCollisionShape.cpp ../../src/BulletCollision/CollisionShapes/btConvexPolyhedron.cpp + ../../src/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp + ../../src/BulletCollision/CollisionShapes/btTriangleCallback.cpp ) ADD_TEST(Test_Collision_PASS Test_Collision) diff --git a/test/collision/main.cpp b/test/collision/main.cpp index 8f85873ff..5be7a91f7 100644 --- a/test/collision/main.cpp +++ b/test/collision/main.cpp @@ -20,9 +20,12 @@ subject to the following restrictions: ///Todo: the test needs proper coverage and using a convex hull point cloud ///Also the GJK, EPA and MPR should be improved, both quality and performance +#include + #include #include "SphereSphereCollision.h" +#include "BulletCollision/CollisionShapes/btHeightfieldTerrainShape.h" #include "BulletCollision/CollisionShapes/btSphereShape.h" #include "BulletCollision/CollisionShapes/btMultiSphereShape.h" @@ -30,6 +33,8 @@ subject to the following restrictions: #include "BulletCollision/NarrowPhaseCollision/btGjkEpa3.h" #include "BulletCollision/NarrowPhaseCollision/btMprPenetration.h" +namespace { + btVector3 MyBulletShapeSupportFunc(const void* shapeAptr, const btVector3& dir, bool includeMargin) { btConvexShape* shape = (btConvexShape*)shapeAptr; @@ -249,6 +254,48 @@ TEST(BulletCollisionTest, AnalyticSphereSphereDistance) testSphereSphereDistance(SSTM_ANALYTIC, 0.00001); } +class TriangleCollector : public btTriangleCallback +{ +public: + std::vector *triangles; + + explicit TriangleCollector(std::vector* triangles) : triangles(triangles) {} + virtual ~TriangleCollector() {} + + virtual void processTriangle(btVector3* triangle, int partId, int triangleIndex) + { + triangles->push_back(*triangle); + } +}; + +TEST(BulletCollisionTest, Heightfield_ProcessAllTriangles_FiltersByUpAxis) +{ + // A flat 2x2 heightfield. + const btScalar heightFieldData[] = { + 10.0, 10.0, + 10.0, 10.0, + }; + btHeightfieldTerrainShape shape( + /*heightStickWidth=*/2, /*heightStickLength=*/2, + &heightFieldData[0], /*heightScale=*/1, + /*minHeight=*/-10.0, /*maxHeight=*/10.0, + /*upAxis=*/2, PHY_FLOAT, /*flipQuadEdges=*/false); + + std::vector triangles; + TriangleCollector collector(&triangles); + + // AABB overlaps with the heightfield on upAxis. + shape.processAllTriangles(&collector, btVector3(0, 0, 0), btVector3(20, 20, 20)); + EXPECT_EQ(triangles.size(), 2); + + // AABB does not overlap with the heightfield on upAxis. + triangles.clear(); + shape.processAllTriangles(&collector, btVector3(0, 0, 0), btVector3(20, 20, 5)); + EXPECT_EQ(triangles.size(), 0); +} + +} // namespace + int main(int argc, char** argv) { #if _MSC_VER