From 1a73485e36400c76c446115b3a871bbb6d742533 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Mon, 29 Jul 2013 15:30:30 -0700 Subject: [PATCH] Improved VoxelNode delete callback based on CR feedback - changed hooks to use a virtual base class approach - switched account of hooks to use a vector instead of home grown solution - added support for VoxelNode to know what VoxelSystem it belongs to --- interface/src/VoxelSystem.cpp | 15 +++++----- interface/src/VoxelSystem.h | 5 ++-- libraries/voxels/src/VoxelNode.cpp | 43 ++++++++------------------- libraries/voxels/src/VoxelNode.h | 29 ++++++++++-------- libraries/voxels/src/VoxelNodeBag.cpp | 11 +++---- libraries/voxels/src/VoxelNodeBag.h | 4 ++- 6 files changed, 48 insertions(+), 59 deletions(-) diff --git a/interface/src/VoxelSystem.cpp b/interface/src/VoxelSystem.cpp index af3f7ce5c2..2d1bfb577c 100644 --- a/interface/src/VoxelSystem.cpp +++ b/interface/src/VoxelSystem.cpp @@ -59,15 +59,13 @@ VoxelSystem::VoxelSystem(float treeScale, int maxVoxels) : pthread_mutex_init(&_bufferWriteLock, NULL); pthread_mutex_init(&_treeLock, NULL); - _hookID = VoxelNode::addDeleteHook(voxelNodeDeleteHook, this); + VoxelNode::addDeleteHook(this); _abandonedVBOSlots = 0; } -void VoxelSystem::voxelNodeDeleteHook(VoxelNode* node, void* extraData) { - VoxelSystem* theSystem = (VoxelSystem*)extraData; - - if (node->isKnownBufferIndex()) { - theSystem->freeBufferIndex(node->getBufferIndex()); +void VoxelSystem::nodeDeleted(VoxelNode* node) { + if (node->isKnownBufferIndex() && (node->getVoxelSystem() == this)) { + freeBufferIndex(node->getBufferIndex()); } } @@ -99,7 +97,7 @@ VoxelSystem::~VoxelSystem() { pthread_mutex_destroy(&_bufferWriteLock); pthread_mutex_destroy(&_treeLock); - VoxelNode::removeDeleteHook(_hookID); + VoxelNode::removeDeleteHook(this); } void VoxelSystem::loadVoxelsFile(const char* fileName, bool wantColorRandomizer) { @@ -401,11 +399,13 @@ int VoxelSystem::updateNodeInArraysAsFullVBO(VoxelNode* node) { // and RGB color for each added vertex updateNodeInArrays(nodeIndex, startVertex, voxelScale, node->getColor()); node->setBufferIndex(nodeIndex); + node->setVoxelSystem(this); _writeVoxelDirtyArray[nodeIndex] = true; // just in case we switch to Partial mode _voxelsInWriteArrays++; // our know vertices in the arrays return 1; // rendered } else { node->setBufferIndex(GLBUFFER_INDEX_UNKNOWN); + node->setVoxelSystem(NULL); } return 0; // not-rendered @@ -440,6 +440,7 @@ int VoxelSystem::updateNodeInArraysAsPartialVBO(VoxelNode* node) { } else { nodeIndex = _voxelsInWriteArrays; node->setBufferIndex(nodeIndex); + node->setVoxelSystem(this); _voxelsInWriteArrays++; } _writeVoxelDirtyArray[nodeIndex] = true; diff --git a/interface/src/VoxelSystem.h b/interface/src/VoxelSystem.h index 1e47cda52a..ca390a5a20 100644 --- a/interface/src/VoxelSystem.h +++ b/interface/src/VoxelSystem.h @@ -28,7 +28,7 @@ class ProgramObject; const int NUM_CHILDREN = 8; -class VoxelSystem : public NodeData { +class VoxelSystem : public NodeData, public VoxelNodeDeleteHook { public: VoxelSystem(float treeScale = TREE_SCALE, int maxVoxels = MAX_VOXELS_PER_SYSTEM); ~VoxelSystem(); @@ -92,6 +92,8 @@ public: CoverageMapV2 myCoverageMapV2; CoverageMap myCoverageMap; + + virtual void nodeDeleted(VoxelNode* node); protected: float _treeScale; @@ -191,7 +193,6 @@ private: int _hookID; std::vector _freeIndexes; - static void voxelNodeDeleteHook(VoxelNode* node, void* extraData); void freeBufferIndex(glBufferIndex index); void clearFreeBufferIndexes(); }; diff --git a/libraries/voxels/src/VoxelNode.cpp b/libraries/voxels/src/VoxelNode.cpp index 4a0f017326..8a76d9dfe8 100644 --- a/libraries/voxels/src/VoxelNode.cpp +++ b/libraries/voxels/src/VoxelNode.cpp @@ -48,6 +48,7 @@ void VoxelNode::init(unsigned char * octalCode) { _subtreeLeafNodeCount = 0; // that's me _glBufferIndex = GLBUFFER_INDEX_UNKNOWN; + _voxelSystem = NULL; _isDirty = true; _shouldRender = false; markWithChangedTime(); @@ -399,43 +400,23 @@ float VoxelNode::distanceToPoint(const glm::vec3& point) const { return distance; } -VoxelNodeDeleteHook VoxelNode::_hooks[VOXEL_NODE_MAX_DELETE_HOOKS]; -void* VoxelNode::_hooksExtraData[VOXEL_NODE_MAX_DELETE_HOOKS]; -int VoxelNode::_hooksInUse = 0; +std::vector VoxelNode::_hooks; -int VoxelNode::addDeleteHook(VoxelNodeDeleteHook hook, void* extraData) { - // If first use, initialize the _hooks array - if (_hooksInUse == 0) { - memset(_hooks, 0, sizeof(_hooks)); - memset(_hooksExtraData, 0, sizeof(_hooksExtraData)); - } - // find first available slot - for (int i = 0; i < VOXEL_NODE_MAX_DELETE_HOOKS; i++) { - if (!_hooks[i]) { - _hooks[i] = hook; - _hooksExtraData[i] = extraData; - _hooksInUse++; - return i; - } - } - // if we got here, then we're out of room in our hooks, return error - return VOXEL_NODE_NO_MORE_HOOKS_AVAILABLE; +void VoxelNode::addDeleteHook(VoxelNodeDeleteHook* hook) { + _hooks.push_back(hook); } -void VoxelNode::removeDeleteHook(int hookID) { - if (_hooks[hookID]) { - _hooks[hookID] = NULL; - _hooksExtraData[hookID] = NULL; - _hooksInUse--; +void VoxelNode::removeDeleteHook(VoxelNodeDeleteHook* hook) { + for (int i = 0; i < _hooks.size(); i++) { + if (_hooks[i] == hook) { + _hooks.erase(_hooks.begin() + i); + return; + } } } void VoxelNode::notifyDeleteHooks() { - if (_hooksInUse > 0) { - for (int i = 0; i < VOXEL_NODE_MAX_DELETE_HOOKS; i++) { - if (_hooks[i]) { - _hooks[i](this, _hooksExtraData[i]); - } - } + for (int i = 0; i < _hooks.size(); i++) { + _hooks[i]->nodeDeleted(this); } } diff --git a/libraries/voxels/src/VoxelNode.h b/libraries/voxels/src/VoxelNode.h index 1efab362de..38e9627848 100644 --- a/libraries/voxels/src/VoxelNode.h +++ b/libraries/voxels/src/VoxelNode.h @@ -14,18 +14,19 @@ #include "ViewFrustum.h" #include "VoxelConstants.h" -class VoxelTree; // forward delclaration -class VoxelNode; // forward delclaration +class VoxelTree; // forward declaration +class VoxelNode; // forward declaration +class VoxelSystem; // forward declaration typedef unsigned char colorPart; typedef unsigned char nodeColor[4]; typedef unsigned char rgbColor[3]; -// Callback function, for delete hook -typedef void (*VoxelNodeDeleteHook)(VoxelNode* node, void* extraData); -const int VOXEL_NODE_MAX_DELETE_HOOKS = 100; -const int VOXEL_NODE_NO_MORE_HOOKS_AVAILABLE = -1; - +// Callers who want delete hook callbacks should implement this class +class VoxelNodeDeleteHook { +public: + virtual void nodeDeleted(VoxelNode* node) = 0; +}; class VoxelNode { public: @@ -77,6 +78,9 @@ public: glBufferIndex getBufferIndex() const { return _glBufferIndex; }; bool isKnownBufferIndex() const { return (_glBufferIndex != GLBUFFER_INDEX_UNKNOWN); }; void setBufferIndex(glBufferIndex index) { _glBufferIndex = index; }; + VoxelSystem* getVoxelSystem() const { return _voxelSystem; }; + void setVoxelSystem(VoxelSystem* voxelSystem) { _voxelSystem = voxelSystem; }; + // Used by VoxelSystem for rendering in/out of view and LOD void setShouldRender(bool shouldRender); @@ -101,8 +105,8 @@ public: const nodeColor& getColor() const { return _trueColor; }; #endif - static int addDeleteHook(VoxelNodeDeleteHook hook, void* extraData = NULL); - static void removeDeleteHook(int hookID); + static void addDeleteHook(VoxelNodeDeleteHook* hook); + static void removeDeleteHook(VoxelNodeDeleteHook* hook); void recalculateSubTreeNodeCount(); unsigned long getSubTreeNodeCount() const { return _subtreeNodeCount; }; @@ -120,6 +124,7 @@ private: bool _falseColored; #endif glBufferIndex _glBufferIndex; + VoxelSystem* _voxelSystem; bool _isDirty; uint64_t _lastChanged; bool _shouldRender; @@ -130,10 +135,8 @@ private: unsigned long _subtreeNodeCount; unsigned long _subtreeLeafNodeCount; float _density; // If leaf: density = 1, if internal node: 0-1 density of voxels inside - - static VoxelNodeDeleteHook _hooks[VOXEL_NODE_MAX_DELETE_HOOKS]; - static void* _hooksExtraData[VOXEL_NODE_MAX_DELETE_HOOKS]; - static int _hooksInUse; + + static std::vector _hooks; }; #endif /* defined(__hifi__VoxelNode__) */ diff --git a/libraries/voxels/src/VoxelNodeBag.cpp b/libraries/voxels/src/VoxelNodeBag.cpp index 713f4de1dd..6b69edd32c 100644 --- a/libraries/voxels/src/VoxelNodeBag.cpp +++ b/libraries/voxels/src/VoxelNodeBag.cpp @@ -13,11 +13,11 @@ VoxelNodeBag::VoxelNodeBag() : _bagElements(NULL), _elementsInUse(0), _sizeOfElementsArray(0) { - _hookID = VoxelNode::addDeleteHook(voxelNodeDeleteHook, this); + VoxelNode::addDeleteHook(this); }; VoxelNodeBag::~VoxelNodeBag() { - VoxelNode::removeDeleteHook(_hookID); + VoxelNode::removeDeleteHook(this); deleteAll(); } @@ -125,9 +125,10 @@ void VoxelNodeBag::remove(VoxelNode* node) { _elementsInUse--; } } -void VoxelNodeBag::voxelNodeDeleteHook(VoxelNode* node, void* extraData) { - VoxelNodeBag* theBag = (VoxelNodeBag*)extraData; - theBag->remove(node); // note: remove can safely handle nodes that aren't in it, so we don't need to check contains() + + +void VoxelNodeBag::nodeDeleted(VoxelNode* node) { + remove(node); // note: remove can safely handle nodes that aren't in it, so we don't need to check contains() } diff --git a/libraries/voxels/src/VoxelNodeBag.h b/libraries/voxels/src/VoxelNodeBag.h index 27bd4e5b60..a29e7678c9 100644 --- a/libraries/voxels/src/VoxelNodeBag.h +++ b/libraries/voxels/src/VoxelNodeBag.h @@ -16,7 +16,7 @@ #include "VoxelNode.h" -class VoxelNodeBag { +class VoxelNodeBag : public VoxelNodeDeleteHook { public: VoxelNodeBag(); @@ -34,6 +34,8 @@ public: static void voxelNodeDeleteHook(VoxelNode* node, void* extraData); + virtual void nodeDeleted(VoxelNode* node); + private: VoxelNode** _bagElements;