From 18d723b6b43b62f57ddeaf362bcc95ebf1afa261 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Tue, 10 Oct 2017 16:17:39 -0700 Subject: [PATCH 1/5] div by zero fixes, detected by address sanitizer --- interface/src/LODManager.cpp | 1 + libraries/animation/src/Rig.cpp | 7 ++++++- .../avatars-renderer/src/avatars-renderer/Avatar.cpp | 3 ++- libraries/shared/src/MovingMinMaxAvg.h | 8 ++++++-- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index d3c8746e16..199f3ea2c6 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -54,6 +54,7 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta float renderTime = batchTime + OVERLAY_AND_SWAP_TIME_BUDGET; float maxTime = glm::max(renderTime, engineRunTime); const float BLEND_TIMESCALE = 0.3f; // sec + const float safeDeltaTime = (deltaTime == 0.0f) ? 0.001f : deltaTime; float blend = BLEND_TIMESCALE / deltaTimeSec; if (blend > 1.0f) { blend = 1.0f; diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp index 0897c26a12..83d258fb08 100644 --- a/libraries/animation/src/Rig.cpp +++ b/libraries/animation/src/Rig.cpp @@ -38,7 +38,12 @@ static std::mutex rigRegistryMutex; static bool isEqual(const glm::vec3& u, const glm::vec3& v) { const float EPSILON = 0.0001f; - return glm::length(u - v) / glm::length(u) <= EPSILON; + float uLen = glm::length(u); + if (uLen == 0.0f) { + return glm::length(v) <= EPSILON; + } else { + return glm::length(u - v) / glm::length(u) <= EPSILON; + } } static bool isEqual(const glm::quat& p, const glm::quat& q) { diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 87d4a2d343..a52582ff45 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -446,7 +446,8 @@ void Avatar::applyPositionDelta(const glm::vec3& delta) { void Avatar::measureMotionDerivatives(float deltaTime) { PerformanceTimer perfTimer("derivatives"); // linear - float invDeltaTime = 1.0f / deltaTime; + const float safeDeltaTime = (deltaTime == 0.0f) ? 0.001f : deltaTime; + float invDeltaTime = 1.0f / safeDeltaTime; // Floating point error prevents us from computing velocity in a naive way // (e.g. vel = (pos - oldPos) / dt) so instead we use _positionOffsetAccumulator. glm::vec3 velocity = _positionDeltaAccumulator * invDeltaTime; diff --git a/libraries/shared/src/MovingMinMaxAvg.h b/libraries/shared/src/MovingMinMaxAvg.h index 580baf7317..782b0dc523 100644 --- a/libraries/shared/src/MovingMinMaxAvg.h +++ b/libraries/shared/src/MovingMinMaxAvg.h @@ -59,8 +59,12 @@ public: _max = other._max; } double totalSamples = _samples + other._samples; - _average = _average * ((double)_samples / totalSamples) - + other._average * ((double)other._samples / totalSamples); + if (totalSamples > 0) { + _average = _average * ((double)_samples / totalSamples) + + other._average * ((double)other._samples / totalSamples); + } else { + _average = 0.0f; + } _samples += other._samples; } From cbb0d27f684b4e0f3c091c6a9268c1e2309426f7 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Tue, 10 Oct 2017 16:20:52 -0700 Subject: [PATCH 2/5] div by zero fix --- interface/src/LODManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index 199f3ea2c6..4bdb13545e 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -55,7 +55,7 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta float maxTime = glm::max(renderTime, engineRunTime); const float BLEND_TIMESCALE = 0.3f; // sec const float safeDeltaTime = (deltaTime == 0.0f) ? 0.001f : deltaTime; - float blend = BLEND_TIMESCALE / deltaTimeSec; + float blend = BLEND_TIMESCALE / safeDeltaTime; if (blend > 1.0f) { blend = 1.0f; } From fe57a209792e06952da19999e3aeb277a13d3822 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Tue, 10 Oct 2017 16:22:49 -0700 Subject: [PATCH 3/5] div by zero fix --- libraries/animation/src/Rig.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp index 83d258fb08..bd19dcc0e8 100644 --- a/libraries/animation/src/Rig.cpp +++ b/libraries/animation/src/Rig.cpp @@ -42,7 +42,7 @@ static bool isEqual(const glm::vec3& u, const glm::vec3& v) { if (uLen == 0.0f) { return glm::length(v) <= EPSILON; } else { - return glm::length(u - v) / glm::length(u) <= EPSILON; + return (glm::length(u - v) / uLen) <= EPSILON; } } From cb01c5cada93ed0d4e02f250c7a9047efcc1d227 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Tue, 10 Oct 2017 16:30:11 -0700 Subject: [PATCH 4/5] compile fix --- interface/src/LODManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index 4bdb13545e..8f01296e1c 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -54,7 +54,7 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta float renderTime = batchTime + OVERLAY_AND_SWAP_TIME_BUDGET; float maxTime = glm::max(renderTime, engineRunTime); const float BLEND_TIMESCALE = 0.3f; // sec - const float safeDeltaTime = (deltaTime == 0.0f) ? 0.001f : deltaTime; + const float safeDeltaTime = (deltaTimeSec == 0.0f) ? 0.001f : deltaTimeSec; float blend = BLEND_TIMESCALE / safeDeltaTime; if (blend > 1.0f) { blend = 1.0f; From ba4c0f189e9600921efa5c4107858775b04b6de3 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Fri, 10 Nov 2017 10:25:29 -0800 Subject: [PATCH 5/5] code review feedback removed discontinuity in safeDeltaTime, which is used to prevent division by zero. --- interface/src/LODManager.cpp | 3 ++- libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index 8f01296e1c..01ccbd0d9a 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -54,7 +54,8 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta float renderTime = batchTime + OVERLAY_AND_SWAP_TIME_BUDGET; float maxTime = glm::max(renderTime, engineRunTime); const float BLEND_TIMESCALE = 0.3f; // sec - const float safeDeltaTime = (deltaTimeSec == 0.0f) ? 0.001f : deltaTimeSec; + const float MIN_DELTA_TIME = 0.001f; + const float safeDeltaTime = glm::max(deltaTimeSec, MIN_DELTA_TIME); float blend = BLEND_TIMESCALE / safeDeltaTime; if (blend > 1.0f) { blend = 1.0f; diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 2335095056..49d2431098 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -453,7 +453,8 @@ void Avatar::applyPositionDelta(const glm::vec3& delta) { void Avatar::measureMotionDerivatives(float deltaTime) { PerformanceTimer perfTimer("derivatives"); // linear - const float safeDeltaTime = (deltaTime == 0.0f) ? 0.001f : deltaTime; + const float MIN_DELTA_TIME = 0.001f; + const float safeDeltaTime = glm::max(deltaTime, MIN_DELTA_TIME); float invDeltaTime = 1.0f / safeDeltaTime; // Floating point error prevents us from computing velocity in a naive way // (e.g. vel = (pos - oldPos) / dt) so instead we use _positionOffsetAccumulator.