merge upstream/master into andrew/inertia

assignment-client/src/octree/OctreeSendThread.cpp

@@ -404,6 +404,13 @@ int OctreeSendThread::packetDistributor(OctreeQueryNode* nodeData, bool viewFrus
 
             bool lastNodeDidntFit = false; // assume each node fits
             if (!nodeData->elementBag.isEmpty()) {
+                
+                quint64 lockWaitStart = usecTimestampNow();
+                _myServer->getOctree()->lockForRead();
+                quint64 lockWaitEnd = usecTimestampNow();
+                lockWaitElapsedUsec = (float)(lockWaitEnd - lockWaitStart);
+                quint64 encodeStart = usecTimestampNow();
+
                 OctreeElement* subTree = nodeData->elementBag.extract();
 
                 /* TODO: Looking for a way to prevent locking and encoding a tree that is not
@@ -447,12 +454,6 @@ int OctreeSendThread::packetDistributor(OctreeQueryNode* nodeData, bool viewFrus
                 // it seems like it may be a good idea to include the lock time as part of the encode time
                 // are reported to client. Since you can encode without the lock
                 nodeData->stats.encodeStarted();
-                
-                quint64 lockWaitStart = usecTimestampNow();
-                _myServer->getOctree()->lockForRead();
-                quint64 lockWaitEnd = usecTimestampNow();
-                lockWaitElapsedUsec = (float)(lockWaitEnd - lockWaitStart);
-                quint64 encodeStart = usecTimestampNow();
 
                 bytesWritten = _myServer->getOctree()->encodeTreeBitstream(subTree, &_packetData, nodeData->elementBag, params);
 

interface/src/avatar/Head.cpp

@@ -54,7 +54,6 @@ Head::Head(Avatar* owningAvatar) :
     _deltaRoll(0.0f),
     _deltaLeanSideways(0.0f),
     _deltaLeanForward(0.0f),
-    _torsoTwist(0.0f),
     _isCameraMoving(false),
     _isLookingAtMe(false),
     _faceModel(this),

interface/src/avatar/Head.h

@@ -76,9 +76,6 @@ public:
     float getFinalLeanSideways() const { return _leanSideways + _deltaLeanSideways; }
     float getFinalLeanForward() const { return _leanForward + _deltaLeanForward; }
     
-    float getTorsoTwist() const { return _torsoTwist; }
-    void setTorsoTwist(float torsoTwist) { _torsoTwist = torsoTwist; }
-    
     glm::quat getEyeRotation(const glm::vec3& eyePosition) const;
     
     const glm::vec3& getRightEyePosition() const { return _rightEyePosition; }
@@ -151,8 +148,6 @@ private:
     // delta lean angles for lean perturbations (driven by collisions)
     float _deltaLeanSideways;
     float _deltaLeanForward;
-    
-    float _torsoTwist;
 
     bool _isCameraMoving;
     bool _isLookingAtMe;

libraries/avatars/src/AvatarData.cpp

@@ -158,15 +158,18 @@ QByteArray AvatarData::toByteArray() {
     destinationBuffer += packFloatRatioToTwoByte(destinationBuffer, _targetScale);
 
     // Head rotation (NOTE: This needs to become a quaternion to save two bytes)
-    destinationBuffer += packFloatAngleToTwoByte(destinationBuffer, _headData->getFinalYaw());
+    glm::vec3 pitchYawRoll = glm::vec3(_headData->getFinalPitch(),
-    destinationBuffer += packFloatAngleToTwoByte(destinationBuffer, _headData->getFinalPitch());
+                                       _headData->getFinalYaw(),
-    destinationBuffer += packFloatAngleToTwoByte(destinationBuffer, _headData->getFinalRoll());
+                                       _headData->getFinalRoll());
-    
+    if (this->isMyAvatar()) {
-    // Head lean X,Z (head lateral and fwd/back motion relative to torso)
+        glm::vec3 lean = glm::vec3(_headData->getFinalLeanForward(),
-    memcpy(destinationBuffer, &_headData->_leanSideways, sizeof(_headData->_leanSideways));
+                                   _headData->getTorsoTwist(),
-    destinationBuffer += sizeof(_headData->_leanSideways);
+                                   _headData->getFinalLeanSideways());
-    memcpy(destinationBuffer, &_headData->_leanForward, sizeof(_headData->_leanForward));
+        pitchYawRoll -= lean;
-    destinationBuffer += sizeof(_headData->_leanForward);
+    }
+    destinationBuffer += packFloatAngleToTwoByte(destinationBuffer, pitchYawRoll.x);
+    destinationBuffer += packFloatAngleToTwoByte(destinationBuffer, pitchYawRoll.y);
+    destinationBuffer += packFloatAngleToTwoByte(destinationBuffer, pitchYawRoll.z);
 
     // Lookat Position
     memcpy(destinationBuffer, &_headData->_lookAtPosition, sizeof(_headData->_lookAtPosition));
@@ -287,18 +290,16 @@ int AvatarData::parseDataAtOffset(const QByteArray& packet, int offset) {
     //     bodyPitch     =  2 (compressed float)
     //     bodyRoll      =  2 (compressed float)
     //     targetScale   =  2 (compressed float)
-    //     headYaw       =  2 (compressed float)
     //     headPitch     =  2 (compressed float)
+    //     headYaw       =  2 (compressed float)
     //     headRoll      =  2 (compressed float)
-    //     leanSideways  =  4
-    //     leanForward   =  4
     //     lookAt        = 12
     //     audioLoudness =  4
     // }
     // + 1 byte for pupilSize
     // + 1 byte for numJoints (0)
-    // = 53 bytes
+    // = 45 bytes
-    int minPossibleSize = 52;
+    int minPossibleSize = 45;
     
     int maxAvailableSize = packet.size() - offset;
     if (minPossibleSize > maxAvailableSize) {
@@ -356,8 +357,8 @@ int AvatarData::parseDataAtOffset(const QByteArray& packet, int offset) {
     { // Head rotation 
         //(NOTE: This needs to become a quaternion to save two bytes)
         float headYaw, headPitch, headRoll;
-        sourceBuffer += unpackFloatAngleFromTwoByte((uint16_t*) sourceBuffer, &headYaw);
         sourceBuffer += unpackFloatAngleFromTwoByte((uint16_t*) sourceBuffer, &headPitch);
+        sourceBuffer += unpackFloatAngleFromTwoByte((uint16_t*) sourceBuffer, &headYaw);
         sourceBuffer += unpackFloatAngleFromTwoByte((uint16_t*) sourceBuffer, &headRoll);
         if (glm::isnan(headYaw) || glm::isnan(headPitch) || glm::isnan(headRoll)) {
             if (shouldLogError(now)) {
@@ -365,27 +366,10 @@ int AvatarData::parseDataAtOffset(const QByteArray& packet, int offset) {
             }
             return maxAvailableSize;
         }
-        _headData->setBaseYaw(headYaw);
         _headData->setBasePitch(headPitch);
+        _headData->setBaseYaw(headYaw);
         _headData->setBaseRoll(headRoll);
     } // 6 bytes
-        
-    // Head lean (relative to pelvis)
-    {
-        float leanSideways, leanForward;
-        memcpy(&leanSideways, sourceBuffer, sizeof(float));
-        sourceBuffer += sizeof(float);
-        memcpy(&leanForward, sourceBuffer, sizeof(float));
-        sourceBuffer += sizeof(float);
-        if (glm::isnan(leanSideways) || glm::isnan(leanForward)) {
-            if (shouldLogError(now)) {
-                qDebug() << "Discard nan AvatarData::leanSideways,leanForward; displayName = '" << _displayName << "'";
-            }
-            return maxAvailableSize;
-        }
-        _headData->_leanSideways = leanSideways;
-        _headData->_leanForward = leanForward;
-    } // 8 bytes
     
     { // Lookat Position
         glm::vec3 lookAt;

libraries/avatars/src/AvatarData.h

@@ -152,6 +152,8 @@ class AvatarData : public QObject {
 public:
     AvatarData();
     virtual ~AvatarData();
+    
+    virtual bool isMyAvatar() { return false; }
 
     const QUuid& getSessionUUID() const { return _sessionUUID; }
 

libraries/avatars/src/HeadData.cpp

@@ -24,6 +24,7 @@ HeadData::HeadData(AvatarData* owningAvatar) :
     _baseRoll(0.0f),
     _leanSideways(0.0f),
     _leanForward(0.0f),
+    _torsoTwist(0.0f),
     _lookAtPosition(0.0f, 0.0f, 0.0f),
     _audioLoudness(0.0f),
     _isFaceshiftConnected(false),

libraries/avatars/src/HeadData.h

@@ -71,11 +71,13 @@ public:
     
     float getLeanSideways() const { return _leanSideways; }
     float getLeanForward() const { return _leanForward; }
+    float getTorsoTwist() const { return _torsoTwist; }
     virtual float getFinalLeanSideways() const { return _leanSideways; }
     virtual float getFinalLeanForward() const { return _leanForward; }
     
     void setLeanSideways(float leanSideways) { _leanSideways = leanSideways; }
     void setLeanForward(float leanForward) { _leanForward = leanForward; }
+    void setTorsoTwist(float torsoTwist) { _torsoTwist = torsoTwist; }
     
     friend class AvatarData;
     
@@ -86,6 +88,7 @@ protected:
     float _baseRoll;
     float _leanSideways;
     float _leanForward;
+    float _torsoTwist;
 
     glm::vec3 _lookAtPosition;
     float _audioLoudness;

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -98,7 +98,9 @@ void EntityTreeRenderer::init() {
 }
 
 QScriptValue EntityTreeRenderer::loadEntityScript(const EntityItemID& entityItemID) {
+    _tree->lockForRead();
     EntityItem* entity = static_cast<EntityTree*>(_tree)->findEntityByEntityItemID(entityItemID);
+    _tree->unlock();
     return loadEntityScript(entity);
 }
 

libraries/entities/src/EntityItem.cpp

@@ -97,6 +97,7 @@ EntityItem::EntityItem(const EntityItemID& entityItemID) {
     _physicsInfo = NULL;
     _dirtyFlags = 0;
     _changedOnServer = 0;
+    _element = NULL;
     initFromEntityItemID(entityItemID);
 }
 
@@ -112,6 +113,7 @@ EntityItem::EntityItem(const EntityItemID& entityItemID, const EntityItemPropert
     _physicsInfo = NULL;
     _dirtyFlags = 0;
     _changedOnServer = 0;
+    _element = NULL;
     initFromEntityItemID(entityItemID);
     setProperties(properties);
 }
@@ -119,6 +121,7 @@ EntityItem::EntityItem(const EntityItemID& entityItemID, const EntityItemPropert
 EntityItem::~EntityItem() {
     // be sure to clean up _physicsInfo before calling this dtor
     assert(_physicsInfo == NULL);
+    assert(_element == NULL);
 }
 
 EntityPropertyFlags EntityItem::getEntityProperties(EncodeBitstreamParams& params) const {

libraries/entities/src/EntityItem.h

@@ -64,6 +64,7 @@ const QString DEFAULT_USER_DATA = QString("");
 /// to all other entity types. In particular: postion, size, rotation, age, lifetime, velocity, gravity. You can not instantiate
 /// one directly, instead you must only construct one of it's derived classes with additional features.
 class EntityItem  {
+    friend class EntityTreeElement;
 
 public:
     enum EntityDirtyFlags {
@@ -302,6 +303,7 @@ public:
     void* getPhysicsInfo() const { return _physicsInfo; }
     void setPhysicsInfo(void* data) { _physicsInfo = data; }
     
+    EntityTreeElement* getElement() const { return _element; }
 protected:
 
     virtual void initFromEntityItemID(const EntityItemID& entityItemID); // maybe useful to allow subclasses to init
@@ -363,6 +365,8 @@ protected:
 
     // DirtyFlags are set whenever a property changes that the EntitySimulation needs to know about.
     uint32_t _dirtyFlags;   // things that have changed from EXTERNAL changes (via script or packet) but NOT from simulation
+
+    EntityTreeElement* _element;    // back pointer to containing Element
 };
 
 

libraries/entities/src/EntityTreeElement.cpp

@@ -682,6 +682,7 @@ void EntityTreeElement::cleanupEntities() {
     uint16_t numberOfEntities = _entityItems->size();
     for (uint16_t i = 0; i < numberOfEntities; i++) {
         EntityItem* entity = (*_entityItems)[i];
+        entity->_element = NULL;
         delete entity;
     }
     _entityItems->clear();
@@ -693,6 +694,7 @@ bool EntityTreeElement::removeEntityWithEntityItemID(const EntityItemID& id) {
     for (uint16_t i = 0; i < numberOfEntities; i++) {
         if ((*_entityItems)[i]->getEntityItemID() == id) {
             foundEntity = true;
+            (*_entityItems)[i]->_element = NULL;
             _entityItems->removeAt(i);
             break;
         }
@@ -701,7 +703,13 @@ bool EntityTreeElement::removeEntityWithEntityItemID(const EntityItemID& id) {
 }
 
 bool EntityTreeElement::removeEntityItem(EntityItem* entity) {
-    return _entityItems->removeAll(entity) > 0;
+    int numEntries = _entityItems->removeAll(entity);
+    if (numEntries > 0) {
+        assert(entity->_element == this);
+        entity->_element = NULL;
+        return true;
+    }
+    return false;
 }
 
 
@@ -808,7 +816,10 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int
 }
 
 void EntityTreeElement::addEntityItem(EntityItem* entity) {
+    assert(entity);
+    assert(entity->_element == NULL);
     _entityItems->push_back(entity);
+    entity->_element = this;
 }
 
 // will average a "common reduced LOD view" from the the child elements...

libraries/entities/src/MovingEntitiesOperator.cpp

@@ -179,7 +179,7 @@ bool MovingEntitiesOperator::preRecursion(OctreeElement* element) {
 
             // If this is one of the old elements we're looking for, then ask it to remove the old entity
             if (!details.oldFound && entityTreeElement == details.oldContainingElement) {
-                entityTreeElement->removeEntityItem(details.entity);
+                // DO NOT remove the entity here.  It will be removed when added to the destination element.
                 _foundOldCount++;
                 //details.oldFound = true; // TODO: would be nice to add this optimization
                 if (_wantDebug) {
@@ -193,8 +193,15 @@ bool MovingEntitiesOperator::preRecursion(OctreeElement* element) {
             // If this element is the best fit for the new bounds of this entity then add the entity to the element
             if (!details.newFound && entityTreeElement->bestFitBounds(details.newCube)) {
                 EntityItemID entityItemID = details.entity->getEntityItemID();
-                entityTreeElement->addEntityItem(details.entity);
+                // remove from the old before adding
-                _tree->setContainingElement(entityItemID, entityTreeElement);
+                EntityTreeElement* oldElement = details.entity->getElement();
+                if (oldElement != entityTreeElement) {
+                    if (oldElement) {
+                        oldElement->removeEntityItem(details.entity);
+                    }
+                    entityTreeElement->addEntityItem(details.entity);
+                    _tree->setContainingElement(entityItemID, entityTreeElement);
+                }
                 _foundNewCount++;
                 //details.newFound = true; // TODO: would be nice to add this optimization
                 if (_wantDebug) {
@@ -227,7 +234,7 @@ bool MovingEntitiesOperator::postRecursion(OctreeElement* element) {
     
 
 
-    // It's not OK to prune if we have the potential of deleting the original containig element.
+    // It's not OK to prune if we have the potential of deleting the original containing element
     // because if we prune the containing element then new might end up reallocating the same memory later 
     // and that will confuse our logic.
     // 

libraries/entities/src/UpdateEntityOperator.cpp

@@ -231,18 +231,19 @@ bool UpdateEntityOperator::preRecursion(OctreeElement* element) {
                     qDebug() << "    *** REMOVING from ELEMENT ***";
                 }
 
-                entityTreeElement->removeEntityItem(_existingEntity); // NOTE: only removes the entity, doesn't delete it
+                // the entity knows what element it's in, so we remove it from that one
+                // NOTE: we know we haven't yet added it to its new element because _removeOld is true
+                EntityTreeElement* oldElement = _existingEntity->getElement();
+                oldElement->removeEntityItem(_existingEntity);
+                _tree->setContainingElement(_entityItemID, NULL);
 
-                // If we haven't yet found the new location, then we need to 
+                if (oldElement != _containingElement) {
-                // make sure to remove our entity to element map, because for
+                    qDebug() << "WARNING entity moved during UpdateEntityOperator recursion";
-                // now we're not in that map
+                    assert(! _containingElement->removeEntityItem(_existingEntity));
-                if (!_foundNew) {
+                }
-                    _tree->setContainingElement(_entityItemID, NULL);
-
-                    if (_wantDebug) {
-                        qDebug() << "    *** REMOVING from MAP ***";
-                    }
 
+                if (_wantDebug) {
+                    qDebug() << "    *** REMOVING from MAP ***";
                 }
             }
             _foundOld = true;
@@ -263,7 +264,6 @@ bool UpdateEntityOperator::preRecursion(OctreeElement* element) {
             qDebug() << "    entityTreeElement->bestFitBounds(_newEntityBox)=" << entityTreeElement->bestFitBounds(_newEntityBox);
         }
 
-
         // If this element is the best fit for the new entity properties, then add/or update it
         if (entityTreeElement->bestFitBounds(_newEntityBox)) {
 
@@ -271,33 +271,14 @@ bool UpdateEntityOperator::preRecursion(OctreeElement* element) {
                 qDebug() << "    *** THIS ELEMENT IS BEST FIT ***";
             }
 
+            EntityTreeElement* oldElement = _existingEntity->getElement();
             // if we are the existing containing element, then we can just do the update of the entity properties
-            if (entityTreeElement == _containingElement) {
+            if (entityTreeElement == oldElement) {
 
                 if (_wantDebug) {
                     qDebug() << "    *** This is the same OLD ELEMENT ***";
                 }
             
-                // TODO: We shouldn't be in a remove old case and also be the new best fit. This indicates that
-                // we have some kind of a logic error in this operator. But, it can handle it properly by setting
-                // the new properties for the entity and moving on. Still going to output a warning that if we
-                // see consistently we will want to address this.
-                if (_removeOld) {
-                    qDebug() << "UNEXPECTED - UpdateEntityOperator - "
-                                "we thought we needed to removeOld, but the old entity is our best fit.";
-                    _removeOld = false;
-                    
-                    // if we thought we were supposed to remove the old item, and we already did, then we need
-                    // to repair this case.
-                    if (_foundOld) {
-                        if (_wantDebug) {
-                            qDebug() << "    *** REPAIRING PREVIOUS REMOVAL from ELEMENT and MAP ***";
-                        }
-                        entityTreeElement->addEntityItem(_existingEntity);
-                        _tree->setContainingElement(_entityItemID, entityTreeElement);
-                    }
-                }
-
                 // set the entity properties and mark our element as changed.
                 _existingEntity->setProperties(_properties);
                 if (_wantDebug) {
@@ -305,14 +286,22 @@ bool UpdateEntityOperator::preRecursion(OctreeElement* element) {
                 }
             } else {
                 // otherwise, this is an add case.
+                if (oldElement) {
+                    oldElement->removeEntityItem(_existingEntity);
+                    if (oldElement != _containingElement) {
+                        qDebug() << "WARNING entity moved during UpdateEntityOperator recursion";
+                    }
+                }
                 entityTreeElement->addEntityItem(_existingEntity);
-                _existingEntity->setProperties(_properties); // still need to update the properties!
                 _tree->setContainingElement(_entityItemID, entityTreeElement);
+
+                _existingEntity->setProperties(_properties); // still need to update the properties!
                 if (_wantDebug) {
                     qDebug() << "    *** ADDING ENTITY to ELEMENT and MAP and SETTING PROPERTIES ***";
                 }
             }
-            _foundNew = true; // we found the new item!
+            _foundNew = true; // we found the new element
+            _removeOld = false; // and it has already been removed from the old
         } else {
             keepSearching = true;
         }

libraries/networking/src/PacketHeaders.cpp

@@ -57,7 +57,7 @@ PacketVersion versionForPacketType(PacketType type) {
         case PacketTypeInjectAudio:
             return 1;
         case PacketTypeAvatarData:
-            return 4;
+            return 5;
         case PacketTypeAvatarIdentity:
             return 1;
         case PacketTypeEnvironmentData:

libraries/shared/src/DependencyManager.h

@@ -13,6 +13,7 @@
 #define hifi_DependencyManager_h
 
 #include <QSharedPointer>
+#include <QDebug>
 
 #include <typeinfo>
 
@@ -22,10 +23,12 @@ public:\
 private:\
     void customDeleter() {\
         QObject* thisObject = dynamic_cast<QObject*>(this);\
-        if (thisObject) {\
+        if (thisObject && thisObject->parent()) {\
             thisObject->deleteLater();\
+            qDebug() << "Delete later:" << #T;\
         } else {\
             delete this;\
+            qDebug() << "Deleted:" << #T;\
         }\
     }\
     friend class DependencyManager;