Merge pull request #7508 from howard-stearns/import-export-parents

Make export/import and copy/paste be parent/child-clean.

assignment-client/src/entities/AssignmentParentFinder.cpp

@@ -11,7 +11,7 @@
 
 #include "AssignmentParentFinder.h"
 
-SpatiallyNestableWeakPointer AssignmentParentFinder::find(QUuid parentID, bool& success) const {
+SpatiallyNestableWeakPointer AssignmentParentFinder::find(QUuid parentID, bool& success, SpatialParentTree* entityTree) const {
     SpatiallyNestableWeakPointer parent;
 
     if (parentID.isNull()) {
@@ -20,7 +20,11 @@ SpatiallyNestableWeakPointer AssignmentParentFinder::find(QUuid parentID, bool&
     }
 
     // search entities
-    parent = _tree->findEntityByEntityItemID(parentID);
+    if (entityTree) {
+        parent = entityTree->findByID(parentID);
+    } else {
+        parent = _tree->findEntityByEntityItemID(parentID);
+    }
     if (parent.expired()) {
         success = false;
     } else {

assignment-client/src/entities/AssignmentParentFinder.h

@@ -25,7 +25,7 @@ class AssignmentParentFinder : public SpatialParentFinder {
 public:
     AssignmentParentFinder(EntityTreePointer tree) : _tree(tree) { }
     virtual ~AssignmentParentFinder() { }
-    virtual SpatiallyNestableWeakPointer find(QUuid parentID, bool& success) const;
+    virtual SpatiallyNestableWeakPointer find(QUuid parentID, bool& success, SpatialParentTree* entityTree = nullptr) const;
 
 protected:
     EntityTreePointer _tree;

interface/src/Application.cpp

@@ -2800,43 +2800,50 @@ void Application::calibrateEyeTracker5Points() {
 }
 #endif
 
-bool Application::exportEntities(const QString& filename, const QVector<EntityItemID>& entityIDs) {
+bool Application::exportEntities(const QString& filename, const QVector<EntityItemID>& entityIDs, const glm::vec3* givenOffset) {
-    QVector<EntityItemPointer> entities;
+    QHash<EntityItemID, EntityItemPointer> entities;
 
     auto entityTree = getEntities()->getTree();
     auto exportTree = std::make_shared<EntityTree>();
     exportTree->createRootElement();
 
     glm::vec3 root(TREE_SCALE, TREE_SCALE, TREE_SCALE);
-    for (auto entityID : entityIDs) {
+    for (auto entityID : entityIDs) { // Gather entities and properties.
         auto entityItem = entityTree->findEntityByEntityItemID(entityID);
         if (!entityItem) {
+            qCWarning(interfaceapp) << "Skipping export of" << entityID << "that is not in scene.";
             continue;
         }
 
-        auto properties = entityItem->getProperties();
+        if (!givenOffset) {
-        auto position = properties.getPosition();
+            EntityItemID parentID = entityItem->getParentID();
-
+            if (parentID.isInvalidID() || !entityIDs.contains(parentID) || !entityTree->findEntityByEntityItemID(parentID)) {
-        root.x = glm::min(root.x, position.x);
+                auto position = entityItem->getPosition(); // If parent wasn't selected, we want absolute position, which isn't in properties.
-        root.y = glm::min(root.y, position.y);
+                root.x = glm::min(root.x, position.x);
-        root.z = glm::min(root.z, position.z);
+                root.y = glm::min(root.y, position.y);
-
+                root.z = glm::min(root.z, position.z);
-        entities << entityItem;
+            }
+        }
+        entities[entityID] = entityItem;
     }
 
     if (entities.size() == 0) {
         return false;
     }
 
-    for (auto entityItem : entities) {
+    if (givenOffset) {
-        auto properties = entityItem->getProperties();
+        root = *givenOffset;
-
+    }
-        properties.setPosition(properties.getPosition() - root);
+    for (EntityItemPointer& entityDatum : entities) {
-        exportTree->addEntity(entityItem->getEntityItemID(), properties);
+        auto properties = entityDatum->getProperties();
+        EntityItemID parentID = properties.getParentID();
+        if (parentID.isInvalidID()) {
+            properties.setPosition(properties.getPosition() - root);
+        } else if (!entities.contains(parentID)) {
+            entityDatum->globalizeProperties(properties, "Parent %3 of %2 %1 is not selected for export.", -root);
+        } // else valid parent -- don't offset
+        exportTree->addEntity(entityDatum->getEntityItemID(), properties);
     }
-
-    // remap IDs on export so that we aren't publishing the IDs of entities in our domain
-    exportTree->remapIDs();
 
     exportTree->writeToJSONFile(filename.toLocal8Bit().constData());
 
@@ -2846,33 +2853,14 @@ bool Application::exportEntities(const QString& filename, const QVector<EntityIt
 }
 
 bool Application::exportEntities(const QString& filename, float x, float y, float z, float scale) {
+    glm::vec3 offset(x, y, z);
     QVector<EntityItemPointer> entities;
-    getEntities()->getTree()->findEntities(AACube(glm::vec3(x, y, z), scale), entities);
+    QVector<EntityItemID> ids;
-
+    getEntities()->getTree()->findEntities(AACube(offset, scale), entities);
-    if (entities.size() > 0) {
+    foreach(EntityItemPointer entity, entities) {
-        glm::vec3 root(x, y, z);
+        ids << entity->getEntityItemID();
-        auto exportTree = std::make_shared<EntityTree>();
-        exportTree->createRootElement();
-
-        for (int i = 0; i < entities.size(); i++) {
-            EntityItemProperties properties = entities.at(i)->getProperties();
-            EntityItemID id = entities.at(i)->getEntityItemID();
-            properties.setPosition(properties.getPosition() - root);
-            exportTree->addEntity(id, properties);
-        }
-
-        // remap IDs on export so that we aren't publishing the IDs of entities in our domain
-        exportTree->remapIDs();
-
-        exportTree->writeToSVOFile(filename.toLocal8Bit().constData());
-    } else {
-        qCDebug(interfaceapp) << "No models were selected";
-        return false;
     }
-
+    return exportEntities(filename, ids, &offset);
-    // restore the main window's active state
-    _window->activateWindow();
-    return true;
 }
 
 void Application::loadSettings() {
@@ -2905,7 +2893,6 @@ bool Application::importEntities(const QString& urlOrFilename) {
 
     bool success = _entityClipboard->readFromURL(urlOrFilename);
     if (success) {
-        _entityClipboard->remapIDs();
         _entityClipboard->reaverageOctreeElements();
     }
     return success;

interface/src/Application.h

@@ -233,7 +233,7 @@ signals:
 
 public slots:
     QVector<EntityItemID> pasteEntities(float x, float y, float z);
-    bool exportEntities(const QString& filename, const QVector<EntityItemID>& entityIDs);
+    bool exportEntities(const QString& filename, const QVector<EntityItemID>& entityIDs, const glm::vec3* givenOffset = nullptr);
     bool exportEntities(const QString& filename, float x, float y, float z, float scale);
     bool importEntities(const QString& url);
 

interface/src/InterfaceParentFinder.cpp

@@ -16,7 +16,7 @@
 
 #include "InterfaceParentFinder.h"
 
-SpatiallyNestableWeakPointer InterfaceParentFinder::find(QUuid parentID, bool& success) const {
+SpatiallyNestableWeakPointer InterfaceParentFinder::find(QUuid parentID, bool& success, SpatialParentTree* entityTree) const {
     SpatiallyNestableWeakPointer parent;
 
     if (parentID.isNull()) {
@@ -25,9 +25,13 @@ SpatiallyNestableWeakPointer InterfaceParentFinder::find(QUuid parentID, bool& s
     }
 
     // search entities
-    EntityTreeRenderer* treeRenderer = qApp->getEntities();
+    if (entityTree) {
-    EntityTreePointer tree = treeRenderer ? treeRenderer->getTree() : nullptr;
+        parent = entityTree->findByID(parentID);
-    parent = tree ? tree->findEntityByEntityItemID(parentID) : nullptr;
+    } else {
+        EntityTreeRenderer* treeRenderer = qApp->getEntities();
+        EntityTreePointer tree = treeRenderer ? treeRenderer->getTree() : nullptr;
+        parent = tree ? tree->findEntityByEntityItemID(parentID) : nullptr;
+    }
     if (!parent.expired()) {
         success = true;
         return parent;

interface/src/InterfaceParentFinder.h

@@ -21,7 +21,7 @@ class InterfaceParentFinder : public SpatialParentFinder {
 public:
     InterfaceParentFinder() { }
     virtual ~InterfaceParentFinder() { }
-    virtual SpatiallyNestableWeakPointer find(QUuid parentID, bool& success) const;
+    virtual SpatiallyNestableWeakPointer find(QUuid parentID, bool& success, SpatialParentTree* entityTree = nullptr) const;
 };
 
 #endif // hifi_InterfaceParentFinder_h

libraries/entities/src/EntityItem.cpp

@@ -1010,6 +1010,10 @@ EntityTreePointer EntityItem::getTree() const {
     return tree;
 }
 
+SpatialParentTree* EntityItem::getParentTree() const {
+    return getTree().get();
+}
+
 bool EntityItem::wantTerseEditLogging() const {
     EntityTreePointer tree = getTree();
     return tree ? tree->wantTerseEditLogging() : false;
@@ -1978,3 +1982,25 @@ void EntityItem::dimensionsChanged() {
     requiresRecalcBoxes();
     SpatiallyNestable::dimensionsChanged(); // Do what you have to do
 }
+
+void EntityItem::globalizeProperties(EntityItemProperties& properties, const QString& messageTemplate, const glm::vec3& offset) const {
+    bool success;
+    auto globalPosition = getPosition(success);
+    if (success) {
+        properties.setPosition(globalPosition + offset);
+        properties.setRotation(getRotation());
+        properties.setDimensions(getDimensions());
+        // Should we do velocities and accelerations, too? This could end up being quite involved, which is why the method exists.
+    } else {
+        properties.setPosition(getQueryAACube().calcCenter() + offset); // best we can do
+    }
+    if (!messageTemplate.isEmpty()) {
+        QString name = properties.getName();
+        if (name.isEmpty()) {
+            name = EntityTypes::getEntityTypeName(properties.getType());
+        }
+        qCWarning(entities) << messageTemplate.arg(getEntityItemID().toString()).arg(name).arg(properties.getParentID().toString());
+    }
+    QUuid empty;
+    properties.setParentID(empty);
+}

libraries/entities/src/EntityItem.h

@@ -86,6 +86,8 @@ public:
 
     /// returns true if something changed
     virtual bool setProperties(const EntityItemProperties& properties);
+    // Update properties with empty parent id and globalized/absolute values (applying offset), and apply (non-empty) log template to args id, name-or-type, parent id.
+    void globalizeProperties(EntityItemProperties& properties, const QString& messageTemplate = QString(), const glm::vec3& offset = glm::vec3(0.0f)) const;
 
     /// Override this in your derived class if you'd like to be informed when something about the state of the entity
     /// has changed. This will be called with properties change or when new data is loaded from a stream
@@ -359,6 +361,7 @@ public:
     void setPhysicsInfo(void* data) { _physicsInfo = data; }
     EntityTreeElementPointer getElement() const { return _element; }
     EntityTreePointer getTree() const;
+    virtual SpatialParentTree* getParentTree() const;
     bool wantTerseEditLogging() const;
 
     glm::mat4 getEntityToWorldMatrix() const;

libraries/entities/src/EntityTree.cpp

@@ -24,7 +24,6 @@
 #include "EntitiesLogging.h"
 #include "RecurseOctreeToMapOperator.h"
 #include "LogHandler.h"
-#include "RemapIDOperator.h"
 
 static const quint64 DELETED_ENTITIES_EXTRA_USECS_TO_CONSIDER = USECS_PER_MSEC * 50;
 
@@ -1317,42 +1316,59 @@ QVector<EntityItemID> EntityTree::sendEntities(EntityEditPacketSender* packetSen
                                                float x, float y, float z) {
     SendEntitiesOperationArgs args;
     args.packetSender = packetSender;
-    args.localTree = localTree;
+    args.ourTree = this;
+    args.otherTree = localTree;
     args.root = glm::vec3(x, y, z);
-    QVector<EntityItemID> newEntityIDs;
+    // If this is called repeatedly (e.g., multiple pastes with the same data), the new elements will clash unless we use new identifiers.
-    args.newEntityIDs = &newEntityIDs;
+    // We need to keep a map so that we can map parent identifiers correctly.
+    QHash<EntityItemID, EntityItemID> map;
+    args.map = &map;
     recurseTreeWithOperation(sendEntitiesOperation, &args);
     packetSender->releaseQueuedMessages();
 
-    return newEntityIDs;
+    return map.values().toVector();
 }
 
 bool EntityTree::sendEntitiesOperation(OctreeElementPointer element, void* extraData) {
     SendEntitiesOperationArgs* args = static_cast<SendEntitiesOperationArgs*>(extraData);
     EntityTreeElementPointer entityTreeElement = std::static_pointer_cast<EntityTreeElement>(element);
-    entityTreeElement->forEachEntity([&](EntityItemPointer entityItem) {
+    std::function<const EntityItemID(EntityItemPointer&)> getMapped = [&](EntityItemPointer& item) -> const EntityItemID {
-        EntityItemID newID(QUuid::createUuid());
+        EntityItemID oldID = item->getEntityItemID();
-        args->newEntityIDs->append(newID);
+        if (args->map->contains(oldID)) { // Already been handled (e.g., as a parent of somebody that we've processed).
-        EntityItemProperties properties = entityItem->getProperties();
+            return args->map->value(oldID);
-        properties.setPosition(properties.getPosition() + args->root);
+        }
+        EntityItemID newID = QUuid::createUuid();
+        EntityItemProperties properties = item->getProperties();
+        EntityItemID oldParentID = properties.getParentID();
+        if (oldParentID.isInvalidID()) {  // no parent
+            properties.setPosition(properties.getPosition() + args->root);
+        } else {
+            EntityItemPointer parentEntity = args->ourTree->findEntityByEntityItemID(oldParentID);
+            if (parentEntity) { // map the parent
+                // Warning: (non-tail) recursion of getMapped could blow the call stack if the parent hierarchy is VERY deep.
+                properties.setParentID(getMapped(parentEntity));
+                // But do not add root offset in this case.
+            } else { // Should not happen, but let's try to be helpful...
+                item->globalizeProperties(properties, "Cannot find %3 parent of %2 %1", args->root);
+            }
+        }
         properties.markAllChanged(); // so the entire property set is considered new, since we're making a new entity
 
         // queue the packet to send to the server
         args->packetSender->queueEditEntityMessage(PacketType::EntityAdd, newID, properties);
 
         // also update the local tree instantly (note: this is not our tree, but an alternate tree)
-        if (args->localTree) {
+        if (args->otherTree) {
-            args->localTree->withWriteLock([&] {
+            args->otherTree->withWriteLock([&] {
-                args->localTree->addEntity(newID, properties);
+                args->otherTree->addEntity(newID, properties);
             });
         }
-    });
+        args->map->insert(oldID, newID);
-    return true;
+        return newID;
-}
+    };
 
-void EntityTree::remapIDs() {
+    entityTreeElement->forEachEntity(getMapped);
-    RemapIDOperator theOperator;
+    return true;
-    recurseTreeWithOperator(&theOperator);
 }
 
 bool EntityTree::writeToMap(QVariantMap& entityDescription, OctreeElementPointer element, bool skipDefaultValues,
@@ -1393,7 +1409,6 @@ bool EntityTree::readFromMap(QVariantMap& map) {
             qCDebug(entities) << "adding Entity failed:" << entityItemID << properties.getType();
         }
     }
-
     return true;
 }
 

libraries/entities/src/EntityTree.h

@@ -16,6 +16,7 @@
 #include <QVector>
 
 #include <Octree.h>
+#include <SpatialParentFinder.h>
 
 class EntityTree;
 typedef std::shared_ptr<EntityTree> EntityTreePointer;
@@ -46,13 +47,14 @@ public:
 class SendEntitiesOperationArgs {
 public:
     glm::vec3 root;
-    EntityTreePointer localTree;
+    EntityTree* ourTree;
+    EntityTreePointer otherTree;
     EntityEditPacketSender* packetSender;
-    QVector<EntityItemID>* newEntityIDs;
+    QHash<EntityItemID, EntityItemID>* map;
 };
 
 
-class EntityTree : public Octree {
+class EntityTree : public Octree, public SpatialParentTree {
     Q_OBJECT
 public:
     EntityTree(bool shouldReaverage = false);
@@ -125,6 +127,7 @@ public:
     EntityItemPointer findClosestEntity(glm::vec3 position, float targetRadius);
     EntityItemPointer findEntityByID(const QUuid& id);
     EntityItemPointer findEntityByEntityItemID(const EntityItemID& entityID);
+    virtual SpatiallyNestablePointer findByID(const QUuid& id) { return findEntityByID(id); }
 
     EntityItemID assignEntityID(const EntityItemID& entityItemID); /// Assigns a known ID for a creator token ID
 
@@ -200,8 +203,6 @@ public:
     bool wantTerseEditLogging() const { return _wantTerseEditLogging; }
     void setWantTerseEditLogging(bool value) { _wantTerseEditLogging = value; }
 
-    void remapIDs();
-
     virtual bool writeToMap(QVariantMap& entityDescription, OctreeElementPointer element, bool skipDefaultValues,
                             bool skipThoseWithBadParents) override;
     virtual bool readFromMap(QVariantMap& entityDescription) override;

libraries/entities/src/RemapIDOperator.cpp

@@ -1,33 +0,0 @@
-//
-//  RemapIDOperator.cpp
-//  libraries/entities/src
-//
-//  Created by Seth Alves on 2015-12-6.
-//  Copyright 2015 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 "EntityTree.h"
-#include "RemapIDOperator.h"
-
-QUuid RemapIDOperator::remap(const QUuid& oldID) {
-    if (oldID.isNull()) {
-        return oldID;
-    }
-    if (!_oldToNew.contains(oldID)) {
-        _oldToNew[oldID] = QUuid::createUuid();
-    }
-    return _oldToNew[oldID];
-}
-
-bool RemapIDOperator::postRecursion(OctreeElementPointer element) {
-    EntityTreeElementPointer entityTreeElement = std::static_pointer_cast<EntityTreeElement>(element);
-    entityTreeElement->forEachEntity([&](EntityItemPointer entityItem) {
-        entityItem->setID(remap(entityItem->getID()));
-        entityItem->setParentID(remap(entityItem->getParentID()));
-    });
-    return true;
-}

libraries/entities/src/RemapIDOperator.h

@@ -1,30 +0,0 @@
-//
-//  RemapIDOperator.h
-//  libraries/entities/src
-//
-//  Created by Seth Alves on 2015-12-6.
-//  Copyright 2015 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
-//
-
-#ifndef hifi_RemapIDOperator_h
-#define hifi_RemapIDOperator_h
-
-#include "Octree.h"
-
-// this will change all the IDs in an EntityTree.  Parent/Child relationships are maintained.
-
-class RemapIDOperator : public RecurseOctreeOperator {
-public:
-    RemapIDOperator() : RecurseOctreeOperator() {}
-    ~RemapIDOperator() {}
-    virtual bool preRecursion(OctreeElementPointer element) { return true; }
-    virtual bool postRecursion(OctreeElementPointer element);
-private:
-    QUuid remap(const QUuid& oldID);
-    QHash<QUuid, QUuid> _oldToNew;
-};
-
-#endif // hifi_RemapIDOperator_h

libraries/shared/src/SpatialParentFinder.h

@@ -19,6 +19,10 @@
 class SpatiallyNestable;
 using SpatiallyNestableWeakPointer = std::weak_ptr<SpatiallyNestable>;
 using SpatiallyNestablePointer = std::shared_ptr<SpatiallyNestable>;
+class SpatialParentTree {
+public:
+    virtual SpatiallyNestablePointer findByID(const QUuid& id) { return nullptr; }
+};
 class SpatialParentFinder : public Dependency {
 
 
@@ -31,7 +35,7 @@ public:
     SpatialParentFinder() { }
     virtual ~SpatialParentFinder() { }
 
-    virtual SpatiallyNestableWeakPointer find(QUuid parentID, bool& success) const = 0;
+    virtual SpatiallyNestableWeakPointer find(QUuid parentID, bool& success, SpatialParentTree* entityTree = nullptr) const = 0;
 };
 
 #endif // hifi_SpatialParentFinder_h

libraries/shared/src/SpatiallyNestable.cpp

@@ -105,7 +105,7 @@ SpatiallyNestablePointer SpatiallyNestable::getParentPointer(bool& success) cons
         success = false;
         return nullptr;
     }
-    _parent = parentFinder->find(parentID, success);
+    _parent = parentFinder->find(parentID, success, getParentTree());
     if (!success) {
         return nullptr;
     }

libraries/shared/src/SpatiallyNestable.h

@@ -140,6 +140,7 @@ public:
     bool isDead() const { return _isDead; }
 
     bool isParentIDValid() const { bool success = false; getParentPointer(success); return success; }
+    virtual SpatialParentTree* getParentTree() const { return nullptr; }
 
 protected:
     const NestableType _nestableType; // EntityItem or an AvatarData