more work on optimized storeModel()

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

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) {
+void ModelItem::setProperties(const ModelItemProperties& properties, bool forceCopy) {
-    properties.copyToModelItem(*this);
+    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;
     }

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<ModelItemID>);
 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;

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 (!_oldModel) {
-
+            //assert(_oldModel);
-        // if we do know the old model, then check to see if the position and/or size has
+            qDebug() << "that's UNEXPECTED, we got a _containingElement, but couldn't find the oldModel!";
-        // 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 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());
+    bool wantDebug = false;
-    qDebug() << "ModelTree::storeModel().... after store... containingElement=" << containingElement;
+    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<const unsigned char*>(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;
+}
+

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 {
+    ModelTreeElement* getContainingElement(const ModelItemID& modelItemID) const;
-        // TODO: do we need to make this thread safe? Or is it acceptable as is
+    void setContainingElement(const ModelItemID& modelItemID, ModelTreeElement* element);
-        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;
-    }
 
 private:
 

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
+                // seems like we shouldn't need this
-                // changed or has not been set?
                 _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();
-
+        // Since we're adding this item to this element, we need to let the tree know about it
-        // 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(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.
+                // TODO: We should optimize this... some things we need to do.
-                //   1) findModelByID() currently recurses the entire tree to find the model, that sucks and needs
+                //   1) it's also inefficient to call readModelDataFromBuffer() twice. It would be better to just
-                //      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
                 //      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;

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;

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!
             }