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

@@ -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) {
+void VoxelSystem::nodeDeleted(VoxelNode* node) {
-    VoxelSystem* theSystem = (VoxelSystem*)extraData;
+    if (node->isKnownBufferIndex() && (node->getVoxelSystem() == this)) {
-    
+        freeBufferIndex(node->getBufferIndex());
-    if (node->isKnownBufferIndex()) {
-        theSystem->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;

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<glBufferIndex> _freeIndexes;
 
-    static void voxelNodeDeleteHook(VoxelNode* node, void* extraData);
     void freeBufferIndex(glBufferIndex index);
     void clearFreeBufferIndexes();
 };

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];
+std::vector<VoxelNodeDeleteHook*> VoxelNode::_hooks;
-void* VoxelNode::_hooksExtraData[VOXEL_NODE_MAX_DELETE_HOOKS];
-int VoxelNode::_hooksInUse = 0;
 
-int VoxelNode::addDeleteHook(VoxelNodeDeleteHook hook, void* extraData) {
+void VoxelNode::addDeleteHook(VoxelNodeDeleteHook* hook) {
-    // If first use, initialize the _hooks array
+    _hooks.push_back(hook);
-    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::removeDeleteHook(int hookID) {
+void VoxelNode::removeDeleteHook(VoxelNodeDeleteHook* hook) {
-    if (_hooks[hookID]) {
+    for (int i = 0; i < _hooks.size(); i++) {
-        _hooks[hookID] = NULL;
+        if (_hooks[i] == hook) {
-        _hooksExtraData[hookID] = NULL;
+            _hooks.erase(_hooks.begin() + i);
-        _hooksInUse--;
+            return;
+        }
     }
 }
 
 void VoxelNode::notifyDeleteHooks() {
-    if (_hooksInUse > 0) {
+    for (int i = 0; i < _hooks.size(); i++) {
-        for (int i = 0; i < VOXEL_NODE_MAX_DELETE_HOOKS; i++) {
+        _hooks[i]->nodeDeleted(this);
-            if (_hooks[i]) {
-                _hooks[i](this, _hooksExtraData[i]);
-            }
-        }
     }
 }

libraries/voxels/src/VoxelNode.h

@@ -14,18 +14,19 @@
 #include "ViewFrustum.h"
 #include "VoxelConstants.h"
 
-class VoxelTree; // forward delclaration
+class VoxelTree; // forward declaration
-class VoxelNode; // forward delclaration
+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
+// Callers who want delete hook callbacks should implement this class
-typedef void (*VoxelNodeDeleteHook)(VoxelNode* node, void* extraData);
+class VoxelNodeDeleteHook {
-const int VOXEL_NODE_MAX_DELETE_HOOKS = 100;
+public:
-const int VOXEL_NODE_NO_MORE_HOOKS_AVAILABLE = -1;
+    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 addDeleteHook(VoxelNodeDeleteHook* hook);
-    static void removeDeleteHook(int hookID);
+    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 std::vector<VoxelNodeDeleteHook*> _hooks;
-    static void*                _hooksExtraData[VOXEL_NODE_MAX_DELETE_HOOKS];
-    static int                  _hooksInUse;
 };
 
 #endif /* defined(__hifi__VoxelNode__) */

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()
 }
 
 

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;