From b7ffb96adf9f53f641b65b72bd46b06f71bdda0d Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 10 Sep 2015 22:28:50 -0700 Subject: [PATCH] Working on thread safety for the entity tree --- .../src/EntityTreeRenderer.cpp | 17 +- libraries/entities/src/EntityItem.cpp | 2 +- libraries/entities/src/EntityTree.cpp | 9 +- libraries/entities/src/EntityTreeElement.cpp | 409 +++++++++--------- libraries/entities/src/EntityTreeElement.h | 20 +- .../src/RecurseOctreeToMapOperator.cpp | 7 +- 6 files changed, 225 insertions(+), 239 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 5ced01e982..c5f7283303 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -642,22 +642,14 @@ void EntityTreeRenderer::renderElement(OctreeElement* element, RenderArgs* args) // we need to iterate the actual entityItems of the element EntityTreeElement* entityTreeElement = static_cast(element); - EntityItems& entityItems = entityTreeElement->getEntities(); - - - uint16_t numberOfEntities = entityItems.size(); - bool isShadowMode = args->_renderMode == RenderArgs::SHADOW_RENDER_MODE; - if (!isShadowMode && _displayModelElementProxy && numberOfEntities > 0) { + if (!isShadowMode && _displayModelElementProxy && entityTreeElement->size() > 0) { renderElementProxy(entityTreeElement, args); } - - for (uint16_t i = 0; i < numberOfEntities; i++) { - EntityItemPointer entityItem = entityItems[i]; - - if (entityItem->isVisible()) { + entityTreeElement->forEachEntity([&](EntityItemPointer entityItem) { + if (entityItem->isVisible()) { // NOTE: Zone Entities are a special case we handle here... if (entityItem->getType() == EntityTypes::Zone) { if (entityItem->contains(_viewState->getAvatarPosition())) { @@ -681,7 +673,8 @@ void EntityTreeRenderer::renderElement(OctreeElement* element, RenderArgs* args) } } } - } + }); + } float EntityTreeRenderer::getSizeScale() const { diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index c4ef8cd910..8ab847448c 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1512,7 +1512,7 @@ bool EntityItem::addAction(EntitySimulation* simulation, EntityActionPointer act withWriteLock([&] { checkWaitingToRemove(simulation); - bool result = addActionInternal(simulation, action); + result = addActionInternal(simulation, action); if (!result) { removeActionInternal(action->getID()); } diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 6e6cb989e5..1c8f59ffc9 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1012,12 +1012,10 @@ QVector EntityTree::sendEntities(EntityEditPacketSender* packetSen bool EntityTree::sendEntitiesOperation(OctreeElement* element, void* extraData) { SendEntitiesOperationArgs* args = static_cast(extraData); EntityTreeElement* entityTreeElement = static_cast(element); - - const EntityItems& entities = entityTreeElement->getEntities(); - for (int i = 0; i < entities.size(); i++) { + entityTreeElement->forEachEntity([&](EntityItemPointer entityItem) { EntityItemID newID(QUuid::createUuid()); args->newEntityIDs->append(newID); - EntityItemProperties properties = entities[i]->getProperties(); + EntityItemProperties properties = entityItem->getProperties(); properties.setPosition(properties.getPosition() + args->root); properties.markAllChanged(); // so the entire property set is considered new, since we're making a new entity @@ -1030,8 +1028,7 @@ bool EntityTree::sendEntitiesOperation(OctreeElement* element, void* extraData) args->localTree->addEntity(newID, properties); }); } - } - + }); return true; } diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index a023f46c5e..f80e4eb6a5 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -24,8 +24,6 @@ EntityTreeElement::EntityTreeElement(unsigned char* octalCode) : OctreeElement() EntityTreeElement::~EntityTreeElement() { _octreeMemoryUsage -= sizeof(EntityTreeElement); - delete _entityItems; - _entityItems = NULL; } // This will be called primarily on addChildAt(), which means we're adding a child of our @@ -39,7 +37,6 @@ OctreeElement* EntityTreeElement::createNewElement(unsigned char* octalCode) { void EntityTreeElement::init(unsigned char* octalCode) { OctreeElement::init(octalCode); - _entityItems = new EntityItems; _octreeMemoryUsage += sizeof(EntityTreeElement); } @@ -71,7 +68,7 @@ void EntityTreeElement::initializeExtraEncodeData(EncodeBitstreamParams& params) // Check to see if this element yet has encode data... if it doesn't create it if (!extraEncodeData->contains(this)) { EntityTreeElementExtraEncodeData* entityTreeElementExtraEncodeData = new EntityTreeElementExtraEncodeData(); - entityTreeElementExtraEncodeData->elementCompleted = (_entityItems->size() == 0); + entityTreeElementExtraEncodeData->elementCompleted = (_entityItems.size() == 0); for (int i = 0; i < NUMBER_OF_CHILDREN; i++) { EntityTreeElement* child = getChildAtIndex(i); if (!child) { @@ -84,10 +81,9 @@ void EntityTreeElement::initializeExtraEncodeData(EncodeBitstreamParams& params) } } } - for (uint16_t i = 0; i < _entityItems->size(); i++) { - EntityItemPointer entity = (*_entityItems)[i]; + forEachEntity([&](EntityItemPointer entity) { entityTreeElementExtraEncodeData->entities.insert(entity->getEntityItemID(), entity->getEntityProperties(params)); - } + }); // TODO: some of these inserts might be redundant!!! extraEncodeData->insert(this, entityTreeElementExtraEncodeData); @@ -248,7 +244,7 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData } else { // if there wasn't one already, then create one entityTreeElementExtraEncodeData = new EntityTreeElementExtraEncodeData(); - entityTreeElementExtraEncodeData->elementCompleted = (_entityItems->size() == 0); + entityTreeElementExtraEncodeData->elementCompleted = !hasContent(); for (int i = 0; i < NUMBER_OF_CHILDREN; i++) { EntityTreeElement* child = getChildAtIndex(i); @@ -262,10 +258,9 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData } } } - for (uint16_t i = 0; i < _entityItems->size(); i++) { - EntityItemPointer entity = (*_entityItems)[i]; + forEachEntity([&](EntityItemPointer entity) { entityTreeElementExtraEncodeData->entities.insert(entity->getEntityItemID(), entity->getEntityProperties(params)); - } + }); } //assert(extraEncodeData); @@ -277,81 +272,84 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData // write our entities out... first determine which of the entities are in view based on our params uint16_t numberOfEntities = 0; uint16_t actualNumberOfEntities = 0; - QVector indexesOfEntitiesToInclude; + int numberOfEntitiesOffset = 0; + withReadLock([&] { + QVector indexesOfEntitiesToInclude; - // It's possible that our element has been previous completed. In this case we'll simply not include any of our - // entities for encoding. This is needed because we encode the element data at the "parent" level, and so we - // need to handle the case where our sibling elements need encoding but we don't. - if (!entityTreeElementExtraEncodeData->elementCompleted) { - for (uint16_t i = 0; i < _entityItems->size(); i++) { - EntityItemPointer entity = (*_entityItems)[i]; - bool includeThisEntity = true; - - if (!params.forceSendScene && entity->getLastChangedOnServer() < params.lastViewFrustumSent) { - includeThisEntity = false; - } - - if (hadElementExtraData) { - includeThisEntity = includeThisEntity && - entityTreeElementExtraEncodeData->entities.contains(entity->getEntityItemID()); - } - - if (includeThisEntity && params.viewFrustum) { - - // we want to use the maximum possible box for this, so that we don't have to worry about the nuance of - // simulation changing what's visible. consider the case where the entity contains an angular velocity - // the entity may not be in view and then in view a frame later, let the client side handle it's view - // frustum culling on rendering. - AACube entityCube = entity->getMaximumAACube(); - if (params.viewFrustum->cubeInFrustum(entityCube) == ViewFrustum::OUTSIDE) { - includeThisEntity = false; // out of view, don't include it + // It's possible that our element has been previous completed. In this case we'll simply not include any of our + // entities for encoding. This is needed because we encode the element data at the "parent" level, and so we + // need to handle the case where our sibling elements need encoding but we don't. + if (!entityTreeElementExtraEncodeData->elementCompleted) { + for (uint16_t i = 0; i < _entityItems.size(); i++) { + EntityItemPointer entity = _entityItems[i]; + bool includeThisEntity = true; + + if (!params.forceSendScene && entity->getLastChangedOnServer() < params.lastViewFrustumSent) { + includeThisEntity = false; + } + + if (hadElementExtraData) { + includeThisEntity = includeThisEntity && + entityTreeElementExtraEncodeData->entities.contains(entity->getEntityItemID()); + } + + if (includeThisEntity && params.viewFrustum) { + + // we want to use the maximum possible box for this, so that we don't have to worry about the nuance of + // simulation changing what's visible. consider the case where the entity contains an angular velocity + // the entity may not be in view and then in view a frame later, let the client side handle it's view + // frustum culling on rendering. + AACube entityCube = entity->getMaximumAACube(); + if (params.viewFrustum->cubeInFrustum(entityCube) == ViewFrustum::OUTSIDE) { + includeThisEntity = false; // out of view, don't include it + } + } + + if (includeThisEntity) { + indexesOfEntitiesToInclude << i; + numberOfEntities++; } } - - if (includeThisEntity) { - indexesOfEntitiesToInclude << i; - numberOfEntities++; - } } - } - int numberOfEntitiesOffset = packetData->getUncompressedByteOffset(); - bool successAppendEntityCount = packetData->appendValue(numberOfEntities); + numberOfEntitiesOffset = packetData->getUncompressedByteOffset(); + bool successAppendEntityCount = packetData->appendValue(numberOfEntities); - if (successAppendEntityCount) { - foreach (uint16_t i, indexesOfEntitiesToInclude) { - EntityItemPointer entity = (*_entityItems)[i]; - LevelDetails entityLevel = packetData->startLevel(); - OctreeElement::AppendState appendEntityState = entity->appendEntityData(packetData, - params, entityTreeElementExtraEncodeData); + if (successAppendEntityCount) { + foreach(uint16_t i, indexesOfEntitiesToInclude) { + EntityItemPointer entity = _entityItems[i]; + LevelDetails entityLevel = packetData->startLevel(); + OctreeElement::AppendState appendEntityState = entity->appendEntityData(packetData, + params, entityTreeElementExtraEncodeData); - // If none of this entity data was able to be appended, then discard it - // and don't include it in our entity count - if (appendEntityState == OctreeElement::NONE) { - packetData->discardLevel(entityLevel); - } else { - // If either ALL or some of it got appended, then end the level (commit it) - // and include the entity in our final count of entities - packetData->endLevel(entityLevel); - actualNumberOfEntities++; - } - - // If the entity item got completely appended, then we can remove it from the extra encode data - if (appendEntityState == OctreeElement::COMPLETED) { - entityTreeElementExtraEncodeData->entities.remove(entity->getEntityItemID()); - } - - // If any part of the entity items didn't fit, then the element is considered partial - // NOTE: if the entity item didn't fit or only partially fit, then the entity item should have - // added itself to the extra encode data. - if (appendEntityState != OctreeElement::COMPLETED) { - appendElementState = OctreeElement::PARTIAL; + // If none of this entity data was able to be appended, then discard it + // and don't include it in our entity count + if (appendEntityState == OctreeElement::NONE) { + packetData->discardLevel(entityLevel); + } else { + // If either ALL or some of it got appended, then end the level (commit it) + // and include the entity in our final count of entities + packetData->endLevel(entityLevel); + actualNumberOfEntities++; + } + + // If the entity item got completely appended, then we can remove it from the extra encode data + if (appendEntityState == OctreeElement::COMPLETED) { + entityTreeElementExtraEncodeData->entities.remove(entity->getEntityItemID()); + } + + // If any part of the entity items didn't fit, then the element is considered partial + // NOTE: if the entity item didn't fit or only partially fit, then the entity item should have + // added itself to the extra encode data. + if (appendEntityState != OctreeElement::COMPLETED) { + appendElementState = OctreeElement::PARTIAL; + } } + } else { + // we we couldn't add the entity count, then we couldn't add anything for this element and we're in a NONE state + appendElementState = OctreeElement::NONE; } - } else { - // we we couldn't add the entity count, then we couldn't add anything for this element and we're in a NONE state - appendElementState = OctreeElement::NONE; - } + }); // If we were provided with extraEncodeData, and we allocated and/or got entityTreeElementExtraEncodeData // then we need to do some additional processing, namely make sure our extraEncodeData is up to date for @@ -478,56 +476,41 @@ bool EntityTreeElement::findDetailedRayIntersection(const glm::vec3& origin, con // only called if we do intersect our bounding cube, but find if we actually intersect with entities... int entityNumber = 0; - - EntityItems::iterator entityItr = _entityItems->begin(); - EntityItems::const_iterator entityEnd = _entityItems->end(); bool somethingIntersected = false; - - //float bestEntityDistance = distance; - - while(entityItr != entityEnd) { - EntityItemPointer entity = (*entityItr); - + forEachEntity([&](EntityItemPointer entity) { AABox entityBox = entity->getAABox(); float localDistance; BoxFace localFace; // if the ray doesn't intersect with our cube, we can stop searching! - if (entityBox.findRayIntersection(origin, direction, localDistance, localFace)) { + if (!entityBox.findRayIntersection(origin, direction, localDistance, localFace)) { + return; + } - // extents is the entity relative, scaled, centered extents of the entity - glm::mat4 rotation = glm::mat4_cast(entity->getRotation()); - glm::mat4 translation = glm::translate(entity->getPosition()); - glm::mat4 entityToWorldMatrix = translation * rotation; - glm::mat4 worldToEntityMatrix = glm::inverse(entityToWorldMatrix); + // extents is the entity relative, scaled, centered extents of the entity + glm::mat4 rotation = glm::mat4_cast(entity->getRotation()); + glm::mat4 translation = glm::translate(entity->getPosition()); + glm::mat4 entityToWorldMatrix = translation * rotation; + glm::mat4 worldToEntityMatrix = glm::inverse(entityToWorldMatrix); - glm::vec3 dimensions = entity->getDimensions(); - glm::vec3 registrationPoint = entity->getRegistrationPoint(); - glm::vec3 corner = -(dimensions * registrationPoint); + glm::vec3 dimensions = entity->getDimensions(); + glm::vec3 registrationPoint = entity->getRegistrationPoint(); + glm::vec3 corner = -(dimensions * registrationPoint); - AABox entityFrameBox(corner, dimensions); + AABox entityFrameBox(corner, dimensions); - glm::vec3 entityFrameOrigin = glm::vec3(worldToEntityMatrix * glm::vec4(origin, 1.0f)); - glm::vec3 entityFrameDirection = glm::vec3(worldToEntityMatrix * glm::vec4(direction, 0.0f)); + glm::vec3 entityFrameOrigin = glm::vec3(worldToEntityMatrix * glm::vec4(origin, 1.0f)); + glm::vec3 entityFrameDirection = glm::vec3(worldToEntityMatrix * glm::vec4(direction, 0.0f)); + + // we can use the AABox's ray intersection by mapping our origin and direction into the entity frame + // and testing intersection there. + if (entityFrameBox.findRayIntersection(entityFrameOrigin, entityFrameDirection, localDistance, localFace)) { + if (localDistance < distance) { + // now ask the entity if we actually intersect + if (entity->supportsDetailedRayIntersection()) { + if (entity->findDetailedRayIntersection(origin, direction, keepSearching, element, localDistance, + localFace, intersectedObject, precisionPicking)) { - // we can use the AABox's ray intersection by mapping our origin and direction into the entity frame - // and testing intersection there. - if (entityFrameBox.findRayIntersection(entityFrameOrigin, entityFrameDirection, localDistance, localFace)) { - if (localDistance < distance) { - // now ask the entity if we actually intersect - if (entity->supportsDetailedRayIntersection()) { - if (entity->findDetailedRayIntersection(origin, direction, keepSearching, element, localDistance, - localFace, intersectedObject, precisionPicking)) { - - if (localDistance < distance) { - distance = localDistance; - face = localFace; - *intersectedObject = (void*)entity.get(); - somethingIntersected = true; - } - } - } else { - // if the entity type doesn't support a detailed intersection, then just return the non-AABox results if (localDistance < distance) { distance = localDistance; face = localFace; @@ -535,75 +518,79 @@ bool EntityTreeElement::findDetailedRayIntersection(const glm::vec3& origin, con somethingIntersected = true; } } + } else { + // if the entity type doesn't support a detailed intersection, then just return the non-AABox results + if (localDistance < distance) { + distance = localDistance; + face = localFace; + *intersectedObject = (void*)entity.get(); + somethingIntersected = true; + } } } } - - ++entityItr; entityNumber++; - } + }); return somethingIntersected; } // TODO: change this to use better bounding shape for entity than sphere bool EntityTreeElement::findSpherePenetration(const glm::vec3& center, float radius, glm::vec3& penetration, void** penetratedObject) const { - EntityItems::iterator entityItr = _entityItems->begin(); - EntityItems::const_iterator entityEnd = _entityItems->end(); - while(entityItr != entityEnd) { - EntityItemPointer entity = (*entityItr); - glm::vec3 entityCenter = entity->getPosition(); - float entityRadius = entity->getRadius(); + bool result = false; + withReadLock([&] { + foreach(EntityItemPointer entity, _entityItems) { + glm::vec3 entityCenter = entity->getPosition(); + float entityRadius = entity->getRadius(); - // don't penetrate yourself - if (entityCenter == center && entityRadius == radius) { - return false; - } + // don't penetrate yourself + if (entityCenter == center && entityRadius == radius) { + return; + } - if (findSphereSpherePenetration(center, radius, entityCenter, entityRadius, penetration)) { - // return true on first valid entity penetration - - *penetratedObject = (void*)(entity.get()); - - return true; + if (findSphereSpherePenetration(center, radius, entityCenter, entityRadius, penetration)) { + // return true on first valid entity penetration + *penetratedObject = (void*)(entity.get()); + + result = true; + return; + } } - ++entityItr; - } - return false; + }); + return result; } EntityItemPointer EntityTreeElement::getClosestEntity(glm::vec3 position) const { EntityItemPointer closestEntity = NULL; float closestEntityDistance = FLT_MAX; - uint16_t numberOfEntities = _entityItems->size(); - for (uint16_t i = 0; i < numberOfEntities; i++) { - float distanceToEntity = glm::distance(position, (*_entityItems)[i]->getPosition()); - if (distanceToEntity < closestEntityDistance) { - closestEntity = (*_entityItems)[i]; + withReadLock([&] { + foreach(EntityItemPointer entity, _entityItems) { + float distanceToEntity = glm::distance2(position, entity->getPosition()); + if (distanceToEntity < closestEntityDistance) { + closestEntity = entity; + } } - } + }); return closestEntity; } // TODO: change this to use better bounding shape for entity than sphere void EntityTreeElement::getEntities(const glm::vec3& searchPosition, float searchRadius, QVector& foundEntities) const { - uint16_t numberOfEntities = _entityItems->size(); - for (uint16_t i = 0; i < numberOfEntities; i++) { - EntityItemPointer entity = (*_entityItems)[i]; - float distance = glm::length(entity->getPosition() - searchPosition); - if (distance < searchRadius + entity->getRadius()) { + float compareRadius = searchRadius * searchRadius; + forEachEntity([&](EntityItemPointer entity) { + // For iteration like this, avoid the use of square roots by comparing distances squared + float distanceSquared = glm::length2(entity->getPosition() - searchPosition); + float otherRadius = entity->getRadius(); + if (distanceSquared < (compareRadius + (otherRadius * otherRadius))) { foundEntities.push_back(entity); } - } + }); } // TODO: change this to use better bounding shape for entity than sphere void EntityTreeElement::getEntities(const AACube& box, QVector& foundEntities) { - EntityItems::iterator entityItr = _entityItems->begin(); - EntityItems::iterator entityEnd = _entityItems->end(); AACube entityCube; - while(entityItr != entityEnd) { - EntityItemPointer entity = (*entityItr); + forEachEntity([&](EntityItemPointer entity) { float radius = entity->getRadius(); // NOTE: we actually do cube-cube collision queries here, which is sloppy but good enough for now // TODO: decide whether to replace entityCube-cube query with sphere-cube (requires a square root @@ -612,64 +599,57 @@ void EntityTreeElement::getEntities(const AACube& box, QVectorsize(); - for (uint16_t i = 0; i < numberOfEntities; i++) { - if ((*_entityItems)[i]->getEntityItemID() == id) { - foundEntity = (*_entityItems)[i]; - break; + withReadLock([&] { + foreach(EntityItemPointer entity, _entityItems) { + if (entity->getEntityItemID() == id) { + foundEntity = entity; + break; + } } - } - return foundEntity; -} - -EntityItemPointer EntityTreeElement::getEntityWithEntityItemID(const EntityItemID& id) { - EntityItemPointer foundEntity = NULL; - uint16_t numberOfEntities = _entityItems->size(); - for (uint16_t i = 0; i < numberOfEntities; i++) { - if ((*_entityItems)[i]->getEntityItemID() == id) { - foundEntity = (*_entityItems)[i]; - break; - } - } + }); return foundEntity; } void EntityTreeElement::cleanupEntities() { - uint16_t numberOfEntities = _entityItems->size(); - for (uint16_t i = 0; i < numberOfEntities; i++) { - EntityItemPointer entity = (*_entityItems)[i]; - entity->_element = NULL; - - // NOTE: We explicitly don't delete the EntityItem here because since we only - // access it by smart pointers, when we remove it from the _entityItems - // we know that it will be deleted. - //delete entity; - } - _entityItems->clear(); + withWriteLock([&] { + foreach(EntityItemPointer entity, _entityItems) { + // NOTE: We explicitly don't delete the EntityItem here because since we only + // access it by smart pointers, when we remove it from the _entityItems + // we know that it will be deleted. + //delete entity; + entity->_element = NULL; + } + _entityItems.clear(); + }); } bool EntityTreeElement::removeEntityWithEntityItemID(const EntityItemID& id) { bool foundEntity = false; - uint16_t numberOfEntities = _entityItems->size(); - for (uint16_t i = 0; i < numberOfEntities; i++) { - if ((*_entityItems)[i]->getEntityItemID() == id) { - foundEntity = true; - (*_entityItems)[i]->_element = NULL; - _entityItems->removeAt(i); - break; + withWriteLock([&] { + uint16_t numberOfEntities = _entityItems.size(); + for (uint16_t i = 0; i < numberOfEntities; i++) { + EntityItemPointer& entity = _entityItems[i]; + if (entity->getEntityItemID() == id) { + foundEntity = true; + entity->_element = NULL; + _entityItems.removeAt(i); + break; + } } - } + }); return foundEntity; } bool EntityTreeElement::removeEntityItem(EntityItemPointer entity) { - int numEntries = _entityItems->removeAll(entity); + int numEntries = 0; + withWriteLock([&] { + numEntries = _entityItems.removeAll(entity); + }); if (numEntries > 0) { assert(entity->_element == this); entity->_element = NULL; @@ -791,7 +771,9 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int void EntityTreeElement::addEntityItem(EntityItemPointer entity) { assert(entity); assert(entity->_element == NULL); - _entityItems->push_back(entity); + withWriteLock([&] { + _entityItems.push_back(entity); + }); entity->_element = this; } @@ -824,30 +806,39 @@ bool EntityTreeElement::pruneChildren() { } void EntityTreeElement::expandExtentsToContents(Extents& extents) { - if (_entityItems->size()) { - for (uint16_t i = 0; i < _entityItems->size(); i++) { - EntityItemPointer entity = (*_entityItems)[i]; + withReadLock([&] { + foreach(EntityItemPointer entity, _entityItems) { extents.add(entity->getAABox()); } - } + }); } +uint16_t EntityTreeElement::size() const { + uint16_t result = 0; + withReadLock([&] { + result = _entityItems.size(); + }); + return result; +} void EntityTreeElement::debugDump() { qCDebug(entities) << "EntityTreeElement..."; qCDebug(entities) << " cube:" << _cube; qCDebug(entities) << " has child elements:" << getChildCount(); - if (_entityItems->size()) { - qCDebug(entities) << " has entities:" << _entityItems->size(); - qCDebug(entities) << "--------------------------------------------------"; - for (uint16_t i = 0; i < _entityItems->size(); i++) { - EntityItemPointer entity = (*_entityItems)[i]; - entity->debugDump(); + + withReadLock([&] { + if (_entityItems.size()) { + qCDebug(entities) << " has entities:" << _entityItems.size(); + qCDebug(entities) << "--------------------------------------------------"; + for (uint16_t i = 0; i < _entityItems.size(); i++) { + EntityItemPointer entity = _entityItems[i]; + entity->debugDump(); + } + qCDebug(entities) << "--------------------------------------------------"; + } else { + qCDebug(entities) << " NO entities!"; } - qCDebug(entities) << "--------------------------------------------------"; - } else { - qCDebug(entities) << " NO entities!"; - } + }); } diff --git a/libraries/entities/src/EntityTreeElement.h b/libraries/entities/src/EntityTreeElement.h index 05d1904384..0fe4175079 100644 --- a/libraries/entities/src/EntityTreeElement.h +++ b/libraries/entities/src/EntityTreeElement.h @@ -74,7 +74,7 @@ public: }; -class EntityTreeElement : public OctreeElement { +class EntityTreeElement : public OctreeElement, ReadWriteLockable { friend class EntityTree; // to allow createElement to new us... EntityTreeElement(unsigned char* octalCode = NULL); @@ -146,10 +146,18 @@ public: virtual bool findSpherePenetration(const glm::vec3& center, float radius, glm::vec3& penetration, void** penetratedObject) const; - const EntityItems& getEntities() const { return *_entityItems; } - EntityItems& getEntities() { return *_entityItems; } - bool hasEntities() const { return _entityItems ? _entityItems->size() > 0 : false; } + template + void forEachEntity(F f) const { + withReadLock([&] { + foreach(EntityItemPointer entityItem, _entityItems) { + f(entityItem); + } + }); + } + + virtual uint16_t size() const; + bool hasEntities() const { return size() > 0; } void setTree(EntityTree* tree) { _myTree = tree; } EntityTree* getTree() const { return _myTree; } @@ -174,8 +182,6 @@ public: EntityItemPointer getEntityWithEntityItemID(const EntityItemID& id) const; void getEntitiesInside(const AACube& box, QVector& foundEntities); - EntityItemPointer getEntityWithEntityItemID(const EntityItemID& id); - void cleanupEntities(); /// called by EntityTree on cleanup this will free all entities bool removeEntityWithEntityItemID(const EntityItemID& id); bool removeEntityItem(EntityItemPointer entity); @@ -204,7 +210,7 @@ public: protected: virtual void init(unsigned char * octalCode); EntityTree* _myTree; - EntityItems* _entityItems; + EntityItems _entityItems; }; #endif // hifi_EntityTreeElement_h diff --git a/libraries/entities/src/RecurseOctreeToMapOperator.cpp b/libraries/entities/src/RecurseOctreeToMapOperator.cpp index 167e3513c1..060e874c85 100644 --- a/libraries/entities/src/RecurseOctreeToMapOperator.cpp +++ b/libraries/entities/src/RecurseOctreeToMapOperator.cpp @@ -43,11 +43,9 @@ bool RecurseOctreeToMapOperator::postRecursion(OctreeElement* element) { EntityItemProperties defaultProperties; EntityTreeElement* entityTreeElement = static_cast(element); - const EntityItems& entities = entityTreeElement->getEntities(); - QVariantList entitiesQList = qvariant_cast(_map["Entities"]); - foreach (EntityItemPointer entityItem, entities) { + entityTreeElement->forEachEntity([&](EntityItemPointer entityItem) { EntityItemProperties properties = entityItem->getProperties(); QScriptValue qScriptValues; if (_skipDefaultValues) { @@ -56,7 +54,8 @@ bool RecurseOctreeToMapOperator::postRecursion(OctreeElement* element) { qScriptValues = EntityItemPropertiesToScriptValue(_engine, properties); } entitiesQList << qScriptValues.toVariant(); - } + }); + _map["Entities"] = entitiesQList; if (element == _top) { _withinTop = false;