From 03c6275268fa32e2806afad7f8356d035a61b241 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 3 Apr 2017 08:44:27 -0700 Subject: [PATCH] more thread safety --- libraries/entities/src/EntityItem.cpp | 43 ++++--- libraries/entities/src/EntityItem.h | 3 +- libraries/entities/src/LightEntityItem.cpp | 140 ++++++++++++++++++--- libraries/entities/src/LightEntityItem.h | 2 +- libraries/entities/src/LineEntityItem.cpp | 61 ++++++++- libraries/entities/src/LineEntityItem.h | 20 ++- libraries/entities/src/ModelEntityItem.h | 2 +- libraries/entities/src/TextEntityItem.h | 2 +- 8 files changed, 216 insertions(+), 57 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 9ba740728b..891196862a 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/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(); 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(); 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(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(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; diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 929fe8a698..e705fcbe2a 100644 --- a/libraries/entities/src/EntityItem.h +++ b/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; diff --git a/libraries/entities/src/LightEntityItem.cpp b/libraries/entities/src/LightEntityItem.cpp index e09822f028..753ff1d3c8 100644 --- a/libraries/entities/src/LightEntityItem.cpp +++ b/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; +} + diff --git a/libraries/entities/src/LightEntityItem.h b/libraries/entities/src/LightEntityItem.h index 3444b11cae..f0ed58f47a 100644 --- a/libraries/entities/src/LightEntityItem.h +++ b/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 diff --git a/libraries/entities/src/LineEntityItem.cpp b/libraries/entities/src/LineEntityItem.cpp index 8ace665616..fe2c803f43 100644 --- a/libraries/entities/src/LineEntityItem.cpp +++ b/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& 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 LineEntityItem::getLinePoints() const { + QVector result; + withReadLock([&] { + result = _points; + }); + return result; +} diff --git a/libraries/entities/src/LineEntityItem.h b/libraries/entities/src/LineEntityItem.h index 8629c94eb4..7a882f6b9d 100644 --- a/libraries/entities/src/LineEntityItem.h +++ b/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& points); bool appendPoint(const glm::vec3& point); - const QVector& getLinePoints() const{ return _points; } + QVector 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; diff --git a/libraries/entities/src/ModelEntityItem.h b/libraries/entities/src/ModelEntityItem.h index e1cb5cd92c..972bf1e18d 100644 --- a/libraries/entities/src/ModelEntityItem.h +++ b/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. diff --git a/libraries/entities/src/TextEntityItem.h b/libraries/entities/src/TextEntityItem.h index 4e49bb8ef0..ee421d567a 100644 --- a/libraries/entities/src/TextEntityItem.h +++ b/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;