From 1164a1b3bf43cba1549cc8303edf5fdf8d74d6ec Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Sun, 12 Feb 2017 08:27:39 -0800 Subject: [PATCH 1/7] some fixes for when an avatar is a child of something else --- assignment-client/src/avatars/AvatarMixer.cpp | 4 +-- interface/src/avatar/Avatar.cpp | 4 +++ libraries/avatars/src/AvatarData.cpp | 35 +++++++++---------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 61164ee8d7..bf85918145 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -405,7 +405,7 @@ void AvatarMixer::broadcastAvatarData() { otherNodeData->getLastReceivedSequenceNumber()); // determine if avatar is in view, to determine how much data to include... - glm::vec3 otherNodeBoxScale = (otherNodeData->getPosition() - otherNodeData->getGlobalBoundingBoxCorner()) * 2.0f; + glm::vec3 otherNodeBoxScale = (otherPosition - otherNodeData->getGlobalBoundingBoxCorner()) * 2.0f; AABox otherNodeBox(otherNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); bool isInView = nodeData->otherAvatarInView(otherNodeBox); @@ -431,7 +431,7 @@ void AvatarMixer::broadcastAvatarData() { auto lastEncodeForOther = nodeData->getLastOtherAvatarEncodeTime(otherNode->getUUID()); QVector& lastSentJointsForOther = nodeData->getLastOtherAvatarSentJoints(otherNode->getUUID()); bool distanceAdjust = true; - glm::vec3 viewerPosition = nodeData->getPosition(); + glm::vec3 viewerPosition = myPosition; auto bytes = otherAvatar.toByteArray(detail, lastEncodeForOther, lastSentJointsForOther, distanceAdjust, viewerPosition, &lastSentJointsForOther); numAvatarDataBytes += avatarPacketList->write(bytes); diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index ab97f563f6..ec82ac9ff8 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -1324,6 +1324,7 @@ void Avatar::setParentID(const QUuid& parentID) { if (!isMyAvatar()) { return; } + QUuid initialParentID = getParentID(); bool success; Transform beforeChangeTransform = getTransform(success); SpatiallyNestable::setParentID(parentID); @@ -1332,6 +1333,9 @@ void Avatar::setParentID(const QUuid& parentID) { if (!success) { qCDebug(interfaceapp) << "Avatar::setParentID failed to reset avatar's location."; } + if (initialParentID != parentID) { + _parentChanged = usecTimestampNow(); + } } } diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index af060429af..89cabbce3e 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -405,6 +405,18 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent _additionalFlagsRateOutbound.increment(numBytes); } + if (hasParentInfo) { + auto startSection = destinationBuffer; + auto parentInfo = reinterpret_cast(destinationBuffer); + QByteArray referentialAsBytes = parentID.toRfc4122(); + memcpy(parentInfo->parentUUID, referentialAsBytes.data(), referentialAsBytes.size()); + parentInfo->parentJointIndex = _parentJointIndex; + destinationBuffer += sizeof(AvatarDataPacket::ParentInfo); + + int numBytes = destinationBuffer - startSection; + _parentInfoRateOutbound.increment(numBytes); + } + if (hasAvatarLocalPosition) { auto startSection = destinationBuffer; auto data = reinterpret_cast(destinationBuffer); @@ -418,18 +430,6 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent _localPositionRateOutbound.increment(numBytes); } - if (hasParentInfo) { - auto startSection = destinationBuffer; - auto parentInfo = reinterpret_cast(destinationBuffer); - QByteArray referentialAsBytes = parentID.toRfc4122(); - memcpy(parentInfo->parentUUID, referentialAsBytes.data(), referentialAsBytes.size()); - parentInfo->parentJointIndex = _parentJointIndex; - destinationBuffer += sizeof(AvatarDataPacket::ParentInfo); - - int numBytes = destinationBuffer - startSection; - _parentInfoRateOutbound.increment(numBytes); - } - // If it is connected, pack up the data if (hasFaceTrackerInfo) { auto startSection = destinationBuffer; @@ -904,19 +904,18 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) { auto newParentID = QUuid::fromRfc4122(byteArray); - if ((_parentID != newParentID) || (_parentJointIndex = parentInfo->parentJointIndex)) { - _parentID = newParentID; - _parentJointIndex = parentInfo->parentJointIndex; + if ((_parentID != newParentID) || (_parentJointIndex != parentInfo->parentJointIndex)) { + setParentID(newParentID); + setParentJointIndex(parentInfo->parentJointIndex); _parentChanged = usecTimestampNow(); } int numBytesRead = sourceBuffer - startSection; _parentInfoRate.increment(numBytesRead); _parentInfoUpdateRate.increment(); - } - else { + } else { // FIXME - this aint totally right, for switching to parent/no-parent - _parentID = QUuid(); + setParentID(QUuid()); } if (hasAvatarLocalPosition) { From 0c2abc988342e61d54594e8447438a00bf9c801c Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Sun, 12 Feb 2017 10:01:30 -0800 Subject: [PATCH 2/7] fix handling of an avatar's parent changing --- libraries/avatars/src/AvatarData.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 89cabbce3e..6440432675 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -258,8 +258,8 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent // local position, and parent info only apply to avatars that are parented. The local position // and the parent info can change independently though, so we track their "changed since" // separately - bool hasParentInfo = hasParent() && (sendAll || parentInfoChangedSince(lastSentTime)); - bool hasAvatarLocalPosition = hasParent() && (sendAll || tranlationChangedSince(lastSentTime)); + bool hasParentInfo = sendAll || parentInfoChangedSince(lastSentTime); + bool hasAvatarLocalPosition = sendAll || tranlationChangedSince(lastSentTime); bool hasFaceTrackerInfo = hasFaceTracker() && (sendAll || faceTrackerInfoChangedSince(lastSentTime)); bool hasJointData = sendAll || !sendMinimum; @@ -884,7 +884,7 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) { sourceBuffer += sizeof(AvatarDataPacket::AdditionalFlags); - if (somethingChanged) { + if (somethingChanged) { _additionalFlagsChanged = usecTimestampNow(); } int numBytesRead = sourceBuffer - startSection; @@ -892,8 +892,6 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) { _additionalFlagsUpdateRate.increment(); } - // FIXME -- make sure to handle the existance of a parent vs a change in the parent... - //bool hasReferential = oneAtBit(bitItems, HAS_REFERENTIAL); if (hasParentInfo) { auto startSection = sourceBuffer; PACKET_READ_CHECK(ParentInfo, sizeof(AvatarDataPacket::ParentInfo)); @@ -904,18 +902,15 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) { auto newParentID = QUuid::fromRfc4122(byteArray); - if ((_parentID != newParentID) || (_parentJointIndex != parentInfo->parentJointIndex)) { - setParentID(newParentID); - setParentJointIndex(parentInfo->parentJointIndex); + if ((getParentID() != newParentID) || (getParentJointIndex() != parentInfo->parentJointIndex)) { + SpatiallyNestable::setParentID(newParentID); + SpatiallyNestable::setParentJointIndex(parentInfo->parentJointIndex); _parentChanged = usecTimestampNow(); } int numBytesRead = sourceBuffer - startSection; _parentInfoRate.increment(numBytesRead); _parentInfoUpdateRate.increment(); - } else { - // FIXME - this aint totally right, for switching to parent/no-parent - setParentID(QUuid()); } if (hasAvatarLocalPosition) { From bc5f563f5786c635b65aa005043c04f16c9fe818 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Sun, 12 Feb 2017 11:37:29 -0800 Subject: [PATCH 3/7] make _parentID and _parentJointIndex private to avoid accidental direct access --- interface/src/ui/overlays/Line3DOverlay.cpp | 8 ++++---- libraries/avatars/src/AvatarData.cpp | 2 +- libraries/entities/src/EntityItem.cpp | 2 +- libraries/shared/src/SpatiallyNestable.h | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/interface/src/ui/overlays/Line3DOverlay.cpp b/interface/src/ui/overlays/Line3DOverlay.cpp index 97e7d825f2..23668bcc25 100644 --- a/interface/src/ui/overlays/Line3DOverlay.cpp +++ b/interface/src/ui/overlays/Line3DOverlay.cpp @@ -38,7 +38,7 @@ Line3DOverlay::~Line3DOverlay() { glm::vec3 Line3DOverlay::getStart() const { bool success; - glm::vec3 worldStart = localToWorld(_start, _parentID, _parentJointIndex, success); + glm::vec3 worldStart = localToWorld(_start, getParentID(), getParentJointIndex(), success); if (!success) { qDebug() << "Line3DOverlay::getStart failed"; } @@ -47,7 +47,7 @@ glm::vec3 Line3DOverlay::getStart() const { glm::vec3 Line3DOverlay::getEnd() const { bool success; - glm::vec3 worldEnd = localToWorld(_end, _parentID, _parentJointIndex, success); + glm::vec3 worldEnd = localToWorld(_end, getParentID(), getParentJointIndex(), success); if (!success) { qDebug() << "Line3DOverlay::getEnd failed"; } @@ -56,7 +56,7 @@ glm::vec3 Line3DOverlay::getEnd() const { void Line3DOverlay::setStart(const glm::vec3& start) { bool success; - _start = worldToLocal(start, _parentID, _parentJointIndex, success); + _start = worldToLocal(start, getParentID(), getParentJointIndex(), success); if (!success) { qDebug() << "Line3DOverlay::setStart failed"; } @@ -64,7 +64,7 @@ void Line3DOverlay::setStart(const glm::vec3& start) { void Line3DOverlay::setEnd(const glm::vec3& end) { bool success; - _end = worldToLocal(end, _parentID, _parentJointIndex, success); + _end = worldToLocal(end, getParentID(), getParentJointIndex(), success); if (!success) { qDebug() << "Line3DOverlay::setEnd failed"; } diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 6440432675..29cbd155fe 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -410,7 +410,7 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent auto parentInfo = reinterpret_cast(destinationBuffer); QByteArray referentialAsBytes = parentID.toRfc4122(); memcpy(parentInfo->parentUUID, referentialAsBytes.data(), referentialAsBytes.size()); - parentInfo->parentJointIndex = _parentJointIndex; + parentInfo->parentJointIndex = getParentJointIndex(); destinationBuffer += sizeof(AvatarDataPacket::ParentInfo); int numBytes = destinationBuffer - startSection; diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 6543af5355..3ef1648fae 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1595,7 +1595,7 @@ void EntityItem::updatePosition(const glm::vec3& value) { } void EntityItem::updateParentID(const QUuid& value) { - if (_parentID != value) { + if (getParentID() != value) { setParentID(value); _dirtyFlags |= Simulation::DIRTY_MOTION_TYPE; // children are forced to be kinematic _dirtyFlags |= Simulation::DIRTY_COLLISION_GROUP; // may need to not collide with own avatar diff --git a/libraries/shared/src/SpatiallyNestable.h b/libraries/shared/src/SpatiallyNestable.h index cd59fb30a0..be285eff53 100644 --- a/libraries/shared/src/SpatiallyNestable.h +++ b/libraries/shared/src/SpatiallyNestable.h @@ -186,9 +186,6 @@ public: protected: const NestableType _nestableType; // EntityItem or an AvatarData QUuid _id; - QUuid _parentID; // what is this thing's transform relative to? - quint16 _parentJointIndex { INVALID_JOINT_INDEX }; // which joint of the parent is this relative to? - mutable SpatiallyNestableWeakPointer _parent; virtual void beParentOfChild(SpatiallyNestablePointer newChild) const; @@ -211,6 +208,9 @@ protected: quint64 _rotationChanged { 0 }; private: + QUuid _parentID; // what is this thing's transform relative to? + quint16 _parentJointIndex { INVALID_JOINT_INDEX }; // which joint of the parent is this relative to? + mutable ReadWriteLockable _transformLock; mutable ReadWriteLockable _idLock; mutable ReadWriteLockable _velocityLock; From 107442eea009dd291ba6c470a758231ffa9ed59c Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Sun, 12 Feb 2017 11:56:38 -0800 Subject: [PATCH 4/7] avoid extra sends to localPosition --- libraries/avatars/src/AvatarData.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 29cbd155fe..27e3c40fa9 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -259,7 +259,9 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent // and the parent info can change independently though, so we track their "changed since" // separately bool hasParentInfo = sendAll || parentInfoChangedSince(lastSentTime); - bool hasAvatarLocalPosition = sendAll || tranlationChangedSince(lastSentTime); + bool hasAvatarLocalPosition = sendAll || + (hasParent() && tranlationChangedSince(lastSentTime)) || + parentInfoChangedSince(lastSentTime); bool hasFaceTrackerInfo = hasFaceTracker() && (sendAll || faceTrackerInfoChangedSince(lastSentTime)); bool hasJointData = sendAll || !sendMinimum; From 27d55d6ff0522e0d02cac5b6d17d5a9c6a8ed0b0 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Mon, 13 Feb 2017 07:30:33 -0800 Subject: [PATCH 5/7] bump avatar-mixer protocol version --- libraries/networking/src/udt/PacketHeaders.cpp | 2 +- libraries/networking/src/udt/PacketHeaders.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index b13b21ba3b..855499c0e7 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -56,7 +56,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::AvatarData: case PacketType::BulkAvatarData: case PacketType::KillAvatar: - return static_cast(AvatarMixerPacketVersion::VariableAvatarData); + return static_cast(AvatarMixerPacketVersion::AvatarAsChildFixes); case PacketType::MessagesData: return static_cast(MessageDataVersion::TextOrBinaryData); case PacketType::ICEServerHeartbeat: diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 0b2c04b031..e198a486f7 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -224,7 +224,8 @@ enum class AvatarMixerPacketVersion : PacketVersion { SessionDisplayName, Unignore, ImmediateSessionDisplayNameUpdates, - VariableAvatarData + VariableAvatarData, + AvatarAsChildFixes }; enum class DomainConnectRequestVersion : PacketVersion { From b333126cbeed41baf24d5432aa667694848bea49 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Mon, 13 Feb 2017 12:20:07 -0800 Subject: [PATCH 6/7] avoid sending localPosition for avatars with no parent --- libraries/avatars/src/AvatarData.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 27e3c40fa9..8bb0b38ae5 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -260,8 +260,8 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent // separately bool hasParentInfo = sendAll || parentInfoChangedSince(lastSentTime); bool hasAvatarLocalPosition = sendAll || - (hasParent() && tranlationChangedSince(lastSentTime)) || - parentInfoChangedSince(lastSentTime); + (hasParent() && (tranlationChangedSince(lastSentTime) || + parentInfoChangedSince(lastSentTime))); bool hasFaceTrackerInfo = hasFaceTracker() && (sendAll || faceTrackerInfoChangedSince(lastSentTime)); bool hasJointData = sendAll || !sendMinimum; @@ -916,7 +916,6 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) { } if (hasAvatarLocalPosition) { - assert(hasParent()); // we shouldn't have local position unless we have a parent auto startSection = sourceBuffer; PACKET_READ_CHECK(AvatarLocalPosition, sizeof(AvatarDataPacket::AvatarLocalPosition)); @@ -928,7 +927,11 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) { } return buffer.size(); } - setLocalPosition(position); + if (hasParent()) { + setLocalPosition(position); + } else { + qCWarning(avatars) << "received localPosition for avatar with no parent"; + } sourceBuffer += sizeof(AvatarDataPacket::AvatarLocalPosition); int numBytesRead = sourceBuffer - startSection; _localPositionRate.increment(numBytesRead); From 5ee484a5923ff1b1d48a9608d3d6a95cbb279bb1 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Mon, 13 Feb 2017 13:36:07 -0800 Subject: [PATCH 7/7] don't send avatar localPosition unless the avatar has a parent, even if sendAll is true --- libraries/avatars/src/AvatarData.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 8bb0b38ae5..49a4355301 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -259,9 +259,9 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent // and the parent info can change independently though, so we track their "changed since" // separately bool hasParentInfo = sendAll || parentInfoChangedSince(lastSentTime); - bool hasAvatarLocalPosition = sendAll || - (hasParent() && (tranlationChangedSince(lastSentTime) || - parentInfoChangedSince(lastSentTime))); + bool hasAvatarLocalPosition = hasParent() && (sendAll || + tranlationChangedSince(lastSentTime) || + parentInfoChangedSince(lastSentTime)); bool hasFaceTrackerInfo = hasFaceTracker() && (sendAll || faceTrackerInfoChangedSince(lastSentTime)); bool hasJointData = sendAll || !sendMinimum;