From e4c6d49c4d0f2d2f28b438994f91c8658e54307c Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 13 Mar 2015 16:36:18 -0700 Subject: [PATCH] ShapeManager can release shape by pointer --- libraries/physics/src/ShapeManager.cpp | 66 ++++++++++++++++++------- libraries/physics/src/ShapeManager.h | 11 +++-- tests/physics/src/ShapeManagerTests.cpp | 38 ++++++++++++-- 3 files changed, 90 insertions(+), 25 deletions(-) diff --git a/libraries/physics/src/ShapeManager.cpp b/libraries/physics/src/ShapeManager.cpp index 02228bf7aa..513fbfa7a5 100644 --- a/libraries/physics/src/ShapeManager.cpp +++ b/libraries/physics/src/ShapeManager.cpp @@ -21,7 +21,7 @@ ShapeManager::~ShapeManager() { int numShapes = _shapeMap.size(); for (int i = 0; i < numShapes; ++i) { ShapeReference* shapeRef = _shapeMap.getAtIndex(i); - delete shapeRef->_shape; + delete shapeRef->shape; } _shapeMap.clear(); } @@ -40,26 +40,27 @@ btCollisionShape* ShapeManager::getShape(const ShapeInfo& info) { DoubleHashKey key = info.getHash(); ShapeReference* shapeRef = _shapeMap.find(key); if (shapeRef) { - shapeRef->_refCount++; - return shapeRef->_shape; + shapeRef->refCount++; + return shapeRef->shape; } btCollisionShape* shape = ShapeInfoUtil::createShapeFromInfo(info); if (shape) { ShapeReference newRef; - newRef._refCount = 1; - newRef._shape = shape; + newRef.refCount = 1; + newRef.shape = shape; + newRef.key = key; _shapeMap.insert(key, newRef); } return shape; } -bool ShapeManager::releaseShape(const ShapeInfo& info) { - DoubleHashKey key = info.getHash(); +// private helper method +bool ShapeManager::releaseShape(const DoubleHashKey& key) { ShapeReference* shapeRef = _shapeMap.find(key); if (shapeRef) { - if (shapeRef->_refCount > 0) { - shapeRef->_refCount--; - if (shapeRef->_refCount == 0) { + if (shapeRef->refCount > 0) { + shapeRef->refCount--; + if (shapeRef->refCount == 0) { _pendingGarbage.push_back(key); const int MAX_GARBAGE_CAPACITY = 127; if (_pendingGarbage.size() > MAX_GARBAGE_CAPACITY) { @@ -78,10 +79,19 @@ bool ShapeManager::releaseShape(const ShapeInfo& info) { return false; } +bool ShapeManager::releaseShape(const ShapeInfo& info) { + return releaseShape(info.getHash()); +} + bool ShapeManager::releaseShape(const btCollisionShape* shape) { - ShapeInfo info; - ShapeInfoUtil::collectInfoFromShape(shape, info); - return releaseShape(info); + int numShapes = _shapeMap.size(); + for (int i = 0; i < numShapes; ++i) { + ShapeReference* shapeRef = _shapeMap.getAtIndex(i); + if (shape == shapeRef->shape) { + return releaseShape(shapeRef->key); + } + } + return false; } void ShapeManager::collectGarbage() { @@ -89,8 +99,8 @@ void ShapeManager::collectGarbage() { for (int i = 0; i < numShapes; ++i) { DoubleHashKey& key = _pendingGarbage[i]; ShapeReference* shapeRef = _shapeMap.find(key); - if (shapeRef && shapeRef->_refCount == 0) { - delete shapeRef->_shape; + if (shapeRef && shapeRef->refCount == 0) { + delete shapeRef->shape; _shapeMap.remove(key); } } @@ -101,7 +111,29 @@ int ShapeManager::getNumReferences(const ShapeInfo& info) const { DoubleHashKey key = info.getHash(); const ShapeReference* shapeRef = _shapeMap.find(key); if (shapeRef) { - return shapeRef->_refCount; + return shapeRef->refCount; } - return -1; + return 0; +} + +int ShapeManager::getNumReferences(const btCollisionShape* shape) const { + int numShapes = _shapeMap.size(); + for (int i = 0; i < numShapes; ++i) { + const ShapeReference* shapeRef = _shapeMap.getAtIndex(i); + if (shape == shapeRef->shape) { + return shapeRef->refCount; + } + } + return 0; +} + +bool ShapeManager::hasShape(const btCollisionShape* shape) const { + int numShapes = _shapeMap.size(); + for (int i = 0; i < numShapes; ++i) { + const ShapeReference* shapeRef = _shapeMap.getAtIndex(i); + if (shape == shapeRef->shape) { + return true; + } + } + return false; } diff --git a/libraries/physics/src/ShapeManager.h b/libraries/physics/src/ShapeManager.h index 3fe80c7b61..e04fa1280c 100644 --- a/libraries/physics/src/ShapeManager.h +++ b/libraries/physics/src/ShapeManager.h @@ -38,12 +38,17 @@ public: // validation methods int getNumShapes() const { return _shapeMap.size(); } int getNumReferences(const ShapeInfo& info) const; + int getNumReferences(const btCollisionShape* shape) const; + bool hasShape(const btCollisionShape* shape) const; private: + bool releaseShape(const DoubleHashKey& key); + struct ShapeReference { - int _refCount; - btCollisionShape* _shape; - ShapeReference() : _refCount(0), _shape(NULL) {} + int refCount; + btCollisionShape* shape; + DoubleHashKey key; + ShapeReference() : refCount(0), shape(NULL) {} }; btHashMap _shapeMap; diff --git a/tests/physics/src/ShapeManagerTests.cpp b/tests/physics/src/ShapeManagerTests.cpp index 6dfae15e78..d86f296d0e 100644 --- a/tests/physics/src/ShapeManagerTests.cpp +++ b/tests/physics/src/ShapeManagerTests.cpp @@ -21,10 +21,8 @@ void ShapeManagerTests::testShapeAccounting() { ShapeInfo info; info.setBox(glm::vec3(1.0f, 1.0f, 1.0f)); - // NOTE: ShapeManager returns -1 as refcount when the shape is unknown, - // which is distinct from "known but with zero references" int numReferences = shapeManager.getNumReferences(info); - if (numReferences != -1) { + if (numReferences != 0) { std::cout << __FILE__ << ":" << __LINE__ << " ERROR: expected ignorant ShapeManager after initialization" << std::endl; } @@ -104,8 +102,7 @@ void ShapeManagerTests::testShapeAccounting() { if (shapeManager.getNumShapes() != 0) { std::cout << __FILE__ << ":" << __LINE__ << " ERROR: expected zero shapes after release" << std::endl; } - numReferences = shapeManager.getNumReferences(info); - if (numReferences != -1) { + if (shapeManager.hasShape(shape)) { std::cout << __FILE__ << ":" << __LINE__ << " ERROR: expected ignorant ShapeManager after garbage collection" << std::endl; } @@ -122,33 +119,64 @@ void ShapeManagerTests::testShapeAccounting() { void ShapeManagerTests::addManyShapes() { ShapeManager shapeManager; + QVector shapes; + int numSizes = 100; float startSize = 1.0f; float endSize = 99.0f; float deltaSize = (endSize - startSize) / (float)numSizes; ShapeInfo info; for (int i = 0; i < numSizes; ++i) { + // make a sphere float s = startSize + (float)i * deltaSize; glm::vec3 scale(s, 1.23f + s, s - 0.573f); info.setBox(0.5f * scale); btCollisionShape* shape = shapeManager.getShape(info); + shapes.push_back(shape); if (!shape) { std::cout << __FILE__ << ":" << __LINE__ << " ERROR: i = " << i << " null box shape for scale = " << scale << std::endl; } + + // make a box float radius = 0.5f * s; info.setSphere(radius); shape = shapeManager.getShape(info); + shapes.push_back(shape); if (!shape) { std::cout << __FILE__ << ":" << __LINE__ << " ERROR: i = " << i << " null sphere shape for radius = " << radius << std::endl; } } + + // verify shape count int numShapes = shapeManager.getNumShapes(); if (numShapes != 2 * numSizes) { std::cout << __FILE__ << ":" << __LINE__ << " ERROR: expected numShapes = " << numSizes << " but found numShapes = " << numShapes << std::endl; } + + // release each shape by pointer + for (int i = 0; i < numShapes; ++i) { + btCollisionShape* shape = shapes[i]; + bool success = shapeManager.releaseShape(shape); + if (!success) { + std::cout << __FILE__ << ":" << __LINE__ + << " ERROR: failed to release shape index " << i << std::endl; + break; + } + } + + // verify zero references + for (int i = 0; i < numShapes; ++i) { + btCollisionShape* shape = shapes[i]; + int numReferences = shapeManager.getNumReferences(shape); + if (numReferences != 0) { + std::cout << __FILE__ << ":" << __LINE__ + << " ERROR: expected zero references for shape " << i + << " but refCount = " << numReferences << std::endl; + } + } } void ShapeManagerTests::addBoxShape() {