Merge pull request #495 from ZappoMan/recursive_voxel_delete

Fixes to delete voxel and color/add voxel to allow for unwinding color reaveraging

libraries/shared/src/SharedUtil.cpp

@@ -188,10 +188,6 @@ bool cmdOptionExists(int argc, const char * argv[],const char* option) {
 //
 // HACK ATTACK: Well, what if this is larger than the MTU? That's the caller's problem, we
 //              just truncate the message
-// Usage:       
-//                  unsigned char* voxelData = pointToVoxel(x,y,z,s,red,green,blue);
-//                  tree->readCodeColorBufferToTree(voxelData);
-//                  delete voxelData;
 //
 // Complaints:  Brad :)
 #define GUESS_OF_VOXELCODE_SIZE 10

libraries/voxels/src/VoxelNode.cpp

@@ -12,6 +12,7 @@
 #include "SharedUtil.h"
 #include "Log.h"
 #include "VoxelNode.h"
+#include "VoxelTree.h"
 #include "VoxelConstants.h"
 #include "OctalCode.h"
 #include "AABox.h"
@@ -46,7 +47,7 @@ void VoxelNode::init(unsigned char * octalCode) {
     _isDirty = true;
     _shouldRender = false;
     _isStagedForDeletion = false;
-    
+    markWithChangedTime();
     calculateAABox();
 }
 
@@ -61,11 +62,28 @@ VoxelNode::~VoxelNode() {
     }
 }
 
+// This method is called by VoxelTree when the subtree below this node
+// is known to have changed. It's intended to be used as a place to do
+// bookkeeping that a node may need to do when the subtree below it has
+// changed. However, you should hopefully make your bookkeeping relatively
+// localized, because this method will get called for every node in an
+// recursive unwinding case like delete or add voxel
+void VoxelNode::handleSubtreeChanged(VoxelTree* myTree) {
+    markWithChangedTime();
+    
+    // here's a good place to do color re-averaging...
+    if (myTree->getShouldReaverage()) {
+        setColorFromAverageOfChildren();
+    }
+}
+
+
 void VoxelNode::setShouldRender(bool shouldRender) {
     // if shouldRender is changing, then consider ourselves dirty
     if (shouldRender != _shouldRender) {
         _shouldRender = shouldRender;
         _isDirty = true;
+        markWithChangedTime();
     }
 }
 
@@ -89,6 +107,7 @@ void VoxelNode::deleteChildAtIndex(int childIndex) {
         delete _children[childIndex];
         _children[childIndex] = NULL;
         _isDirty = true;
+        markWithChangedTime();
         _childCount--;
     }
 }
@@ -99,17 +118,20 @@ VoxelNode* VoxelNode::removeChildAtIndex(int childIndex) {
     if (_children[childIndex]) {
         _children[childIndex] = NULL;
         _isDirty = true;
+        markWithChangedTime();
         _childCount--;
     }
     return returnedChild;
 }
 
-void VoxelNode::addChildAtIndex(int childIndex) {
+VoxelNode* VoxelNode::addChildAtIndex(int childIndex) {
     if (!_children[childIndex]) {
         _children[childIndex] = new VoxelNode(childOctalCode(_octalCode, childIndex));
         _isDirty = true;
+        markWithChangedTime();
         _childCount++;
     }
+    return _children[childIndex];
 }
 
 // handles staging or deletion of all deep children
@@ -135,6 +157,7 @@ void VoxelNode::safeDeepDeleteChildAtIndex(int childIndex, bool& stagedForDeleti
             deleteChildAtIndex(childIndex);
             _isDirty = true;
         } 
+        markWithChangedTime();
     }
 }
 
@@ -178,6 +201,7 @@ void VoxelNode::setFalseColor(colorPart red, colorPart green, colorPart blue) {
         _currentColor[2] = blue;
         _currentColor[3] = 1; // XXXBHG - False colors are always considered set
         _isDirty = true;
+        markWithChangedTime();
     }
 }
 
@@ -189,6 +213,7 @@ void VoxelNode::setFalseColored(bool isFalseColored) {
         }
         _falseColored = isFalseColored; 
         _isDirty = true;
+        markWithChangedTime();
     }
 };
 
@@ -202,6 +227,7 @@ void VoxelNode::setColor(const nodeColor& color) {
             memcpy(&_currentColor,&color,sizeof(nodeColor));
         }
         _isDirty = true;
