From 0846eb8ec68730af06629a6cea7ab7cffb39d86c Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 30 Jan 2019 13:59:37 -0800 Subject: [PATCH 1/5] attempt to allow position edits in releaseGrab entity-method --- interface/src/avatar/MyAvatar.cpp | 10 ++++ .../src/avatars-renderer/Avatar.cpp | 3 ++ libraries/physics/src/EntityMotionState.cpp | 49 ++++++++++++++----- libraries/shared/src/Grab.h | 14 ++++-- libraries/shared/src/SpatiallyNestable.cpp | 7 ++- libraries/shared/src/SpatiallyNestable.h | 2 +- 6 files changed, 68 insertions(+), 17 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 92d9270d20..a0dd817742 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -5300,6 +5300,16 @@ void MyAvatar::releaseGrab(const QUuid& grabID) { bool tellHandler { false }; _avatarGrabsLock.withWriteLock([&] { + + std::map::iterator itr; + itr = _avatarGrabs.find(grabID); + if (itr != _avatarGrabs.end()) { + GrabPointer grab = itr->second; + if (grab) { + grab->setDeleted(true); + } + } + if (_avatarGrabData.remove(grabID)) { _grabsToDelete.push_back(grabID); tellHandler = true; diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 07c1ca9a32..4bfaea0617 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -412,6 +412,9 @@ void Avatar::accumulateGrabPositions(std::map& g if (!grab || !grab->getActionID().isNull()) { continue; // the accumulated value isn't used, in this case. } + if (grab->getDeleted()) { + continue; + } glm::vec3 jointTranslation = getAbsoluteJointTranslationInObjectFrame(grab->getParentJointIndex()); glm::quat jointRotation = getAbsoluteJointRotationInObjectFrame(grab->getParentJointIndex()); diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index a82931064a..5a658e1f01 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -114,18 +114,45 @@ void EntityMotionState::updateServerPhysicsVariables() { } } +// void EntityMotionState::handleDeactivation() { +// // copy _server data to entity +// Transform localTransform = _entity->getLocalTransform(); +// // localTransform.setTranslation(_serverPosition); +// // localTransform.setRotation(_serverRotation); +// _entity->setLocalTransformAndVelocities(localTransform, ENTITY_ITEM_ZERO_VEC3, ENTITY_ITEM_ZERO_VEC3); +// // and also to RigidBody +// btTransform worldTrans; +// worldTrans.setOrigin(glmToBullet(_entity->getWorldPosition())); +// worldTrans.setRotation(glmToBullet(_entity->getWorldOrientation())); +// _body->setWorldTransform(worldTrans); +// // no need to update velocities... should already be zero +// } + void EntityMotionState::handleDeactivation() { - // copy _server data to entity - Transform localTransform = _entity->getLocalTransform(); - localTransform.setTranslation(_serverPosition); - localTransform.setRotation(_serverRotation); - _entity->setLocalTransformAndVelocities(localTransform, ENTITY_ITEM_ZERO_VEC3, ENTITY_ITEM_ZERO_VEC3); - // and also to RigidBody - btTransform worldTrans; - worldTrans.setOrigin(glmToBullet(_entity->getWorldPosition())); - worldTrans.setRotation(glmToBullet(_entity->getWorldOrientation())); - _body->setWorldTransform(worldTrans); - // no need to update velocities... should already be zero + if (_entity->getDirtyFlags() & (Simulation::DIRTY_TRANSFORM | Simulation::DIRTY_VELOCITIES)) { + // Some non-physical event (script-call or network-packet) has modified the entity's transform and/or velocities + // at the last minute before deactivation --> the values stored in _server* and _body are stale. + // We assume the EntityMotionState is the last to know, so we copy from EntityItem and let things sort themselves out. + Transform localTransform; + _entity->getLocalTransformAndVelocities(localTransform, _serverVelocity, _serverAngularVelocity); + _serverPosition = localTransform.getTranslation(); + _serverRotation = localTransform.getRotation(); + _serverAcceleration = _entity->getAcceleration(); + _serverActionData = _entity->getDynamicData(); + _lastStep = ObjectMotionState::getWorldSimulationStep(); + } else { + // copy _server data to entity + Transform localTransform = _entity->getLocalTransform(); + localTransform.setTranslation(_serverPosition); + localTransform.setRotation(_serverRotation); + _entity->setLocalTransformAndVelocities(localTransform, ENTITY_ITEM_ZERO_VEC3, ENTITY_ITEM_ZERO_VEC3); + // and also to RigidBody + btTransform worldTrans; + worldTrans.setOrigin(glmToBullet(_entity->getWorldPosition())); + worldTrans.setRotation(glmToBullet(_entity->getWorldOrientation())); + _body->setWorldTransform(worldTrans); + // no need to update velocities... should already be zero + } } // virtual diff --git a/libraries/shared/src/Grab.h b/libraries/shared/src/Grab.h index 5765d6fd0e..59602439f7 100644 --- a/libraries/shared/src/Grab.h +++ b/libraries/shared/src/Grab.h @@ -26,16 +26,16 @@ public: void accumulate(glm::vec3 position, glm::quat orientation) { _position += position; _orientation = orientation; // XXX - count++; + _count++; } - glm::vec3 finalizePosition() { return count > 0 ? _position * (1.0f / count) : glm::vec3(0.0f); } + glm::vec3 finalizePosition() { return _count > 0 ? _position * (1.0f / _count) : glm::vec3(0.0f); } glm::quat finalizeOrientation() { return _orientation; } // XXX protected: glm::vec3 _position; glm::quat _orientation; - int count { 0 }; + int _count { 0 }; }; class Grab { @@ -48,7 +48,8 @@ public: _parentJointIndex(newParentJointIndex), _hand(newHand), _positionalOffset(newPositionalOffset), - _rotationalOffset(newRotationalOffset) {} + _rotationalOffset(newRotationalOffset), + _deleted(false) {} QByteArray toByteArray(); bool fromByteArray(const QByteArray& grabData); @@ -61,6 +62,7 @@ public: _positionalOffset = other->_positionalOffset; _rotationalOffset = other->_rotationalOffset; _actionID = other->_actionID; + _deleted = other->_deleted; return *this; } @@ -85,6 +87,9 @@ public: glm::quat getRotationalOffset() const { return _rotationalOffset; } void setRotationalOffset(glm::quat rotationalOffset) { _rotationalOffset = rotationalOffset; } + bool getDeleted() const { return _deleted; } + void setDeleted(bool value) { _deleted = value; } + protected: QUuid _actionID; // if an action is created in bullet for this grab, this is the ID QUuid _ownerID; // avatar ID of grabber @@ -93,6 +98,7 @@ protected: QString _hand; // "left" or "right" glm::vec3 _positionalOffset; // relative to joint glm::quat _rotationalOffset; // relative to joint + bool _deleted { false }; // scheduled for deletion }; diff --git a/libraries/shared/src/SpatiallyNestable.cpp b/libraries/shared/src/SpatiallyNestable.cpp index c524e3183b..dd0d749919 100644 --- a/libraries/shared/src/SpatiallyNestable.cpp +++ b/libraries/shared/src/SpatiallyNestable.cpp @@ -1390,7 +1390,12 @@ void SpatiallyNestable::removeGrab(GrabPointer grab) { bool SpatiallyNestable::hasGrabs() { bool result { false }; _grabsLock.withReadLock([&] { - result = !_grabs.isEmpty(); + foreach (const GrabPointer &grab, _grabs) { + if (grab && !grab->getDeleted()) { + result = true; + break; + } + } }); return result; } diff --git a/libraries/shared/src/SpatiallyNestable.h b/libraries/shared/src/SpatiallyNestable.h index 319f07236b..ed432647fd 100644 --- a/libraries/shared/src/SpatiallyNestable.h +++ b/libraries/shared/src/SpatiallyNestable.h @@ -241,7 +241,7 @@ protected: quint64 _rotationChanged { 0 }; mutable ReadWriteLockable _grabsLock; - QSet _grabs; + QSet _grabs; // upon this thing private: SpatiallyNestable() = delete; From 3ab2db96b6ecd0d43d15dad43143f81a52c93218 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 30 Jan 2019 14:42:47 -0800 Subject: [PATCH 2/5] deactivate grab action when grab is released --- interface/src/avatar/MyAvatar.cpp | 5 ++++- .../avatars-renderer/src/avatars-renderer/Avatar.cpp | 2 +- libraries/entities/src/EntityDynamicInterface.h | 1 + libraries/entities/src/EntityItem.cpp | 10 ++++++++++ libraries/entities/src/EntityItem.h | 1 + libraries/shared/src/Grab.h | 10 +++++----- libraries/shared/src/SpatiallyNestable.cpp | 2 +- libraries/shared/src/SpatiallyNestable.h | 1 + 8 files changed, 24 insertions(+), 8 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index a0dd817742..0530ba8eb2 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -5306,7 +5306,10 @@ void MyAvatar::releaseGrab(const QUuid& grabID) { if (itr != _avatarGrabs.end()) { GrabPointer grab = itr->second; if (grab) { - grab->setDeleted(true); + grab->setReleased(true); + bool success; + SpatiallyNestablePointer target = SpatiallyNestable::findByID(grab->getTargetID(), success); + target->disableGrab(grab); } } diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 4bfaea0617..b8626c813e 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -412,7 +412,7 @@ void Avatar::accumulateGrabPositions(std::map& g if (!grab || !grab->getActionID().isNull()) { continue; // the accumulated value isn't used, in this case. } - if (grab->getDeleted()) { + if (grab->getReleased()) { continue; } diff --git a/libraries/entities/src/EntityDynamicInterface.h b/libraries/entities/src/EntityDynamicInterface.h index 836dae2057..c911eda471 100644 --- a/libraries/entities/src/EntityDynamicInterface.h +++ b/libraries/entities/src/EntityDynamicInterface.h @@ -59,6 +59,7 @@ public: virtual bool isReadyForAdd() const { return true; } bool isActive() { return _active; } + void deactivate() { _active = false; } virtual void removeFromSimulation(EntitySimulationPointer simulation) const = 0; virtual EntityItemWeakPointer getOwnerEntity() const = 0; diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 41e4f43a5d..1640e97ff4 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -3506,3 +3506,13 @@ void EntityItem::removeGrab(GrabPointer grab) { } disableNoBootstrap(); } + +void EntityItem::disableGrab(GrabPointer grab) { + QUuid actionID = grab->getActionID(); + if (!actionID.isNull()) { + EntityDynamicPointer action = _grabActions.value(actionID); + if (action) { + action->deactivate(); + } + } +} diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index ec7ad78313..27b207b6f3 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -561,6 +561,7 @@ public: virtual void addGrab(GrabPointer grab) override; virtual void removeGrab(GrabPointer grab) override; + virtual void disableGrab(GrabPointer grab) override; signals: void requestRenderUpdate(); diff --git a/libraries/shared/src/Grab.h b/libraries/shared/src/Grab.h index 59602439f7..f16a80befa 100644 --- a/libraries/shared/src/Grab.h +++ b/libraries/shared/src/Grab.h @@ -49,7 +49,7 @@ public: _hand(newHand), _positionalOffset(newPositionalOffset), _rotationalOffset(newRotationalOffset), - _deleted(false) {} + _released(false) {} QByteArray toByteArray(); bool fromByteArray(const QByteArray& grabData); @@ -62,7 +62,7 @@ public: _positionalOffset = other->_positionalOffset; _rotationalOffset = other->_rotationalOffset; _actionID = other->_actionID; - _deleted = other->_deleted; + _released = other->_released; return *this; } @@ -87,8 +87,8 @@ public: glm::quat getRotationalOffset() const { return _rotationalOffset; } void setRotationalOffset(glm::quat rotationalOffset) { _rotationalOffset = rotationalOffset; } - bool getDeleted() const { return _deleted; } - void setDeleted(bool value) { _deleted = value; } + bool getReleased() const { return _released; } + void setReleased(bool value) { _released = value; } protected: QUuid _actionID; // if an action is created in bullet for this grab, this is the ID @@ -98,7 +98,7 @@ protected: QString _hand; // "left" or "right" glm::vec3 _positionalOffset; // relative to joint glm::quat _rotationalOffset; // relative to joint - bool _deleted { false }; // scheduled for deletion + bool _released { false }; // released and scheduled for deletion }; diff --git a/libraries/shared/src/SpatiallyNestable.cpp b/libraries/shared/src/SpatiallyNestable.cpp index dd0d749919..d3ed79faf4 100644 --- a/libraries/shared/src/SpatiallyNestable.cpp +++ b/libraries/shared/src/SpatiallyNestable.cpp @@ -1391,7 +1391,7 @@ bool SpatiallyNestable::hasGrabs() { bool result { false }; _grabsLock.withReadLock([&] { foreach (const GrabPointer &grab, _grabs) { - if (grab && !grab->getDeleted()) { + if (grab && !grab->getReleased()) { result = true; break; } diff --git a/libraries/shared/src/SpatiallyNestable.h b/libraries/shared/src/SpatiallyNestable.h index ed432647fd..e7a449f73f 100644 --- a/libraries/shared/src/SpatiallyNestable.h +++ b/libraries/shared/src/SpatiallyNestable.h @@ -218,6 +218,7 @@ public: virtual void addGrab(GrabPointer grab); virtual void removeGrab(GrabPointer grab); + virtual void disableGrab(GrabPointer grab) {}; bool hasGrabs(); virtual QUuid getEditSenderID(); From f33e5ba5ae5d0df55018128489b4c115ef6b8111 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 30 Jan 2019 14:47:38 -0800 Subject: [PATCH 3/5] cleanup --- libraries/physics/src/EntityMotionState.cpp | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 5a658e1f01..ce9cb20c21 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -114,20 +114,6 @@ void EntityMotionState::updateServerPhysicsVariables() { } } -// void EntityMotionState::handleDeactivation() { -// // copy _server data to entity -// Transform localTransform = _entity->getLocalTransform(); -// // localTransform.setTranslation(_serverPosition); -// // localTransform.setRotation(_serverRotation); -// _entity->setLocalTransformAndVelocities(localTransform, ENTITY_ITEM_ZERO_VEC3, ENTITY_ITEM_ZERO_VEC3); -// // and also to RigidBody -// btTransform worldTrans; -// worldTrans.setOrigin(glmToBullet(_entity->getWorldPosition())); -// worldTrans.setRotation(glmToBullet(_entity->getWorldOrientation())); -// _body->setWorldTransform(worldTrans); -// // no need to update velocities... should already be zero -// } - void EntityMotionState::handleDeactivation() { if (_entity->getDirtyFlags() & (Simulation::DIRTY_TRANSFORM | Simulation::DIRTY_VELOCITIES)) { // Some non-physical event (script-call or network-packet) has modified the entity's transform and/or velocities From c6f44234f8afdc1f30f5c4ac2b1b317c8027ef75 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 30 Jan 2019 14:56:02 -0800 Subject: [PATCH 4/5] avoid possible crash --- interface/src/avatar/MyAvatar.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 0530ba8eb2..2eae7aa181 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -5309,7 +5309,9 @@ void MyAvatar::releaseGrab(const QUuid& grabID) { grab->setReleased(true); bool success; SpatiallyNestablePointer target = SpatiallyNestable::findByID(grab->getTargetID(), success); - target->disableGrab(grab); + if (target) { + target->disableGrab(grab); + } } } From 30d9fe705ebee66d6d30a3233959a699c3ad0010 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 30 Jan 2019 14:56:50 -0800 Subject: [PATCH 5/5] avoid possible crash --- interface/src/avatar/MyAvatar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 2eae7aa181..3f32f96795 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -5309,7 +5309,7 @@ void MyAvatar::releaseGrab(const QUuid& grabID) { grab->setReleased(true); bool success; SpatiallyNestablePointer target = SpatiallyNestable::findByID(grab->getTargetID(), success); - if (target) { + if (target && success) { target->disableGrab(grab); } }