From 1438643a5fc2d59f14a16385da1bd31434ee956c Mon Sep 17 00:00:00 2001 From: Sam Gateau Date: Thu, 13 Sep 2018 23:48:50 -0700 Subject: [PATCH 1/3] Adding extra sanity checks on all time dependant values to avoid bad behavior maybe --- interface/src/LODManager.cpp | 37 +++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index 2b5ca9ae8c..63f76ce705 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -45,10 +45,11 @@ const float LOD_ADJUST_RUNNING_AVG_TIMESCALE = 0.08f; // sec const float LOD_BATCH_TO_PRESENT_CUSHION_TIME = 3.0f; // msec void LODManager::setRenderTimes(float presentTime, float engineRunTime, float batchTime, float gpuTime) { - _presentTime = presentTime; - _engineRunTime = engineRunTime; - _batchTime = batchTime; - _gpuTime = gpuTime; + // Make sure the sampled time are positive values + _presentTime = std::max(0.f, presentTime); + _engineRunTime = std::max(0.f, engineRunTime); + _batchTime = std::max(0.f, batchTime); + _gpuTime = std::max(0.f, gpuTime); } void LODManager::autoAdjustLOD(float realTimeDelta) { @@ -64,16 +65,29 @@ void LODManager::autoAdjustLOD(float realTimeDelta) { auto presentTime = (_presentTime > _batchTime + LOD_BATCH_TO_PRESENT_CUSHION_TIME ? _batchTime + LOD_BATCH_TO_PRESENT_CUSHION_TIME : _presentTime); float maxRenderTime = glm::max(glm::max(presentTime, _engineRunTime), _gpuTime); - // compute time-weighted running average maxRenderTime - // Note: we MUST clamp the blend to 1.0 for stability - float nowBlend = (realTimeDelta < LOD_ADJUST_RUNNING_AVG_TIMESCALE) ? realTimeDelta / LOD_ADJUST_RUNNING_AVG_TIMESCALE : 1.0f; - _nowRenderTime = (1.0f - nowBlend) * _nowRenderTime + nowBlend * maxRenderTime; // msec + // maxRenderTime must be a realistic valid duration in order for the regulation to work correctly. + // We make sure it s a non zero positive value (1.0ms) under 1 sec + maxRenderTime = std::max(1.0f, std::min(maxRenderTime, (float)MSECS_PER_SECOND)); + // realTimeDelta must be a realistic valid duration in order for the regulation to work correctly. + // We make sure it a positive value under 1 sec + // note that if real time delta is very small we will early exit to avoid division by zero + realTimeDelta = std::max(0.0f, std::min(realTimeDelta, 1.0f)); + + // compute time-weighted running average render time (now and smooth) + // We MUST clamp the blend between 0.0 and 1.0 for stability + float nowBlend = (realTimeDelta < LOD_ADJUST_RUNNING_AVG_TIMESCALE) ? realTimeDelta / LOD_ADJUST_RUNNING_AVG_TIMESCALE : 1.0f; float smoothBlend = (realTimeDelta < LOD_ADJUST_RUNNING_AVG_TIMESCALE * _smoothScale) ? realTimeDelta / (LOD_ADJUST_RUNNING_AVG_TIMESCALE * _smoothScale) : 1.0f; + + _nowRenderTime = (1.0f - nowBlend) * _nowRenderTime + nowBlend * maxRenderTime; // msec _smoothRenderTime = (1.0f - smoothBlend) * _smoothRenderTime + smoothBlend * maxRenderTime; // msec - if (!_automaticLODAdjust || _nowRenderTime == 0.0f || _smoothRenderTime == 0.0f) { - // early exit + // We must sanity check for the output average evaluated to be in a valid range to avoid issues + _nowRenderTime = std::max(0.0f, std::min(_nowRenderTime, (float)MSECS_PER_SECOND)); + _smoothRenderTime = std::max(0.0f, std::min(_smoothRenderTime, (float)MSECS_PER_SECOND)); + + // Early exit if not regulating or if the render time doesn't matter + if (!_automaticLODAdjust || realTimeDelta == 0.f || _nowRenderTime == 0.0f || _smoothRenderTime == 0.0f) { return; } @@ -130,7 +144,8 @@ void LODManager::autoAdjustLOD(float realTimeDelta) { glm::clamp(integral, -1.0f, 1.0f); // Compute derivative - auto derivative = (error - previous_error) / dt; + // if dt is never zero becasuerrealTImeDelta would have early exit above, but if it was let's zero the derivative term + auto derivative = (dt <= 0.f ? 0.0f : (error - previous_error) / dt); // remember history _pidHistory.x = error; From eccca053f92f776027c086c806b7c8d45b2b0bf8 Mon Sep 17 00:00:00 2001 From: Sam Gateau Date: Thu, 13 Sep 2018 23:58:40 -0700 Subject: [PATCH 2/3] Clean changes --- interface/src/LODManager.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index 63f76ce705..fc34b7ccf2 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -79,15 +79,15 @@ void LODManager::autoAdjustLOD(float realTimeDelta) { float nowBlend = (realTimeDelta < LOD_ADJUST_RUNNING_AVG_TIMESCALE) ? realTimeDelta / LOD_ADJUST_RUNNING_AVG_TIMESCALE : 1.0f; float smoothBlend = (realTimeDelta < LOD_ADJUST_RUNNING_AVG_TIMESCALE * _smoothScale) ? realTimeDelta / (LOD_ADJUST_RUNNING_AVG_TIMESCALE * _smoothScale) : 1.0f; - _nowRenderTime = (1.0f - nowBlend) * _nowRenderTime + nowBlend * maxRenderTime; // msec - _smoothRenderTime = (1.0f - smoothBlend) * _smoothRenderTime + smoothBlend * maxRenderTime; // msec - + //Evaluate the running averages for the render time // We must sanity check for the output average evaluated to be in a valid range to avoid issues + _nowRenderTime = (1.0f - nowBlend) * _nowRenderTime + nowBlend * maxRenderTime; // msec _nowRenderTime = std::max(0.0f, std::min(_nowRenderTime, (float)MSECS_PER_SECOND)); + _smoothRenderTime = (1.0f - smoothBlend) * _smoothRenderTime + smoothBlend * maxRenderTime; // msec _smoothRenderTime = std::max(0.0f, std::min(_smoothRenderTime, (float)MSECS_PER_SECOND)); - // Early exit if not regulating or if the render time doesn't matter - if (!_automaticLODAdjust || realTimeDelta == 0.f || _nowRenderTime == 0.0f || _smoothRenderTime == 0.0f) { + // Early exit if not regulating or if the simulation or render times don't matter + if (!_automaticLODAdjust || realTimeDelta <= 0.f || _nowRenderTime <= 0.0f || _smoothRenderTime <= 0.0f) { return; } @@ -144,7 +144,7 @@ void LODManager::autoAdjustLOD(float realTimeDelta) { glm::clamp(integral, -1.0f, 1.0f); // Compute derivative - // if dt is never zero becasuerrealTImeDelta would have early exit above, but if it was let's zero the derivative term + // if dt is never zero because realTimeDelta would have early exit above, but if it was let's zero the derivative term auto derivative = (dt <= 0.f ? 0.0f : (error - previous_error) / dt); // remember history From b35f09feadec905fcadf7afa56f8ba208426d0a5 Mon Sep 17 00:00:00 2001 From: sam gateau Date: Fri, 14 Sep 2018 08:43:35 -0700 Subject: [PATCH 3/3] Address review comments --- interface/src/LODManager.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index fc34b7ccf2..729bfd9e45 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -46,10 +46,10 @@ const float LOD_BATCH_TO_PRESENT_CUSHION_TIME = 3.0f; // msec void LODManager::setRenderTimes(float presentTime, float engineRunTime, float batchTime, float gpuTime) { // Make sure the sampled time are positive values - _presentTime = std::max(0.f, presentTime); - _engineRunTime = std::max(0.f, engineRunTime); - _batchTime = std::max(0.f, batchTime); - _gpuTime = std::max(0.f, gpuTime); + _presentTime = std::max(0.0f, presentTime); + _engineRunTime = std::max(0.0f, engineRunTime); + _batchTime = std::max(0.0f, batchTime); + _gpuTime = std::max(0.0f, gpuTime); } void LODManager::autoAdjustLOD(float realTimeDelta) { @@ -87,7 +87,7 @@ void LODManager::autoAdjustLOD(float realTimeDelta) { _smoothRenderTime = std::max(0.0f, std::min(_smoothRenderTime, (float)MSECS_PER_SECOND)); // Early exit if not regulating or if the simulation or render times don't matter - if (!_automaticLODAdjust || realTimeDelta <= 0.f || _nowRenderTime <= 0.0f || _smoothRenderTime <= 0.0f) { + if (!_automaticLODAdjust || realTimeDelta <= 0.0f || _nowRenderTime <= 0.0f || _smoothRenderTime <= 0.0f) { return; } @@ -144,8 +144,8 @@ void LODManager::autoAdjustLOD(float realTimeDelta) { glm::clamp(integral, -1.0f, 1.0f); // Compute derivative - // if dt is never zero because realTimeDelta would have early exit above, but if it was let's zero the derivative term - auto derivative = (dt <= 0.f ? 0.0f : (error - previous_error) / dt); + // dt is never zero because realTimeDelta would have early exit above, but if it ever was let's zero the derivative term + auto derivative = (dt <= 0.0f ? 0.0f : (error - previous_error) / dt); // remember history _pidHistory.x = error;