+        markWithChangedTime();
     }
 }
 #endif

libraries/voxels/src/VoxelNode.h

@@ -9,10 +9,13 @@
 #ifndef __hifi__VoxelNode__
 #define __hifi__VoxelNode__
 
+#include <SharedUtil.h>
 #include "AABox.h"
 #include "ViewFrustum.h"
 #include "VoxelConstants.h"
 
+class VoxelTree; // forward delclaration
+
 typedef unsigned char colorPart;
 typedef unsigned char nodeColor[4];
 typedef unsigned char rgbColor[3];
@@ -26,6 +29,7 @@ private:
 #endif
     glBufferIndex _glBufferIndex;
     bool _isDirty;
+    double _lastChanged;
     bool _shouldRender;
     bool _isStagedForDeletion;
     AABox _box;
@@ -46,7 +50,7 @@ public:
     VoxelNode* getChildAtIndex(int childIndex) const { return _children[childIndex]; };
     void deleteChildAtIndex(int childIndex);
     VoxelNode* removeChildAtIndex(int childIndex);
-    void addChildAtIndex(int childIndex);
+    VoxelNode* addChildAtIndex(int childIndex);
     void safeDeepDeleteChildAtIndex(int childIndex, bool& stagedForDeletion); // handles staging or deletion of all descendents
 
     void setColorFromAverageOfChildren();
@@ -75,6 +79,10 @@ public:
     void printDebugDetails(const char* label) const;
     bool isDirty() const { return _isDirty; };
     void clearDirtyBit() { _isDirty = false; };
+    bool hasChangedSince(double time) const { return (_lastChanged > time);  };
+    void markWithChangedTime() { _lastChanged = usecTimestampNow();  };
+    void handleSubtreeChanged(VoxelTree* myTree);
+    
     glBufferIndex getBufferIndex() const { return _glBufferIndex; };
     bool isKnownBufferIndex() const { return (_glBufferIndex != GLBUFFER_INDEX_UNKNOWN); };
     void setBufferIndex(glBufferIndex index) { _glBufferIndex = index; };

libraries/voxels/src/VoxelTree.cpp

@@ -265,60 +265,133 @@ void VoxelTree::deleteVoxelAt(float x, float y, float z, float s, bool stage) {
 }
 
 
+class DeleteVoxelCodeFromTreeArgs {
+public:
+    bool            stage;
+    bool            collapseEmptyTrees;
+    unsigned char*  codeBuffer;
+    int             lengthOfCode;
+    bool            deleteLastChild;
+    bool            pathChanged;
+};
+
 // Note: uses the codeColorBuffer format, but the color's are ignored, because
 // this only finds and deletes the node from the tree.
 void VoxelTree::deleteVoxelCodeFromTree(unsigned char* codeBuffer, bool stage, bool collapseEmptyTrees) {
-    VoxelNode* parentNode = NULL;
-    VoxelNode* nodeToDelete = nodeForOctalCode(rootNode, codeBuffer, &parentNode);
-    // If the node exists...
-    int lengthInBytes = bytesRequiredForCodeLength(*codeBuffer); // includes octet count, not color!
+    // recurse the tree while decoding the codeBuffer, once you find the node in question, recurse
+    // back and implement color reaveraging, and marking of lastChanged
+    DeleteVoxelCodeFromTreeArgs args;
+    args.stage              = stage;
+    args.collapseEmptyTrees = collapseEmptyTrees;
+    args.codeBuffer         = codeBuffer;
+    args.lengthOfCode       = numberOfThreeBitSectionsInCode(codeBuffer);
+    args.deleteLastChild    = false;
+    args.pathChanged        = false;
+    
+    VoxelNode* node = rootNode;
+    deleteVoxelCodeFromTreeRecursion(node, &args);
+}
 
