From b9005a9e7653cba52284718e9931a1e1166137d5 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 16 May 2017 14:32:55 -0700 Subject: [PATCH 01/17] fix up queryAACubes before sending imported entities to server --- libraries/entities/src/EntityTree.cpp | 20 +++++++++++--------- libraries/entities/src/EntityTree.h | 1 - 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 457b7c21f6..35f3b44fd4 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1574,7 +1574,6 @@ QByteArray EntityTree::remapActionDataIDs(QByteArray actionData, QHash EntityTree::sendEntities(EntityEditPacketSender* packetSender, EntityTreePointer localTree, float x, float y, float z) { SendEntitiesOperationArgs args; - args.packetSender = packetSender; args.ourTree = this; args.otherTree = localTree; args.root = glm::vec3(x, y, z); @@ -1585,21 +1584,29 @@ QVector EntityTree::sendEntities(EntityEditPacketSender* packetSen withReadLock([&] { recurseTreeWithOperation(sendEntitiesOperation, &args); }); - packetSender->releaseQueuedMessages(); // the values from map are used as the list of successfully "sent" entities. If some didn't actually make it, // pull them out. Bogus entries could happen if part of the imported data makes some reference to an entity - // that isn't in the data being imported. + // that isn't in the data being imported. For those that made it, fix up their queryAACubes and send an + // add-entity packet to the server. QHash::iterator i = map.begin(); while (i != map.end()) { EntityItemID newID = i.value(); - if (localTree->findEntityByEntityItemID(newID)) { + EntityItemPointer entity = localTree->findEntityByEntityItemID(newID); + if (entity) { + entity->checkAndAdjustQueryAACube(); + // queue the packet to send to the server + EntityItemProperties properties = entity->getProperties(); + properties.markAllChanged(); // so the entire property set is considered new, since we're making a new entity + packetSender->queueEditEntityMessage(PacketType::EntityAdd, localTree, newID, properties); i++; } else { i = map.erase(i); } } + packetSender->releaseQueuedMessages(); + return map.values().toVector(); } @@ -1652,14 +1659,9 @@ bool EntityTree::sendEntitiesOperation(OctreeElementPointer element, void* extra // set creation time to "now" for imported entities properties.setCreated(usecTimestampNow()); - properties.markAllChanged(); // so the entire property set is considered new, since we're making a new entity - EntityTreeElementPointer entityTreeElement = std::static_pointer_cast(element); EntityTreePointer tree = entityTreeElement->getTree(); - // queue the packet to send to the server - args->packetSender->queueEditEntityMessage(PacketType::EntityAdd, tree, newID, properties); - // also update the local tree instantly (note: this is not our tree, but an alternate tree) if (args->otherTree) { args->otherTree->withWriteLock([&] { diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index d7e069b005..6137d14aac 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -53,7 +53,6 @@ public: glm::vec3 root; EntityTree* ourTree; EntityTreePointer otherTree; - EntityEditPacketSender* packetSender; QHash* map; }; From d90843428df2579be44fd9ac305727da39eb8e92 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 16 May 2017 16:54:16 -0700 Subject: [PATCH 02/17] when importing entities, make a queryAACube that takes children into account --- libraries/entities/src/EntityTree.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 35f3b44fd4..2ece07e86d 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1594,7 +1594,7 @@ QVector EntityTree::sendEntities(EntityEditPacketSender* packetSen EntityItemID newID = i.value(); EntityItemPointer entity = localTree->findEntityByEntityItemID(newID); if (entity) { - entity->checkAndAdjustQueryAACube(); + entity->computePuffedQueryAACube(); // queue the packet to send to the server EntityItemProperties properties = entity->getProperties(); properties.markAllChanged(); // so the entire property set is considered new, since we're making a new entity From ec02887ae615fd8bd75444e457b3abc2afcf506e Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 18 May 2017 13:20:44 -0700 Subject: [PATCH 03/17] attempt to get imported entities into the correct octree-element --- libraries/entities/src/EntityItem.cpp | 9 ++++--- libraries/entities/src/EntityTree.cpp | 30 ++++++++++++++++++---- libraries/shared/src/SpatiallyNestable.cpp | 3 ++- libraries/shared/src/SpatiallyNestable.h | 3 ++- 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 14122594fe..c9a26a9f4d 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1369,9 +1369,12 @@ bool EntityItem::setProperties(const EntityItemProperties& properties) { SET_ENTITY_PROPERTY_FROM_PROPERTIES(lastEditedBy, setLastEditedBy); AACube saveQueryAACube = _queryAACube; - checkAndAdjustQueryAACube(); - if (saveQueryAACube != _queryAACube) { - somethingChanged = true; + if (checkAndAdjustQueryAACube()) { + if (saveQueryAACube != _queryAACube) { + somethingChanged = true; + } + } else { + markAncestorMissing(true); } // Now check the sub classes diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index c6a42c2f3a..7d353f69a1 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1584,25 +1584,46 @@ QVector EntityTree::sendEntities(EntityEditPacketSender* packetSen args.ourTree = this; args.otherTree = localTree; args.root = glm::vec3(x, y, z); - // If this is called repeatedly (e.g., multiple pastes with the same data), the new elements will clash unless we use new identifiers. - // We need to keep a map so that we can map parent identifiers correctly. + // If this is called repeatedly (e.g., multiple pastes with the same data), the new elements will clash unless we + // use new identifiers. We need to keep a map so that we can map parent identifiers correctly. QHash map; args.map = ↦ withReadLock([&] { recurseTreeWithOperation(sendEntitiesOperation, &args); }); - // the values from map are used as the list of successfully "sent" entities. If some didn't actually make it, + // The values from map are used as the list of successfully "sent" entities. If some didn't actually make it, // pull them out. Bogus entries could happen if part of the imported data makes some reference to an entity // that isn't in the data being imported. For those that made it, fix up their queryAACubes and send an // add-entity packet to the server. + + // fix the queryAACubes of any children that were read in before their parents, get them into the correct element + MovingEntitiesOperator moveOperator(localTree); QHash::iterator i = map.begin(); while (i != map.end()) { EntityItemID newID = i.value(); EntityItemPointer entity = localTree->findEntityByEntityItemID(newID); if (entity) { - entity->computePuffedQueryAACube(); + entity->forceQueryAACubeUpdate(); + moveOperator.addEntityToMoveList(entity, entity->getQueryAACube()); + i++; + } else { + i = map.erase(i); + } + } + if (moveOperator.hasMovingEntities()) { + PerformanceTimer perfTimer("recurseTreeWithOperator"); + localTree->recurseTreeWithOperator(&moveOperator); + } + + // send add-entity packets to the server + i = map.begin(); + while (i != map.end()) { + EntityItemID newID = i.value(); + EntityItemPointer entity = localTree->findEntityByEntityItemID(newID); + if (entity) { // queue the packet to send to the server + entity->computePuffedQueryAACube(); EntityItemProperties properties = entity->getProperties(); properties.markAllChanged(); // so the entire property set is considered new, since we're making a new entity packetSender->queueEditEntityMessage(PacketType::EntityAdd, localTree, newID, properties); @@ -1611,7 +1632,6 @@ QVector EntityTree::sendEntities(EntityEditPacketSender* packetSen i = map.erase(i); } } - packetSender->releaseQueuedMessages(); return map.values().toVector(); diff --git a/libraries/shared/src/SpatiallyNestable.cpp b/libraries/shared/src/SpatiallyNestable.cpp index 9c7e216cb6..4fe9cda825 100644 --- a/libraries/shared/src/SpatiallyNestable.cpp +++ b/libraries/shared/src/SpatiallyNestable.cpp @@ -946,12 +946,13 @@ AACube SpatiallyNestable::getMaximumAACube(bool& success) const { return AACube(getPosition(success) - glm::vec3(defaultAACubeSize / 2.0f), defaultAACubeSize); } -void SpatiallyNestable::checkAndAdjustQueryAACube() { +bool SpatiallyNestable::checkAndAdjustQueryAACube() { bool success; AACube maxAACube = getMaximumAACube(success); if (success && (!_queryAACubeSet || !_queryAACube.contains(maxAACube))) { setQueryAACube(maxAACube); } + return success; } void SpatiallyNestable::setQueryAACube(const AACube& queryAACube) { diff --git a/libraries/shared/src/SpatiallyNestable.h b/libraries/shared/src/SpatiallyNestable.h index e8961dba98..6589a44307 100644 --- a/libraries/shared/src/SpatiallyNestable.h +++ b/libraries/shared/src/SpatiallyNestable.h @@ -102,11 +102,12 @@ public: virtual glm::vec3 getParentAngularVelocity(bool& success) const; virtual AACube getMaximumAACube(bool& success) const; - virtual void checkAndAdjustQueryAACube(); + virtual bool checkAndAdjustQueryAACube(); virtual bool computePuffedQueryAACube(); virtual void setQueryAACube(const AACube& queryAACube); virtual bool queryAABoxNeedsUpdate() const; + void forceQueryAACubeUpdate() { _queryAACubeSet = false; } virtual AACube getQueryAACube(bool& success) const; virtual AACube getQueryAACube() const; From ef556fae9b8292309bf88e5f60afe6e851d97af3 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 18 May 2017 14:13:33 -0700 Subject: [PATCH 04/17] when an entity's parent wasn't known and then becomes known, patch up the rigid-body and the render-item bounds --- libraries/entities/src/EntityItem.cpp | 2 +- libraries/entities/src/EntityItem.h | 3 ++- libraries/entities/src/EntityTree.cpp | 3 +++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index c9a26a9f4d..43ba881432 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1624,7 +1624,7 @@ void EntityItem::updateParentID(const QUuid& value) { setParentID(value); // children are forced to be kinematic // may need to not collide with own avatar - markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP); + markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP | Simulation::DIRTY_POSITION); } } diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 1896893b52..73107a4b42 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -475,6 +475,8 @@ public: virtual bool getMeshes(MeshProxyList& result) { return true; } + virtual void locationChanged(bool tellPhysics = true) override; + protected: void setSimulated(bool simulated) { _simulated = simulated; } @@ -482,7 +484,6 @@ protected: const QByteArray getDynamicDataInternal() const; void setDynamicDataInternal(QByteArray dynamicData); - virtual void locationChanged(bool tellPhysics = true) override; virtual void dimensionsChanged() override; EntityTypes::EntityType _type; diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 7d353f69a1..820b3d279e 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1249,6 +1249,9 @@ void EntityTree::fixupMissingParents() { if (entity->isParentIDValid()) { // this entity's parent was previously not known, and now is. Update its location in the EntityTree... doMove = true; + // the bounds on the render-item may need to be updated, the rigid body in the physics engine may + // need to be moved. + entity->locationChanged(true); } else if (getIsServer() && _avatarIDs.contains(entity->getParentID())) { // this is a child of an avatar, which the entity server will never have // a SpatiallyNestable object for. Add it to a list for cleanup when the avatar leaves. From 1b0eeb9c2c8cc01a1e87a343cb205451fc0f383d Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 18 May 2017 14:35:39 -0700 Subject: [PATCH 05/17] try harder to put things in place when their parent entities are discovered --- libraries/entities/src/EntityItem.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 43ba881432..7f7c8b63cb 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1616,6 +1616,7 @@ void EntityItem::updatePosition(const glm::vec3& value) { entity->markDirtyFlags(Simulation::DIRTY_POSITION); } }); + locationChanged(); } } @@ -1625,6 +1626,13 @@ void EntityItem::updateParentID(const QUuid& value) { // children are forced to be kinematic // may need to not collide with own avatar markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP | Simulation::DIRTY_POSITION); + forEachDescendant([&](SpatiallyNestablePointer object) { + if (object->getNestableType() == NestableType::Entity) { + EntityItemPointer entity = std::static_pointer_cast(object); + entity->markDirtyFlags(Simulation::DIRTY_POSITION); + } + }); + locationChanged(); } } From 4007133423cf3a342d11a3ef9cdb1e7c826ce533 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 18 May 2017 14:59:26 -0700 Subject: [PATCH 06/17] collision hulls are still not always in the right place --- libraries/entities/src/EntityTree.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 820b3d279e..d85bdc11bf 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1251,6 +1251,15 @@ void EntityTree::fixupMissingParents() { doMove = true; // the bounds on the render-item may need to be updated, the rigid body in the physics engine may // need to be moved. + entity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | + Simulation::DIRTY_COLLISION_GROUP | + Simulation::DIRTY_POSITION); + entity->forEachDescendant([&](SpatiallyNestablePointer object) { + if (object->getNestableType() == NestableType::Entity) { + EntityItemPointer entity = std::static_pointer_cast(object); + entity->markDirtyFlags(Simulation::DIRTY_POSITION); + } + }); entity->locationChanged(true); } else if (getIsServer() && _avatarIDs.contains(entity->getParentID())) { // this is a child of an avatar, which the entity server will never have From 0c12baa2584f101c279b5b019dd3f367451c7ae0 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 19 May 2017 09:57:42 -0700 Subject: [PATCH 07/17] call entityChanged after settings flags --- libraries/entities/src/EntityItem.cpp | 16 ++++++++++++++++ libraries/entities/src/EntityTree.cpp | 6 ++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 7f7c8b63cb..b9ef606867 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1609,11 +1609,19 @@ void EntityItem::updateRegistrationPoint(const glm::vec3& value) { void EntityItem::updatePosition(const glm::vec3& value) { if (getLocalPosition() != value) { setLocalPosition(value); + + EntityTreePointer tree = getTree(); + if (!tree) { + return; + } + markDirtyFlags(Simulation::DIRTY_POSITION); + tree->entityChanged(getThisPointer()); forEachDescendant([&](SpatiallyNestablePointer object) { if (object->getNestableType() == NestableType::Entity) { EntityItemPointer entity = std::static_pointer_cast(object); entity->markDirtyFlags(Simulation::DIRTY_POSITION); + tree->entityChanged(entity); } }); locationChanged(); @@ -1623,13 +1631,21 @@ void EntityItem::updatePosition(const glm::vec3& value) { void EntityItem::updateParentID(const QUuid& value) { if (getParentID() != value) { setParentID(value); + + EntityTreePointer tree = getTree(); + if (!tree) { + return; + } + // children are forced to be kinematic // may need to not collide with own avatar markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP | Simulation::DIRTY_POSITION); + tree->entityChanged(getThisPointer()); forEachDescendant([&](SpatiallyNestablePointer object) { if (object->getNestableType() == NestableType::Entity) { EntityItemPointer entity = std::static_pointer_cast(object); entity->markDirtyFlags(Simulation::DIRTY_POSITION); + tree->entityChanged(entity); } }); locationChanged(); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index d85bdc11bf..4f9320b931 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1254,10 +1254,12 @@ void EntityTree::fixupMissingParents() { entity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP | Simulation::DIRTY_POSITION); + entityChanged(entity); entity->forEachDescendant([&](SpatiallyNestablePointer object) { if (object->getNestableType() == NestableType::Entity) { - EntityItemPointer entity = std::static_pointer_cast(object); - entity->markDirtyFlags(Simulation::DIRTY_POSITION); + EntityItemPointer descendantEntity = std::static_pointer_cast(object); + descendantEntity->markDirtyFlags(Simulation::DIRTY_POSITION); + entityChanged(descendantEntity); } }); entity->locationChanged(true); From 77ce8a19cf6a0df3824bdd1078f6e9921a4ad92f Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 19 May 2017 10:11:02 -0700 Subject: [PATCH 08/17] call entityChanged after settings flags --- libraries/entities/src/EntityItem.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index b9ef606867..873bc6f33b 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1644,7 +1644,9 @@ void EntityItem::updateParentID(const QUuid& value) { forEachDescendant([&](SpatiallyNestablePointer object) { if (object->getNestableType() == NestableType::Entity) { EntityItemPointer entity = std::static_pointer_cast(object); - entity->markDirtyFlags(Simulation::DIRTY_POSITION); + entity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | + Simulation::DIRTY_COLLISION_GROUP | + Simulation::DIRTY_POSITION); tree->entityChanged(entity); } }); From 7220fe0ad8493522cb4c9369c0c425f4ad7c8131 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 19 May 2017 10:40:24 -0700 Subject: [PATCH 09/17] set more physics flags dirty when an entity parent is found --- libraries/entities/src/EntityTree.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 4f9320b931..ebad27a063 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1258,7 +1258,9 @@ void EntityTree::fixupMissingParents() { entity->forEachDescendant([&](SpatiallyNestablePointer object) { if (object->getNestableType() == NestableType::Entity) { EntityItemPointer descendantEntity = std::static_pointer_cast(object); - descendantEntity->markDirtyFlags(Simulation::DIRTY_POSITION); + descendantEntity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | + Simulation::DIRTY_COLLISION_GROUP | + Simulation::DIRTY_POSITION); entityChanged(descendantEntity); } }); From 735e4b7d05ecc672827b96036bff2eadfd6937e6 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 19 May 2017 10:59:59 -0700 Subject: [PATCH 10/17] trying to get child collision hulls to be in the right place after import --- libraries/entities/src/EntityItem.cpp | 4 ++-- libraries/entities/src/EntityTree.cpp | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 873bc6f33b..44b0869d85 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1639,14 +1639,14 @@ void EntityItem::updateParentID(const QUuid& value) { // children are forced to be kinematic // may need to not collide with own avatar - markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP | Simulation::DIRTY_POSITION); + markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP | Simulation::DIRTY_TRANSFORM); tree->entityChanged(getThisPointer()); forEachDescendant([&](SpatiallyNestablePointer object) { if (object->getNestableType() == NestableType::Entity) { EntityItemPointer entity = std::static_pointer_cast(object); entity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP | - Simulation::DIRTY_POSITION); + Simulation::DIRTY_TRANSFORM); tree->entityChanged(entity); } }); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index ebad27a063..6f790bdd87 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1253,14 +1253,14 @@ void EntityTree::fixupMissingParents() { // need to be moved. entity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP | - Simulation::DIRTY_POSITION); + Simulation::DIRTY_TRANSFORM); entityChanged(entity); entity->forEachDescendant([&](SpatiallyNestablePointer object) { if (object->getNestableType() == NestableType::Entity) { EntityItemPointer descendantEntity = std::static_pointer_cast(object); descendantEntity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP | - Simulation::DIRTY_POSITION); + Simulation::DIRTY_TRANSFORM); entityChanged(descendantEntity); } }); @@ -1620,6 +1620,10 @@ QVector EntityTree::sendEntities(EntityEditPacketSender* packetSen EntityItemID newID = i.value(); EntityItemPointer entity = localTree->findEntityByEntityItemID(newID); if (entity) { + if (!entity->getParentID().isNull()) { + QWriteLocker locker(&_missingParentLock); + _missingParent.append(entity); + } entity->forceQueryAACubeUpdate(); moveOperator.addEntityToMoveList(entity, entity->getQueryAACube()); i++; From 936d0e2d500b5e4b4020ebc9c4c8cfb2cf3f3d45 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 19 May 2017 12:13:43 -0700 Subject: [PATCH 11/17] be more careful about fixing up entities which arrived before their parents --- libraries/entities/src/EntityTree.cpp | 112 ++++++++++++-------------- 1 file changed, 50 insertions(+), 62 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 6f790bdd87..84dc3844e3 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -122,7 +122,7 @@ void EntityTree::postAddEntity(EntityItemPointer entity) { _simulation->addEntity(entity); } - if (!entity->isParentIDValid()) { + if (!entity->getParentID().isNull()) { QWriteLocker locker(&_missingParentLock); _missingParent.append(entity); } @@ -1212,73 +1212,61 @@ void EntityTree::entityChanged(EntityItemPointer entity) { void EntityTree::fixupMissingParents() { MovingEntitiesOperator moveOperator(getThisPointer()); - QList missingParents; - { - QWriteLocker locker(&_missingParentLock); - QMutableVectorIterator iter(_missingParent); - while (iter.hasNext()) { - EntityItemWeakPointer entityWP = iter.next(); - EntityItemPointer entity = entityWP.lock(); - if (entity) { - if (entity->isParentIDValid()) { - iter.remove(); - } else { - missingParents.append(entity); - } - } else { - // entity was deleted before we found its parent. - iter.remove(); + QWriteLocker locker(&_missingParentLock); + QMutableVectorIterator iter(_missingParent); + while (iter.hasNext()) { + EntityItemWeakPointer entityWP = iter.next(); + EntityItemPointer entity = entityWP.lock(); + if (!entity) { + // entity was deleted before we found its parent + iter.remove(); + } + bool queryAACubeSuccess; + AACube newCube = entity->getQueryAACube(queryAACubeSuccess); + if (queryAACubeSuccess) { + // make sure queryAACube encompasses maxAACube + bool maxAACubeSuccess; + AACube maxAACube = entity->getMaximumAACube(maxAACubeSuccess); + if (maxAACubeSuccess && !newCube.contains(maxAACube)) { + newCube = maxAACube; } } - } - for (EntityItemPointer entity : missingParents) { - if (entity) { - bool queryAACubeSuccess; - AACube newCube = entity->getQueryAACube(queryAACubeSuccess); - if (queryAACubeSuccess) { - // make sure queryAACube encompasses maxAACube - bool maxAACubeSuccess; - AACube maxAACube = entity->getMaximumAACube(maxAACubeSuccess); - if (maxAACubeSuccess && !newCube.contains(maxAACube)) { - newCube = maxAACube; + bool doMove = false; + if (entity->isParentIDValid()) { + iter.remove(); // this entity is all hooked up; we can remove it from the list + // this entity's parent was previously not known, and now is. Update its location in the EntityTree... + doMove = true; + // the bounds on the render-item may need to be updated, the rigid body in the physics engine may + // need to be moved. + entity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | + Simulation::DIRTY_COLLISION_GROUP | + Simulation::DIRTY_TRANSFORM); + entityChanged(entity); + entity->forEachDescendant([&](SpatiallyNestablePointer object) { + if (object->getNestableType() == NestableType::Entity) { + EntityItemPointer descendantEntity = std::static_pointer_cast(object); + descendantEntity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | + Simulation::DIRTY_COLLISION_GROUP | + Simulation::DIRTY_TRANSFORM); + entityChanged(descendantEntity); } + }); + entity->locationChanged(true); + } else if (getIsServer() && _avatarIDs.contains(entity->getParentID())) { + // this is a child of an avatar, which the entity server will never have + // a SpatiallyNestable object for. Add it to a list for cleanup when the avatar leaves. + if (!_childrenOfAvatars.contains(entity->getParentID())) { + _childrenOfAvatars[entity->getParentID()] = QSet(); } + _childrenOfAvatars[entity->getParentID()] += entity->getEntityItemID(); + doMove = true; + iter.remove(); // and pull it out of the list + } - bool doMove = false; - if (entity->isParentIDValid()) { - // this entity's parent was previously not known, and now is. Update its location in the EntityTree... - doMove = true; - // the bounds on the render-item may need to be updated, the rigid body in the physics engine may - // need to be moved. - entity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | - Simulation::DIRTY_COLLISION_GROUP | - Simulation::DIRTY_TRANSFORM); - entityChanged(entity); - entity->forEachDescendant([&](SpatiallyNestablePointer object) { - if (object->getNestableType() == NestableType::Entity) { - EntityItemPointer descendantEntity = std::static_pointer_cast(object); - descendantEntity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | - Simulation::DIRTY_COLLISION_GROUP | - Simulation::DIRTY_TRANSFORM); - entityChanged(descendantEntity); - } - }); - entity->locationChanged(true); - } else if (getIsServer() && _avatarIDs.contains(entity->getParentID())) { - // this is a child of an avatar, which the entity server will never have - // a SpatiallyNestable object for. Add it to a list for cleanup when the avatar leaves. - if (!_childrenOfAvatars.contains(entity->getParentID())) { - _childrenOfAvatars[entity->getParentID()] = QSet(); - } - _childrenOfAvatars[entity->getParentID()] += entity->getEntityItemID(); - doMove = true; - } - - if (queryAACubeSuccess && doMove) { - moveOperator.addEntityToMoveList(entity, newCube); - entity->markAncestorMissing(false); - } + if (queryAACubeSuccess && doMove) { + moveOperator.addEntityToMoveList(entity, newCube); + entity->markAncestorMissing(false); } } From 5c93dbd20d35c79b7a73cad5f30d3cd352b23560 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 19 May 2017 13:48:29 -0700 Subject: [PATCH 12/17] be more careful about fixing up entities which arrived before their parents --- libraries/entities/src/AddEntityOperator.cpp | 4 ---- libraries/entities/src/EntityItem.cpp | 4 +--- libraries/entities/src/EntityTree.cpp | 13 +++++-------- libraries/shared/src/SpatiallyNestable.h | 4 ---- 4 files changed, 6 insertions(+), 19 deletions(-) diff --git a/libraries/entities/src/AddEntityOperator.cpp b/libraries/entities/src/AddEntityOperator.cpp index 1c39b40495..7cf4e4a472 100644 --- a/libraries/entities/src/AddEntityOperator.cpp +++ b/libraries/entities/src/AddEntityOperator.cpp @@ -27,10 +27,6 @@ AddEntityOperator::AddEntityOperator(EntityTreePointer tree, EntityItemPointer n bool success; auto queryCube = _newEntity->getQueryAACube(success); - if (!success) { - _newEntity->markAncestorMissing(true); - } - _newEntityBox = queryCube.clamp((float)(-HALF_TREE_SCALE), (float)HALF_TREE_SCALE); } diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 44b0869d85..800bae5617 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1373,11 +1373,9 @@ bool EntityItem::setProperties(const EntityItemProperties& properties) { if (saveQueryAACube != _queryAACube) { somethingChanged = true; } - } else { - markAncestorMissing(true); } - // Now check the sub classes + // Now check the sub classes somethingChanged |= setSubClassProperties(properties); // Finally notify if change detected diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 84dc3844e3..403d981409 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -295,7 +295,7 @@ bool EntityTree::updateEntityWithElement(EntityItemPointer entity, const EntityI _missingParent.append(childEntity); continue; } - if (!childEntity->isParentIDValid()) { + if (!childEntity->getParentID().isNull()) { QWriteLocker locker(&_missingParentLock); _missingParent.append(childEntity); } @@ -383,9 +383,7 @@ EntityItemPointer EntityTree::addEntity(const EntityItemID& entityID, const Enti // Recurse the tree and store the entity in the correct tree element AddEntityOperator theOperator(getThisPointer(), result); recurseTreeWithOperator(&theOperator); - if (result->getAncestorMissing()) { - // we added the entity, but didn't know about all its ancestors, so it went into the wrong place. - // add it to a list of entities needing to be fixed once their parents are known. + if (!result->getParentID().isNull()) { QWriteLocker locker(&_missingParentLock); _missingParent.append(result); } @@ -1221,11 +1219,11 @@ void EntityTree::fixupMissingParents() { // entity was deleted before we found its parent iter.remove(); } - bool queryAACubeSuccess; + bool queryAACubeSuccess { false }; + bool maxAACubeSuccess { false }; AACube newCube = entity->getQueryAACube(queryAACubeSuccess); if (queryAACubeSuccess) { // make sure queryAACube encompasses maxAACube - bool maxAACubeSuccess; AACube maxAACube = entity->getMaximumAACube(maxAACubeSuccess); if (maxAACubeSuccess && !newCube.contains(maxAACube)) { newCube = maxAACube; @@ -1233,7 +1231,7 @@ void EntityTree::fixupMissingParents() { } bool doMove = false; - if (entity->isParentIDValid()) { + if (entity->isParentIDValid() && maxAACubeSuccess) { // maxAACubeSuccess of true means all ancestors are known iter.remove(); // this entity is all hooked up; we can remove it from the list // this entity's parent was previously not known, and now is. Update its location in the EntityTree... doMove = true; @@ -1266,7 +1264,6 @@ void EntityTree::fixupMissingParents() { if (queryAACubeSuccess && doMove) { moveOperator.addEntityToMoveList(entity, newCube); - entity->markAncestorMissing(false); } } diff --git a/libraries/shared/src/SpatiallyNestable.h b/libraries/shared/src/SpatiallyNestable.h index 6589a44307..b98ab4c358 100644 --- a/libraries/shared/src/SpatiallyNestable.h +++ b/libraries/shared/src/SpatiallyNestable.h @@ -158,9 +158,6 @@ public: SpatiallyNestablePointer getThisPointer() const; - void markAncestorMissing(bool value) { _missingAncestor = value; } - bool getAncestorMissing() { return _missingAncestor; } - void forEachChild(std::function actor); void forEachDescendant(std::function actor); @@ -207,7 +204,6 @@ protected: mutable AACube _queryAACube; mutable bool _queryAACubeSet { false }; - bool _missingAncestor { false }; quint64 _scaleChanged { 0 }; quint64 _translationChanged { 0 }; quint64 _rotationChanged { 0 }; From 54dafa0f83af630cab79f91247bd2fb609bab15b Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 19 May 2017 14:14:00 -0700 Subject: [PATCH 13/17] be more careful about fixing up entities which arrived before their parents --- libraries/entities/src/EntityTree.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 403d981409..e88575fc10 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1219,6 +1219,8 @@ void EntityTree::fixupMissingParents() { // entity was deleted before we found its parent iter.remove(); } + + entity->requiresRecalcBoxes(); bool queryAACubeSuccess { false }; bool maxAACubeSuccess { false }; AACube newCube = entity->getQueryAACube(queryAACubeSuccess); From 386c76545ce960c3c025fec023345c2117de268b Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 19 May 2017 14:21:05 -0700 Subject: [PATCH 14/17] be more careful about fixing up entities which arrived before their parents --- libraries/entities/src/EntityItem.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 73107a4b42..dbfb99b06b 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -187,7 +187,7 @@ public: const Transform getTransformToCenter(bool& success) const; - inline void requiresRecalcBoxes(); + void requiresRecalcBoxes(); // Hyperlink related getters and setters QString getHref() const; From 67f222cb3f507656de689e3d6ffc9c9eb8d324c5 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 19 May 2017 15:19:16 -0700 Subject: [PATCH 15/17] be more careful about fixing up entities which arrived before their parents --- libraries/entities/src/EntityTree.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index e88575fc10..c25685ba81 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1218,6 +1218,7 @@ void EntityTree::fixupMissingParents() { if (!entity) { // entity was deleted before we found its parent iter.remove(); + continue; } entity->requiresRecalcBoxes(); From 5c94147f40ff095795196a37c4ce7dece06ce903 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 19 May 2017 17:09:42 -0700 Subject: [PATCH 16/17] children collision hulls appear to be in the right place, now --- libraries/entities/src/EntityTree.cpp | 32 +++++++++++--------- libraries/entities/src/EntityTree.h | 8 +++-- libraries/entities/src/EntityTreeElement.cpp | 8 ++++- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index c25685ba81..a98e07b9e3 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -123,15 +123,14 @@ void EntityTree::postAddEntity(EntityItemPointer entity) { } if (!entity->getParentID().isNull()) { - QWriteLocker locker(&_missingParentLock); - _missingParent.append(entity); + addToNeedsParentFixupList(entity); } _isDirty = true; emit addingEntity(entity->getEntityItemID()); // find and hook up any entities with this entity as a (previously) missing parent - fixupMissingParents(); + fixupNeedsParentFixups(); } bool EntityTree::updateEntity(const EntityItemID& entityID, const EntityItemProperties& properties, const SharedNodePointer& senderNode) { @@ -291,13 +290,11 @@ bool EntityTree::updateEntityWithElement(EntityItemPointer entity, const EntityI bool success; AACube queryCube = childEntity->getQueryAACube(success); if (!success) { - QWriteLocker locker(&_missingParentLock); - _missingParent.append(childEntity); + addToNeedsParentFixupList(childEntity); continue; } if (!childEntity->getParentID().isNull()) { - QWriteLocker locker(&_missingParentLock); - _missingParent.append(childEntity); + addToNeedsParentFixupList(childEntity); } UpdateEntityOperator theChildOperator(getThisPointer(), containingElement, childEntity, queryCube); @@ -384,8 +381,7 @@ EntityItemPointer EntityTree::addEntity(const EntityItemID& entityID, const Enti AddEntityOperator theOperator(getThisPointer(), result); recurseTreeWithOperator(&theOperator); if (!result->getParentID().isNull()) { - QWriteLocker locker(&_missingParentLock); - _missingParent.append(result); + addToNeedsParentFixupList(result); } postAddEntity(result); @@ -1207,11 +1203,13 @@ void EntityTree::entityChanged(EntityItemPointer entity) { } } -void EntityTree::fixupMissingParents() { + +void EntityTree::fixupNeedsParentFixups() { MovingEntitiesOperator moveOperator(getThisPointer()); - QWriteLocker locker(&_missingParentLock); - QMutableVectorIterator iter(_missingParent); + QWriteLocker locker(&_needsParentFixupLock); + + QMutableVectorIterator iter(_needsParentFixup); while (iter.hasNext()) { EntityItemWeakPointer entityWP = iter.next(); EntityItemPointer entity = entityWP.lock(); @@ -1283,8 +1281,13 @@ void EntityTree::deleteDescendantsOfAvatar(QUuid avatarID) { } } +void EntityTree::addToNeedsParentFixupList(EntityItemPointer entity) { + QWriteLocker locker(&_needsParentFixupLock); + _needsParentFixup.append(entity); +} + void EntityTree::update() { - fixupMissingParents(); + fixupNeedsParentFixups(); if (_simulation) { withWriteLock([&] { _simulation->updateEntities(); @@ -1609,8 +1612,7 @@ QVector EntityTree::sendEntities(EntityEditPacketSender* packetSen EntityItemPointer entity = localTree->findEntityByEntityItemID(newID); if (entity) { if (!entity->getParentID().isNull()) { - QWriteLocker locker(&_missingParentLock); - _missingParent.append(entity); + addToNeedsParentFixupList(entity); } entity->forceQueryAACubeUpdate(); moveOperator.addEntityToMoveList(entity, entity->getQueryAACube()); diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 6137d14aac..2703060719 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -271,6 +271,8 @@ public: void forgetAvatarID(QUuid avatarID) { _avatarIDs -= avatarID; } void deleteDescendantsOfAvatar(QUuid avatarID); + void addToNeedsParentFixupList(EntityItemPointer entity); + void notifyNewCollisionSoundURL(const QString& newCollisionSoundURL, const EntityItemID& entityID); static const float DEFAULT_MAX_TMP_ENTITY_LIFETIME; @@ -351,9 +353,9 @@ protected: quint64 _maxEditDelta = 0; quint64 _treeResetTime = 0; - void fixupMissingParents(); // try to hook members of _missingParent to parent instances - QVector _missingParent; // entites with a parentID but no (yet) known parent instance - mutable QReadWriteLock _missingParentLock; + void fixupNeedsParentFixups(); // try to hook members of _needsParentFixup to parent instances + QVector _needsParentFixup; // entites with a parentID but no (yet) known parent instance + mutable QReadWriteLock _needsParentFixupLock; // we maintain a list of avatarIDs to notice when an entity is a child of one. QSet _avatarIDs; // IDs of avatars connected to entity server diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 98287541e3..0dc42717f5 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -208,7 +208,7 @@ void EntityTreeElement::elementEncodeComplete(EncodeBitstreamParams& params) con // why would this ever fail??? // If we've encoding this element before... but we're coming back a second time in an attempt to - // encoud our parent... this might happen. + // encode our parent... this might happen. if (extraEncodeData->contains(childElement.get())) { EntityTreeElementExtraEncodeDataPointer childExtraEncodeData = std::static_pointer_cast((*extraEncodeData)[childElement.get()]); @@ -981,6 +981,7 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int // 3) remember the old cube for the entity so we can mark it as dirty if (entityItem) { QString entityScriptBefore = entityItem->getScript(); + QUuid parentIDBefore = entityItem->getParentID(); QString entityServerScriptsBefore = entityItem->getServerScripts(); quint64 entityScriptTimestampBefore = entityItem->getScriptTimestamp(); bool bestFitBefore = bestFitEntityBounds(entityItem); @@ -1018,6 +1019,11 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int _myTree->emitEntityServerScriptChanging(entityItemID, reload); // the entity server script has changed } + QUuid parentIDAfter = entityItem->getParentID(); + if (parentIDBefore != parentIDAfter) { + _myTree->addToNeedsParentFixupList(entityItem); + } + } else { entityItem = EntityTypes::constructEntityItem(dataAt, bytesLeftToRead, args); if (entityItem) { From a442181859485403b003a977c25511f40b9724fb Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Sat, 20 May 2017 08:51:44 -0700 Subject: [PATCH 17/17] remove some redundancy --- libraries/entities/src/EntityItem.cpp | 44 +++++++++------------------ 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 800bae5617..fc1b6490cd 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1369,10 +1369,9 @@ bool EntityItem::setProperties(const EntityItemProperties& properties) { SET_ENTITY_PROPERTY_FROM_PROPERTIES(lastEditedBy, setLastEditedBy); AACube saveQueryAACube = _queryAACube; - if (checkAndAdjustQueryAACube()) { - if (saveQueryAACube != _queryAACube) { - somethingChanged = true; - } + checkAndAdjustQueryAACube(); + if (saveQueryAACube != _queryAACube) { + somethingChanged = true; } // Now check the sub classes @@ -1609,46 +1608,33 @@ void EntityItem::updatePosition(const glm::vec3& value) { setLocalPosition(value); EntityTreePointer tree = getTree(); - if (!tree) { - return; - } - markDirtyFlags(Simulation::DIRTY_POSITION); - tree->entityChanged(getThisPointer()); + if (tree) { + tree->entityChanged(getThisPointer()); + } forEachDescendant([&](SpatiallyNestablePointer object) { if (object->getNestableType() == NestableType::Entity) { EntityItemPointer entity = std::static_pointer_cast(object); entity->markDirtyFlags(Simulation::DIRTY_POSITION); - tree->entityChanged(entity); + if (tree) { + tree->entityChanged(entity); + } } }); - locationChanged(); } } void EntityItem::updateParentID(const QUuid& value) { if (getParentID() != value) { setParentID(value); - - EntityTreePointer tree = getTree(); - if (!tree) { - return; - } - // children are forced to be kinematic // may need to not collide with own avatar - markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP | Simulation::DIRTY_TRANSFORM); - tree->entityChanged(getThisPointer()); - forEachDescendant([&](SpatiallyNestablePointer object) { - if (object->getNestableType() == NestableType::Entity) { - EntityItemPointer entity = std::static_pointer_cast(object); - entity->markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | - Simulation::DIRTY_COLLISION_GROUP | - Simulation::DIRTY_TRANSFORM); - tree->entityChanged(entity); - } - }); - locationChanged(); + markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP); + + EntityTreePointer tree = getTree(); + if (tree) { + tree->addToNeedsParentFixupList(getThisPointer()); + } } }