Fix race condition accessing QString sub-properties in zones

This commit is contained in:
Brad Davis 2017-09-13 18:06:21 -07:00
parent 84998a3360
commit f4d8216501
4 changed files with 62 additions and 33 deletions

View file

@ -176,6 +176,10 @@ void ZoneEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& scen
_lastRotation = entity->getRotation(); _lastRotation = entity->getRotation();
_lastDimensions = entity->getDimensions(); _lastDimensions = entity->getDimensions();
_keyLightProperties = entity->getKeyLightProperties();
_stageProperties = entity->getStageProperties();
_skyboxProperties = entity->getSkyboxProperties();
#if 0 #if 0
if (_lastShapeURL != _typedEntity->getCompoundShapeURL()) { if (_lastShapeURL != _typedEntity->getCompoundShapeURL()) {
@ -196,14 +200,14 @@ void ZoneEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& scen
} }
#endif #endif
updateKeyZoneItemFromEntity(entity); updateKeyZoneItemFromEntity();
if (sunChanged) { if (sunChanged) {
updateKeySunFromEntity(entity); updateKeySunFromEntity();
} }
if (sunChanged || skyboxChanged) { if (sunChanged || skyboxChanged) {
updateKeyAmbientFromEntity(entity); updateKeyAmbientFromEntity();
} }
if (backgroundChanged || skyboxChanged) { if (backgroundChanged || skyboxChanged) {
updateKeyBackgroundFromEntity(entity); updateKeyBackgroundFromEntity(entity);
@ -265,19 +269,19 @@ bool ZoneEntityRenderer::needsRenderUpdateFromTypedEntity(const TypedEntityPoint
return false; return false;
} }
void ZoneEntityRenderer::updateKeySunFromEntity(const TypedEntityPointer& entity) { void ZoneEntityRenderer::updateKeySunFromEntity() {
const auto& sunLight = editSunLight(); const auto& sunLight = editSunLight();
sunLight->setType(model::Light::SUN); sunLight->setType(model::Light::SUN);
sunLight->setPosition(_lastPosition); sunLight->setPosition(_lastPosition);
sunLight->setOrientation(_lastRotation); sunLight->setOrientation(_lastRotation);
// Set the keylight // Set the keylight
sunLight->setColor(ColorUtils::toVec3(entity->getKeyLightProperties().getColor())); sunLight->setColor(ColorUtils::toVec3(_keyLightProperties.getColor()));
sunLight->setIntensity(entity->getKeyLightProperties().getIntensity()); sunLight->setIntensity(_keyLightProperties.getIntensity());
sunLight->setDirection(entity->getKeyLightProperties().getDirection()); sunLight->setDirection(_keyLightProperties.getDirection());
} }
void ZoneEntityRenderer::updateKeyAmbientFromEntity(const TypedEntityPointer& entity) { void ZoneEntityRenderer::updateKeyAmbientFromEntity() {
const auto& ambientLight = editAmbientLight(); const auto& ambientLight = editAmbientLight();
ambientLight->setType(model::Light::AMBIENT); ambientLight->setType(model::Light::AMBIENT);
ambientLight->setPosition(_lastPosition); ambientLight->setPosition(_lastPosition);
@ -285,24 +289,24 @@ void ZoneEntityRenderer::updateKeyAmbientFromEntity(const TypedEntityPointer& en
// Set the keylight // Set the keylight
ambientLight->setAmbientIntensity(entity->getKeyLightProperties().getAmbientIntensity()); ambientLight->setAmbientIntensity(_keyLightProperties.getAmbientIntensity());
if (entity->getKeyLightProperties().getAmbientURL().isEmpty()) { if (_keyLightProperties.getAmbientURL().isEmpty()) {
setAmbientURL(entity->getSkyboxProperties().getURL()); setAmbientURL(_skyboxProperties.getURL());
} else { } else {
setAmbientURL(entity->getKeyLightProperties().getAmbientURL()); setAmbientURL(_keyLightProperties.getAmbientURL());
} }
} }
void ZoneEntityRenderer::updateKeyBackgroundFromEntity(const TypedEntityPointer& entity) { void ZoneEntityRenderer::updateKeyBackgroundFromEntity(const TypedEntityPointer& entity) {
editBackground(); editBackground();
setBackgroundMode(entity->getBackgroundMode()); setBackgroundMode(entity->getBackgroundMode());
setSkyboxColor(entity->getSkyboxProperties().getColorVec3()); setSkyboxColor(_skyboxProperties.getColorVec3());
setProceduralUserData(entity->getUserData()); setProceduralUserData(entity->getUserData());
setSkyboxURL(entity->getSkyboxProperties().getURL()); setSkyboxURL(_skyboxProperties.getURL());
} }
void ZoneEntityRenderer::updateKeyZoneItemFromEntity(const TypedEntityPointer& entity) { void ZoneEntityRenderer::updateKeyZoneItemFromEntity() {
/* TODO: Implement the sun model behavior / Keep this code here for reference, this is how we /* TODO: Implement the sun model behavior / Keep this code here for reference, this is how we
{ {
// Set the stage // Set the stage

View file

@ -41,9 +41,9 @@ protected:
virtual void doRenderUpdateAsynchronousTyped(const TypedEntityPointer& entity) override; virtual void doRenderUpdateAsynchronousTyped(const TypedEntityPointer& entity) override;
private: private:
void updateKeyZoneItemFromEntity(const TypedEntityPointer& entity); void updateKeyZoneItemFromEntity();
void updateKeySunFromEntity(const TypedEntityPointer& entity); void updateKeySunFromEntity();
void updateKeyAmbientFromEntity(const TypedEntityPointer& entity); void updateKeyAmbientFromEntity();
void updateKeyBackgroundFromEntity(const TypedEntityPointer& entity); void updateKeyBackgroundFromEntity(const TypedEntityPointer& entity);
void updateAmbientMap(); void updateAmbientMap();
void updateSkyboxMap(); void updateSkyboxMap();
@ -89,6 +89,10 @@ private:
bool _needAmbientUpdate{ true }; bool _needAmbientUpdate{ true };
bool _needBackgroundUpdate{ true }; bool _needBackgroundUpdate{ true };
KeyLightPropertyGroup _keyLightProperties;
StagePropertyGroup _stageProperties;
SkyboxPropertyGroup _skyboxProperties;
// More attributes used for rendering: // More attributes used for rendering:
QString _ambientTextureURL; QString _ambientTextureURL;
NetworkTexturePointer _ambientTexture; NetworkTexturePointer _ambientTexture;

View file

@ -49,8 +49,10 @@ ZoneEntityItem::ZoneEntityItem(const EntityItemID& entityItemID) : EntityItem(en
EntityItemProperties ZoneEntityItem::getProperties(EntityPropertyFlags desiredProperties) const { EntityItemProperties ZoneEntityItem::getProperties(EntityPropertyFlags desiredProperties) const {
EntityItemProperties properties = EntityItem::getProperties(desiredProperties); // get the properties from our base class EntityItemProperties properties = EntityItem::getProperties(desiredProperties); // get the properties from our base class
// Contains a QString property, must be synchronized
_keyLightProperties.getProperties(properties); withReadLock([&] {
_keyLightProperties.getProperties(properties);
});
_stageProperties.getProperties(properties); _stageProperties.getProperties(properties);
@ -58,7 +60,10 @@ EntityItemProperties ZoneEntityItem::getProperties(EntityPropertyFlags desiredPr
COPY_ENTITY_PROPERTY_TO_PROPERTIES(compoundShapeURL, getCompoundShapeURL); COPY_ENTITY_PROPERTY_TO_PROPERTIES(compoundShapeURL, getCompoundShapeURL);
COPY_ENTITY_PROPERTY_TO_PROPERTIES(backgroundMode, getBackgroundMode); COPY_ENTITY_PROPERTY_TO_PROPERTIES(backgroundMode, getBackgroundMode);
_skyboxProperties.getProperties(properties); // Contains a QString property, must be synchronized
withReadLock([&] {
_skyboxProperties.getProperties(properties);
});
COPY_ENTITY_PROPERTY_TO_PROPERTIES(flyingAllowed, getFlyingAllowed); COPY_ENTITY_PROPERTY_TO_PROPERTIES(flyingAllowed, getFlyingAllowed);
COPY_ENTITY_PROPERTY_TO_PROPERTIES(ghostingAllowed, getGhostingAllowed); COPY_ENTITY_PROPERTY_TO_PROPERTIES(ghostingAllowed, getGhostingAllowed);
@ -88,8 +93,10 @@ bool ZoneEntityItem::setProperties(const EntityItemProperties& properties) {
bool ZoneEntityItem::setSubClassProperties(const EntityItemProperties& properties) { bool ZoneEntityItem::setSubClassProperties(const EntityItemProperties& properties) {
bool somethingChanged = EntityItem::setSubClassProperties(properties); // set the properties in our base class bool somethingChanged = EntityItem::setSubClassProperties(properties); // set the properties in our base class
// Contains a QString property, must be synchronized
_keyLightPropertiesChanged = _keyLightProperties.setProperties(properties); withWriteLock([&] {
_keyLightPropertiesChanged = _keyLightProperties.setProperties(properties);
});
_stagePropertiesChanged = _stageProperties.setProperties(properties); _stagePropertiesChanged = _stageProperties.setProperties(properties);
@ -101,11 +108,13 @@ bool ZoneEntityItem::setSubClassProperties(const EntityItemProperties& propertie
SET_ENTITY_PROPERTY_FROM_PROPERTIES(ghostingAllowed, setGhostingAllowed); SET_ENTITY_PROPERTY_FROM_PROPERTIES(ghostingAllowed, setGhostingAllowed);
SET_ENTITY_PROPERTY_FROM_PROPERTIES(filterURL, setFilterURL); SET_ENTITY_PROPERTY_FROM_PROPERTIES(filterURL, setFilterURL);
_skyboxPropertiesChanged = _skyboxProperties.setProperties(properties); // Contains a QString property, must be synchronized
withWriteLock([&] {
_skyboxPropertiesChanged = _skyboxProperties.setProperties(properties);
});
somethingChanged = somethingChanged || _keyLightPropertiesChanged || _stagePropertiesChanged || _skyboxPropertiesChanged; somethingChanged = somethingChanged || _keyLightPropertiesChanged || _stagePropertiesChanged || _skyboxPropertiesChanged;
return somethingChanged; return somethingChanged;
} }
@ -116,8 +125,12 @@ int ZoneEntityItem::readEntitySubclassDataFromBuffer(const unsigned char* data,
int bytesRead = 0; int bytesRead = 0;
const unsigned char* dataAt = data; const unsigned char* dataAt = data;
int bytesFromKeylight = _keyLightProperties.readEntitySubclassDataFromBuffer(dataAt, (bytesLeftToRead - bytesRead), args, int bytesFromKeylight;
propertyFlags, overwriteLocalData, _keyLightPropertiesChanged); withWriteLock([&] {
bytesFromKeylight = _keyLightProperties.readEntitySubclassDataFromBuffer(dataAt, (bytesLeftToRead - bytesRead), args,
propertyFlags, overwriteLocalData, _keyLightPropertiesChanged);
});
somethingChanged = somethingChanged || _keyLightPropertiesChanged; somethingChanged = somethingChanged || _keyLightPropertiesChanged;
bytesRead += bytesFromKeylight; bytesRead += bytesFromKeylight;
dataAt += bytesFromKeylight; dataAt += bytesFromKeylight;
@ -132,8 +145,11 @@ int ZoneEntityItem::readEntitySubclassDataFromBuffer(const unsigned char* data,
READ_ENTITY_PROPERTY(PROP_COMPOUND_SHAPE_URL, QString, setCompoundShapeURL); READ_ENTITY_PROPERTY(PROP_COMPOUND_SHAPE_URL, QString, setCompoundShapeURL);
READ_ENTITY_PROPERTY(PROP_BACKGROUND_MODE, BackgroundMode, setBackgroundMode); READ_ENTITY_PROPERTY(PROP_BACKGROUND_MODE, BackgroundMode, setBackgroundMode);
int bytesFromSkybox = _skyboxProperties.readEntitySubclassDataFromBuffer(dataAt, (bytesLeftToRead - bytesRead), args, int bytesFromSkybox;
propertyFlags, overwriteLocalData, _skyboxPropertiesChanged); withWriteLock([&] {
bytesFromSkybox = _skyboxProperties.readEntitySubclassDataFromBuffer(dataAt, (bytesLeftToRead - bytesRead), args,
propertyFlags, overwriteLocalData, _skyboxPropertiesChanged);
});
somethingChanged = somethingChanged || _skyboxPropertiesChanged; somethingChanged = somethingChanged || _skyboxPropertiesChanged;
bytesRead += bytesFromSkybox; bytesRead += bytesFromSkybox;
dataAt += bytesFromSkybox; dataAt += bytesFromSkybox;
@ -150,13 +166,18 @@ int ZoneEntityItem::readEntitySubclassDataFromBuffer(const unsigned char* data,
EntityPropertyFlags ZoneEntityItem::getEntityProperties(EncodeBitstreamParams& params) const { EntityPropertyFlags ZoneEntityItem::getEntityProperties(EncodeBitstreamParams& params) const {
EntityPropertyFlags requestedProperties = EntityItem::getEntityProperties(params); EntityPropertyFlags requestedProperties = EntityItem::getEntityProperties(params);
requestedProperties += _keyLightProperties.getEntityProperties(params); withReadLock([&] {
requestedProperties += _keyLightProperties.getEntityProperties(params);
});
requestedProperties += PROP_SHAPE_TYPE; requestedProperties += PROP_SHAPE_TYPE;
requestedProperties += PROP_COMPOUND_SHAPE_URL; requestedProperties += PROP_COMPOUND_SHAPE_URL;
requestedProperties += PROP_BACKGROUND_MODE; requestedProperties += PROP_BACKGROUND_MODE;
requestedProperties += _stageProperties.getEntityProperties(params); requestedProperties += _stageProperties.getEntityProperties(params);
requestedProperties += _skyboxProperties.getEntityProperties(params);
withReadLock([&] {
requestedProperties += _skyboxProperties.getEntityProperties(params);
});
requestedProperties += PROP_FLYING_ALLOWED; requestedProperties += PROP_FLYING_ALLOWED;
requestedProperties += PROP_GHOSTING_ALLOWED; requestedProperties += PROP_GHOSTING_ALLOWED;

View file

@ -63,12 +63,12 @@ public:
QString getCompoundShapeURL() const; QString getCompoundShapeURL() const;
virtual void setCompoundShapeURL(const QString& url); virtual void setCompoundShapeURL(const QString& url);
const KeyLightPropertyGroup& getKeyLightProperties() const { return _keyLightProperties; } KeyLightPropertyGroup getKeyLightProperties() const { return resultWithReadLock<KeyLightPropertyGroup>([&] { return _keyLightProperties; }); }
void setBackgroundMode(BackgroundMode value) { _backgroundMode = value; _backgroundPropertiesChanged = true; } void setBackgroundMode(BackgroundMode value) { _backgroundMode = value; _backgroundPropertiesChanged = true; }
BackgroundMode getBackgroundMode() const { return _backgroundMode; } BackgroundMode getBackgroundMode() const { return _backgroundMode; }
const SkyboxPropertyGroup& getSkyboxProperties() const { return _skyboxProperties; } SkyboxPropertyGroup getSkyboxProperties() const { return resultWithReadLock<SkyboxPropertyGroup>([&] { return _skyboxProperties; }); }
const StagePropertyGroup& getStageProperties() const { return _stageProperties; } const StagePropertyGroup& getStageProperties() const { return _stageProperties; }
bool getFlyingAllowed() const { return _flyingAllowed; } bool getFlyingAllowed() const { return _flyingAllowed; }