From 0f4e7fb53236b7e9cbaf5536e92b93dfdb0b4508 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 3 Oct 2017 18:03:25 -0700 Subject: [PATCH 1/3] workaround for bad physics polling on login --- interface/src/Application.cpp | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 0fc8c46cdc..f1e1f918e2 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5639,10 +5639,10 @@ bool Application::nearbyEntitiesAreReadyForPhysics() { return false; } + AABox avatarBox(getMyAvatar()->getPosition() - glm::vec3(0.5f * PHYSICS_READY_RANGE), glm::vec3(PHYSICS_READY_RANGE)); QVector entities; entityTree->withReadLock([&] { - AABox box(getMyAvatar()->getPosition() - glm::vec3(PHYSICS_READY_RANGE), glm::vec3(2 * PHYSICS_READY_RANGE)); - entityTree->findEntities(box, entities); + entityTree->findEntities(avatarBox, entities); }); // For reasons I haven't found, we don't necessarily have the full scene when we receive a stats packet. Apply @@ -5662,11 +5662,18 @@ bool Application::nearbyEntitiesAreReadyForPhysics() { bool result = true; foreach (EntityItemPointer entity, entities) { if (entity->shouldBePhysical() && !entity->isReadyToComputeShape()) { - static QString repeatedMessage = - LogHandler::getInstance().addRepeatedMessageRegex("Physics disabled until entity loads: .*"); - qCDebug(interfaceapp) << "Physics disabled until entity loads: " << entity->getID() << entity->getName(); - // don't break here because we want all the relevant entities to start their downloads - result = false; + // BUG: the findEntities() query above is sometimes returning objects that don't actually overlap + // TODO: investigate and fix findQueries() but in the meantime... + // WORKAROUND: test the overlap of each entity to verify it matters + bool success = false; + AACube entityCube = entity->getQueryAACube(success); + if (success && avatarBox.touches(entityCube)) { + static QString repeatedMessage = + LogHandler::getInstance().addRepeatedMessageRegex("Physics disabled until entity loads: .*"); + qCDebug(interfaceapp) << "Physics disabled until entity loads: " << entity->getID() << entity->getName(); + // don't break here because we want all the relevant entities to start their downloads + result = false; + } } } return result; From 0bcecdbe668466b09e209c40c3ca70b331f89292 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 5 Oct 2017 11:17:40 -0700 Subject: [PATCH 2/3] be picky when finding nearby entities at login --- interface/src/Application.cpp | 49 +++++++++++++------- libraries/entities/src/EntityTree.cpp | 6 +++ libraries/entities/src/EntityTree.h | 5 ++ libraries/entities/src/EntityTreeElement.cpp | 8 ++++ libraries/entities/src/EntityTreeElement.h | 6 +++ 5 files changed, 58 insertions(+), 16 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index f1e1f918e2..835a2fed56 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5639,14 +5639,38 @@ bool Application::nearbyEntitiesAreReadyForPhysics() { return false; } - AABox avatarBox(getMyAvatar()->getPosition() - glm::vec3(0.5f * PHYSICS_READY_RANGE), glm::vec3(PHYSICS_READY_RANGE)); + // We don't want to use EntityTree::findEntities(AABox, ...) method because that scan will snarf parented entities + // whose bounding boxes cannot be computed (it is too loose for our purposes here). Instead we manufacture + // custom filters and use the general-purpose EntityTree::findEntities(filter, ...) QVector entities; entityTree->withReadLock([&] { - entityTree->findEntities(avatarBox, entities); + AABox avatarBox(getMyAvatar()->getPosition() - glm::vec3(PHYSICS_READY_RANGE), glm::vec3(2 * PHYSICS_READY_RANGE)); + + // create two functions that use avatarBox (entityScan and elementScan), the second calls the first + std::function entityScan = [=](EntityItemPointer& entity) { + if (entity->shouldBePhysical()) { + bool success = false; + AABox entityBox = entity->getAABox(success); + // important: bail for entities that cannot supply a valid AABox + return success && avatarBox.touches(entityBox); + } + return false; + }; + std::function elementScan = [&](const OctreeElementPointer& element, void* unused) { + if (element->getAACube().touches(avatarBox)) { + EntityTreeElementPointer entityTreeElement = std::static_pointer_cast(element); + entityTreeElement->getEntities(entityScan, entities); + return true; + } + return false; + }; + + // Pass the second function to the general-purpose EntityTree::findEntities() + // which will traverse the tree, apply the two filter functions (to element, then to entities) + // as it traverses. The end result will be a list of entities that match. + entityTree->findEntities(elementScan, entities); }); - // For reasons I haven't found, we don't necessarily have the full scene when we receive a stats packet. Apply - // a heuristic to try to decide when we actually know about all of the nearby entities. uint32_t nearbyCount = entities.size(); if (nearbyCount == _nearbyEntitiesCountAtLastPhysicsCheck) { _nearbyEntitiesStabilityCount++; @@ -5662,18 +5686,11 @@ bool Application::nearbyEntitiesAreReadyForPhysics() { bool result = true; foreach (EntityItemPointer entity, entities) { if (entity->shouldBePhysical() && !entity->isReadyToComputeShape()) { - // BUG: the findEntities() query above is sometimes returning objects that don't actually overlap - // TODO: investigate and fix findQueries() but in the meantime... - // WORKAROUND: test the overlap of each entity to verify it matters - bool success = false; - AACube entityCube = entity->getQueryAACube(success); - if (success && avatarBox.touches(entityCube)) { - static QString repeatedMessage = - LogHandler::getInstance().addRepeatedMessageRegex("Physics disabled until entity loads: .*"); - qCDebug(interfaceapp) << "Physics disabled until entity loads: " << entity->getID() << entity->getName(); - // don't break here because we want all the relevant entities to start their downloads - result = false; - } + static QString repeatedMessage = + LogHandler::getInstance().addRepeatedMessageRegex("Physics disabled until entity loads: .*"); + qCDebug(interfaceapp) << "Physics disabled until entity loads: " << entity->getID() << entity->getName(); + // don't break here because we want all the relevant entities to start their downloads + result = false; } } return result; diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index bf37a08386..fa386ae090 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -864,6 +864,12 @@ void EntityTree::findEntities(const ViewFrustum& frustum, QVector& foundEntities) { + recurseTreeWithOperation(elementFilter, nullptr); +} + EntityItemPointer EntityTree::findEntityByID(const QUuid& id) { EntityItemID entityID(id); return findEntityByEntityItemID(entityID); diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index d0448f438a..53e36bc7c7 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -165,6 +165,11 @@ public: /// \param foundEntities[out] vector of EntityItemPointer void findEntities(const ViewFrustum& frustum, QVector& foundEntities); + /// finds all entities that match scanOperator + /// \parameter scanOperator function that scans entities that match criteria + /// \parameter foundEntities[out] vector of EntityItemPointer + void findEntities(RecurseOctreeOperation& scanOperator, QVector& foundEntities); + void addNewlyCreatedHook(NewlyCreatedEntityHook* hook); void removeNewlyCreatedHook(NewlyCreatedEntityHook* hook); diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 0c33855a61..2696377028 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -869,6 +869,14 @@ void EntityTreeElement::getEntities(const ViewFrustum& frustum, QVector& foundEntities) { + forEachEntity([&](EntityItemPointer entity) { + if (filter(entity)) { + foundEntities.push_back(entity); + } + }); +} + EntityItemPointer EntityTreeElement::getEntityWithEntityItemID(const EntityItemID& id) const { EntityItemPointer foundEntity = NULL; withReadLock([&] { diff --git a/libraries/entities/src/EntityTreeElement.h b/libraries/entities/src/EntityTreeElement.h index c7fb80c330..cafae9941a 100644 --- a/libraries/entities/src/EntityTreeElement.h +++ b/libraries/entities/src/EntityTreeElement.h @@ -27,6 +27,7 @@ class EntityTreeElement; using EntityItems = QVector; using EntityTreeElementWeakPointer = std::weak_ptr; using EntityTreeElementPointer = std::shared_ptr; +using EntityItemFilter = std::function; class EntityTreeUpdateArgs { public: @@ -199,6 +200,11 @@ public: /// \param entities[out] vector of non-const EntityItemPointer void getEntities(const ViewFrustum& frustum, QVector& foundEntities); + /// finds all entities that match filter + /// \param filter function that adds matching entities to foundEntities + /// \param entities[out] vector of non-const EntityItemPointer + void getEntities(EntityItemFilter& filter, QVector& foundEntities); + EntityItemPointer getEntityWithID(uint32_t id) const; EntityItemPointer getEntityWithEntityItemID(const EntityItemID& id) const; void getEntitiesInside(const AACube& box, QVector& foundEntities); From bbde1bcd632767466478fdc001350f7257f5cd80 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 9 Oct 2017 10:42:42 -0700 Subject: [PATCH 3/3] move stuff out of writelock when possible --- interface/src/Application.cpp | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 835a2fed56..527e3607f5 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5643,20 +5643,18 @@ bool Application::nearbyEntitiesAreReadyForPhysics() { // whose bounding boxes cannot be computed (it is too loose for our purposes here). Instead we manufacture // custom filters and use the general-purpose EntityTree::findEntities(filter, ...) QVector entities; - entityTree->withReadLock([&] { - AABox avatarBox(getMyAvatar()->getPosition() - glm::vec3(PHYSICS_READY_RANGE), glm::vec3(2 * PHYSICS_READY_RANGE)); - - // create two functions that use avatarBox (entityScan and elementScan), the second calls the first - std::function entityScan = [=](EntityItemPointer& entity) { - if (entity->shouldBePhysical()) { - bool success = false; - AABox entityBox = entity->getAABox(success); - // important: bail for entities that cannot supply a valid AABox - return success && avatarBox.touches(entityBox); - } - return false; - }; - std::function elementScan = [&](const OctreeElementPointer& element, void* unused) { + AABox avatarBox(getMyAvatar()->getPosition() - glm::vec3(PHYSICS_READY_RANGE), glm::vec3(2 * PHYSICS_READY_RANGE)); + // create two functions that use avatarBox (entityScan and elementScan), the second calls the first + std::function entityScan = [=](EntityItemPointer& entity) { + if (entity->shouldBePhysical()) { + bool success = false; + AABox entityBox = entity->getAABox(success); + // important: bail for entities that cannot supply a valid AABox + return success && avatarBox.touches(entityBox); + } + return false; + }; + std::function elementScan = [&](const OctreeElementPointer& element, void* unused) { if (element->getAACube().touches(avatarBox)) { EntityTreeElementPointer entityTreeElement = std::static_pointer_cast(element); entityTreeElement->getEntities(entityScan, entities); @@ -5665,6 +5663,7 @@ bool Application::nearbyEntitiesAreReadyForPhysics() { return false; }; + entityTree->withReadLock([&] { // Pass the second function to the general-purpose EntityTree::findEntities() // which will traverse the tree, apply the two filter functions (to element, then to entities) // as it traverses. The end result will be a list of entities that match.