From c998ddbb9e7e56826ae59b68d88d50229e092944 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 17 Dec 2018 17:11:27 -0800 Subject: [PATCH] ScriptableAvatar::setAvatarEntityData() works --- .../src/avatars/ScriptableAvatar.cpp | 165 ++++++++++++++---- .../src/avatars/ScriptableAvatar.h | 22 ++- interface/src/avatar/MyAvatar.cpp | 48 ++--- .../src/avatars-renderer/Avatar.cpp | 27 +-- .../entities/src/EntityItemProperties.cpp | 2 + 5 files changed, 202 insertions(+), 62 deletions(-) diff --git a/assignment-client/src/avatars/ScriptableAvatar.cpp b/assignment-client/src/avatars/ScriptableAvatar.cpp index 8e88d6c236..c61e41fbbe 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.cpp +++ b/assignment-client/src/avatars/ScriptableAvatar.cpp @@ -252,26 +252,79 @@ void ScriptableAvatar::setHasAudioEnabledFaceMovement(bool hasAudioEnabledFaceMo _headData->setHasAudioEnabledFaceMovement(hasAudioEnabledFaceMovement); } -void ScriptableAvatar::updateAvatarEntity(const QUuid& id, const QByteArray& data) { - /* TODO: fix this - if (data.isNull()) { - // interpret this as a DELETE - std::map::iterator itr = _entities.find(id); - if (itr != _entities.end()) { - _entities.erase(itr); - clearAvatarEntity(id); +AvatarEntityMap ScriptableAvatar::getAvatarEntityData() const { + // DANGER: Now that we store the AvatarEntityData in packed format this call is potentially Very Expensive! + // Avoid calling this method if possible. + AvatarEntityMap data; + QUuid sessionID = getID(); + _avatarEntitiesLock.withReadLock([&] { + for (const auto& itr : _entities) { + QUuid id = itr.first; + EntityItemPointer entity = itr.second; + EntityItemProperties properties = entity->getProperties(); + QByteArray blob; + EntityItemProperties::propertiesToBlob(_scriptEngine, sessionID, properties, blob); + data[id] = blob; } - } else { - EntityItemPointer entity; - EntityItemProperties properties; - bool honorReadOnly = true; - properties.copyFromScriptValue(data, honorReadOnly); + }); + return data; +} - std::map::iterator itr = _entities.find(id); - if (itr == _entities.end()) { - // this is an ADD - entity = EntityTypes::constructEntityItem(id, properties); +void ScriptableAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { + // Note: this is an invokable Script call + // avatarEntityData is expected to be a map of QByteArrays that represent EntityItemProperties objects from JavaScript + // + if (avatarEntityData.size() > MAX_NUM_AVATAR_ENTITIES) { + // the data is suspect + qCDebug(avatars) << "discard suspect avatarEntityData with size =" << avatarEntityData.size(); + return; + } + + // convert binary data to EntityItemProperties + // NOTE: this operation is NOT efficient + std::map newProperties; + AvatarEntityMap::const_iterator dataItr = avatarEntityData.begin(); + while (dataItr != avatarEntityData.end()) { + EntityItemProperties properties; + const QByteArray& blob = dataItr.value(); + if (!blob.isNull() && EntityItemProperties::blobToProperties(_scriptEngine, blob, properties)) { + newProperties[dataItr.key()] = properties; + } + ++dataItr; + } + + // delete existing entities not found in avatarEntityData + std::vector idsToClear; + _avatarEntitiesLock.withWriteLock([&] { + std::map::iterator entityItr = _entities.begin(); + while (entityItr != _entities.end()) { + QUuid id = entityItr->first; + std::map::const_iterator propertiesItr = newProperties.find(id); + if (propertiesItr == newProperties.end()) { + idsToClear.push_back(id); + entityItr = _entities.erase(entityItr); + } else { + ++entityItr; + } + } + }); + + // add or update entities + _avatarEntitiesLock.withWriteLock([&] { + std::map::const_iterator propertiesItr = newProperties.begin(); + while (propertiesItr != newProperties.end()) { + QUuid id = propertiesItr->first; + const EntityItemProperties& properties = propertiesItr->second; + std::map::iterator entityItr = _entities.find(id); + EntityItemPointer entity; + if (entityItr != _entities.end()) { + entity = entityItr->second; + entity->setProperties(properties); + } else { + entity = EntityTypes::constructEntityItem(id, properties); + } if (entity) { + // build outgoing payload OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); EncodeBitstreamParams params; EntityTreeElementExtraEncodeDataPointer extra { nullptr }; @@ -281,24 +334,74 @@ void ScriptableAvatar::updateAvatarEntity(const QUuid& id, const QByteArray& dat _entities[id] = entity; QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); storeAvatarEntityDataPayload(id, tempArray); + } else { + // payload doesn't fit + entityItr = _entities.find(id); + if (entityItr != _entities.end()) { + _entities.erase(entityItr); + idsToClear.push_back(id); + } + } } - } else { - // this is an UPDATE - entity = itr->second; - bool somethingChanged = entity->setProperties(properties); - if (somethingChanged) { - OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); - EncodeBitstreamParams params; - EntityTreeElementExtraEncodeDataPointer extra { nullptr }; - OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + ++propertiesItr; + } + }); - if (appendState == OctreeElement::COMPLETED) { - QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); - storeAvatarEntityDataPayload(id, tempArray); - } + // clear deleted traits + for (const auto& id : idsToClear) { + clearAvatarEntity(id); + } +} + +void ScriptableAvatar::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) { + if (entityData.isNull()) { + // interpret this as a DELETE + std::map::iterator itr = _entities.find(entityID); + if (itr != _entities.end()) { + _entities.erase(itr); + clearAvatarEntity(entityID); + } + return; + } + + EntityItemPointer entity; + EntityItemProperties properties; + if (!EntityItemProperties::blobToProperties(_scriptEngine, entityData, properties)) { + // entityData is corrupt + return; + } + + std::map::iterator itr = _entities.find(entityID); + if (itr == _entities.end()) { + // this is an ADD + entity = EntityTypes::constructEntityItem(entityID, properties); + if (entity) { + OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); + EncodeBitstreamParams params; + EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + + if (appendState == OctreeElement::COMPLETED) { + _entities[entityID] = entity; + QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + storeAvatarEntityDataPayload(entityID, tempArray); + } + } + } else { + // this is an UPDATE + entity = itr->second; + bool somethingChanged = entity->setProperties(properties); + if (somethingChanged) { + OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); + EncodeBitstreamParams params; + EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + + if (appendState == OctreeElement::COMPLETED) { + QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + storeAvatarEntityDataPayload(entityID, tempArray); } } } - */ } diff --git a/assignment-client/src/avatars/ScriptableAvatar.h b/assignment-client/src/avatars/ScriptableAvatar.h index b0e41d28eb..df949f8bff 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.h +++ b/assignment-client/src/avatars/ScriptableAvatar.h @@ -185,7 +185,26 @@ public: bool getHasProceduralEyeFaceMovement() const override { return _headData->getHasProceduralEyeFaceMovement(); } void setHasAudioEnabledFaceMovement(bool hasAudioEnabledFaceMovement); bool getHasAudioEnabledFaceMovement() const override { return _headData->getHasAudioEnabledFaceMovement(); } - void updateAvatarEntity(const QUuid& id, const QByteArray& data) override; + + /**jsdoc + * Potentially Very Expensive. Do not use. + * @function Avatar.getAvatarEntityData + * @returns {object} + */ + Q_INVOKABLE AvatarEntityMap getAvatarEntityData() const override; + + /**jsdoc + * @function MyAvatar.setAvatarEntityData + * @param {object} avatarEntityData + */ + Q_INVOKABLE void setAvatarEntityData(const AvatarEntityMap& avatarEntityData) override; + + /**jsdoc + * @function MyAvatar.updateAvatarEntity + * @param {Uuid} entityID + * @param {string} entityData + */ + Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) override; public slots: void update(float deltatime); @@ -204,6 +223,7 @@ private: QHash _fstJointIndices; ///< 1-based, since zero is returned for missing keys QStringList _fstJointNames; ///< in order of depth-first traversal QUrl _skeletonFBXURL; + mutable QScriptEngine _scriptEngine; std::map _entities; /// Loads the joint indices, names from the FST file (if any) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 18494e2c42..67b933e18a 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -2353,8 +2353,11 @@ void MyAvatar::clearAvatarEntities() { auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - AvatarEntityMap avatarEntities = getAvatarEntityData(); - for (auto entityID : avatarEntities.keys()) { + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); + for (const auto& entityID : avatarEntityIDs) { entityTree->withWriteLock([&entityID, &entityTree] { // remove this entity first from the entity tree entityTree->deleteEntity(entityID, true, true); @@ -2369,10 +2372,12 @@ void MyAvatar::clearAvatarEntities() { void MyAvatar::removeWearableAvatarEntities() { auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - if (entityTree) { - AvatarEntityMap avatarEntities = getAvatarEntityData(); - for (auto entityID : avatarEntities.keys()) { + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); + for (const auto& entityID : avatarEntityIDs) { auto entity = entityTree->findEntityByID(entityID); if (entity && isWearableEntity(entity)) { entityTree->withWriteLock([&entityID, &entityTree] { @@ -2394,8 +2399,11 @@ QVariantList MyAvatar::getAvatarEntitiesVariant() { auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; if (entityTree) { - AvatarEntityMap avatarEntities = getAvatarEntityData(); - for (auto entityID : avatarEntities.keys()) { + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); + for (const auto& entityID : avatarEntityIDs) { auto entity = entityTree->findEntityByID(entityID); if (!entity) { continue; @@ -2795,17 +2803,17 @@ void MyAvatar::setAttachmentData(const QVector& attachmentData) } QVector MyAvatar::getAttachmentData() const { - QVector avatarData; - auto avatarEntities = getAvatarEntityData(); - AvatarEntityMap::const_iterator dataItr = avatarEntities.begin(); - while (dataItr != avatarEntities.end()) { - QUuid entityID = dataItr.key(); + QVector attachmentData; + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); + for (const auto& entityID : avatarEntityIDs) { auto properties = DependencyManager::get()->getEntityProperties(entityID); AttachmentData data = entityPropertiesToAttachmentData(properties); - avatarData.append(data); - dataItr++; + attachmentData.append(data); } - return avatarData; + return attachmentData; } QVariantList MyAvatar::getAttachmentsVariant() const { @@ -2834,16 +2842,16 @@ void MyAvatar::setAttachmentsVariant(const QVariantList& variant) { } bool MyAvatar::findAvatarEntity(const QString& modelURL, const QString& jointName, QUuid& entityID) { - auto avatarEntities = getAvatarEntityData(); - AvatarEntityMap::const_iterator dataItr = avatarEntities.begin(); - while (dataItr != avatarEntities.end()) { - entityID = dataItr.key(); + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); + for (const auto& entityID : avatarEntityIDs) { auto props = DependencyManager::get()->getEntityProperties(entityID); if (props.getModelURL() == modelURL && (jointName.isEmpty() || props.getParentJointIndex() == getJointIndex(jointName))) { return true; } - dataItr++; } return false; } diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index b41e1381ef..feb95f1dd0 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -340,10 +340,13 @@ void Avatar::updateAvatarEntities() { return; } + PackedAvatarEntityMap packedAvatarEntityData; + _avatarEntitiesLock.withReadLock([&] { + packedAvatarEntityData = _packedAvatarEntityData; + }); entityTree->withWriteLock([&] { - AvatarEntityMap avatarEntities = getAvatarEntityData(); - AvatarEntityMap::const_iterator dataItr = avatarEntities.begin(); - while (dataItr != avatarEntities.end()) { + AvatarEntityMap::const_iterator dataItr = packedAvatarEntityData.begin(); + while (dataItr != packedAvatarEntityData.end()) { // compute hash of data. TODO? cache this? QByteArray data = dataItr.value(); uint32_t newHash = qHash(data); @@ -381,10 +384,11 @@ void Avatar::updateAvatarEntities() { ++dataItr; EntityItemProperties properties; - { - int32_t bytesLeftToRead = data.size(); - unsigned char* dataAt = (unsigned char*)(data.data()); - properties.constructFromBuffer(dataAt, bytesLeftToRead); + int32_t bytesLeftToRead = data.size(); + unsigned char* dataAt = (unsigned char*)(data.data()); + if (!properties.constructFromBuffer(dataAt, bytesLeftToRead)) { + // properties are corrupt + continue; } properties.setEntityHostType(entity::HostType::AVATAR); @@ -454,7 +458,7 @@ void Avatar::updateAvatarEntities() { } } } - if (avatarEntities.size() != _avatarEntityForRecording.size()) { + if (packedAvatarEntityData.size() != _avatarEntityForRecording.size()) { createRecordingIDs(); } }); @@ -466,9 +470,12 @@ void Avatar::removeAvatarEntitiesFromTree() { auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; if (entityTree) { + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); entityTree->withWriteLock([&] { - AvatarEntityMap avatarEntities = getAvatarEntityData(); - for (auto entityID : avatarEntities.keys()) { + for (const auto& entityID : avatarEntityIDs) { entityTree->deleteEntity(entityID, true, true); } }); diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index d466de0507..3b1e9a3e2b 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -4613,6 +4613,7 @@ void EntityItemProperties::convertToCloneProperties(const EntityItemID& entityID } bool EntityItemProperties::blobToProperties(QScriptEngine& scriptEngine, const QByteArray& blob, EntityItemProperties& properties) { + // DANGER: this method is NOT efficient. // begin recipe for converting unfortunately-formatted-binary-blob to EntityItemProperties QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(blob); if (!jsonProperties.isObject()) { @@ -4628,6 +4629,7 @@ bool EntityItemProperties::blobToProperties(QScriptEngine& scriptEngine, const Q } void EntityItemProperties::propertiesToBlob(QScriptEngine& scriptEngine, const QUuid& myAvatarID, const EntityItemProperties& properties, QByteArray& blob) { + // DANGER: this method is NOT efficient. // begin recipe for extracting unfortunately-formatted-binary-blob from EntityItem QScriptValue scriptValue = EntityItemNonDefaultPropertiesToScriptValue(&scriptEngine, properties); QVariant variantProperties = scriptValue.toVariant();