more thread safety

libraries/entities/src/EntityItem.cpp

@@ -683,7 +683,7 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
                 // However, for now, when the server uses a newer time than what we sent, listen to what we're told.
                 if (overwriteLocalData) weOwnSimulation = false;
             } else if (_simulationOwner.set(newSimOwner)) {
-                _dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID;
+                markDirtyFlags(Simulation::DIRTY_SIMULATOR_ID);
                 somethingChanged = true;
                 // recompute weOwnSimulation for later
                 weOwnSimulation = _simulationOwner.matchesValidID(myNodeID);
@@ -695,19 +695,19 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
             weOwnSimulation = true;
             if (!_simulationOwner.isNull()) {
                 // someone else really did own it
-                _dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID;
+                markDirtyFlags(Simulation::DIRTY_SIMULATOR_ID);
                 somethingChanged = true;
                 _simulationOwner.clearCurrentOwner();
             }
         } else if (newSimOwner.matchesValidID(myNodeID) && !_hasBidOnSimulation) {
             // entity-server tells us that we have simulation ownership while we never requested this for this EntityItem,
             // this could happen when the user reloads the cache and entity tree.
-            _dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID;
+            markDirtyFlags(Simulation::DIRTY_SIMULATOR_ID);
             somethingChanged = true;
             _simulationOwner.clearCurrentOwner();
             weOwnSimulation = false;
         } else if (_simulationOwner.set(newSimOwner)) {
-            _dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID;
+            markDirtyFlags(Simulation::DIRTY_SIMULATOR_ID);
             somethingChanged = true;
             // recompute weOwnSimulation for later
             weOwnSimulation = _simulationOwner.matchesValidID(myNodeID);
@@ -994,7 +994,7 @@ SharedSoundPointer EntityItem::getCollisionSound() {
 }
 
 void EntityItem::simulate(const quint64& now) {
-    if (0 == getLastSimulated()) {
+    if (getLastSimulated() == 0) {
         setLastSimulated(now);
     }
 
@@ -1042,7 +1042,7 @@ void EntityItem::simulate(const quint64& now) {
     if (!stepKinematicMotion(timeElapsed)) {
         // this entity is no longer moving
         // flag it to transition from KINEMATIC to STATIC
-        _dirtyFlags |= Simulation::DIRTY_MOTION_TYPE;
+        markDirtyFlags(Simulation::DIRTY_MOTION_TYPE);
         setAcceleration(Vectors::ZERO);
     }
     setLastSimulated(now);
@@ -1294,7 +1294,7 @@ void EntityItem::getAllTerseUpdateProperties(EntityItemProperties& properties) c
 }
 
 void EntityItem::pokeSimulationOwnership() {
-    _dirtyFlags |= Simulation::DIRTY_SIMULATION_OWNERSHIP_FOR_POKE;
+    markDirtyFlags(Simulation::DIRTY_SIMULATION_OWNERSHIP_FOR_POKE);
     auto nodeList = DependencyManager::get<NodeList>();
     if (_simulationOwner.matchesValidID(nodeList->getSessionUUID())) {
         // we already own it
@@ -1306,7 +1306,7 @@ void EntityItem::pokeSimulationOwnership() {
 }
 
 void EntityItem::grabSimulationOwnership() {
-    _dirtyFlags |= Simulation::DIRTY_SIMULATION_OWNERSHIP_FOR_GRAB;
+    markDirtyFlags(Simulation::DIRTY_SIMULATION_OWNERSHIP_FOR_GRAB);
     auto nodeList = DependencyManager::get<NodeList>();
     if (_simulationOwner.matchesValidID(nodeList->getSessionUUID())) {
         // we already own it
@@ -1599,18 +1599,18 @@ float EntityItem::getVolumeEstimate() const {
 void EntityItem::updateRegistrationPoint(const glm::vec3& value) {
     if (value != _registrationPoint) {
         setRegistrationPoint(value);
-        _dirtyFlags |= Simulation::DIRTY_SHAPE;
+        markDirtyFlags(Simulation::DIRTY_SHAPE);
     }
 }
 
 void EntityItem::updatePosition(const glm::vec3& value) {
     if (getLocalPosition() != value) {
         setLocalPosition(value);
-        _dirtyFlags |= Simulation::DIRTY_POSITION;
+        markDirtyFlags(Simulation::DIRTY_POSITION);
         forEachDescendant([&](SpatiallyNestablePointer object) {
             if (object->getNestableType() == NestableType::Entity) {
                 EntityItemPointer entity = std::static_pointer_cast<EntityItem>(object);
-                entity->_dirtyFlags |= Simulation::DIRTY_POSITION;
+                entity->markDirtyFlags(Simulation::DIRTY_POSITION);
             }
         });
     }
@@ -1619,8 +1619,9 @@ void EntityItem::updatePosition(const glm::vec3& value) {
 void EntityItem::updateParentID(const QUuid& value) {
     if (getParentID() != value) {
         setParentID(value);
-        _dirtyFlags |= Simulation::DIRTY_MOTION_TYPE; // children are forced to be kinematic
-        _dirtyFlags |= Simulation::DIRTY_COLLISION_GROUP; // may need to not collide with own avatar
+        // children are forced to be kinematic
+        // may need to not collide with own avatar
+        markDirtyFlags(Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP);
     }
 }
 
@@ -1634,7 +1635,7 @@ void EntityItem::updatePositionFromNetwork(const glm::vec3& value) {
 void EntityItem::updateDimensions(const glm::vec3& value) {
     if (getDimensions() != value) {
         setDimensions(value);
-        _dirtyFlags |= (Simulation::DIRTY_SHAPE | Simulation::DIRTY_MASS);
+        markDirtyFlags(Simulation::DIRTY_SHAPE | Simulation::DIRTY_MASS);
     }
 }
 
@@ -1645,8 +1646,7 @@ void EntityItem::updateRotation(const glm::quat& rotation) {
         forEachDescendant([&](SpatiallyNestablePointer object) {
             if (object->getNestableType() == NestableType::Entity) {
                 EntityItemPointer entity = std::static_pointer_cast<EntityItem>(object);
-                entity->_dirtyFlags |= Simulation::DIRTY_ROTATION;
-                entity->_dirtyFlags |= Simulation::DIRTY_POSITION;
+                entity->markDirtyFlags(Simulation::DIRTY_ROTATION | Simulation::DIRTY_POSITION);
             }
         });
     }
@@ -1913,7 +1913,7 @@ void EntityItem::updateSimulationOwner(const SimulationOwner& owner) {
     }
 
     if (_simulationOwner.set(owner)) {
-        _dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID;
+        markDirtyFlags(Simulation::DIRTY_SIMULATOR_ID);
     }
 }
 
@@ -1926,7 +1926,7 @@ void EntityItem::clearSimulationOwnership() {
     // don't bother setting the DIRTY_SIMULATOR_ID flag because:
     // (a) when entity-server calls clearSimulationOwnership() the dirty-flags are meaningless (only used by interface)
     // (b) the interface only calls clearSimulationOwnership() in a context that already knows best about dirty flags
-    //_dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID;
+    //markDirtyFlags(Simulation::DIRTY_SIMULATOR_ID);
 
 }
 
@@ -2776,6 +2776,13 @@ uint32_t EntityItem::getDirtyFlags() const {
     return result;
 }
 
+
+void EntityItem::markDirtyFlags(uint32_t mask) {
+    withWriteLock([&] {
+        _dirtyFlags |= mask;
+    });
+}
+
 void EntityItem::clearDirtyFlags(uint32_t mask) { 
     withWriteLock([&] {
         _dirtyFlags &= ~mask;

libraries/entities/src/EntityItem.h

@@ -367,6 +367,7 @@ public:
     virtual void setShapeType(ShapeType type) { /* do nothing */ }
 
     uint32_t getDirtyFlags() const;
+    void markDirtyFlags(uint32_t mask);
     void clearDirtyFlags(uint32_t mask = 0xffffffff);
 
     bool isMoving() const;
@@ -509,7 +510,7 @@ protected:
     // NOTE: _volumeMultiplier is used to allow some mass properties code exist in the EntityItem base class
     // rather than in all of the derived classes.  If we ever collapse these classes to one we could do it a
     // different way.
-    float _volumeMultiplier { 1.0f };
+    const float _volumeMultiplier { 1.0f };
     glm::vec3 _gravity;
     glm::vec3 _acceleration;
     float _damping;

libraries/entities/src/LightEntityItem.cpp

@@ -69,38 +69,59 @@ EntityItemProperties LightEntityItem::getProperties(EntityPropertyFlags desiredP
 }
 
 void LightEntityItem::setFalloffRadius(float value) {
-    _falloffRadius = glm::max(value, 0.0f);
-    _lightPropertiesChanged = true;
+    value = glm::max(value, 0.0f);
+    if (value == getFalloffRadius()) {
+        return;
+    }
+    withWriteLock([&] {
+        _falloffRadius = value;
+        _lightPropertiesChanged = true;
+    });
 }
 
 void LightEntityItem::setIsSpotlight(bool value) {
-    if (value != _isSpotlight) {
-        _isSpotlight = value;
-
-        glm::vec3 dimensions = getDimensions();
-        if (_isSpotlight) {
-            const float length = dimensions.z;
-            const float width = length * glm::sin(glm::radians(_cutoff));
-            setDimensions(glm::vec3(width, width, length));
-        } else {
-            float maxDimension = glm::compMax(dimensions);
-            setDimensions(glm::vec3(maxDimension, maxDimension, maxDimension));
-        }
-        _lightPropertiesChanged = true;
+    if (value == getIsSpotlight()) {
+        return;
     }
+
+    glm::vec3 dimensions = getDimensions();
+    glm::vec3 newDimensions;
+    if (value) {
+        const float length = dimensions.z;
+        const float width = length * glm::sin(glm::radians(getCutoff()));
+        newDimensions = glm::vec3(width, width, length);
+    } else {
+        newDimensions = glm::vec3(glm::compMax(dimensions));
+    }
+
+    withWriteLock([&] {
+        _isSpotlight = value;
+        _lightPropertiesChanged = true;
+    });
+    setDimensions(newDimensions);
 }
 
 void LightEntityItem::setCutoff(float value) {
-    _cutoff = glm::clamp(value, 0.0f, 90.0f);
+    value = glm::clamp(value, 0.0f, 90.0f);
+    if (value == getCutoff()) {
+        return;
+    }
 
-    if (_isSpotlight) {
+    withWriteLock([&] {
+        _cutoff = value;
+    });
+
+    if (getIsSpotlight()) {
         // If we are a spotlight, adjusting the cutoff will affect the area we encapsulate,
         // so update the dimensions to reflect this.
         const float length = getDimensions().z;
         const float width = length * glm::sin(glm::radians(_cutoff));
         setDimensions(glm::vec3(width, width, length));
     }
-    _lightPropertiesChanged = true;
+    
+    withWriteLock([&] {
+        _lightPropertiesChanged = true;
+    });
 }
 
 bool LightEntityItem::setProperties(const EntityItemProperties& properties) {
@@ -205,5 +226,86 @@ void LightEntityItem::appendSubclassData(OctreePacketData* packetData, EncodeBit
 
 void LightEntityItem::somethingChangedNotification() {
     EntityItem::somethingChangedNotification();
-    _lightPropertiesChanged = false;
+    withWriteLock([&] {
+        _lightPropertiesChanged = false;
+    });
 }
+
+const rgbColor& LightEntityItem::getColor() const { 
+    return _color; 
+}
+
+xColor LightEntityItem::getXColor() const {
+    xColor color = { _color[RED_INDEX], _color[GREEN_INDEX], _color[BLUE_INDEX] }; return color;
+}
+
+void LightEntityItem::setColor(const rgbColor& value) { 
+    withWriteLock([&] {
+        memcpy(_color, value, sizeof(_color));
+        _lightPropertiesChanged = true;
+    });
+}
+
+void LightEntityItem::setColor(const xColor& value) {
+    withWriteLock([&] {
+        _color[RED_INDEX] = value.red;
+        _color[GREEN_INDEX] = value.green;
+        _color[BLUE_INDEX] = value.blue;
+        _lightPropertiesChanged = true;
+    });
+}
+
+bool LightEntityItem::getIsSpotlight() const {
+    bool result;
+    withReadLock([&] {
+        result = _isSpotlight;
+    });
+    return result;
+}
+
+float LightEntityItem::getIntensity() const { 
+    float result;
+    withReadLock([&] {
+        result = _intensity;
+    });
+    return result;
+}
+
+void LightEntityItem::setIntensity(float value) {
+    withWriteLock([&] {
+        _intensity = value;
+        _lightPropertiesChanged = true;
+    });
+}
+
+float LightEntityItem::getFalloffRadius() const { 
+    float result;
+    withReadLock([&] {
+        result = _falloffRadius;
+    });
+    return result;
+}
+
+float LightEntityItem::getExponent() const { 
+    float result;
+    withReadLock([&] {
+        result = _exponent;
+    });
+    return result;
+}
+
+void LightEntityItem::setExponent(float value) {
+    withWriteLock([&] {
+        _exponent = value;
+        _lightPropertiesChanged = true;
+    });
+}
+
+float LightEntityItem::getCutoff() const { 
+    float result;
+    withReadLock([&] {
+        result = _cutoff;
+    });
+    return result;
+}
+

libraries/entities/src/LightEntityItem.h

@@ -97,7 +97,7 @@ public:
     static bool getLightsArePickable() { return _lightsArePickable; }
     static void setLightsArePickable(bool value) { _lightsArePickable = value; }
     
-protected:
+private:
 
 
     // properties of a light

libraries/entities/src/LineEntityItem.cpp

@@ -88,8 +88,10 @@ bool LineEntityItem::appendPoint(const glm::vec3& point) {
         qCDebug(entities) << "Point is outside entity's bounding box";
         return false;
     }
-    _points << point;
-    _pointsChanged = true;
+    withWriteLock([&] {
+        _points << point;
+        _pointsChanged = true;
+    });
     return true;
 }
 
@@ -105,8 +107,11 @@ bool LineEntityItem::setLinePoints(const QVector<glm::vec3>& points) {
             return false;
         }
     }
-    _points = points;
-    _pointsChanged = true;
+
+    withWriteLock([&] {
+        _points = points;
+        _pointsChanged = true;
+    });
     return true;
 }
 
@@ -159,3 +164,51 @@ void LineEntityItem::debugDump() const {
     qCDebug(entities) << "       getLastEdited:" << debugTime(getLastEdited(), now);
 }
 
+
+const rgbColor& LineEntityItem::getColor() const { 
+    return _color; 
+}
+
+xColor LineEntityItem::getXColor() const { 
+    xColor result;
+    withReadLock([&] {
+        result = { _color[RED_INDEX], _color[GREEN_INDEX], _color[BLUE_INDEX] };
+    });
+    return result; 
+}
+
+void LineEntityItem::setColor(const rgbColor& value) { 
+    withWriteLock([&] {
+        memcpy(_color, value, sizeof(_color));
+    });
+}
+
+void LineEntityItem::setColor(const xColor& value) {
+    withWriteLock([&] {
+        _color[RED_INDEX] = value.red;
+        _color[GREEN_INDEX] = value.green;
+        _color[BLUE_INDEX] = value.blue;
+    });
+}
+
+void LineEntityItem::setLineWidth(float lineWidth) { 
+    withWriteLock([&] {
+        _lineWidth = lineWidth;
+    });
+}
+
+float LineEntityItem::getLineWidth() const { 
+    float result;
+    withReadLock([&] {
+        result = _lineWidth;
+    });
+    return result;
+}
+
+QVector<glm::vec3> LineEntityItem::getLinePoints() const { 
+    QVector<glm::vec3> result;
+    withReadLock([&] {
+        result = _points;
+    });
+    return result;
+}

libraries/entities/src/LineEntityItem.h

@@ -42,23 +42,19 @@ class LineEntityItem : public EntityItem {
                                                  EntityPropertyFlags& propertyFlags, bool overwriteLocalData,
                                                  bool& somethingChanged) override;
 
-    const rgbColor& getColor() const { return _color; }
-    xColor getXColor() const { xColor color = { _color[RED_INDEX], _color[GREEN_INDEX], _color[BLUE_INDEX] }; return color; }
+    const rgbColor& getColor() const;
+    xColor getXColor() const;
 
-    void setColor(const rgbColor& value) { memcpy(_color, value, sizeof(_color)); }
-    void setColor(const xColor& value) {
-        _color[RED_INDEX] = value.red;
-        _color[GREEN_INDEX] = value.green;
-        _color[BLUE_INDEX] = value.blue;
-    }
+    void setColor(const rgbColor& value);
+    void setColor(const xColor& value);
 
-    void setLineWidth(float lineWidth){ _lineWidth = lineWidth; }
-    float getLineWidth() const{ return _lineWidth; }
+    void setLineWidth(float lineWidth);
+    float getLineWidth() const;
 
     bool setLinePoints(const QVector<glm::vec3>& points);
     bool appendPoint(const glm::vec3& point);
 
-    const QVector<glm::vec3>& getLinePoints() const{ return _points; }
+    QVector<glm::vec3> getLinePoints() const;
 
     virtual ShapeType getShapeType() const override { return SHAPE_TYPE_NONE; }
 
@@ -74,7 +70,7 @@ class LineEntityItem : public EntityItem {
     static const float DEFAULT_LINE_WIDTH;
     static const int MAX_POINTS_PER_LINE;
 
- protected:
+ private:
     rgbColor _color;
     float _lineWidth;
     bool _pointsChanged;

libraries/entities/src/ModelEntityItem.h

@@ -130,7 +130,7 @@ private:
     void setAnimationSettings(const QString& value); // only called for old bitstream format
     ShapeType computeTrueShapeType() const;
 
-protected:
+private:
     // these are used:
     // - to bounce joint data from an animation into the model/rig.
     // - to relay changes from scripts to model/rig.

libraries/entities/src/TextEntityItem.h

@@ -80,7 +80,7 @@ public:
     bool getFaceCamera() const;
     void setFaceCamera(bool value);
 
-protected:
+private:
     QString _text;
     float _lineHeight;
     rgbColor _textColor;