Merge pull request #11355 from jherico/crash/zone-keylight

Fix race condition accessing QString sub-properties in zones
This commit is contained in:
Thijs Wenker 2017-09-14 16:03:05 +02:00 committed by GitHub
commit 523d099f70
4 changed files with 62 additions and 33 deletions

View file

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

View file

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

View file

@ -49,8 +49,10 @@ ZoneEntityItem::ZoneEntityItem(const EntityItemID& entityItemID) : EntityItem(en
EntityItemProperties ZoneEntityItem::getProperties(EntityPropertyFlags desiredProperties) const {
EntityItemProperties properties = EntityItem::getProperties(desiredProperties); // get the properties from our base class
_keyLightProperties.getProperties(properties);
// Contains a QString property, must be synchronized
withReadLock([&] {
_keyLightProperties.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(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(ghostingAllowed, getGhostingAllowed);
@ -88,8 +93,10 @@ bool ZoneEntityItem::setProperties(const EntityItemProperties& properties) {
bool ZoneEntityItem::setSubClassProperties(const EntityItemProperties& properties) {
bool somethingChanged = EntityItem::setSubClassProperties(properties); // set the properties in our base class
_keyLightPropertiesChanged = _keyLightProperties.setProperties(properties);
// Contains a QString property, must be synchronized
withWriteLock([&] {
_keyLightPropertiesChanged = _keyLightProperties.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(filterURL, setFilterURL);
_skyboxPropertiesChanged = _skyboxProperties.setProperties(properties);
// Contains a QString property, must be synchronized
withWriteLock([&] {
_skyboxPropertiesChanged = _skyboxProperties.setProperties(properties);
});
somethingChanged = somethingChanged || _keyLightPropertiesChanged || _stagePropertiesChanged || _skyboxPropertiesChanged;
return somethingChanged;
}
@ -116,8 +125,12 @@ int ZoneEntityItem::readEntitySubclassDataFromBuffer(const unsigned char* data,
int bytesRead = 0;
const unsigned char* dataAt = data;
int bytesFromKeylight = _keyLightProperties.readEntitySubclassDataFromBuffer(dataAt, (bytesLeftToRead - bytesRead), args,
propertyFlags, overwriteLocalData, _keyLightPropertiesChanged);
int bytesFromKeylight;
withWriteLock([&] {
bytesFromKeylight = _keyLightProperties.readEntitySubclassDataFromBuffer(dataAt, (bytesLeftToRead - bytesRead), args,
propertyFlags, overwriteLocalData, _keyLightPropertiesChanged);
});
somethingChanged = somethingChanged || _keyLightPropertiesChanged;
bytesRead += 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_BACKGROUND_MODE, BackgroundMode, setBackgroundMode);
int bytesFromSkybox = _skyboxProperties.readEntitySubclassDataFromBuffer(dataAt, (bytesLeftToRead - bytesRead), args,
propertyFlags, overwriteLocalData, _skyboxPropertiesChanged);
int bytesFromSkybox;
withWriteLock([&] {
bytesFromSkybox = _skyboxProperties.readEntitySubclassDataFromBuffer(dataAt, (bytesLeftToRead - bytesRead), args,
propertyFlags, overwriteLocalData, _skyboxPropertiesChanged);
});
somethingChanged = somethingChanged || _skyboxPropertiesChanged;
bytesRead += bytesFromSkybox;
dataAt += bytesFromSkybox;
@ -150,13 +166,18 @@ int ZoneEntityItem::readEntitySubclassDataFromBuffer(const unsigned char* data,
EntityPropertyFlags ZoneEntityItem::getEntityProperties(EncodeBitstreamParams& params) const {
EntityPropertyFlags requestedProperties = EntityItem::getEntityProperties(params);
requestedProperties += _keyLightProperties.getEntityProperties(params);
withReadLock([&] {
requestedProperties += _keyLightProperties.getEntityProperties(params);
});
requestedProperties += PROP_SHAPE_TYPE;
requestedProperties += PROP_COMPOUND_SHAPE_URL;
requestedProperties += PROP_BACKGROUND_MODE;
requestedProperties += _stageProperties.getEntityProperties(params);
requestedProperties += _skyboxProperties.getEntityProperties(params);
withReadLock([&] {
requestedProperties += _skyboxProperties.getEntityProperties(params);
});
requestedProperties += PROP_FLYING_ALLOWED;
requestedProperties += PROP_GHOSTING_ALLOWED;

View file

@ -63,12 +63,12 @@ public:
QString getCompoundShapeURL() const;
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; }
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; }
bool getFlyingAllowed() const { return _flyingAllowed; }