-    // if the code we got back matches our target, then we know we can actually delete it
-    if (memcmp(nodeToDelete->getOctalCode(), codeBuffer, lengthInBytes) == 0) {
-        if (parentNode) {
-            int childIndex = branchIndexWithDescendant(parentNode->getOctalCode(), codeBuffer);
-            if (stage) {
-                nodeToDelete->stageForDeletion();
-            } else {
-                parentNode->deleteChildAtIndex(childIndex);
-                if (_shouldReaverage) {
-                    parentNode->setColorFromAverageOfChildren();
-                }
-            }
+void VoxelTree::deleteVoxelCodeFromTreeRecursion(VoxelNode* node, void* extraData) {
+    DeleteVoxelCodeFromTreeArgs* args = (DeleteVoxelCodeFromTreeArgs*)extraData;
 
-            // If we're in collapseEmptyTrees mode, and we're the last child of this parent, then delete the parent.
-            // This will collapse the empty tree above us. 
-            if (collapseEmptyTrees && parentNode->getChildCount() == 0) {
-                // Can't delete the root this way.
-                if (parentNode != rootNode) {
-                    deleteVoxelCodeFromTree(parentNode->getOctalCode(), stage, collapseEmptyTrees);
-                }
-            }
-            _isDirty = true;
-        }
-    } else if (nodeToDelete->isLeaf()) {
+    int lengthOfNodeCode = numberOfThreeBitSectionsInCode(node->getOctalCode());
+
+    // Since we traverse the tree in code order, we know that if our code 
+    // matches, then we've reached  our target node.
+    if (lengthOfNodeCode == args->lengthOfCode) {
+        // we've reached our target, depending on how we're called we may be able to operate on it
+        // if we're in "stage" mode, then we can could have the node staged, otherwise we can't really delete 
+        // it here, we need to recurse up, and delete it there. So we handle these cases the same to keep
+        // the logic consistent.
+        args->deleteLastChild = true;
+        return;
+    }
+
+    // Ok, we know we haven't reached our target node yet, so keep looking
+    int childIndex = branchIndexWithDescendant(node->getOctalCode(), args->codeBuffer);
+    VoxelNode* childNode = node->getChildAtIndex(childIndex);
+    
+    // If there is no child at the target location, and the current parent node is a colored leaf,
+    // then it means we were asked to delete a child out of a larger leaf voxel. 
+    // We support this by breaking up the parent voxel into smaller pieces.
+    if (!childNode && node->isLeaf() && node->isColored()) {
         // we need to break up ancestors until we get to the right level
-        VoxelNode* ancestorNode = nodeToDelete;
+        VoxelNode* ancestorNode = node;
         while (true) {
-            int index = branchIndexWithDescendant(ancestorNode->getOctalCode(), codeBuffer);
-            for (int i = 0; i < 8; i++) {
+            int index = branchIndexWithDescendant(ancestorNode->getOctalCode(), args->codeBuffer);
+            for (int i = 0; i < NUMBER_OF_CHILDREN; i++) {
                 if (i != index) {
                     ancestorNode->addChildAtIndex(i);
-                    if (nodeToDelete->isColored()) {
-                        ancestorNode->getChildAtIndex(i)->setColor(nodeToDelete->getColor());
+                    if (node->isColored()) {
+                        ancestorNode->getChildAtIndex(i)->setColor(node->getColor());
                     }
                 }
             }
-            if (*ancestorNode->getOctalCode() == *codeBuffer - 1) {
+            int lengthOfancestorNode = numberOfThreeBitSectionsInCode(ancestorNode->getOctalCode());
+            
+            // If we've reached the parent of the target, then stop breaking up children
+            if (lengthOfancestorNode == (args->lengthOfCode - 1)) {
                 break;
             }
             ancestorNode->addChildAtIndex(index);
             ancestorNode = ancestorNode->getChildAtIndex(index);
-            if (nodeToDelete->isColored()) {
-                ancestorNode->setColor(nodeToDelete->getColor());
+            if (node->isColored()) {
+                ancestorNode->setColor(node->getColor());
             }
         }
         _isDirty = true;
+        args->pathChanged = true;
+        
+        // ends recursion, unwinds up stack
+        return;
+    }
+    
+    // if we don't have a child and we reach this point, then we actually know that the parent
+    // isn't a colored leaf, and the child branch doesn't exist, so there's nothing to do below and
+    // we can safely return, ending the recursion and unwinding
+    if (!childNode) {
+        //printLog("new___deleteVoxelCodeFromTree() child branch doesn't exist, but parent is not a leaf, just unwind\n");
+        return;
+    }
+
+    // If we got this far then we have a child for the branch we're looking for, but we're not there yet
+    // recurse till we get there
+    deleteVoxelCodeFromTreeRecursion(childNode, args);
+
+    // If the lower level determined it needs to be deleted, then we should delete now.
+    if (args->deleteLastChild) {
+        if (args->stage) {
+            childNode->stageForDeletion();
+        } else {
+            node->deleteChildAtIndex(childIndex); // note: this will track dirtiness and lastChanged for this node
+            if (_shouldReaverage) {
+                node->setColorFromAverageOfChildren();
+            }
+        }
+
+        // track our tree dirtiness
+        _isDirty = true;
+        
+        // track that path has changed
+        args->pathChanged = true;
+
+        // If we're in collapseEmptyTrees mode, and this was the last child of this node, then we also want
+        // to delete this node.  This will collapse the empty tree above us. 
+        if (args->collapseEmptyTrees && node->getChildCount() == 0) {
+            // Can't delete the root this way.
+            if (node == rootNode) {
+                args->deleteLastChild = false; // reset so that further up the unwinding chain we don't do anything
+            }
+        } else {
+            args->deleteLastChild = false; // reset so that further up the unwinding chain we don't do anything
+        }
+    }
+    
+    // If the lower level did some work, then we need to let this node know, so it can
+    // do any bookkeeping it wants to, like color re-averaging, time stamp marking, etc
+    if (args->pathChanged) {
+        node->handleSubtreeChanged(this);
     }
 }
 
@@ -329,50 +402,105 @@ void VoxelTree::eraseAllVoxels() {
     _isDirty = true;
 }
 
-void VoxelTree::readCodeColorBufferToTree(unsigned char *codeColorBuffer, bool destructive) {
-    VoxelNode* lastCreatedNode = nodeForOctalCode(rootNode, codeColorBuffer, NULL);
-    // create the node if it does not exist
-    if (*lastCreatedNode->getOctalCode() != *codeColorBuffer) {
-        lastCreatedNode = createMissingNode(lastCreatedNode, codeColorBuffer);
-        _isDirty = true;
-    } else {
-        // if it does exist, make sure it has no children
-        for (int i = 0; i < 8; i++) {
-            if (lastCreatedNode->getChildAtIndex(i)) {
-                if (destructive) {
-                    lastCreatedNode->deleteChildAtIndex(i);
-                } else {
-                    printLog("WARNING! operation would require deleting child at index %d, add Voxel ignored!\n ", i);
-                }
+class ReadCodeColorBufferToTreeArgs {
+public:
+    unsigned char*  codeColorBuffer;
+    int             lengthOfCode;
+    bool            destructive;
+    bool            pathChanged;
+};
+
+void VoxelTree::readCodeColorBufferToTree(unsigned char* codeColorBuffer, bool destructive) {
+    ReadCodeColorBufferToTreeArgs args;
+    args.codeColorBuffer = codeColorBuffer;
+    args.lengthOfCode    = numberOfThreeBitSectionsInCode(codeColorBuffer);
+    args.destructive     = destructive;
+    args.pathChanged     = false;
+
+    
+    VoxelNode* node = rootNode;
+    
+    readCodeColorBufferToTreeRecursion(node, &args);
+}
+
+
+void VoxelTree::readCodeColorBufferToTreeRecursion(VoxelNode* node, void* extraData) {
+    ReadCodeColorBufferToTreeArgs* args = (ReadCodeColorBufferToTreeArgs*)extraData;
+
+    int lengthOfNodeCode = numberOfThreeBitSectionsInCode(node->getOctalCode());
+
+    // Since we traverse the tree in code order, we know that if our code 
+    // matches, then we've reached  our target node.
+    if (lengthOfNodeCode == args->lengthOfCode) {
+        // we've reached our target -- we might have found our node, but that node might have children.
+        // in this case, we only allow you to set the color if you explicitly asked for a destructive
+        // write.
+        if (!node->isLeaf() && args->destructive) {
+            // if it does exist, make sure it has no children
+            for (int i = 0; i < NUMBER_OF_CHILDREN; i++) {
+                node->deleteChildAtIndex(i);
+            }
+        } else {
+            if (!node->isLeaf()) {
+                printLog("WARNING! operation would require deleting children, add Voxel ignored!\n ");
             }
         }
+        
+        // If we get here, then it means, we either had a true leaf to begin with, or we were in
+        // destructive mode and we deleted all the child trees. So we can color.
+        if (node->isLeaf()) {
+            // give this node its color
+            int octalCodeBytes = bytesRequiredForCodeLength(args->lengthOfCode);
+
+            nodeColor newColor;
+            memcpy(newColor, args->codeColorBuffer + octalCodeBytes, SIZE_OF_COLOR_DATA);
+            newColor[SIZE_OF_COLOR_DATA] = 1;
+            node->setColor(newColor);
+            
+            // It's possible we just reset the node to it's exact same color, in
+            // which case we don't consider this to be dirty...
+            if (node->isDirty()) {
+                // track our tree dirtiness
+                _isDirty = true;
+                // track that path has changed
+                args->pathChanged = true;
+            }
+        }
+        return;
     }
 
-    if (lastCreatedNode->isLeaf()) {
-        // give this node its color
-        int octalCodeBytes = bytesRequiredForCodeLength(*codeColorBuffer);
+    // Ok, we know we haven't reached our target node yet, so keep looking
+    int childIndex = branchIndexWithDescendant(node->getOctalCode(), args->codeColorBuffer);
+    VoxelNode* childNode = node->getChildAtIndex(childIndex);
+    
+    // If the branch we need to traverse does not exist, then create it on the way down...
+    if (!childNode) {
+        childNode = node->addChildAtIndex(childIndex);
+    }
 
-        nodeColor newColor;
-        memcpy(newColor, codeColorBuffer + octalCodeBytes, 3);
-        newColor[3] = 1;
-        lastCreatedNode->setColor(newColor);
-        if (lastCreatedNode->isDirty()) {
-            _isDirty = true;
-        }
+    // recurse...
+    readCodeColorBufferToTreeRecursion(childNode, args);
+
+    // Unwinding...
+    
+    // If the lower level did some work, then we need to let this node know, so it can
+    // do any bookkeeping it wants to, like color re-averaging, time stamp marking, etc
+    if (args->pathChanged) {
+        node->handleSubtreeChanged(this);
     }
 }
 
 void VoxelTree::processRemoveVoxelBitstream(unsigned char * bitstream, int bufferSizeBytes) {
 	//unsigned short int itemNumber = (*((unsigned short int*)&bitstream[sizeof(PACKET_HEADER)]));
 	int atByte = sizeof(short int) + sizeof(PACKET_HEADER);
-	unsigned char* pVoxelData = (unsigned char*)&bitstream[atByte];
+	unsigned char* voxelCode = (unsigned char*)&bitstream[atByte];
 	while (atByte < bufferSizeBytes) {
-		unsigned char octets = (unsigned char)*pVoxelData;
-		int voxelDataSize = bytesRequiredForCodeLength(octets)+3; // 3 for color!
+		int codeLength = numberOfThreeBitSectionsInCode(voxelCode);
+		int voxelDataSize = bytesRequiredForCodeLength(codeLength) + SIZE_OF_COLOR_DATA;
 
-		deleteVoxelCodeFromTree(pVoxelData, ACTUALLY_DELETE, COLLAPSE_EMPTY_TREE);
+		deleteVoxelCodeFromTree(voxelCode, ACTUALLY_DELETE, COLLAPSE_EMPTY_TREE);
 
-		pVoxelData+=voxelDataSize;
+		voxelCode+=voxelDataSize;
 		atByte+=voxelDataSize;
 	}
     reaverageVoxelColors(rootNode); // Fix our colors!! Need to call it on rootNode

libraries/voxels/src/VoxelTree.h

@@ -10,7 +10,6 @@
 #define __hifi__VoxelTree__
 
 #include "SimpleMovingAverage.h"
-
 #include "ViewFrustum.h"
 #include "VoxelNode.h"
 #include "VoxelNodeBag.h"
@@ -100,7 +99,12 @@ public:
     void copySubTreeIntoNewTree(VoxelNode* startNode, VoxelTree* destinationTree, bool rebaseToRoot);
     void copyFromTreeIntoSubTree(VoxelTree* sourceTree, VoxelNode* destinationNode);
     
+    bool getShouldReaverage() const { return _shouldReaverage; }
+    
 private:
+    void deleteVoxelCodeFromTreeRecursion(VoxelNode* node, void* extraData);
+    void readCodeColorBufferToTreeRecursion(VoxelNode* node, void* extraData);
+
     int encodeTreeBitstreamRecursion(int maxEncodeLevel, int& currentEncodeLevel,
                                      VoxelNode* node, unsigned char* outputBuffer, int availableBytes, VoxelNodeBag& bag, 
                                      const ViewFrustum* viewFrustum, bool includeColor, bool includeExistsBits,