From f3adb8a2f70264af4eb50b93b314065ce88d059b Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 17 Dec 2015 11:16:59 -0800 Subject: [PATCH 1/4] fix offset math in hold action --- interface/src/avatar/AvatarActionHold.cpp | 55 +++++++++++------------ 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/interface/src/avatar/AvatarActionHold.cpp b/interface/src/avatar/AvatarActionHold.cpp index 34b959575d..b2343c8bce 100644 --- a/interface/src/avatar/AvatarActionHold.cpp +++ b/interface/src/avatar/AvatarActionHold.cpp @@ -48,38 +48,38 @@ void AvatarActionHold::prepareForPhysicsSimulation() { auto avatarManager = DependencyManager::get(); auto holdingAvatar = std::static_pointer_cast(avatarManager->getAvatarBySessionID(_holderID)); - if (!holdingAvatar) { + if (!holdingAvatar || !holdingAvatar->isMyAvatar()) { return; } withWriteLock([&]{ - if (_ignoreIK && holdingAvatar->isMyAvatar()) { + if (_ignoreIK) { return; } - if (holdingAvatar->isMyAvatar()) { - glm::vec3 palmPosition; - glm::quat palmRotation; - if (_hand == "right") { - palmPosition = holdingAvatar->getRightPalmPosition(); - palmRotation = holdingAvatar->getRightPalmRotation(); - } else { - palmPosition = holdingAvatar->getLeftPalmPosition(); - palmRotation = holdingAvatar->getLeftPalmRotation(); - } - glm::vec3 avatarRigidBodyPosition; - glm::quat avatarRigidBodyRotation; - getAvatarRigidBodyLocation(avatarRigidBodyPosition, avatarRigidBodyRotation); - - // determine the difference in translation and rotation between the avatar's - // rigid body and the palm position. The avatar's rigid body will be moved by bullet - // between this call and the call to getTarget, below. A call to get*PalmPosition in - // getTarget would get the palm position of the previous location of the avatar (because - // bullet has moved the av's rigid body but the rigid body's location has not yet been - // copied out into the Avatar class. - _palmOffsetFromRigidBody = palmPosition - avatarRigidBodyPosition; - _palmRotationFromRigidBody = glm::inverse(avatarRigidBodyRotation) * palmRotation; + glm::vec3 palmPosition; + glm::quat palmRotation; + if (_hand == "right") { + palmPosition = holdingAvatar->getRightPalmPosition(); + palmRotation = holdingAvatar->getRightPalmRotation(); + } else { + palmPosition = holdingAvatar->getLeftPalmPosition(); + palmRotation = holdingAvatar->getLeftPalmRotation(); } + + glm::vec3 avatarRigidBodyPosition; + glm::quat avatarRigidBodyRotation; + getAvatarRigidBodyLocation(avatarRigidBodyPosition, avatarRigidBodyRotation); + + // determine the difference in translation and rotation between the avatar's + // rigid body and the palm position. The avatar's rigid body will be moved by bullet + // between this call and the call to getTarget, below. A call to get*PalmPosition in + // getTarget would get the palm position of the previous location of the avatar (because + // bullet has moved the av's rigid body but the rigid body's location has not yet been + // copied out into the Avatar class. + glm::quat avatarRotationInverse = glm::inverse(avatarRigidBodyRotation); + _palmOffsetFromRigidBody = avatarRotationInverse * (palmPosition - avatarRigidBodyPosition); + _palmRotationFromRigidBody = avatarRotationInverse * palmRotation; }); } @@ -116,13 +116,8 @@ std::shared_ptr AvatarActionHold::getTarget(glm::quat& rotation, glm::ve // and the data in the Avatar class is stale. This means that the result of get*PalmPosition will // be stale. Instead, determine the current palm position with the current avatar's rigid body // location and the saved offsets. - palmPosition = avatarRigidBodyPosition + _palmOffsetFromRigidBody; + palmPosition = avatarRigidBodyPosition + avatarRigidBodyRotation * _palmOffsetFromRigidBody; palmRotation = avatarRigidBodyRotation * _palmRotationFromRigidBody; - if (isRightHand) { - palmRotation = holdingAvatar->getRightPalmRotation(); - } else { - palmRotation = holdingAvatar->getLeftPalmRotation(); - } } else { if (isRightHand) { palmPosition = holdingAvatar->getRightPalmPosition(); From b80fa1c8064c23bb3709593dafcef968cb7e1aa3 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 17 Dec 2015 11:46:41 -0800 Subject: [PATCH 2/4] code review --- interface/src/avatar/AvatarActionHold.cpp | 29 +++++++++++++++++++---- interface/src/avatar/AvatarActionHold.h | 3 ++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/interface/src/avatar/AvatarActionHold.cpp b/interface/src/avatar/AvatarActionHold.cpp index b2343c8bce..be562e2773 100644 --- a/interface/src/avatar/AvatarActionHold.cpp +++ b/interface/src/avatar/AvatarActionHold.cpp @@ -78,8 +78,16 @@ void AvatarActionHold::prepareForPhysicsSimulation() { // bullet has moved the av's rigid body but the rigid body's location has not yet been // copied out into the Avatar class. glm::quat avatarRotationInverse = glm::inverse(avatarRigidBodyRotation); - _palmOffsetFromRigidBody = avatarRotationInverse * (palmPosition - avatarRigidBodyPosition); - _palmRotationFromRigidBody = avatarRotationInverse * palmRotation; + + // the offset should be in the frame of the avatar, but something about the order + // things are updated makes this wrong: + // _palmOffsetFromRigidBody = avatarRotationInverse * (palmPosition - avatarRigidBodyPosition); + // I'll leave it here as a comment in case avatar handling changes. + _palmOffsetFromRigidBody = palmPosition - avatarRigidBodyPosition; + + // rotation should also be needed, but again, the order of updates makes this unneeded. leaving + // code here for future reference. + // _palmRotationFromRigidBody = avatarRotationInverse * palmRotation; }); } @@ -116,8 +124,21 @@ std::shared_ptr AvatarActionHold::getTarget(glm::quat& rotation, glm::ve // and the data in the Avatar class is stale. This means that the result of get*PalmPosition will // be stale. Instead, determine the current palm position with the current avatar's rigid body // location and the saved offsets. - palmPosition = avatarRigidBodyPosition + avatarRigidBodyRotation * _palmOffsetFromRigidBody; - palmRotation = avatarRigidBodyRotation * _palmRotationFromRigidBody; + + // this line is more correct but breaks for the current way avatar data is updated. + // palmPosition = avatarRigidBodyPosition + avatarRigidBodyRotation * _palmOffsetFromRigidBody; + // instead, use this for now: + palmPosition = avatarRigidBodyPosition + _palmOffsetFromRigidBody; + + // the item jitters the least by getting the rotation based on the opinion of Avatar.h rather + // than that of the rigid body. leaving this next line here for future reference: + // palmRotation = avatarRigidBodyRotation * _palmRotationFromRigidBody; + + if (isRightHand) { + palmRotation = holdingAvatar->getRightPalmRotation(); + } else { + palmRotation = holdingAvatar->getLeftPalmRotation(); + } } else { if (isRightHand) { palmPosition = holdingAvatar->getRightPalmPosition(); diff --git a/interface/src/avatar/AvatarActionHold.h b/interface/src/avatar/AvatarActionHold.h index fc8baf6dcc..b97ec59780 100644 --- a/interface/src/avatar/AvatarActionHold.h +++ b/interface/src/avatar/AvatarActionHold.h @@ -61,7 +61,8 @@ private: glm::vec3 _previousPositionalDelta; glm::vec3 _palmOffsetFromRigidBody; - glm::quat _palmRotationFromRigidBody; + // leaving this here for future refernece. + // glm::quat _palmRotationFromRigidBody; }; #endif // hifi_AvatarActionHold_h From 0b781a3589be3a3b604054f36e531714d7aff94a Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 17 Dec 2015 18:39:05 -0800 Subject: [PATCH 3/4] fix warnings --- interface/src/avatar/Avatar.cpp | 2 +- interface/src/avatar/AvatarActionHold.cpp | 2 +- interface/src/ui/ApplicationCompositor.cpp | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index f8040754d7..6094a0b9fe 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -157,7 +157,7 @@ void Avatar::animateScaleChanges(float deltaTime) { // snap to the end when we get close enough const float MIN_RELATIVE_SCALE_ERROR = 0.03f; - if (fabsf(_targetScale - currentScale) / _targetScale < 0.03f) { + if (fabsf(_targetScale - currentScale) / _targetScale < MIN_RELATIVE_SCALE_ERROR) { animatedScale = _targetScale; } diff --git a/interface/src/avatar/AvatarActionHold.cpp b/interface/src/avatar/AvatarActionHold.cpp index be562e2773..689d557c48 100644 --- a/interface/src/avatar/AvatarActionHold.cpp +++ b/interface/src/avatar/AvatarActionHold.cpp @@ -77,7 +77,7 @@ void AvatarActionHold::prepareForPhysicsSimulation() { // getTarget would get the palm position of the previous location of the avatar (because // bullet has moved the av's rigid body but the rigid body's location has not yet been // copied out into the Avatar class. - glm::quat avatarRotationInverse = glm::inverse(avatarRigidBodyRotation); + //glm::quat avatarRotationInverse = glm::inverse(avatarRigidBodyRotation); // the offset should be in the frame of the avatar, but something about the order // things are updated makes this wrong: diff --git a/interface/src/ui/ApplicationCompositor.cpp b/interface/src/ui/ApplicationCompositor.cpp index e5ecdfe217..abc3520c83 100644 --- a/interface/src/ui/ApplicationCompositor.cpp +++ b/interface/src/ui/ApplicationCompositor.cpp @@ -34,7 +34,6 @@ static const quint64 MSECS_TO_USECS = 1000ULL; static const quint64 TOOLTIP_DELAY = 500 * MSECS_TO_USECS; -static const float RETICLE_COLOR[] = { 0.0f, 198.0f / 255.0f, 244.0f / 255.0f }; static const float reticleSize = TWO_PI / 100.0f; static const float CURSOR_PIXEL_SIZE = 32.0f; From a7824a06d0b2d3bfa9d4fba2da1761d8e7bbf384 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 17 Dec 2015 18:51:27 -0800 Subject: [PATCH 4/4] fix Assignment copy constructor warning --- libraries/networking/src/Assignment.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/Assignment.cpp b/libraries/networking/src/Assignment.cpp index 4c72a16f1d..e6163c776b 100644 --- a/libraries/networking/src/Assignment.cpp +++ b/libraries/networking/src/Assignment.cpp @@ -91,7 +91,7 @@ Assignment::Assignment(ReceivedMessage& message) : #endif -Assignment::Assignment(const Assignment& otherAssignment) { +Assignment::Assignment(const Assignment& otherAssignment) : QObject() { _uuid = otherAssignment._uuid; _command = otherAssignment._command; _type = otherAssignment._type;