From 2441536de387637b48531fcbec6121d68206d968 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 18 Apr 2017 13:37:36 -0700 Subject: [PATCH] remove Avatar dependency on AvatarMotionState --- interface/src/avatar/Avatar.cpp | 25 ++++--- interface/src/avatar/Avatar.h | 14 ++-- interface/src/avatar/AvatarManager.cpp | 76 ++++++++++++++-------- interface/src/avatar/AvatarManager.h | 4 +- interface/src/avatar/AvatarMotionState.cpp | 6 +- interface/src/avatar/AvatarMotionState.h | 8 +-- libraries/physics/src/PhysicsEngine.cpp | 1 + 7 files changed, 79 insertions(+), 55 deletions(-) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index 2234630504..91d8dfb447 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -149,11 +149,6 @@ Avatar::~Avatar() { }); } - if (_motionState) { - delete _motionState; - _motionState = nullptr; - } - auto geometryCache = DependencyManager::get(); if (geometryCache) { geometryCache->releaseID(_nameRectGeometryID); @@ -1202,8 +1197,8 @@ int Avatar::parseDataFromBuffer(const QByteArray& buffer) { const float MOVE_DISTANCE_THRESHOLD = 0.001f; _moving = glm::distance(oldPosition, getPosition()) > MOVE_DISTANCE_THRESHOLD; - if (_moving && _motionState) { - _motionState->addDirtyFlags(Simulation::DIRTY_POSITION); + if (_moving) { + addPhysicsFlags(Simulation::DIRTY_POSITION); } if (_moving || _hasNewJointData) { locationChanged(); @@ -1325,14 +1320,18 @@ void Avatar::getCapsule(glm::vec3& start, glm::vec3& end, float& radius) { radius = halfExtents.x; } -void Avatar::setMotionState(AvatarMotionState* motionState) { - _motionState = motionState; -} - // virtual void Avatar::rebuildCollisionShape() { - if (_motionState) { - _motionState->addDirtyFlags(Simulation::DIRTY_SHAPE); + addPhysicsFlags(Simulation::DIRTY_SHAPE); +} + +void Avatar::setPhysicsCallback(AvatarPhysicsCallback cb) { + _physicsCallback = cb; +} + +void Avatar::addPhysicsFlags(uint32_t flags) { + if (_physicsCallback) { + _physicsCallback(flags); } } diff --git a/interface/src/avatar/Avatar.h b/interface/src/avatar/Avatar.h index 116e5aad11..17b5ea926e 100644 --- a/interface/src/avatar/Avatar.h +++ b/interface/src/avatar/Avatar.h @@ -11,6 +11,7 @@ #ifndef hifi_Avatar_h #define hifi_Avatar_h +#include #include #include @@ -48,9 +49,10 @@ enum ScreenTintLayer { NUM_SCREEN_TINT_LAYERS }; -class AvatarMotionState; class Texture; +using AvatarPhysicsCallback = std::function; + class Avatar : public AvatarData { Q_OBJECT @@ -190,7 +192,7 @@ public: virtual void computeShapeInfo(ShapeInfo& shapeInfo); void getCapsule(glm::vec3& start, glm::vec3& end, float& radius); - AvatarMotionState* getMotionState() { return _motionState; } + //AvatarMotionState* getMotionState() { return _motionState; } using SpatiallyNestable::setPosition; virtual void setPosition(const glm::vec3& position) override; @@ -248,8 +250,12 @@ public: void addToScene(AvatarSharedPointer self, const render::ScenePointer& scene); void ensureInScene(AvatarSharedPointer self, const render::ScenePointer& scene); bool isInScene() const { return render::Item::isValidID(_renderItemID); } + bool isMoving() const { return _moving; } - void setMotionState(AvatarMotionState* motionState); + //void setMotionState(AvatarMotionState* motionState); + void setPhysicsCallback(AvatarPhysicsCallback cb); + void addPhysicsFlags(uint32_t flags); + bool isInPhysicsSimulation() const { return _physicsCallback != nullptr; } public slots: @@ -366,7 +372,7 @@ private: int _voiceSphereID; - AvatarMotionState* _motionState = nullptr; + AvatarPhysicsCallback _physicsCallback { nullptr }; }; #endif // hifi_Avatar_h diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 35a5681213..41cf797eba 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -74,6 +74,7 @@ AvatarManager::AvatarManager(QObject* parent) : } AvatarManager::~AvatarManager() { + assert(_motionStates.empty()); } void AvatarManager::init() { @@ -128,10 +129,9 @@ float AvatarManager::getAvatarUpdateRate(const QUuid& sessionID, const QString& float AvatarManager::getAvatarSimulationRate(const QUuid& sessionID, const QString& rateName) const { auto avatar = std::static_pointer_cast(getAvatarBySessionID(sessionID)); - return avatar ? avatar->getSimulationRate(rateName) : 0.0f; + return avatar ? avatar->getSimulationRate(rateName) : 0.0f; } - void AvatarManager::updateOtherAvatars(float deltaTime) { // lock the hash for read to check the size QReadLocker lock(&_hashLock); @@ -189,16 +189,15 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { if (_shouldRender) { avatar->ensureInScene(avatar, qApp->getMain3DScene()); } - if (!avatar->getMotionState()) { + if (!avatar->isInPhysicsSimulation()) { ShapeInfo shapeInfo; avatar->computeShapeInfo(shapeInfo); btCollisionShape* shape = const_cast(ObjectMotionState::getShapeManager()->getShape(shapeInfo)); if (shape) { - // don't add to the simulation now, instead put it on a list to be added later - AvatarMotionState* motionState = new AvatarMotionState(avatar.get(), shape); - avatar->setMotionState(motionState); + AvatarMotionState* motionState = new AvatarMotionState(avatar, shape); + avatar->setPhysicsCallback([=] (uint32_t flags) { motionState->addDirtyFlags(flags); }); + _motionStates.insert(avatar.get(), motionState); _motionStatesToAddToPhysics.insert(motionState); - _motionStatesThatMightUpdate.insert(motionState); } } avatar->animateScaleChanges(deltaTime); @@ -283,30 +282,34 @@ void AvatarManager::simulateAvatarFades(float deltaTime) { const float MIN_FADE_SCALE = MIN_AVATAR_SCALE; QReadLocker locker(&_hashLock); - QVector::iterator itr = _avatarsToFade.begin(); - while (itr != _avatarsToFade.end()) { - auto avatar = std::static_pointer_cast(*itr); + QVector::iterator avatarItr = _avatarsToFade.begin(); + while (avatarItr != _avatarsToFade.end()) { + auto avatar = std::static_pointer_cast(*avatarItr); avatar->setTargetScale(avatar->getUniformScale() * SHRINK_RATE); avatar->animateScaleChanges(deltaTime); if (avatar->getTargetScale() <= MIN_FADE_SCALE) { - // fading to zero is such a rare event we push unique transaction for each one + // fading to zero is such a rare event we push a unique transaction for each if (avatar->isInScene()) { const render::ScenePointer& scene = qApp->getMain3DScene(); render::Transaction transaction; - avatar->removeFromScene(*itr, scene, transaction); + avatar->removeFromScene(*avatarItr, scene, transaction); scene->enqueueTransaction(transaction); } - // only remove from _avatarsToFade if we're sure its motionState has been removed from PhysicsEngine - if (_motionStatesToRemoveFromPhysics.empty()) { - itr = _avatarsToFade.erase(itr); - } else { - ++itr; + // delete the motionState + // TODO: use SharedPointer technology to make this happen automagically + assert(!avatar->isInPhysicsSimulation()); + AvatarMotionStateMap::iterator motionStateItr = _motionStates.find(avatar.get()); + if (motionStateItr != _motionStates.end()) { + delete *motionStateItr; + _motionStates.erase(motionStateItr); } + + avatarItr = _avatarsToFade.erase(avatarItr); } else { const bool inView = true; // HACK avatar->simulate(deltaTime, inView); - ++itr; + ++avatarItr; } } } @@ -315,19 +318,24 @@ AvatarSharedPointer AvatarManager::newSharedAvatar() { return std::make_shared(qApp->thread(), std::make_shared()); } +void AvatarManager::removeAvatarFromPhysicsSimulation(Avatar* avatar) { + assert(avatar); + avatar->setPhysicsCallback(nullptr); + AvatarMotionStateMap::iterator itr = _motionStates.find(avatar); + if (itr != _motionStates.end()) { + AvatarMotionState* motionState = *itr; + _motionStatesToAddToPhysics.remove(motionState); + _motionStatesToRemoveFromPhysics.push_back(motionState); + } +} + void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason) { AvatarHashMap::handleRemovedAvatar(removedAvatar, removalReason); // removedAvatar is a shared pointer to an AvatarData but we need to get to the derived Avatar // class in this context so we can call methods that don't exist at the base class. auto avatar = std::static_pointer_cast(removedAvatar); - - AvatarMotionState* motionState = avatar->getMotionState(); - if (motionState) { - _motionStatesThatMightUpdate.remove(motionState); - _motionStatesToAddToPhysics.remove(motionState); - _motionStatesToRemoveFromPhysics.push_back(motionState); - } + removeAvatarFromPhysicsSimulation(avatar.get()); if (removalReason == KillAvatarReason::TheirAvatarEnteredYourBubble) { emit DependencyManager::get()->enteredIgnoreRadius(); @@ -362,11 +370,21 @@ void AvatarManager::clearOtherAvatars() { ++avatarIterator; } } + assert(scene); scene->enqueueTransaction(transaction); _myAvatar->clearLookAtTargetAvatar(); } void AvatarManager::deleteAllAvatars() { + // delete motionStates + // TODO: use shared_ptr technology to make this work automagically + AvatarMotionStateMap::iterator motionStateItr = _motionStates.begin(); + while (motionStateItr != _motionStates.end()) { + delete *motionStateItr; + ++motionStateItr; + } + _motionStates.clear(); + QReadLocker locker(&_hashLock); AvatarHash::iterator avatarIterator = _avatarHash.begin(); while (avatarIterator != _avatarHash.end()) { @@ -391,10 +409,12 @@ void AvatarManager::getObjectsToAddToPhysics(VectorOfMotionStates& result) { void AvatarManager::getObjectsToChange(VectorOfMotionStates& result) { result.clear(); - for (auto state : _motionStatesThatMightUpdate) { - if (state->_dirtyFlags > 0) { - result.push_back(state); + AvatarMotionStateMap::iterator motionStateItr = _motionStates.begin(); + while (motionStateItr != _motionStates.end()) { + if ((*motionStateItr)->getIncomingDirtyFlags() != 0) { + result.push_back(*motionStateItr); } + ++motionStateItr; } } diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h index 1832bc7126..5c8935417b 100644 --- a/interface/src/avatar/AvatarManager.h +++ b/interface/src/avatar/AvatarManager.h @@ -93,11 +93,13 @@ private: void simulateAvatarFades(float deltaTime); AvatarSharedPointer newSharedAvatar() override; + void removeAvatarFromPhysicsSimulation(Avatar* avatar); void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason = KillAvatarReason::NoReason) override; QVector _avatarsToFade; - QSet _motionStatesThatMightUpdate; + using AvatarMotionStateMap = QMap; + AvatarMotionStateMap _motionStates; VectorOfMotionStates _motionStatesToRemoveFromPhysics; SetOfMotionStates _motionStatesToAddToPhysics; diff --git a/interface/src/avatar/AvatarMotionState.cpp b/interface/src/avatar/AvatarMotionState.cpp index 335245670b..ffa99e3990 100644 --- a/interface/src/avatar/AvatarMotionState.cpp +++ b/interface/src/avatar/AvatarMotionState.cpp @@ -17,7 +17,7 @@ #include "AvatarMotionState.h" #include "BulletUtil.h" -AvatarMotionState::AvatarMotionState(Avatar* avatar, const btCollisionShape* shape) : ObjectMotionState(shape), _avatar(avatar) { +AvatarMotionState::AvatarMotionState(AvatarSharedPointer avatar, const btCollisionShape* shape) : ObjectMotionState(shape), _avatar(avatar) { assert(_avatar); _type = MOTIONSTATE_TYPE_AVATAR; if (_shape) { @@ -49,7 +49,7 @@ PhysicsMotionType AvatarMotionState::computePhysicsMotionType() const { // virtual and protected const btCollisionShape* AvatarMotionState::computeNewShape() { ShapeInfo shapeInfo; - _avatar->computeShapeInfo(shapeInfo); + std::static_pointer_cast(_avatar)->computeShapeInfo(shapeInfo); return getShapeManager()->getShape(shapeInfo); } @@ -130,7 +130,7 @@ glm::vec3 AvatarMotionState::getObjectAngularVelocity() const { // virtual glm::vec3 AvatarMotionState::getObjectGravity() const { - return _avatar->getAcceleration(); + return std::static_pointer_cast(_avatar)->getAcceleration(); } // virtual diff --git a/interface/src/avatar/AvatarMotionState.h b/interface/src/avatar/AvatarMotionState.h index 98b2b69373..a8dd7327ca 100644 --- a/interface/src/avatar/AvatarMotionState.h +++ b/interface/src/avatar/AvatarMotionState.h @@ -20,7 +20,7 @@ class Avatar; class AvatarMotionState : public ObjectMotionState { public: - AvatarMotionState(Avatar* avatar, const btCollisionShape* shape); + AvatarMotionState(AvatarSharedPointer avatar, const btCollisionShape* shape); virtual PhysicsMotionType getMotionType() const override { return _motionType; } @@ -74,11 +74,7 @@ protected: virtual bool isReadyToComputeShape() const override { return true; } virtual const btCollisionShape* computeNewShape() override; - // The AvatarMotionState keeps a RAW backpointer to its Avatar because all AvatarMotionState - // instances are "owned" by their corresponding Avatar instance and are deleted in the Avatar dtor. - // In other words, it is impossible for the Avatar to be deleted out from under its MotionState. - // In conclusion: weak pointer shennanigans would be pure overhead. - Avatar* _avatar; // do NOT use smartpointer here, no need for weakpointer + AvatarSharedPointer _avatar; uint32_t _dirtyFlags; }; diff --git a/libraries/physics/src/PhysicsEngine.cpp b/libraries/physics/src/PhysicsEngine.cpp index ca6889485a..87a15eb264 100644 --- a/libraries/physics/src/PhysicsEngine.cpp +++ b/libraries/physics/src/PhysicsEngine.cpp @@ -220,6 +220,7 @@ void PhysicsEngine::removeObjects(const SetOfMotionStates& objects) { body->setMotionState(nullptr); delete body; } + object->clearIncomingDirtyFlags(); } }