From 3221e1dbd59c4b333e4285427a3b0a595acf732e Mon Sep 17 00:00:00 2001 From: Clement Date: Tue, 19 Mar 2019 19:39:17 -0700 Subject: [PATCH] Simplify packing/unpacking for easier extension --- .../src/avatars/AvatarMixerClientData.cpp | 2 +- .../src/avatars/AvatarMixerSlave.cpp | 6 +- libraries/avatars/src/AvatarData.cpp | 151 +++++------------- libraries/avatars/src/AvatarData.h | 22 ++- libraries/avatars/src/AvatarTraits.cpp | 135 ++++++++++++++++ libraries/avatars/src/AvatarTraits.h | 27 ++-- libraries/avatars/src/ClientTraitsHandler.cpp | 6 +- 7 files changed, 209 insertions(+), 140 deletions(-) create mode 100644 libraries/avatars/src/AvatarTraits.cpp diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 557c5c9fe3..dfbeca96ce 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -341,7 +341,7 @@ void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(const SlaveSharedDa // the returned set traits packet uses the trait version from the incoming packet // so the client knows they should not overwrite if they have since changed the trait - _avatar->packTrait(AvatarTraits::SkeletonModelURL, *packet, traitVersion); + AvatarTraits::packVersionedTrait(AvatarTraits::SkeletonModelURL, *packet, traitVersion, *_avatar); auto nodeList = DependencyManager::get(); nodeList->sendPacket(std::move(packet), sendingNode); diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index e59c81f4b7..fdbdf21607 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -139,7 +139,8 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis if (lastReceivedVersion > lastSentVersionRef) { bytesWritten += addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); // there is an update to this trait, add it to the traits packet - bytesWritten += sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); + bytesWritten += AvatarTraits::packVersionedTrait(traitType, traitsPacketList, + lastReceivedVersion, *sendingAvatar); // update the last sent version lastSentVersionRef = lastReceivedVersion; // Remember which versions we sent in this particular packet @@ -194,7 +195,8 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis bytesWritten += addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); // this instance version exists and has never been sent or is newer so we need to send it - bytesWritten += sendingAvatar->packTraitInstance(traitType, instanceID, traitsPacketList, receivedVersion); + bytesWritten += AvatarTraits::packVersionedTraitInstance(traitType, instanceID, traitsPacketList, + receivedVersion, *sendingAvatar); if (sentInstanceIt != sentIDValuePairs.end()) { sentInstanceIt->value = receivedVersion; diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 39dfaa8a1a..9c923580f9 100755 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1990,42 +1990,16 @@ QUrl AvatarData::getWireSafeSkeletonModelURL() const { } } -qint64 AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, - AvatarTraits::TraitVersion traitVersion) { - - qint64 bytesWritten = 0; - - if (traitType == AvatarTraits::SkeletonModelURL) { - - QByteArray encodedSkeletonURL = getWireSafeSkeletonModelURL().toEncoded(); - - if (encodedSkeletonURL.size() > AvatarTraits::MAXIMUM_TRAIT_SIZE) { - qWarning() << "Refusing to pack simple trait" << traitType << "of size" << encodedSkeletonURL.size() - << "bytes since it exceeds the maximum size" << AvatarTraits::MAXIMUM_TRAIT_SIZE << "bytes"; - return 0; - } - - bytesWritten += destination.writePrimitive(traitType); - - if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) { - bytesWritten += destination.writePrimitive(traitVersion); - } - - AvatarTraits::TraitWireSize encodedURLSize = encodedSkeletonURL.size(); - bytesWritten += destination.writePrimitive(encodedURLSize); - - bytesWritten += destination.write(encodedSkeletonURL); - } - - return bytesWritten; +QByteArray AvatarData::packSkeletonModelURL() const { + return getWireSafeSkeletonModelURL().toEncoded(); } +void AvatarData::unpackSkeletonModelURL(const QByteArray& data) { + auto skeletonModelURL = QUrl::fromEncoded(data); + setSkeletonModelURL(skeletonModelURL); +} -qint64 AvatarData::packAvatarEntityTraitInstance(AvatarTraits::TraitType traitType, - AvatarTraits::TraitInstanceID traitInstanceID, - ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) { - qint64 bytesWritten = 0; - +QByteArray AvatarData::packAvatarEntityTraitInstance(AvatarTraits::TraitInstanceID traitInstanceID) { // grab a read lock on the avatar entities and check for entity data for the given ID QByteArray entityBinaryData; _avatarEntitiesLock.withReadLock([this, &entityBinaryData, &traitInstanceID] { @@ -2034,104 +2008,48 @@ qint64 AvatarData::packAvatarEntityTraitInstance(AvatarTraits::TraitType traitTy } }); - if (entityBinaryData.size() > AvatarTraits::MAXIMUM_TRAIT_SIZE) { - qWarning() << "Refusing to pack instanced trait" << traitType << "of size" << entityBinaryData.size() - << "bytes since it exceeds the maximum size " << AvatarTraits::MAXIMUM_TRAIT_SIZE << "bytes"; - return 0; - } - - bytesWritten += destination.writePrimitive(traitType); - - if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) { - bytesWritten += destination.writePrimitive(traitVersion); - } - - bytesWritten += destination.write(traitInstanceID.toRfc4122()); - - if (!entityBinaryData.isNull()) { - AvatarTraits::TraitWireSize entityBinarySize = entityBinaryData.size(); - - bytesWritten += destination.writePrimitive(entityBinarySize); - bytesWritten += destination.write(entityBinaryData); - } else { - bytesWritten += destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE); - } - - return bytesWritten; + return entityBinaryData; } - -qint64 AvatarData::packGrabTraitInstance(AvatarTraits::TraitType traitType, - AvatarTraits::TraitInstanceID traitInstanceID, - ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) { - qint64 bytesWritten = 0; - +QByteArray AvatarData::packGrabTraitInstance(AvatarTraits::TraitInstanceID traitInstanceID) { // grab a read lock on the avatar grabs and check for grab data for the given ID QByteArray grabBinaryData; - _avatarGrabsLock.withReadLock([this, &grabBinaryData, &traitInstanceID] { if (_avatarGrabData.contains(traitInstanceID)) { grabBinaryData = _avatarGrabData[traitInstanceID]; } }); - if (grabBinaryData.size() > AvatarTraits::MAXIMUM_TRAIT_SIZE) { - qWarning() << "Refusing to pack instanced trait" << traitType << "of size" << grabBinaryData.size() - << "bytes since it exceeds the maximum size " << AvatarTraits::MAXIMUM_TRAIT_SIZE << "bytes"; - return 0; - } - - bytesWritten += destination.writePrimitive(traitType); - - if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) { - bytesWritten += destination.writePrimitive(traitVersion); - } - - bytesWritten += destination.write(traitInstanceID.toRfc4122()); - - if (!grabBinaryData.isNull()) { - AvatarTraits::TraitWireSize grabBinarySize = grabBinaryData.size(); - - bytesWritten += destination.writePrimitive(grabBinarySize); - bytesWritten += destination.write(grabBinaryData); - } else { - bytesWritten += destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE); - } - - return bytesWritten; + return grabBinaryData; } -qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID traitInstanceID, - ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) { - qint64 bytesWritten = 0; +QByteArray AvatarData::packTrait(AvatarTraits::TraitType traitType) const { + QByteArray traitBinaryData; + // Call packer function + if (traitType == AvatarTraits::SkeletonModelURL) { + traitBinaryData = packSkeletonModelURL(); + } + + return traitBinaryData; +} + +QByteArray AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID traitInstanceID) { + QByteArray traitBinaryData; + + // Call packer function if (traitType == AvatarTraits::AvatarEntity) { - bytesWritten += packAvatarEntityTraitInstance(traitType, traitInstanceID, destination, traitVersion); + traitBinaryData = packAvatarEntityTraitInstance(traitInstanceID); } else if (traitType == AvatarTraits::Grab) { - bytesWritten += packGrabTraitInstance(traitType, traitInstanceID, destination, traitVersion); + traitBinaryData = packGrabTraitInstance(traitInstanceID); } - return bytesWritten; -} - -void AvatarData::prepareResetTraitInstances() { - if (_clientTraitsHandler) { - _avatarEntitiesLock.withReadLock([this]{ - foreach (auto entityID, _packedAvatarEntityData.keys()) { - _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID); - } - foreach (auto grabID, _avatarGrabData.keys()) { - _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::Grab, grabID); - } - }); - } + return traitBinaryData; } void AvatarData::processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData) { if (traitType == AvatarTraits::SkeletonModelURL) { - // get the URL from the binary data - auto skeletonModelURL = QUrl::fromEncoded(traitBinaryData); - setSkeletonModelURL(skeletonModelURL); + unpackSkeletonModelURL(traitBinaryData); } } @@ -2152,6 +2070,19 @@ void AvatarData::processDeletedTraitInstance(AvatarTraits::TraitType traitType, } } +void AvatarData::prepareResetTraitInstances() { + if (_clientTraitsHandler) { + _avatarEntitiesLock.withReadLock([this]{ + foreach (auto entityID, _packedAvatarEntityData.keys()) { + _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID); + } + foreach (auto grabID, _avatarGrabData.keys()) { + _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::Grab, grabID); + } + }); + } +} + QByteArray AvatarData::identityByteArray(bool setIsReplicated) const { QByteArray identityData; QDataStream identityStream(&identityData, QIODevice::Append); diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 00e7e67923..dd6f0a9efd 100755 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -1134,18 +1134,16 @@ public: // identityChanged returns true if identity has changed, false otherwise. Similarly for displayNameChanged and skeletonModelUrlChange. void processAvatarIdentity(QDataStream& packetStream, bool& identityChanged, bool& displayNameChanged); - qint64 packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, - AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION); - qint64 packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID, - ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION); - - void prepareResetTraitInstances(); + QByteArray packTrait(AvatarTraits::TraitType traitType) const; + QByteArray packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID); void processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData); void processTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID, QByteArray traitBinaryData); void processDeletedTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID); + void prepareResetTraitInstances(); + QByteArray identityByteArray(bool setIsReplicated = false) const; QUrl getWireSafeSkeletonModelURL() const; @@ -1596,13 +1594,13 @@ protected: bool hasParent() const { return !getParentID().isNull(); } bool hasFaceTracker() const { return _headData ? _headData->_isFaceTrackerConnected : false; } - qint64 packAvatarEntityTraitInstance(AvatarTraits::TraitType traitType, - AvatarTraits::TraitInstanceID traitInstanceID, - ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion); - qint64 packGrabTraitInstance(AvatarTraits::TraitType traitType, - AvatarTraits::TraitInstanceID traitInstanceID, - ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion); + QByteArray packSkeletonModelURL() const; + QByteArray packAvatarEntityTraitInstance(AvatarTraits::TraitInstanceID traitInstanceID); + QByteArray packGrabTraitInstance(AvatarTraits::TraitInstanceID traitInstanceID); + void unpackSkeletonModelURL(const QByteArray& data); + + // isReplicated will be true on downstream Avatar Mixers and their clients, but false on the upstream "master" // Audio Mixer that the replicated avatar is connected to. bool _isReplicated{ false }; diff --git a/libraries/avatars/src/AvatarTraits.cpp b/libraries/avatars/src/AvatarTraits.cpp new file mode 100644 index 0000000000..724f30e2f3 --- /dev/null +++ b/libraries/avatars/src/AvatarTraits.cpp @@ -0,0 +1,135 @@ +// +// AvatarTraits.cpp +// libraries/avatars/src +// +// Created by Clement Brisset on 3/19/19. +// Copyright 2019 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "AvatarTraits.h" + +#include + +#include "AvatarData.h" + +namespace AvatarTraits { + + qint64 packTrait(TraitType traitType, ExtendedIODevice& destination, const AvatarData& avatar) { + // Call packer function + auto traitBinaryData = avatar.packTrait(traitType); + auto traitBinaryDataSize = traitBinaryData.size(); + + // Verify packed data + if (traitBinaryDataSize > MAXIMUM_TRAIT_SIZE) { + qWarning() << "Refusing to pack simple trait" << traitType << "of size" << traitBinaryDataSize + << "bytes since it exceeds the maximum size" << MAXIMUM_TRAIT_SIZE << "bytes"; + return 0; + } + + // Write packed data to stream + qint64 bytesWritten = 0; + bytesWritten += destination.writePrimitive((TraitType)traitType); + bytesWritten += destination.writePrimitive((TraitWireSize)traitBinaryDataSize); + bytesWritten += destination.write(traitBinaryData); + return bytesWritten; + } + + qint64 packVersionedTrait(TraitType traitType, ExtendedIODevice& destination, + TraitVersion traitVersion, const AvatarData& avatar) { + // Call packer function + auto traitBinaryData = avatar.packTrait(traitType); + auto traitBinaryDataSize = traitBinaryData.size(); + + // Verify packed data + if (traitBinaryDataSize > MAXIMUM_TRAIT_SIZE) { + qWarning() << "Refusing to pack simple trait" << traitType << "of size" << traitBinaryDataSize + << "bytes since it exceeds the maximum size" << MAXIMUM_TRAIT_SIZE << "bytes"; + return 0; + } + + // Write packed data to stream + qint64 bytesWritten = 0; + bytesWritten += destination.writePrimitive((TraitType)traitType); + bytesWritten += destination.writePrimitive((TraitVersion)traitVersion); + bytesWritten += destination.writePrimitive((TraitWireSize)traitBinaryDataSize); + bytesWritten += destination.write(traitBinaryData); + return bytesWritten; + } + + + qint64 packTraitInstance(TraitType traitType, TraitInstanceID traitInstanceID, + ExtendedIODevice& destination, AvatarData& avatar) { + // Call packer function + auto traitBinaryData = avatar.packTraitInstance(traitType, traitInstanceID); + auto traitBinaryDataSize = traitBinaryData.size(); + + + // Verify packed data + if (traitBinaryDataSize > AvatarTraits::MAXIMUM_TRAIT_SIZE) { + qWarning() << "Refusing to pack instanced trait" << traitType << "of size" << traitBinaryDataSize + << "bytes since it exceeds the maximum size " << AvatarTraits::MAXIMUM_TRAIT_SIZE << "bytes"; + return 0; + } + + // Write packed data to stream + qint64 bytesWritten = 0; + bytesWritten += destination.writePrimitive((TraitType)traitType); + bytesWritten += destination.write(traitInstanceID.toRfc4122()); + + if (!traitBinaryData.isNull()) { + bytesWritten += destination.writePrimitive((TraitWireSize)traitBinaryDataSize); + bytesWritten += destination.write(traitBinaryData); + } else { + bytesWritten += destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE); + } + + return bytesWritten; + } + + qint64 packVersionedTraitInstance(TraitType traitType, TraitInstanceID traitInstanceID, + ExtendedIODevice& destination, TraitVersion traitVersion, + AvatarData& avatar) { + // Call packer function + auto traitBinaryData = avatar.packTraitInstance(traitType, traitInstanceID); + auto traitBinaryDataSize = traitBinaryData.size(); + + + // Verify packed data + if (traitBinaryDataSize > AvatarTraits::MAXIMUM_TRAIT_SIZE) { + qWarning() << "Refusing to pack instanced trait" << traitType << "of size" << traitBinaryDataSize + << "bytes since it exceeds the maximum size " << AvatarTraits::MAXIMUM_TRAIT_SIZE << "bytes"; + return 0; + } + + // Write packed data to stream + qint64 bytesWritten = 0; + bytesWritten += destination.writePrimitive((TraitType)traitType); + bytesWritten += destination.writePrimitive((TraitVersion)traitVersion); + bytesWritten += destination.write(traitInstanceID.toRfc4122()); + + if (!traitBinaryData.isNull()) { + bytesWritten += destination.writePrimitive((TraitWireSize)traitBinaryDataSize); + bytesWritten += destination.write(traitBinaryData); + } else { + bytesWritten += destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE); + } + + return bytesWritten; + } + + + qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination, + TraitVersion traitVersion) { + qint64 bytesWritten = 0; + bytesWritten += destination.writePrimitive(traitType); + if (traitVersion > DEFAULT_TRAIT_VERSION) { + bytesWritten += destination.writePrimitive(traitVersion); + } + bytesWritten += destination.write(instanceID.toRfc4122()); + bytesWritten += destination.writePrimitive(DELETED_TRAIT_SIZE); + return bytesWritten; + } +}; diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index 5542acd37f..adff34f351 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -19,6 +19,9 @@ #include +class ExtendedIODevice; +class AvatarData; + namespace AvatarTraits { enum TraitType : int8_t { // Null trait @@ -36,6 +39,7 @@ namespace AvatarTraits { // Traits count TotalTraitTypes }; + const int NUM_SIMPLE_TRAITS = (int)FirstInstancedTrait; const int NUM_INSTANCED_TRAITS = (int)TotalTraitTypes - (int)FirstInstancedTrait; const int NUM_TRAITS = (int)TotalTraitTypes; @@ -58,22 +62,19 @@ namespace AvatarTraits { const TraitMessageSequence FIRST_TRAIT_SEQUENCE = 0; const TraitMessageSequence MAX_TRAIT_SEQUENCE = INT64_MAX; - inline qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination, - TraitVersion traitVersion = NULL_TRAIT_VERSION) { - qint64 bytesWritten = 0; + qint64 packTrait(TraitType traitType, ExtendedIODevice& destination, const AvatarData& avatar); + qint64 packVersionedTrait(TraitType traitType, ExtendedIODevice& destination, + TraitVersion traitVersion, const AvatarData& avatar); - bytesWritten += destination.writePrimitive(traitType); + qint64 packTraitInstance(TraitType traitType, TraitInstanceID traitInstanceID, + ExtendedIODevice& destination, AvatarData& avatar); + qint64 packVersionedTraitInstance(TraitType traitType, TraitInstanceID traitInstanceID, + ExtendedIODevice& destination, TraitVersion traitVersion, + AvatarData& avatar); - if (traitVersion > DEFAULT_TRAIT_VERSION) { - bytesWritten += destination.writePrimitive(traitVersion); - } + qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination, + TraitVersion traitVersion = NULL_TRAIT_VERSION); - bytesWritten += destination.write(instanceID.toRfc4122()); - - bytesWritten += destination.writePrimitive(DELETED_TRAIT_SIZE); - - return bytesWritten; - } }; #endif // hifi_AvatarTraits_h diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp index c9e3b67f16..a2d21fed54 100644 --- a/libraries/avatars/src/ClientTraitsHandler.cpp +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -106,7 +106,7 @@ int ClientTraitsHandler::sendChangedTraitsToMixer() { auto traitType = static_cast(std::distance(traitStatusesCopy.simpleCBegin(), simpleIt)); if (initialSend || *simpleIt == Updated) { - bytesWritten += _owningAvatar->packTrait(traitType, *traitsPacketList); + bytesWritten += AvatarTraits::packTrait(traitType, *traitsPacketList, *_owningAvatar); if (traitType == AvatarTraits::SkeletonModelURL) { @@ -125,7 +125,9 @@ int ClientTraitsHandler::sendChangedTraitsToMixer() { || instanceIDValuePair.value == Updated) { // this is a changed trait we need to send or we haven't send out trait information yet // ask the owning avatar to pack it - bytesWritten += _owningAvatar->packTraitInstance(instancedIt->traitType, instanceIDValuePair.id, *traitsPacketList); + bytesWritten += AvatarTraits::packTraitInstance(instancedIt->traitType, instanceIDValuePair.id, + *traitsPacketList, *_owningAvatar); + } else if (!initialSend && instanceIDValuePair.value == Deleted) { // pack delete for this trait instance bytesWritten += AvatarTraits::packInstancedTraitDelete(instancedIt->traitType, instanceIDValuePair.id,