From 5e65e3991cc5c7fc17e81137a40ff7a35cc0d280 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Tue, 20 Dec 2016 21:03:10 -0800 Subject: [PATCH] CR feedback --- assignment-client/src/Agent.cpp | 3 ++- assignment-client/src/avatars/AvatarMixer.cpp | 10 ++++++---- .../src/avatars/AvatarMixerClientData.cpp | 4 ---- .../src/avatars/AvatarMixerClientData.h | 1 - interface/src/avatar/MyAvatar.cpp | 6 +++--- interface/src/avatar/MyAvatar.h | 2 +- libraries/avatars/src/AvatarData.cpp | 12 ++++++++---- libraries/avatars/src/AvatarData.h | 9 ++++++++- 8 files changed, 28 insertions(+), 19 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index dc6b2427f2..e79085244f 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -499,7 +499,8 @@ void Agent::processAgentAvatar() { if (!_scriptEngine->isFinished() && _isAvatar) { auto scriptedAvatar = DependencyManager::get(); - QByteArray avatarByteArray = scriptedAvatar->toByteArray(true, randFloat() < AVATAR_SEND_FULL_UPDATE_RATIO); + QByteArray avatarByteArray = scriptedAvatar->toByteArray((randFloat() < AVATAR_SEND_FULL_UPDATE_RATIO) + ? AvatarData::SendAllData : AvatarData::CullSmallData); scriptedAvatar->doneEncoding(true); static AvatarDataSequenceNumber sequenceNumber = 0; diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 540526524e..d8d0b10fea 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -386,17 +386,19 @@ void AvatarMixer::broadcastAvatarData() { // determine if avatar is in view, to determine how much data to include... glm::vec3 otherNodeBoxScale = (otherNodeData->getPosition() - otherNodeData->getGlobalBoundingBoxCorner()) * 2.0f; AABox otherNodeBox(otherNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); - bool sendMinimumForOutOfView = !nodeData->otherAvatarInView(otherNodeBox); - if (sendMinimumForOutOfView) { + AvatarData::AvatarDataDetail detail; + if (!nodeData->otherAvatarInView(otherNodeBox)) { + detail = AvatarData::MinimumData; nodeData->incrementAvatarOutOfView(); } else { + detail = distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO + ? AvatarData::SendAllData : AvatarData::IncludeSmallData; nodeData->incrementAvatarInView(); } numAvatarDataBytes += avatarPacketList->write(otherNode->getUUID().toRfc4122()); - numAvatarDataBytes += - avatarPacketList->write(otherAvatar.toByteArray(false, distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO, sendMinimumForOutOfView)); + numAvatarDataBytes += avatarPacketList->write(otherAvatar.toByteArray(detail)); avatarPacketList->endSegment(); }); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 162ef2c462..e6e2fc3849 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -62,10 +62,6 @@ void AvatarMixerClientData::readViewFrustumPacket(const QByteArray& message) { _currentViewFrustum.fromByteArray(message); } -bool AvatarMixerClientData::otherAvatarInView(const glm::vec3& otherAvatar) { - return !_currentViewFrustumIsValid || _currentViewFrustum.pointIntersectsFrustum(otherAvatar); -} - bool AvatarMixerClientData::otherAvatarInView(const AABox& otherAvatarBox) { return !_currentViewFrustumIsValid || _currentViewFrustum.boxIntersectsKeyhole(otherAvatarBox); } diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 6b916750b4..b894932ba7 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -95,7 +95,6 @@ public: void readViewFrustumPacket(const QByteArray& message); bool otherAvatarInView(const AABox& otherAvatarBox); - bool otherAvatarInView(const glm::vec3& otherAvatar); void resetInViewStats() { _recentOtherAvatarsInView = _recentOtherAvatarsOutOfView = 0; } void incrementAvatarInView() { _recentOtherAvatarsInView++; } diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 9547d769fa..eebcee8e4c 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -226,7 +226,7 @@ void MyAvatar::simulateAttachments(float deltaTime) { // don't update attachments here, do it in harvestResultsFromPhysicsSimulation() } -QByteArray MyAvatar::toByteArray(bool cullSmallChanges, bool sendAll, bool sendMinimum) { +QByteArray MyAvatar::toByteArray(AvatarDataDetail dataDetail) { CameraMode mode = qApp->getCamera()->getMode(); _globalPosition = getPosition(); _globalBoundingBoxCorner.x = _characterController.getCapsuleRadius(); @@ -237,12 +237,12 @@ QByteArray MyAvatar::toByteArray(bool cullSmallChanges, bool sendAll, bool sendM // fake the avatar position that is sent up to the AvatarMixer glm::vec3 oldPosition = getPosition(); setPosition(getSkeletonPosition()); - QByteArray array = AvatarData::toByteArray(cullSmallChanges, sendAll, sendMinimum); + QByteArray array = AvatarData::toByteArray(dataDetail); // copy the correct position back setPosition(oldPosition); return array; } - return AvatarData::toByteArray(cullSmallChanges, sendAll, sendMinimum); + return AvatarData::toByteArray(dataDetail); } void MyAvatar::centerBody() { diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 14770fa14f..0e5ce0fe7b 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -333,7 +333,7 @@ private: glm::vec3 getWorldBodyPosition() const; glm::quat getWorldBodyOrientation() const; - QByteArray toByteArray(bool cullSmallChanges, bool sendAll, bool sendMinimum) override; + QByteArray toByteArray(AvatarDataDetail dataDetail) override; void simulate(float deltaTime); void updateFromTrackers(float deltaTime); virtual void render(RenderArgs* renderArgs, const glm::vec3& cameraPositio) override; diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 73df7a2f9e..d77f70fbe9 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -181,7 +181,11 @@ void AvatarData::setHandPosition(const glm::vec3& handPosition) { _handPosition = glm::inverse(getOrientation()) * (handPosition - getPosition()); } -QByteArray AvatarData::toByteArray(bool cullSmallChanges, bool sendAll, bool sendMinimum) { + +QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail) { + bool cullSmallChanges = (dataDetail == CullSmallData); + bool sendAll = (dataDetail == SendAllData); + bool sendMinimum = (dataDetail == MinimumData); // TODO: DRY this up to a shared method // that can pack any type given the number of bytes // and return the number of bytes to push the pointer @@ -1208,9 +1212,9 @@ void AvatarData::sendAvatarDataPacket() { // about 2% of the time, we send a full update (meaning, we transmit all the joint data), even if nothing has changed. // this is to guard against a joint moving once, the packet getting lost, and the joint never moving again. - bool sendFullUpdate = randFloat() < AVATAR_SEND_FULL_UPDATE_RATIO; - QByteArray avatarByteArray = toByteArray(true, sendFullUpdate); - doneEncoding(true); + QByteArray avatarByteArray = toByteArray((randFloat() < AVATAR_SEND_FULL_UPDATE_RATIO) ? SendAllData : CullSmallData); + + doneEncoding(true); // FIXME - doneEncoding() takes a bool for culling small changes, that's janky! static AvatarDataSequenceNumber sequenceNumber = 0; diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 1ad6d32810..db06d52092 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -209,7 +209,14 @@ public: glm::vec3 getHandPosition() const; void setHandPosition(const glm::vec3& handPosition); - virtual QByteArray toByteArray(bool cullSmallChanges, bool sendAll, bool sendMinimum = false); + typedef enum { + MinimumData, + CullSmallData, + IncludeSmallData, + SendAllData + } AvatarDataDetail; + + virtual QByteArray toByteArray(AvatarDataDetail dataDetail); virtual void doneEncoding(bool cullSmallChanges); /// \return true if an error should be logged