From 07a4dc3a7fc5e7e522de6c9a51ebfb71e54094a3 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 14 Oct 2015 12:49:06 -0700 Subject: [PATCH] more lock fixing --- interface/src/avatar/AvatarActionHold.cpp | 15 +--- libraries/entities/src/EntityItem.cpp | 9 ++ libraries/physics/src/ObjectAction.cpp | 89 ++++++++++++-------- libraries/physics/src/ObjectActionOffset.cpp | 62 ++++++++------ libraries/physics/src/ObjectActionSpring.cpp | 82 ++++++++++-------- 5 files changed, 155 insertions(+), 102 deletions(-) diff --git a/interface/src/avatar/AvatarActionHold.cpp b/interface/src/avatar/AvatarActionHold.cpp index 7d38eb0f23..ce34306df4 100644 --- a/interface/src/avatar/AvatarActionHold.cpp +++ b/interface/src/avatar/AvatarActionHold.cpp @@ -86,13 +86,9 @@ bool AvatarActionHold::updateArguments(QVariantMap arguments) { QString hand; QUuid holderID; bool needUpdate = false; - bool updateArgumentsSuceeded = true; + bool somethingChanged = ObjectAction::updateArguments(arguments); withReadLock([&]{ - if (!ObjectAction::updateArguments(arguments)) { - updateArgumentsSuceeded = false; - return; - } bool ok = true; relativePosition = EntityActionInterface::extractVec3Argument("hold", arguments, "relativePosition", ok, false); if (!ok) { @@ -121,7 +117,8 @@ bool AvatarActionHold::updateArguments(QVariantMap arguments) { auto myAvatar = DependencyManager::get()->getMyAvatar(); holderID = myAvatar->getSessionUUID(); - if (relativePosition != _relativePosition + if (somethingChanged || + relativePosition != _relativePosition || relativeRotation != _relativeRotation || timeScale != _linearTimeScale || hand != _hand @@ -130,10 +127,6 @@ bool AvatarActionHold::updateArguments(QVariantMap arguments) { } }); - if (!updateArgumentsSuceeded) { - return false; - } - if (needUpdate) { withWriteLock([&] { _relativePosition = relativePosition; @@ -221,7 +214,7 @@ void AvatarActionHold::deserialize(QByteArray serializedArguments) { qDebug() << "deserialize hold: " << _holderID << _relativePosition.x << _relativePosition.y << _relativePosition.z - << _hand; + << _hand << _expires; _active = true; }); diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 56d68e910e..8fe5bea125 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1579,6 +1579,9 @@ bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulation* s } EntityActionPointer action = _objectActions[actionID]; + + qDebug() << "EntityItem::removeActionInternal clearing _ownerEntity"; + action->setOwnerEntity(nullptr); _objectActions.remove(actionID); @@ -1601,6 +1604,7 @@ bool EntityItem::clearActions(EntitySimulation* simulation) { const QUuid id = i.key(); EntityActionPointer action = _objectActions[id]; i = _objectActions.erase(i); + qDebug() << "EntityItem::clearActions clearing _ownerEntity"; action->setOwnerEntity(nullptr); action->removeFromSimulation(simulation); } @@ -1644,6 +1648,8 @@ void EntityItem::deserializeActionsInternal() { // Keep track of which actions got added or updated by the new actionData QSet updated; + qDebug() << "EntityItem::deserializeActionsInternal" << serializedActions.size(); + foreach(QByteArray serializedAction, serializedActions) { QDataStream serializedActionStream(serializedAction); EntityActionType actionType; @@ -1722,8 +1728,11 @@ void EntityItem::setActionData(QByteArray actionData) { void EntityItem::setActionDataInternal(QByteArray actionData) { assertWriteLocked(); if (_allActionsDataCache != actionData) { + qDebug() << "EntityItem::setActionDataInternal yes"; _allActionsDataCache = actionData; deserializeActionsInternal(); + } else { + qDebug() << "EntityItem::setActionDataInternal no"; } checkWaitingToRemove(); } diff --git a/libraries/physics/src/ObjectAction.cpp b/libraries/physics/src/ObjectAction.cpp index 2923bc20a9..2f0de6d0ab 100644 --- a/libraries/physics/src/ObjectAction.cpp +++ b/libraries/physics/src/ObjectAction.cpp @@ -63,35 +63,48 @@ void ObjectAction::updateAction(btCollisionWorld* collisionWorld, btScalar delta } bool ObjectAction::updateArguments(QVariantMap arguments) { - bool lifetimeSet = true; - float lifetime = EntityActionInterface::extractFloatArgument("action", arguments, "lifetime", lifetimeSet, false); - if (lifetimeSet) { - quint64 now = usecTimestampNow(); - _expires = now + (quint64)(lifetime * USECS_PER_SECOND); - } else { - _expires = 0; - } + bool somethingChanged = false; - bool tagSet = true; - QString tag = EntityActionInterface::extractStringArgument("action", arguments, "tag", tagSet, false); - if (tagSet) { - _tag = tag; - } else { - tag = ""; - } + withWriteLock([&]{ + quint64 previousExpires = _expires; + QString previousTag = _tag; - return true; + bool lifetimeSet = true; + float lifetime = EntityActionInterface::extractFloatArgument("action", arguments, "lifetime", lifetimeSet, false); + if (lifetimeSet) { + quint64 now = usecTimestampNow(); + _expires = now + (quint64)(lifetime * USECS_PER_SECOND); + } else { + _expires = 0; + } + + bool tagSet = true; + QString tag = EntityActionInterface::extractStringArgument("action", arguments, "tag", tagSet, false); + if (tagSet) { + _tag = tag; + } else { + tag = ""; + } + + if (previousExpires != _expires || previousTag != _tag) { + somethingChanged = true; + } + }); + + return somethingChanged; } QVariantMap ObjectAction::getArguments() { QVariantMap arguments; - if (_expires == 0) { - arguments["lifetime"] = 0.0f; - } else { - quint64 now = usecTimestampNow(); - arguments["lifetime"] = (float)(_expires - now) / (float)USECS_PER_SECOND; - } - arguments["tag"] = _tag; + withReadLock([&]{ + if (_expires == 0) { + arguments["lifetime"] = 0.0f; + } else { + quint64 now = usecTimestampNow(); + arguments["lifetime"] = (float)(_expires - now) / (float)USECS_PER_SECOND; + } + arguments["tag"] = _tag; + }); return arguments; } @@ -100,20 +113,30 @@ void ObjectAction::debugDraw(btIDebugDraw* debugDrawer) { } void ObjectAction::removeFromSimulation(EntitySimulation* simulation) const { - simulation->removeAction(_id); + QUuid myID; + withReadLock([&]{ + myID = _id; + }); + simulation->removeAction(myID); } btRigidBody* ObjectAction::getRigidBody() { - auto ownerEntity = _ownerEntity.lock(); - if (!ownerEntity) { - return nullptr; + ObjectMotionState* motionState = nullptr; + withReadLock([&]{ + auto ownerEntity = _ownerEntity.lock(); + if (!ownerEntity) { + return; + } + void* physicsInfo = ownerEntity->getPhysicsInfo(); + if (!physicsInfo) { + return; + } + motionState = static_cast(physicsInfo); + }); + if (motionState) { + return motionState->getRigidBody(); } - void* physicsInfo = ownerEntity->getPhysicsInfo(); - if (!physicsInfo) { - return nullptr; - } - ObjectMotionState* motionState = static_cast(physicsInfo); - return motionState->getRigidBody(); + return nullptr; } glm::vec3 ObjectAction::getPosition() { diff --git a/libraries/physics/src/ObjectActionOffset.cpp b/libraries/physics/src/ObjectActionOffset.cpp index 448ec34689..ad5f49b3fb 100644 --- a/libraries/physics/src/ObjectActionOffset.cpp +++ b/libraries/physics/src/ObjectActionOffset.cpp @@ -80,35 +80,46 @@ void ObjectActionOffset::updateActionWorker(btScalar deltaTimeStep) { bool ObjectActionOffset::updateArguments(QVariantMap arguments) { - if (!ObjectAction::updateArguments(arguments)) { - return false; - } - bool ok = true; - glm::vec3 pointToOffsetFrom = - EntityActionInterface::extractVec3Argument("offset action", arguments, "pointToOffsetFrom", ok, true); - if (!ok) { - pointToOffsetFrom = _pointToOffsetFrom; - } + glm::vec3 pointToOffsetFrom; + float linearTimeScale; + float linearDistance; - ok = true; - float linearTimeScale = - EntityActionInterface::extractFloatArgument("offset action", arguments, "linearTimeScale", ok, false); - if (!ok) { - linearTimeScale = _linearTimeScale; - } + bool needUpdate = false; + bool somethingChanged = ObjectAction::updateArguments(arguments); - ok = true; - float linearDistance = - EntityActionInterface::extractFloatArgument("offset action", arguments, "linearDistance", ok, false); - if (!ok) { - linearDistance = _linearDistance; - } + withReadLock([&]{ + bool ok = true; + pointToOffsetFrom = + EntityActionInterface::extractVec3Argument("offset action", arguments, "pointToOffsetFrom", ok, true); + if (!ok) { + pointToOffsetFrom = _pointToOffsetFrom; + } - // only change stuff if something actually changed - if (_pointToOffsetFrom != pointToOffsetFrom - || _linearTimeScale != linearTimeScale - || _linearDistance != linearDistance) { + ok = true; + linearTimeScale = + EntityActionInterface::extractFloatArgument("offset action", arguments, "linearTimeScale", ok, false); + if (!ok) { + linearTimeScale = _linearTimeScale; + } + ok = true; + linearDistance = + EntityActionInterface::extractFloatArgument("offset action", arguments, "linearDistance", ok, false); + if (!ok) { + linearDistance = _linearDistance; + } + + // only change stuff if something actually changed + if (somethingChanged || + _pointToOffsetFrom != pointToOffsetFrom || + _linearTimeScale != linearTimeScale || + _linearDistance != linearDistance) { + needUpdate = true; + } + }); + + + if (needUpdate) { withWriteLock([&] { _pointToOffsetFrom = pointToOffsetFrom; _linearTimeScale = linearTimeScale; @@ -118,6 +129,7 @@ bool ObjectActionOffset::updateArguments(QVariantMap arguments) { activateBody(); }); } + return true; } diff --git a/libraries/physics/src/ObjectActionSpring.cpp b/libraries/physics/src/ObjectActionSpring.cpp index 8b936a96e9..6c7507a9db 100644 --- a/libraries/physics/src/ObjectActionSpring.cpp +++ b/libraries/physics/src/ObjectActionSpring.cpp @@ -107,43 +107,52 @@ void ObjectActionSpring::updateActionWorker(btScalar deltaTimeStep) { const float MIN_TIMESCALE = 0.1f; + bool ObjectActionSpring::updateArguments(QVariantMap arguments) { - if (!ObjectAction::updateArguments(arguments)) { - return false; - } - // targets are required, spring-constants are optional - bool ok = true; - glm::vec3 positionalTarget = - EntityActionInterface::extractVec3Argument("spring action", arguments, "targetPosition", ok, false); - if (!ok) { - positionalTarget = _positionalTarget; - } - ok = true; - float linearTimeScale = - EntityActionInterface::extractFloatArgument("spring action", arguments, "linearTimeScale", ok, false); - if (!ok || linearTimeScale <= 0.0f) { - linearTimeScale = _linearTimeScale; - } + glm::vec3 positionalTarget; + float linearTimeScale; + glm::quat rotationalTarget; + float angularTimeScale; - ok = true; - glm::quat rotationalTarget = - EntityActionInterface::extractQuatArgument("spring action", arguments, "targetRotation", ok, false); - if (!ok) { - rotationalTarget = _rotationalTarget; - } + bool needUpdate = false; + bool somethingChanged = ObjectAction::updateArguments(arguments); + withReadLock([&]{ + // targets are required, spring-constants are optional + bool ok = true; + positionalTarget = EntityActionInterface::extractVec3Argument("spring action", arguments, "targetPosition", ok, false); + if (!ok) { + positionalTarget = _positionalTarget; + } + ok = true; + linearTimeScale = EntityActionInterface::extractFloatArgument("spring action", arguments, "linearTimeScale", ok, false); + if (!ok || linearTimeScale <= 0.0f) { + linearTimeScale = _linearTimeScale; + } - ok = true; - float angularTimeScale = - EntityActionInterface::extractFloatArgument("spring action", arguments, "angularTimeScale", ok, false); - if (!ok) { - angularTimeScale = _angularTimeScale; - } + ok = true; + rotationalTarget = EntityActionInterface::extractQuatArgument("spring action", arguments, "targetRotation", ok, false); + if (!ok) { + rotationalTarget = _rotationalTarget; + } - if (positionalTarget != _positionalTarget - || linearTimeScale != _linearTimeScale - || rotationalTarget != _rotationalTarget - || angularTimeScale != _angularTimeScale) { - // something changed + ok = true; + angularTimeScale = + EntityActionInterface::extractFloatArgument("spring action", arguments, "angularTimeScale", ok, false); + if (!ok) { + angularTimeScale = _angularTimeScale; + } + + if (somethingChanged || + positionalTarget != _positionalTarget || + linearTimeScale != _linearTimeScale || + rotationalTarget != _rotationalTarget || + angularTimeScale != _angularTimeScale) { + // something changed + needUpdate = true; + } + }); + + if (needUpdate) { withWriteLock([&] { _positionalTarget = positionalTarget; _linearTimeScale = glm::max(MIN_TIMESCALE, glm::abs(linearTimeScale)); @@ -151,8 +160,15 @@ bool ObjectActionSpring::updateArguments(QVariantMap arguments) { _angularTimeScale = glm::max(MIN_TIMESCALE, glm::abs(angularTimeScale)); _active = true; activateBody(); + + auto ownerEntity = _ownerEntity.lock(); + if (ownerEntity) { + ownerEntity->setActionDataDirty(true); + } + }); } + return true; }