From edd256c6004732eed5abe05bb35a8ebb1455843e Mon Sep 17 00:00:00 2001 From: Nissim Date: Thu, 19 Oct 2017 13:36:32 -0700 Subject: [PATCH 1/8] Fixed crash on invalid HazeMode. --- libraries/entities/src/EntityItemProperties.cpp | 14 ++++++++++++-- libraries/entities/src/ZoneEntityItem.cpp | 10 +++++++++- libraries/entities/src/ZoneEntityItem.h | 2 +- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 7ad2f22144..cfe9bda1c0 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -228,11 +228,21 @@ const std::array COMPONENT_MODES = { { } }; QString EntityItemProperties::getHazeModeAsString() const { - return COMPONENT_MODES[_hazeMode].second; + // return "inherit" if _hazeMode is not valid + if (_hazeMode >= 0 && _hazeMode < COMPONENT_MODE_ITEM_COUNT) { + return COMPONENT_MODES[_hazeMode].second; + } else { + return COMPONENT_MODES[0].second; + } } QString EntityItemProperties::getHazeModeString(uint32_t mode) { - return COMPONENT_MODES[mode].second; + // return "inherit" if mode is not valid + if (mode >= 0 && mode < COMPONENT_MODE_ITEM_COUNT) { + return COMPONENT_MODES[mode].second; + } else { + return COMPONENT_MODES[0].second; + } } void EntityItemProperties::setHazeModeFromString(const QString& hazeMode) { diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 077024c3ab..540a31f6c4 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -44,6 +44,7 @@ ZoneEntityItem::ZoneEntityItem(const EntityItemID& entityItemID) : EntityItem(en _compoundShapeURL = DEFAULT_COMPOUND_SHAPE_URL; _backgroundMode = BACKGROUND_MODE_INHERIT; + _hazeMode = (uint32_t)COMPONENT_MODE_INHERIT; } EntityItemProperties ZoneEntityItem::getProperties(EntityPropertyFlags desiredProperties) const { @@ -320,8 +321,15 @@ void ZoneEntityItem::resetRenderingPropertiesChanged() { }); } +#pragma optimize("", off) void ZoneEntityItem::setHazeMode(const uint32_t value) { - _hazeMode = value; + if (_hazeMode >= 0 && _hazeMode < COMPONENT_MODE_ITEM_COUNT) { + _hazeMode = value; + } + else { + _hazeMode = 0; + } + _hazePropertiesChanged = true; } diff --git a/libraries/entities/src/ZoneEntityItem.h b/libraries/entities/src/ZoneEntityItem.h index 066bd5518f..877ca1303c 100644 --- a/libraries/entities/src/ZoneEntityItem.h +++ b/libraries/entities/src/ZoneEntityItem.h @@ -146,7 +146,7 @@ protected: BackgroundMode _backgroundMode = BACKGROUND_MODE_INHERIT; - uint8_t _hazeMode{ (uint8_t)COMPONENT_MODE_INHERIT }; + uint32_t _hazeMode{ (uint32_t)COMPONENT_MODE_INHERIT }; float _hazeRange{ HazePropertyGroup::DEFAULT_HAZE_RANGE }; xColor _hazeColor{ HazePropertyGroup::DEFAULT_HAZE_COLOR }; From 9af87d46f2e361afdc209738cfccac0b62798236 Mon Sep 17 00:00:00 2001 From: Nissim Date: Thu, 19 Oct 2017 13:51:33 -0700 Subject: [PATCH 2/8] Removed optimize pragma. --- libraries/entities/src/ZoneEntityItem.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 540a31f6c4..d1b397d1f8 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -321,7 +321,6 @@ void ZoneEntityItem::resetRenderingPropertiesChanged() { }); } -#pragma optimize("", off) void ZoneEntityItem::setHazeMode(const uint32_t value) { if (_hazeMode >= 0 && _hazeMode < COMPONENT_MODE_ITEM_COUNT) { _hazeMode = value; From e314ee432e9ca5393d0237a09c22fcd1f87f4520 Mon Sep 17 00:00:00 2001 From: Nissim Date: Thu, 19 Oct 2017 14:20:47 -0700 Subject: [PATCH 3/8] Fixed gcc warning. --- libraries/entities/src/EntityItemProperties.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 80a1d0827e..adcfa412cf 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -229,7 +229,7 @@ const std::array COMPONENT_MODES = { { QString EntityItemProperties::getHazeModeAsString() const { // return "inherit" if _hazeMode is not valid - if (_hazeMode >= 0 && _hazeMode < COMPONENT_MODE_ITEM_COUNT) { + if (_hazeMode < COMPONENT_MODE_ITEM_COUNT) { return COMPONENT_MODES[_hazeMode].second; } else { return COMPONENT_MODES[0].second; @@ -238,7 +238,7 @@ QString EntityItemProperties::getHazeModeAsString() const { QString EntityItemProperties::getHazeModeString(uint32_t mode) { // return "inherit" if mode is not valid - if (mode >= 0 && mode < COMPONENT_MODE_ITEM_COUNT) { + if (mode < COMPONENT_MODE_ITEM_COUNT) { return COMPONENT_MODES[mode].second; } else { return COMPONENT_MODES[0].second; From d01ecb71dfc607b6e32d87df3c0065fdd89bc63c Mon Sep 17 00:00:00 2001 From: Nissim Date: Thu, 19 Oct 2017 14:41:02 -0700 Subject: [PATCH 4/8] Fixed gcc warning. --- libraries/entities/src/ZoneEntityItem.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index d1b397d1f8..822c0341e6 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -322,7 +322,7 @@ void ZoneEntityItem::resetRenderingPropertiesChanged() { } void ZoneEntityItem::setHazeMode(const uint32_t value) { - if (_hazeMode >= 0 && _hazeMode < COMPONENT_MODE_ITEM_COUNT) { + if (value < COMPONENT_MODE_ITEM_COUNT) { _hazeMode = value; } else { From 0f519da76e4a13d962e07ae77138527185bc64ec Mon Sep 17 00:00:00 2001 From: Nissim Date: Thu, 19 Oct 2017 17:16:36 -0700 Subject: [PATCH 5/8] Additional protection for mode. --- libraries/model/src/model/Stage.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/libraries/model/src/model/Stage.cpp b/libraries/model/src/model/Stage.cpp index 75a94f0cea..189aa9a184 100644 --- a/libraries/model/src/model/Stage.cpp +++ b/libraries/model/src/model/Stage.cpp @@ -13,6 +13,7 @@ #include #include #include +#include using namespace model; @@ -256,8 +257,12 @@ void SunSkyStage::setSkybox(const SkyboxPointer& skybox) { invalidate(); } -// Haze -void SunSkyStage::setHazeMode(uint32_t mode) { - _hazeMode = mode; +void SunSkyStage::setHazeMode(uint32_t hazeMode) { + if (hazeMode < COMPONENT_MODE_ITEM_COUNT) { + _hazeMode = hazeMode; + } else { + _hazeMode = 0; + } + invalidate(); } From b0ca1353e4e055f8110749f15ac5e93e2d3f8bdb Mon Sep 17 00:00:00 2001 From: Nissim Date: Fri, 20 Oct 2017 13:32:27 -0700 Subject: [PATCH 6/8] Implemented Haze mode default. --- libraries/entities/src/ZoneEntityItem.cpp | 1 - libraries/entities/src/ZoneEntityItem.h | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 822c0341e6..3b5bcb2450 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -44,7 +44,6 @@ ZoneEntityItem::ZoneEntityItem(const EntityItemID& entityItemID) : EntityItem(en _compoundShapeURL = DEFAULT_COMPOUND_SHAPE_URL; _backgroundMode = BACKGROUND_MODE_INHERIT; - _hazeMode = (uint32_t)COMPONENT_MODE_INHERIT; } EntityItemProperties ZoneEntityItem::getProperties(EntityPropertyFlags desiredProperties) const { diff --git a/libraries/entities/src/ZoneEntityItem.h b/libraries/entities/src/ZoneEntityItem.h index 877ca1303c..ddbb2ed914 100644 --- a/libraries/entities/src/ZoneEntityItem.h +++ b/libraries/entities/src/ZoneEntityItem.h @@ -138,6 +138,8 @@ public: static const bool DEFAULT_GHOSTING_ALLOWED; static const QString DEFAULT_FILTER_URL; + static const uint32_t DEFAULT_HAZE_MODE{ (uint32_t)COMPONENT_MODE_INHERIT }; + protected: KeyLightPropertyGroup _keyLightProperties; @@ -146,7 +148,7 @@ protected: BackgroundMode _backgroundMode = BACKGROUND_MODE_INHERIT; - uint32_t _hazeMode{ (uint32_t)COMPONENT_MODE_INHERIT }; + uint32_t _hazeMode{ DEFAULT_HAZE_MODE }; float _hazeRange{ HazePropertyGroup::DEFAULT_HAZE_RANGE }; xColor _hazeColor{ HazePropertyGroup::DEFAULT_HAZE_COLOR }; From 00c99d3d02dd84322eca21bc01121e9ff0015e78 Mon Sep 17 00:00:00 2001 From: Nissim Date: Fri, 20 Oct 2017 13:53:53 -0700 Subject: [PATCH 7/8] Corrected wrong else format. --- libraries/entities/src/ZoneEntityItem.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 3b5bcb2450..4b1bf53bcc 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -323,8 +323,7 @@ void ZoneEntityItem::resetRenderingPropertiesChanged() { void ZoneEntityItem::setHazeMode(const uint32_t value) { if (value < COMPONENT_MODE_ITEM_COUNT) { _hazeMode = value; - } - else { + } else { _hazeMode = 0; } From 5d0deaf9462201848a86f3f8763e0dae20e4cec4 Mon Sep 17 00:00:00 2001 From: Nissim Date: Fri, 20 Oct 2017 14:27:27 -0700 Subject: [PATCH 8/8] Code review fixes. --- libraries/entities/src/EntityItemProperties.cpp | 4 ++-- libraries/entities/src/ZoneEntityItem.cpp | 5 +---- libraries/model/src/model/Stage.cpp | 5 +---- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index adcfa412cf..51ed66bb23 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -232,7 +232,7 @@ QString EntityItemProperties::getHazeModeAsString() const { if (_hazeMode < COMPONENT_MODE_ITEM_COUNT) { return COMPONENT_MODES[_hazeMode].second; } else { - return COMPONENT_MODES[0].second; + return COMPONENT_MODES[COMPONENT_MODE_INHERIT].second; } } @@ -241,7 +241,7 @@ QString EntityItemProperties::getHazeModeString(uint32_t mode) { if (mode < COMPONENT_MODE_ITEM_COUNT) { return COMPONENT_MODES[mode].second; } else { - return COMPONENT_MODES[0].second; + return COMPONENT_MODES[COMPONENT_MODE_INHERIT].second; } } diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 4b1bf53bcc..588c1f9386 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -323,11 +323,8 @@ void ZoneEntityItem::resetRenderingPropertiesChanged() { void ZoneEntityItem::setHazeMode(const uint32_t value) { if (value < COMPONENT_MODE_ITEM_COUNT) { _hazeMode = value; - } else { - _hazeMode = 0; + _hazePropertiesChanged = true; } - - _hazePropertiesChanged = true; } uint32_t ZoneEntityItem::getHazeMode() const { diff --git a/libraries/model/src/model/Stage.cpp b/libraries/model/src/model/Stage.cpp index 189aa9a184..cd2312122c 100644 --- a/libraries/model/src/model/Stage.cpp +++ b/libraries/model/src/model/Stage.cpp @@ -260,9 +260,6 @@ void SunSkyStage::setSkybox(const SkyboxPointer& skybox) { void SunSkyStage::setHazeMode(uint32_t hazeMode) { if (hazeMode < COMPONENT_MODE_ITEM_COUNT) { _hazeMode = hazeMode; - } else { - _hazeMode = 0; + invalidate(); } - - invalidate(); }