From 4aaa9ca02f10f6d51c524e2423a5e59fa1889a62 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Mon, 30 Nov 2015 15:23:49 -0800 Subject: [PATCH] Make OctreeElementBag safer to use --- .../src/octree/OctreeQueryNode.cpp | 6 ++-- .../src/octree/OctreeSendThread.cpp | 4 +-- libraries/entities/src/EntityTreeElement.cpp | 2 +- libraries/entities/src/EntityTreeElement.h | 2 +- libraries/octree/src/Octree.cpp | 6 ++-- libraries/octree/src/Octree.h | 17 +++++------ libraries/octree/src/OctreeElement.h | 3 +- libraries/octree/src/OctreeElementBag.cpp | 28 +++++++++---------- libraries/octree/src/OctreeElementBag.h | 17 ++++++----- 9 files changed, 37 insertions(+), 48 deletions(-) diff --git a/assignment-client/src/octree/OctreeQueryNode.cpp b/assignment-client/src/octree/OctreeQueryNode.cpp index 10f1d2ec14..cff2c7ee2e 100644 --- a/assignment-client/src/octree/OctreeQueryNode.cpp +++ b/assignment-client/src/octree/OctreeQueryNode.cpp @@ -336,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++; @@ -346,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..84caf9ce81 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -417,7 +417,7 @@ int OctreeSendThread::packetDistributor(OctreeQueryNode* nodeData, bool viewFrus quint64 startInside = usecTimestampNow(); bool lastNodeDidntFit = false; // assume each node fits - if (!nodeData->elementBag.isEmpty()) { + if (OctreeElementPointer subTree = nodeData->elementBag.extract()) { quint64 lockWaitStart = usecTimestampNow(); _myServer->getOctree()->withReadLock([&]{ @@ -425,8 +425,6 @@ int OctreeSendThread::packetDistributor(OctreeQueryNode* nodeData, bool viewFrus lockWaitElapsedUsec = (float)(lockWaitEnd - lockWaitStart); quint64 encodeStart = usecTimestampNow(); - OctreeElementPointer subTree = nodeData->elementBag.extract(); - /* 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/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 8403db8b2f..ff2e97e8fe 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -153,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.h b/libraries/octree/src/OctreeElement.h index c686970c3a..3c25ec0850 100644 --- a/libraries/octree/src/OctreeElement.h +++ b/libraries/octree/src/OctreeElement.h @@ -32,7 +32,6 @@ using AtomicUIntStat = std::atomic; class EncodeBitstreamParams; class Octree; class OctreeElement; -class OctreeElementBag; class OctreePacketData; class ReadBitstreamToTreeParams; class Shape; @@ -91,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 diff --git a/libraries/octree/src/OctreeElementBag.cpp b/libraries/octree/src/OctreeElementBag.cpp index 167c3560d6..4634c05a06 100644 --- a/libraries/octree/src/OctreeElementBag.cpp +++ b/libraries/octree/src/OctreeElementBag.cpp @@ -13,29 +13,29 @@ #include 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.get(), element); + _bagElements.push(element); } OctreeElementPointer OctreeElementBag::extract() { OctreeElementPointer result; // Find the first element still alive - while (!_bagElements.empty() && !result) { - auto it = _bagElements.begin(); - result = it->lock(); - _bagElements.erase(it); + 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.get()); -} - -void OctreeElementBag::remove(OctreeElementPointer element) { - _bagElements.remove(element.get()); -} diff --git a/libraries/octree/src/OctreeElementBag.h b/libraries/octree/src/OctreeElementBag.h index c0bfa15472..02423b640c 100644 --- a/libraries/octree/src/OctreeElementBag.h +++ b/libraries/octree/src/OctreeElementBag.h @@ -16,25 +16,24 @@ #ifndef hifi_OctreeElementBag_h #define hifi_OctreeElementBag_h +#include + #include "OctreeElement.h" -#include class OctreeElementBag { + using Bag = std::queue; + public: - 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(); private: - QHash _bagElements; + Bag _bagElements; }; -typedef QMap OctreeElementExtraEncodeData; +using OctreeElementExtraEncodeData = QMap; #endif // hifi_OctreeElementBag_h