From fb34d1330553ba4d5aef79bf92c77247167e6967 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Thu, 19 Jun 2014 14:30:44 -0700 Subject: [PATCH] more work on optimized storeModel() --- examples/editModels.js | 11 +++ libraries/models/src/ModelItem.cpp | 51 ++++++---- libraries/models/src/ModelItem.h | 11 ++- libraries/models/src/ModelTree.cpp | 109 ++++++++++++++++++---- libraries/models/src/ModelTree.h | 15 +-- libraries/models/src/ModelTreeElement.cpp | 74 +++++---------- libraries/models/src/ModelTreeElement.h | 1 - libraries/octree/src/Octree.cpp | 5 +- 8 files changed, 175 insertions(+), 102 deletions(-) diff --git a/examples/editModels.js b/examples/editModels.js index eebcd075fa..5959b6eef5 100644 --- a/examples/editModels.js +++ b/examples/editModels.js @@ -1110,6 +1110,17 @@ function handeMenuEvent(menuItem){ var oldValue = properties[propertyName]; var newValue = Window.prompt("New value for: " + propertyName, oldValue); if (newValue != "") { + if (propertyName == "color") { + if (newValue == "red") { + newValue = { red: 255, green: 0, blue: 0 }; + } else if (newValue == "green") { + newValue = { red: 0, green: 255, blue: 0 }; + } else if (newValue == "blue") { + newValue = { red: 0, green: 0, blue: 255 }; + } else { + newValue = { red: 0, green: 0, blue: 0 }; + } + } properties[propertyName] = newValue; Models.editModel(editModelID, properties); } diff --git a/libraries/models/src/ModelItem.cpp b/libraries/models/src/ModelItem.cpp index 83b6ac525f..bf40e6bb44 100644 --- a/libraries/models/src/ModelItem.cpp +++ b/libraries/models/src/ModelItem.cpp @@ -104,7 +104,7 @@ ModelItem::ModelItem(const ModelItemID& modelItemID) { ModelItem::ModelItem(const ModelItemID& modelItemID, const ModelItemProperties& properties) { initFromModelItemID(modelItemID); - setProperties(properties); + setProperties(properties, true); // force copy } ModelItem::~ModelItem() { @@ -649,6 +649,8 @@ int ModelItem::readModelDataFromBuffer(const unsigned char* data, int bytesLeftT memcpy(&_id, dataAt, sizeof(_id)); dataAt += sizeof(_id); bytesRead += sizeof(_id); + _creatorTokenID = UNKNOWN_MODEL_TOKEN; // if we know the id, then we don't care about the creator token + _newlyCreated = false; // type - TODO: updated to using ByteCountCoding quint8 type; @@ -800,7 +802,6 @@ ModelItem ModelItem::fromEditPacket(const unsigned char* data, int length, int& newModelItem.setCreatorTokenID(creatorTokenID); newModelItem._newlyCreated = true; - } else { // look up the existing modelItem const ModelItem* existingModelItem = tree->findModelByID(editID, true); @@ -834,8 +835,14 @@ ModelItem ModelItem::fromEditPacket(const unsigned char* data, int length, int& memcpy(&packetContainsBits, dataAt, sizeof(packetContainsBits)); dataAt += sizeof(packetContainsBits); processedBytes += sizeof(packetContainsBits); - } + // only applies to editing of existing models + if (!packetContainsBits) { + //qDebug() << "edit packet didn't contain any information ignore it..."; + valid = false; + return newModelItem; + } + } // radius if (isNewModelItem || ((packetContainsBits & MODEL_PACKET_CONTAINS_RADIUS) == MODEL_PACKET_CONTAINS_RADIUS)) { @@ -1258,8 +1265,8 @@ ModelItemProperties ModelItem::getProperties() const { return properties; } -void ModelItem::setProperties(const ModelItemProperties& properties) { - properties.copyToModelItem(*this); +void ModelItem::setProperties(const ModelItemProperties& properties, bool forceCopy) { + properties.copyToModelItem(*this, forceCopy); } ModelItemProperties::ModelItemProperties() : @@ -1294,6 +1301,16 @@ ModelItemProperties::ModelItemProperties() : { } +void ModelItemProperties::debugDump() const { + qDebug() << "ModelItemProperties..."; + qDebug() << " _id=" << _id; + qDebug() << " _idSet=" << _idSet; + qDebug() << " _position=" << _position.x << "," << _position.y << "," << _position.z; + qDebug() << " _radius=" << _radius; + qDebug() << " _modelURL=" << _modelURL; + qDebug() << " _animationURL=" << _animationURL; +} + uint16_t ModelItemProperties::getChangedBits() const { uint16_t changedBits = 0; @@ -1513,59 +1530,59 @@ void ModelItemProperties::copyFromScriptValue(const QScriptValue &object) { _lastEdited = usecTimestampNow(); } -void ModelItemProperties::copyToModelItem(ModelItem& modelItem) const { +void ModelItemProperties::copyToModelItem(ModelItem& modelItem, bool forceCopy) const { bool somethingChanged = false; - if (_positionChanged) { + if (_positionChanged || forceCopy) { modelItem.setPosition(_position / (float) TREE_SCALE); somethingChanged = true; } - if (_colorChanged) { + if (_colorChanged || forceCopy) { modelItem.setColor(_color); somethingChanged = true; } - if (_radiusChanged) { + if (_radiusChanged || forceCopy) { modelItem.setRadius(_radius / (float) TREE_SCALE); somethingChanged = true; } - if (_shouldDieChanged) { + if (_shouldDieChanged || forceCopy) { modelItem.setShouldDie(_shouldDie); somethingChanged = true; } - if (_modelURLChanged) { + if (_modelURLChanged || forceCopy) { modelItem.setModelURL(_modelURL); somethingChanged = true; } - if (_modelRotationChanged) { + if (_modelRotationChanged || forceCopy) { modelItem.setModelRotation(_modelRotation); somethingChanged = true; } - if (_animationURLChanged) { + if (_animationURLChanged || forceCopy) { modelItem.setAnimationURL(_animationURL); somethingChanged = true; } - if (_animationIsPlayingChanged) { + if (_animationIsPlayingChanged || forceCopy) { modelItem.setAnimationIsPlaying(_animationIsPlaying); somethingChanged = true; } - if (_animationFrameIndexChanged) { + if (_animationFrameIndexChanged || forceCopy) { modelItem.setAnimationFrameIndex(_animationFrameIndex); somethingChanged = true; } - if (_animationFPSChanged) { + if (_animationFPSChanged || forceCopy) { modelItem.setAnimationFPS(_animationFPS); somethingChanged = true; } - if (_glowLevelChanged) { + if (_glowLevelChanged || forceCopy) { modelItem.setGlowLevel(_glowLevel); somethingChanged = true; } diff --git a/libraries/models/src/ModelItem.h b/libraries/models/src/ModelItem.h index 108e1b916c..5ca3fd390e 100644 --- a/libraries/models/src/ModelItem.h +++ b/libraries/models/src/ModelItem.h @@ -93,7 +93,7 @@ public: QScriptValue copyToScriptValue(QScriptEngine* engine) const; void copyFromScriptValue(const QScriptValue& object); - void copyToModelItem(ModelItem& modelItem) const; + void copyToModelItem(ModelItem& modelItem, bool forceCopy = false) const; void copyFromModelItem(const ModelItem& modelItem); const glm::vec3& getPosition() const { return _position; } @@ -133,6 +133,8 @@ public: glm::vec3 getMinimumPoint() const { return _position - glm::vec3(_radius, _radius, _radius); } glm::vec3 getMaximumPoint() const { return _position + glm::vec3(_radius, _radius, _radius); } + void debugDump() const; + private: glm::vec3 _position; xColor _color; @@ -200,6 +202,11 @@ inline bool operator==(const ModelItemID& a, const ModelItemID& b) { return a.id == b.id; } +inline QDebug operator<<(QDebug debug, const ModelItemID& id) { + debug << "[ id:" << id.id << ", creatorTokenID:" << id.creatorTokenID << "]"; + return debug; +} + Q_DECLARE_METATYPE(ModelItemID); Q_DECLARE_METATYPE(QVector); QScriptValue ModelItemIDtoScriptValue(QScriptEngine* engine, const ModelItemID& properties); @@ -291,7 +298,7 @@ public: void setAnimationFPS(float value) { _animationFPS = value; } void setGlowLevel(float glowLevel) { _glowLevel = glowLevel; } - void setProperties(const ModelItemProperties& properties); + void setProperties(const ModelItemProperties& properties, bool forceCopy = false); OctreeElement::AppendState appendModelData(OctreePacketData* packetData, EncodeBitstreamParams& params, ModelTreeElementExtraEncodeData* modelTreeElementExtraEncodeData) const; diff --git a/libraries/models/src/ModelTree.cpp b/libraries/models/src/ModelTree.cpp index 35aaf96895..e22e6838da 100644 --- a/libraries/models/src/ModelTree.cpp +++ b/libraries/models/src/ModelTree.cpp @@ -104,17 +104,16 @@ StoreModelOperator::StoreModelOperator(ModelTree* tree, const ModelItem& searchM { // check our tree, to determine if this model is known _containingElement = _tree->getContainingElement(searchModel.getModelItemID()); - - qDebug() << "StoreModelOperator::StoreModelOperator().... _containingElement=" << _containingElement; - if (_containingElement) { _oldModel = _containingElement->getModelWithModelItemID(searchModel.getModelItemID()); - assert(_oldModel); - - // if we do know the old model, then check to see if the position and/or size has - // changed. If it hasn't changed, then we can be confident that the new properties - // won't change the element that the model will be stored in - if (_oldModel->getAACube() == _newModel.getAACube()) { + if (!_oldModel) { + //assert(_oldModel); + qDebug() << "that's UNEXPECTED, we got a _containingElement, but couldn't find the oldModel!"; + } + + // If this containing element would be the best fit for our new model, then just do the new + // portion of the store pass, since the change path will be the same for both parts of the update + if (_containingElement->bestFitModelBounds(_newModel)) { _foundOld = true; } } else { @@ -166,8 +165,13 @@ bool StoreModelOperator::PreRecursion(OctreeElement* element) { // If this is the element we're looking for, then ask it to remove the old model // and we can stop searching. if (modelTreeElement == _containingElement) { - qDebug() << "StoreModelOperator::PreRecursion().. calling modelTreeElement->removeModelWithModelItemID()..."; - modelTreeElement->removeModelWithModelItemID(_newModel.getModelItemID()); + + // If the containgElement IS NOT the best fit for the new model properties + // then we need to remove it, and the updateModel below will store it in the + // correct element. + if (!_containingElement->bestFitModelBounds(_newModel)) { + modelTreeElement->removeModelWithModelItemID(_newModel.getModelItemID()); + } _foundOld = true; } else { // if this isn't the element we're looking for, then keep searching @@ -182,7 +186,7 @@ bool StoreModelOperator::PreRecursion(OctreeElement* element) { // Note: updateModel() will only operate on correctly found models and/or add them // to the element if they SHOULD be stored there. if (modelTreeElement->updateModel(_newModel)) { - qDebug() << "model was updated!"; + //qDebug() << "StoreModelOperator::PreRecursion()... model was updated!"; _foundNew = true; // NOTE: don't change the keepSearching here, if it came in here // false then we stay false, if it came in here true, then it @@ -236,26 +240,62 @@ void ModelTree::storeModel(const ModelItem& model, const SharedNodePointer& send recurseTreeWithOperator(&theOperator); _isDirty = true; - ModelTreeElement* containingElement = getContainingElement(model.getModelItemID()); - qDebug() << "ModelTree::storeModel().... after store... containingElement=" << containingElement; + bool wantDebug = false; + if (wantDebug) { + ModelTreeElement* containingElement = getContainingElement(model.getModelItemID()); + qDebug() << "ModelTree::storeModel().... after store... containingElement=" << containingElement; + } } void ModelTree::updateModel(const ModelItemID& modelID, const ModelItemProperties& properties) { ModelItem updateItem(modelID); + + bool wantDebug = false; + if (wantDebug) { + qDebug() << "ModelTree::updateModel(modelID, properties) line:" << __LINE__ << "updateItem:"; + updateItem.debugDump(); + } // since the properties might not be complete, they may only contain some values, // we need to first see if we already have the model in our tree, and make a copy of // its existing properties first ModelTreeElement* containingElement = getContainingElement(modelID); + if (wantDebug) { + qDebug() << "ModelTree::updateModel(modelID, properties) containingElement=" << containingElement; + } + if (containingElement) { const ModelItem* oldModel = containingElement->getModelWithModelItemID(modelID); if (oldModel) { - updateItem.setProperties(oldModel->getProperties()); + ModelItemProperties oldProps = oldModel->getProperties(); + if (wantDebug) { + qDebug() << "ModelTree::updateModel(modelID, properties) ********** COPY PROPERTIES FROM oldModel=" << oldModel << "*******************"; + qDebug() << "ModelTree::updateModel(modelID, properties) oldModel=" << oldModel; + oldProps.debugDump(); + qDebug() << "ModelTree::updateModel(modelID, properties) line:" << __LINE__ << "about to call updateItem.setProperties(oldProps);"; + } + updateItem.setProperties(oldProps, true); // force copy + + if (wantDebug) { + qDebug() << "ModelTree::updateModel(modelID, properties) line:" << __LINE__ << "updateItem:"; + updateItem.debugDump(); + } + + } else { + if (wantDebug) { + qDebug() << "ModelTree::updateModel(modelID, properties) WAIT WHAT!!! COULDN'T FIND oldModel=" << oldModel; + } } } updateItem.setProperties(properties); + + if (wantDebug) { + qDebug() << "ModelTree::updateModel(modelID, properties) line:" << __LINE__ << "updateItem:"; + updateItem.debugDump(); + } + storeModel(updateItem); } @@ -295,6 +335,9 @@ bool ModelTree::findAndUpdateModelItemIDOperation(OctreeElement* element, void* } void ModelTree::handleAddModelResponse(const QByteArray& packet) { + + qDebug() << "ModelTree::handleAddModelResponse()..."; + int numBytesPacketHeader = numBytesForPacketHeader(packet); const unsigned char* dataAt = reinterpret_cast(packet.data()) + numBytesPacketHeader; @@ -307,6 +350,9 @@ void ModelTree::handleAddModelResponse(const QByteArray& packet) { memcpy(&modelID, dataAt, sizeof(modelID)); dataAt += sizeof(modelID); + qDebug() << " creatorTokenID=" << creatorTokenID; + qDebug() << " modelID=" << modelID; + // update models in our tree bool assumeModelFound = !getIsViewing(); // if we're not a viewing tree, then we don't have to find the actual model FindAndUpdateModelItemIDArgs args = { @@ -317,7 +363,7 @@ void ModelTree::handleAddModelResponse(const QByteArray& packet) { getIsViewing() }; - const bool wantDebug = false; + const bool wantDebug = true; if (wantDebug) { qDebug() << "looking for creatorTokenID=" << creatorTokenID << " modelID=" << modelID << " getIsViewing()=" << getIsViewing(); @@ -491,6 +537,15 @@ const ModelItem* ModelTree::findModelByID(uint32_t id, bool alreadyLocked) { return args.foundModel; } +const ModelItem* ModelTree::findModelByModelItemID(const ModelItemID& modelID) { + const ModelItem* foundModel = NULL; + ModelTreeElement* containingElement = getContainingElement(modelID); + if (containingElement) { + foundModel = containingElement->getModelWithModelItemID(modelID); + } + return foundModel; +} + int ModelTree::processEditPacketData(PacketType packetType, const unsigned char* packetData, int packetLength, const unsigned char* editData, int maxLength, const SharedNodePointer& senderNode) { @@ -502,9 +557,7 @@ int ModelTree::processEditPacketData(PacketType packetType, const unsigned char* bool isValid; ModelItem newModel = ModelItem::fromEditPacket(editData, maxLength, processedBytes, this, isValid); if (isValid) { - lockForWrite(); storeModel(newModel, senderNode); - unlock(); if (newModel.isNewlyCreated()) { notifyNewlyCreatedModel(newModel, senderNode); } @@ -766,3 +819,23 @@ void ModelTree::processEraseMessage(const QByteArray& dataByteArray, const Share recurseTreeWithOperation(findAndDeleteOperation, &args); } } + + +ModelTreeElement* ModelTree::getContainingElement(const ModelItemID& modelItemID) const { + //qDebug() << "_modelToElementMap=" << _modelToElementMap; + + // TODO: do we need to make this thread safe? Or is it acceptable as is + if (_modelToElementMap.contains(modelItemID)) { + return _modelToElementMap.value(modelItemID); + } + return NULL; +} + +void ModelTree::setContainingElement(const ModelItemID& modelItemID, ModelTreeElement* element) { + // TODO: do we need to make this thread safe? Or is it acceptable as is + _modelToElementMap[modelItemID] = element; + + //qDebug() << "setContainingElement() modelItemID=" << modelItemID << "element=" << element; + //qDebug() << "AFTER _modelToElementMap=" << _modelToElementMap; +} + diff --git a/libraries/models/src/ModelTree.h b/libraries/models/src/ModelTree.h index 177a619a45..e44485f29c 100644 --- a/libraries/models/src/ModelTree.h +++ b/libraries/models/src/ModelTree.h @@ -56,6 +56,7 @@ public: void deleteModel(const ModelItemID& modelID); const ModelItem* findClosestModel(glm::vec3 position, float targetRadius); const ModelItem* findModelByID(uint32_t id, bool alreadyLocked = false); + const ModelItem* findModelByModelItemID(const ModelItemID& modelID); /// finds all models that touch a sphere /// \param center the center of the sphere @@ -87,18 +88,8 @@ public: return _fbxService ? _fbxService->getGeometryForModel(modelItem) : NULL; } - ModelTreeElement* getContainingElement(const ModelItemID& modelItemID) const { - // TODO: do we need to make this thread safe? Or is it acceptable as is - if (_modelToElementMap.contains(modelItemID)) { - return _modelToElementMap.value(modelItemID); - } - return NULL; - } - - void setContainingElement(const ModelItemID& modelItemID, ModelTreeElement* element) { - // TODO: do we need to make this thread safe? Or is it acceptable as is - _modelToElementMap[modelItemID] = element; - } + ModelTreeElement* getContainingElement(const ModelItemID& modelItemID) const; + void setContainingElement(const ModelItemID& modelItemID, ModelTreeElement* element); private: diff --git a/libraries/models/src/ModelTreeElement.cpp b/libraries/models/src/ModelTreeElement.cpp index f9aecb1e5a..06db647843 100644 --- a/libraries/models/src/ModelTreeElement.cpp +++ b/libraries/models/src/ModelTreeElement.cpp @@ -226,7 +226,7 @@ void ModelTreeElement::update(ModelTreeUpdateArgs& args) { markWithChangedTime(); // TODO: is this a good place to change the containing element map??? - //_myTree->setContainingElement(model.getModelItemID(), this); + _myTree->setContainingElement(model.getModelItemID(), NULL); } else { ++modelItr; @@ -380,8 +380,7 @@ bool ModelTreeElement::updateModel(const ModelItem& model) { thisModel.copyChangedProperties(model); markWithChangedTime(); - // TODO: can this be optimized to only set the containing element in cases where it could have - // changed or has not been set? + // seems like we shouldn't need this _myTree->setContainingElement(model.getModelItemID(), this); } else { if (wantDebug) { @@ -400,9 +399,7 @@ bool ModelTreeElement::updateModel(const ModelItem& model) { if (bestFitModelBounds(model)) { _modelItems->push_back(model); markWithChangedTime(); - - // TODO: can this be optimized to only set the containing element in cases where it could have - // changed or has not been set? + // Since we're adding this item to this element, we need to let the tree know about it _myTree->setContainingElement(model.getModelItemID(), this); return true; } @@ -410,42 +407,8 @@ bool ModelTreeElement::updateModel(const ModelItem& model) { return false; } -bool ModelTreeElement::updateModel(const ModelItemID& modelID, const ModelItemProperties& properties) { - uint16_t numberOfModels = _modelItems->size(); - for (uint16_t i = 0; i < numberOfModels; i++) { - // note: unlike storeModel() which is called from inbound packets, this is only called by local editors - // and therefore we can be confident that this change is higher priority and should be honored - ModelItem& thisModel = (*_modelItems)[i]; - - bool found = false; - if (modelID.isKnownID) { - found = thisModel.getID() == modelID.id; - } else { - found = thisModel.getCreatorTokenID() == modelID.creatorTokenID; - } - if (found) { - thisModel.setProperties(properties); - markWithChangedTime(); // mark our element as changed.. - - // TODO: can this be optimized to only set the containing element in cases where it could have - // changed or has not been set? - _myTree->setContainingElement(modelID, this); - - const bool wantDebug = false; - if (wantDebug) { - uint64_t now = usecTimestampNow(); - int elapsed = now - thisModel.getLastEdited(); - - qDebug() << "ModelTreeElement::updateModel() AFTER update... edited AGO=" << elapsed << - "now=" << now << " thisModel.getLastEdited()=" << thisModel.getLastEdited(); - } - return true; - } - } - return false; -} - void ModelTreeElement::updateModelItemID(FindAndUpdateModelItemIDArgs* args) { + bool wantDebug = false; uint16_t numberOfModels = _modelItems->size(); for (uint16_t i = 0; i < numberOfModels; i++) { ModelItem& thisModel = (*_modelItems)[i]; @@ -453,14 +416,30 @@ void ModelTreeElement::updateModelItemID(FindAndUpdateModelItemIDArgs* args) { if (!args->creatorTokenFound) { // first, we're looking for matching creatorTokenIDs, if we find that, then we fix it to know the actual ID if (thisModel.getCreatorTokenID() == args->creatorTokenID) { + if (wantDebug) { + qDebug() << "ModelTreeElement::updateModelItemID()... found the model... updating it's ID... " + << "creatorTokenID=" << args->creatorTokenID + << "modelID=" << args->modelID; + } + thisModel.setID(args->modelID); args->creatorTokenFound = true; + + // TODO: We probably need to fix up the tree's map of ids to models! } } // if we're in an isViewing tree, we also need to look for an kill any viewed models if (!args->viewedModelFound && args->isViewing) { if (thisModel.getCreatorTokenID() == UNKNOWN_MODEL_TOKEN && thisModel.getID() == args->modelID) { + + if (wantDebug) { + qDebug() << "ModelTreeElement::updateModelItemID()... VIEWED MODEL FOUND??? " + << "args->creatorTokenID=" << args->creatorTokenID + << "thisModel.getCreatorTokenID()=" << thisModel.getCreatorTokenID() + << "args->modelID=" << args->modelID; + } + _modelItems->removeAt(i); // remove the model at this index numberOfModels--; // this means we have 1 fewer model in this list i--; // and we actually want to back up i as well. @@ -603,24 +582,17 @@ int ModelTreeElement::readElementDataFromBuffer(const unsigned char* data, int b << modelItemID.id << "creatorTokenID=" << modelItemID.creatorTokenID; } - // TODO: We need to optimize this... some things we need to do. - // 1) findModelByID() currently recurses the entire tree to find the model, that sucks and needs - // to be fixed to use a hash for faster ID lookup - // 2) it's also inefficient to call readModelDataFromBuffer() twice. It would be better to just + // TODO: We should optimize this... some things we need to do. + // 1) it's also inefficient to call readModelDataFromBuffer() twice. It would be better to just // pull out the model item ID, then look it up, then read the rest of the data in the buffer - const ModelItem* existingModelItem = _myTree->findModelByID(modelItemID.id, true); + const ModelItem* existingModelItem = _myTree->findModelByModelItemID(modelItemID); if (existingModelItem) { // copy original properties... tempModel.copyChangedProperties(*existingModelItem); // reread only the changed properties bytesForThisModel = tempModel.readModelDataFromBuffer(dataAt, bytesLeftToRead, args); } - - // here! - qDebug() << "ModelTreeElement::readElementDataFromBuffer() calling _myTree->storeModel(tempModel)"; _myTree->storeModel(tempModel); - - dataAt += bytesForThisModel; bytesLeftToRead -= bytesForThisModel; bytesRead += bytesForThisModel; diff --git a/libraries/models/src/ModelTreeElement.h b/libraries/models/src/ModelTreeElement.h index 760711e98a..3533e37dcc 100644 --- a/libraries/models/src/ModelTreeElement.h +++ b/libraries/models/src/ModelTreeElement.h @@ -121,7 +121,6 @@ public: void setTree(ModelTree* tree) { _myTree = tree; } bool updateModel(const ModelItem& model); - bool updateModel(const ModelItemID& modelID, const ModelItemProperties& properties); void updateModelItemID(FindAndUpdateModelItemIDArgs* args); const ModelItem* getClosestModel(glm::vec3 position) const; diff --git a/libraries/octree/src/Octree.cpp b/libraries/octree/src/Octree.cpp index 59f7aeb1fd..d5f2afaedb 100644 --- a/libraries/octree/src/Octree.cpp +++ b/libraries/octree/src/Octree.cpp @@ -159,7 +159,7 @@ bool Octree::recurseElementWithOperator(OctreeElement* element, RecurseOctreeOpe for (int i = 0; i < NUMBER_OF_CHILDREN; i++) { OctreeElement* child = element->getChildAtIndex(i); - // If there is now child at that location, the Operator may want to create a child at that location. + // If there is no child at that location, the Operator may want to create a child at that location. // So give the operator a chance to do so.... if (!child) { child = operatorObject->PossiblyCreateChildAt(element, i); @@ -278,6 +278,8 @@ int Octree::readElementData(OctreeElement* destinationElement, const unsigned ch int childIndex = 0; bytesRead += args.includeExistsBits ? sizeof(childrenInTreeMask) + sizeof(childMask) : sizeof(childMask); + //qDebug() << "Octree::readElementData()... childrenInTreeMask=" << childrenInTreeMask; + while (bytesLeftToRead - bytesRead > 0 && childIndex < NUMBER_OF_CHILDREN) { // check the exists mask to see if we have a child to traverse into @@ -303,6 +305,7 @@ int Octree::readElementData(OctreeElement* destinationElement, const unsigned ch // now also check the childrenInTreeMask, if the mask is missing the bit, then it means we need to delete this child // subtree/element, because it shouldn't actually exist in the tree. if (!oneAtBit(childrenInTreeMask, i) && destinationElement->getChildAtIndex(i)) { + //qDebug() << "Octree::readElementData()... pruning our tree for child i=" << i; destinationElement->safeDeepDeleteChildAtIndex(i); _isDirty = true; // by definition! }