diff --git a/assignment-client/src/octree/OctreeQueryNode.cpp b/assignment-client/src/octree/OctreeQueryNode.cpp index f70ff62f91..cff2c7ee2e 100644 --- a/assignment-client/src/octree/OctreeQueryNode.cpp +++ b/assignment-client/src/octree/OctreeQueryNode.cpp @@ -59,7 +59,6 @@ OctreeQueryNode::~OctreeQueryNode() { void OctreeQueryNode::nodeKilled() { _isShuttingDown = true; - elementBag.unhookNotifications(); // if our node is shutting down, then we no longer need octree element notifications if (_octreeSendThread) { // just tell our thread we want to shutdown, this is asynchronous, and fast, we don't need or want it to block // while the thread actually shuts down @@ -69,7 +68,6 @@ void OctreeQueryNode::nodeKilled() { void OctreeQueryNode::forceNodeShutdown() { _isShuttingDown = true; - elementBag.unhookNotifications(); // if our node is shutting down, then we no longer need octree element notifications if (_octreeSendThread) { // we really need to force our thread to shutdown, this is synchronous, we will block while the thread actually // shuts down because we really need it to shutdown, and it's ok if we wait for it to complete @@ -338,8 +336,7 @@ void OctreeQueryNode::dumpOutOfView() { int stillInView = 0; int outOfView = 0; OctreeElementBag tempBag; - while (!elementBag.isEmpty()) { - OctreeElementPointer elementToCheck = elementBag.extract(); + while (OctreeElementPointer elementToCheck = elementBag.extract()) { if (elementToCheck->isInView(_currentViewFrustum)) { tempBag.insert(elementToCheck); stillInView++; @@ -348,8 +345,7 @@ void OctreeQueryNode::dumpOutOfView() { } } if (stillInView > 0) { - while (!tempBag.isEmpty()) { - OctreeElementPointer elementToKeepInBag = tempBag.extract(); + while (OctreeElementPointer elementToKeepInBag = tempBag.extract()) { if (elementToKeepInBag->isInView(_currentViewFrustum)) { elementBag.insert(elementToKeepInBag); } diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 7c8d8f0e01..94d82b463e 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -424,8 +424,11 @@ int OctreeSendThread::packetDistributor(OctreeQueryNode* nodeData, bool viewFrus quint64 lockWaitEnd = usecTimestampNow(); lockWaitElapsedUsec = (float)(lockWaitEnd - lockWaitStart); quint64 encodeStart = usecTimestampNow(); - + OctreeElementPointer subTree = nodeData->elementBag.extract(); + if (!subTree) { + return; + } /* TODO: Looking for a way to prevent locking and encoding a tree that is not // going to result in any packets being sent... diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index 6e640942e7..6775e56820 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -17,7 +17,6 @@ #include #include -#include #include "OctreeQueryNode.h" diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 9eac14e4b4..5ac892de94 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -44,12 +44,7 @@ void EntityTree::createRootElement() { } OctreeElementPointer EntityTree::createNewElement(unsigned char* octalCode) { - EntityTreeElementPointer newElement = EntityTreeElementPointer(new EntityTreeElement(octalCode), - // see comment int EntityTreeElement::createNewElement - [=](EntityTreeElement* dyingElement) { - EntityTreeElementPointer tmpSharedPointer(dyingElement); - dyingElement->notifyDeleteHooks(); - }); + auto newElement = EntityTreeElementPointer(new EntityTreeElement(octalCode)); newElement->setTree(std::static_pointer_cast(shared_from_this())); return std::static_pointer_cast(newElement); } diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 02552ef488..ff2e97e8fe 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -28,29 +28,8 @@ EntityTreeElement::~EntityTreeElement() { _octreeMemoryUsage -= sizeof(EntityTreeElement); } -// This will be called primarily on addChildAt(), which means we're adding a child of our -// own type to our own tree. This means we should initialize that child with any tree and type -// specific settings that our children must have. OctreeElementPointer EntityTreeElement::createNewElement(unsigned char* octalCode) { - EntityTreeElementPointer newChild = - EntityTreeElementPointer(new EntityTreeElement(octalCode), - // This is a little bit horrible, but I haven't found a better way. The OctreeElement - // destructor used to call notifyDeleteHooks(), which calls zero or more of - // OctreeElementDeleteHook::elementDeleted - // which (now) expects an OctreeElementPointer argument. The destructor doesn't have - // access to the shared pointer (which has had its reference count drop to zero, - // or the destructor wouldn't have been called). The destructor also can't - // make a new shared pointer -- shared_from_this() is forbidden in a destructor, and - // using OctreeElementPointer(this) also fails. So, I've installed a custom deleter: - [=](EntityTreeElement* dyingElement) { - // make a new shared pointer with a reference count of 1 (and no custom deleter) - EntityTreeElementPointer tmpSharedPointer(dyingElement); - // call notifyDeleteHooks which will use shared_from_this() to get this same - // shared pointer, for use with the elementDeleted calls. - dyingElement->notifyDeleteHooks(); - // And now tmpSharedPointer's reference count drops to zero and the - // normal destructors are called. - }); + auto newChild = EntityTreeElementPointer(new EntityTreeElement(octalCode)); newChild->setTree(_myTree); return newChild; } @@ -174,7 +153,7 @@ void EntityTreeElement::updateEncodedData(int childIndex, AppendState childAppen -void EntityTreeElement::elementEncodeComplete(EncodeBitstreamParams& params, OctreeElementBag* bag) const { +void EntityTreeElement::elementEncodeComplete(EncodeBitstreamParams& params) const { const bool wantDebug = false; if (wantDebug) { diff --git a/libraries/entities/src/EntityTreeElement.h b/libraries/entities/src/EntityTreeElement.h index 57e0bea696..d8a182156d 100644 --- a/libraries/entities/src/EntityTreeElement.h +++ b/libraries/entities/src/EntityTreeElement.h @@ -122,7 +122,7 @@ public: virtual bool shouldIncludeChildData(int childIndex, EncodeBitstreamParams& params) const; virtual bool shouldRecurseChildTree(int childIndex, EncodeBitstreamParams& params) const; virtual void updateEncodedData(int childIndex, AppendState childAppendState, EncodeBitstreamParams& params) const; - virtual void elementEncodeComplete(EncodeBitstreamParams& params, OctreeElementBag* bag) const; + virtual void elementEncodeComplete(EncodeBitstreamParams& params) const; bool alreadyFullyEncoded(EncodeBitstreamParams& params) const; diff --git a/libraries/octree/src/Octree.cpp b/libraries/octree/src/Octree.cpp index 6f73be360f..c02a034778 100644 --- a/libraries/octree/src/Octree.cpp +++ b/libraries/octree/src/Octree.cpp @@ -1736,7 +1736,7 @@ int Octree::encodeTreeBitstreamRecursion(OctreeElementPointer element, // If our element is completed let the element know so it can do any cleanup it of extra wants if (elementAppendState == OctreeElement::COMPLETED) { - element->elementEncodeComplete(params, &bag); + element->elementEncodeComplete(params); } return bytesAtThisLevel; @@ -2104,9 +2104,7 @@ void Octree::writeToSVOFile(const char* fileName, OctreeElementPointer element) int bytesWritten = 0; bool lastPacketWritten = false; - while (!elementBag.isEmpty()) { - OctreeElementPointer subTree = elementBag.extract(); - + while (OctreeElementPointer subTree = elementBag.extract()) { EncodeBitstreamParams params(INT_MAX, IGNORE_VIEW_FRUSTUM, WANT_COLOR, NO_EXISTS_BITS); withReadLock([&] { params.extraEncodeData = &extraEncodeData; diff --git a/libraries/octree/src/Octree.h b/libraries/octree/src/Octree.h index 5ebb991d49..d9cf17d7de 100644 --- a/libraries/octree/src/Octree.h +++ b/libraries/octree/src/Octree.h @@ -18,16 +18,6 @@ #include #include -class CoverageMap; -class ReadBitstreamToTreeParams; -class Octree; -class OctreeElement; -class OctreeElementBag; -class OctreePacketData; -class Shape; -typedef std::shared_ptr OctreePointer; - - #include #include @@ -38,6 +28,13 @@ typedef std::shared_ptr OctreePointer; #include "OctreePacketData.h" #include "OctreeSceneStats.h" +class CoverageMap; +class ReadBitstreamToTreeParams; +class Octree; +class OctreeElement; +class OctreePacketData; +class Shape; +using OctreePointer = std::shared_ptr; extern QVector PERSIST_EXTENSIONS; diff --git a/libraries/octree/src/OctreeElement.cpp b/libraries/octree/src/OctreeElement.cpp index 5f03627f1a..f16e1dc88d 100644 --- a/libraries/octree/src/OctreeElement.cpp +++ b/libraries/octree/src/OctreeElement.cpp @@ -95,10 +95,6 @@ void OctreeElement::init(unsigned char * octalCode) { } OctreeElement::~OctreeElement() { - // We can't call notifyDeleteHooks from here: - // notifyDeleteHooks(); - // see comment in EntityTreeElement::createNewElement. - assert(_deleteHooksNotified); _voxelNodeCount--; if (isLeaf()) { _voxelNodeLeafCount--; @@ -115,7 +111,6 @@ OctreeElement::~OctreeElement() { void OctreeElement::markWithChangedTime() { _lastChanged = usecTimestampNow(); - notifyUpdateHooks(); // if the node has changed, notify our hooks } // This method is called by Octree when the subtree below this node @@ -522,57 +517,6 @@ float OctreeElement::distanceToPoint(const glm::vec3& point) const { return distance; } -QReadWriteLock OctreeElement::_deleteHooksLock; -std::vector OctreeElement::_deleteHooks; - -void OctreeElement::addDeleteHook(OctreeElementDeleteHook* hook) { - _deleteHooksLock.lockForWrite(); - _deleteHooks.push_back(hook); - _deleteHooksLock.unlock(); -} - -void OctreeElement::removeDeleteHook(OctreeElementDeleteHook* hook) { - _deleteHooksLock.lockForWrite(); - for (unsigned int i = 0; i < _deleteHooks.size(); i++) { - if (_deleteHooks[i] == hook) { - _deleteHooks.erase(_deleteHooks.begin() + i); - break; - } - } - _deleteHooksLock.unlock(); -} - -void OctreeElement::notifyDeleteHooks() { - _deleteHooksLock.lockForRead(); - for (unsigned int i = 0; i < _deleteHooks.size(); i++) { - _deleteHooks[i]->elementDeleted(shared_from_this()); - } - _deleteHooksLock.unlock(); - _deleteHooksNotified = true; -} - -std::vector OctreeElement::_updateHooks; - -void OctreeElement::addUpdateHook(OctreeElementUpdateHook* hook) { - _updateHooks.push_back(hook); -} - -void OctreeElement::removeUpdateHook(OctreeElementUpdateHook* hook) { - for (unsigned int i = 0; i < _updateHooks.size(); i++) { - if (_updateHooks[i] == hook) { - _updateHooks.erase(_updateHooks.begin() + i); - return; - } - } -} - -void OctreeElement::notifyUpdateHooks() { - for (unsigned int i = 0; i < _updateHooks.size(); i++) { - _updateHooks[i]->elementUpdated(shared_from_this()); - } -} - - bool OctreeElement::findSpherePenetration(const glm::vec3& center, float radius, glm::vec3& penetration, void** penetratedObject) const { // center and radius are in meters, so we have to scale the _cube into world-frame diff --git a/libraries/octree/src/OctreeElement.h b/libraries/octree/src/OctreeElement.h index d705b64acd..3c25ec0850 100644 --- a/libraries/octree/src/OctreeElement.h +++ b/libraries/octree/src/OctreeElement.h @@ -32,28 +32,15 @@ using AtomicUIntStat = std::atomic; class EncodeBitstreamParams; class Octree; class OctreeElement; -class OctreeElementBag; -class OctreeElementDeleteHook; class OctreePacketData; class ReadBitstreamToTreeParams; class Shape; class VoxelSystem; -typedef std::shared_ptr OctreeElementPointer; -typedef std::shared_ptr ConstOctreeElementPointer; -typedef std::shared_ptr OctreePointer; - -// Callers who want delete hook callbacks should implement this class -class OctreeElementDeleteHook { -public: - virtual void elementDeleted(OctreeElementPointer element) = 0; -}; - -// Callers who want update hook callbacks should implement this class -class OctreeElementUpdateHook { -public: - virtual void elementUpdated(OctreeElementPointer element) = 0; -}; +using OctreeElementPointer = std::shared_ptr; +using OctreeElementWeakPointer = std::weak_ptr; +using ConstOctreeElementPointer = std::shared_ptr; +using OctreePointer = std::shared_ptr; class OctreeElement: public std::enable_shared_from_this { @@ -103,7 +90,7 @@ public: virtual bool shouldRecurseChildTree(int childIndex, EncodeBitstreamParams& params) const { return true; } virtual void updateEncodedData(int childIndex, AppendState childAppendState, EncodeBitstreamParams& params) const { } - virtual void elementEncodeComplete(EncodeBitstreamParams& params, OctreeElementBag* bag) const { } + virtual void elementEncodeComplete(EncodeBitstreamParams& params) const { } /// Override to serialize the state of this element. This is used for persistance and for transmission across the network. virtual AppendState appendElementData(OctreePacketData* packetData, EncodeBitstreamParams& params) const @@ -181,12 +168,6 @@ public: bool matchesSourceUUID(const QUuid& sourceUUID) const; static uint16_t getSourceNodeUUIDKey(const QUuid& sourceUUID); - static void addDeleteHook(OctreeElementDeleteHook* hook); - static void removeDeleteHook(OctreeElementDeleteHook* hook); - - static void addUpdateHook(OctreeElementUpdateHook* hook); - static void removeUpdateHook(OctreeElementUpdateHook* hook); - static void resetPopulationStatistics(); static unsigned long getNodeCount() { return _voxelNodeCount; } static unsigned long getInternalNodeCount() { return _voxelNodeCount - _voxelNodeLeafCount; } @@ -244,8 +225,6 @@ protected: void setChildAtIndex(int childIndex, OctreeElementPointer child); void calculateAACube(); - void notifyDeleteHooks(); - void notifyUpdateHooks(); AACube _cube; /// Client and server, axis aligned box for bounds of this voxel, 48 bytes @@ -287,14 +266,6 @@ protected: _unknownBufferIndex : 1, _childrenExternal : 1; /// Client only, is this voxel's VBO buffer the unknown buffer index, 1 bit - bool _deleteHooksNotified = false; - - static QReadWriteLock _deleteHooksLock; - static std::vector _deleteHooks; - - //static QReadWriteLock _updateHooksLock; - static std::vector _updateHooks; - static AtomicUIntStat _voxelNodeCount; static AtomicUIntStat _voxelNodeLeafCount; diff --git a/libraries/octree/src/OctreeElementBag.cpp b/libraries/octree/src/OctreeElementBag.cpp index 5af63c7bb1..4634c05a06 100644 --- a/libraries/octree/src/OctreeElementBag.cpp +++ b/libraries/octree/src/OctreeElementBag.cpp @@ -12,54 +12,30 @@ #include "OctreeElementBag.h" #include -OctreeElementBag::OctreeElementBag() : - _bagElements() -{ - OctreeElement::addDeleteHook(this); - _hooked = true; -} - -OctreeElementBag::~OctreeElementBag() { - unhookNotifications(); - deleteAll(); -} - -void OctreeElementBag::unhookNotifications() { - if (_hooked) { - OctreeElement::removeDeleteHook(this); - _hooked = false; - } -} - -void OctreeElementBag::elementDeleted(OctreeElementPointer element) { - remove(element); // note: remove can safely handle nodes that aren't in it, so we don't need to check contains() -} - - void OctreeElementBag::deleteAll() { - _bagElements.clear(); + _bagElements = Bag(); } +bool OctreeElementBag::isEmpty() { + // Pop all expired front elements + while (!_bagElements.empty() && _bagElements.front().expired()) { + _bagElements.pop(); + } + + return _bagElements.empty(); +} void OctreeElementBag::insert(OctreeElementPointer element) { - _bagElements.insert(element); + _bagElements.push(element); } OctreeElementPointer OctreeElementBag::extract() { - OctreeElementPointer result = NULL; + OctreeElementPointer result; - if (_bagElements.size() > 0) { - QSet::iterator front = _bagElements.begin(); - result = *front; - _bagElements.erase(front); + // Find the first element still alive + while (!result && !_bagElements.empty()) { + result = _bagElements.front().lock(); // Grab head's shared_ptr + _bagElements.pop(); } return result; } - -bool OctreeElementBag::contains(OctreeElementPointer element) { - return _bagElements.contains(element); -} - -void OctreeElementBag::remove(OctreeElementPointer element) { - _bagElements.remove(element); -} diff --git a/libraries/octree/src/OctreeElementBag.h b/libraries/octree/src/OctreeElementBag.h index 8ef01b44a2..02423b640c 100644 --- a/libraries/octree/src/OctreeElementBag.h +++ b/libraries/octree/src/OctreeElementBag.h @@ -16,31 +16,24 @@ #ifndef hifi_OctreeElementBag_h #define hifi_OctreeElementBag_h +#include + #include "OctreeElement.h" -class OctreeElementBag : public OctreeElementDeleteHook { - +class OctreeElementBag { + using Bag = std::queue; + public: - OctreeElementBag(); - ~OctreeElementBag(); - void insert(OctreeElementPointer element); // put a element into the bag OctreeElementPointer extract(); // pull a element out of the bag (could come in any order) - bool contains(OctreeElementPointer element); // is this element in the bag? - void remove(OctreeElementPointer element); // remove a specific element from the bag - bool isEmpty() const { return _bagElements.isEmpty(); } - int count() const { return _bagElements.size(); } - + bool isEmpty(); + void deleteAll(); - virtual void elementDeleted(OctreeElementPointer element); - - void unhookNotifications(); private: - QSet _bagElements; - bool _hooked; + Bag _bagElements; }; -typedef QMap OctreeElementExtraEncodeData; +using OctreeElementExtraEncodeData = QMap; #endif // hifi_OctreeElementBag_h