From 32e4967ee0a66e8b2f9eba8a06e53b5d553b0d19 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Tue, 8 Dec 2015 16:38:05 -0800 Subject: [PATCH] cleanup --- interface/src/Application.cpp | 37 ++++--------------- interface/src/Application.h | 10 +---- interface/src/avatar/Avatar.cpp | 4 +- interface/src/avatar/AvatarManager.cpp | 23 ++---------- .../display-plugins/OpenGLDisplayPlugin.cpp | 3 +- .../src/display-plugins/OpenGLDisplayPlugin.h | 3 -- libraries/plugins/src/plugins/DisplayPlugin.h | 1 - libraries/shared/src/PIDController.cpp | 8 ++-- libraries/shared/src/PIDController.h | 5 +-- 9 files changed, 20 insertions(+), 74 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index f56231ada8..78882342df 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1086,7 +1086,6 @@ void Application::paintGL() { // update fps moving average uint64_t now = usecTimestampNow(); - qint64 sinceSync = getActiveDisplayPlugin()->getTimeSinceSync(); static uint64_t lastPaintBegin{ now }; uint64_t diff = now - lastPaintBegin; float instantaneousFps = 0.0f; @@ -1123,31 +1122,6 @@ void Application::paintGL() { _inPaint = true; Finally clearFlagLambda([this] { _inPaint = false; }); - //_lastPaintWait = (float)(now - _paintWaitStart) / (float)USECS_PER_SECOND; - //_lastInstantaneousFps = instantaneousFps; - // Some LOD-like controls need to know a smoothly varying "potential" frame rate that doesn't - // include time waiting for vsync, and which can report a number above target if we've got the headroom. - // For example, if we're shooting for 75fps and paintWait is 3.3333ms (= 75% * 13.33ms), our deducedNonVSyncFps - // would be 100fps. In principle, a paintWait of zero would have deducedNonVSyncFps=75. - // Here we make a guess for deducedNonVSyncFps = 1 / deducedNonVSyncPeriod. - // - // Time between previous paintGL call and this one, which can vary not only with vSync misses, but also with QT timing. - // We're using this as a proxy for the time between vsync and displayEnd, below. (Not exact, but tends to be the same over time.) - // This is not the same as update(deltaTime), because the latter attempts to throttle to 60hz and also clamps to 1/4 second. -/* const float actualPeriod = diff / (float)USECS_PER_SECOND; // same as 1/instantaneousFps but easier for compiler to optimize - // Note that _lastPaintWait (stored at end of last call) is for the same paint cycle. - float deducedNonVSyncPeriod = actualPeriod - _lastPaintWait + _marginForDeducedFramePeriod; // plus a some non-zero time for machinery we can't measure - // We don't know how much time to allow for that, but if we went over the target period, we know it's at least the portion - // of paintWait up to the next vSync. This gives us enough of a penalty so that when actualPeriod crosses two cycles, - // the key part (and not an exagerated part) of _lastPaintWait is accounted for. - const float targetPeriod = getTargetFramePeriod(); - if (_lastPaintWait > EPSILON && actualPeriod > targetPeriod) { - // Don't use C++ remainder(). It's authors are mathematically insane. - deducedNonVSyncPeriod += fmod(targetPeriod, _lastPaintWait); - } - _lastDeducedNonVSyncFps = 1.0f / deducedNonVSyncPeriod; - */ - auto displayPlugin = getActiveDisplayPlugin(); // FIXME not needed anymore? _offscreenContext->makeCurrent(); @@ -1422,11 +1396,14 @@ void Application::paintGL() { batch.resetStages(); }); } - _paintWaitStart = usecTimestampNow(); - _lastPaintWait = (float)sinceSync / (float)MSECS_PER_SECOND; + + // Some LOD-like controls need to know a smoothly varying "potential" frame rate that doesn't + // include time waiting for sync, and which can report a number above target if we've got the headroom. + // In my tests, the following is mostly less than 0.5ms, and never more than 3ms. I don't think its worth measuring during runtime. + static const float paintWaitAndQTTimerAllowance = 0.001; // seconds + // Store both values now for use by next cycle. _lastInstantaneousFps = instantaneousFps; - _lastDeducedNonVSyncFps = 1.0f / (((_paintWaitStart - now) / (float)USECS_PER_SECOND) + ((float)sinceSync / (float)MSECS_PER_SECOND)); - // qCDebug(interfaceapp) << "pw/now/sync" << _paintWaitStart << now << sinceSync << "period" << (((_paintWaitStart - now) / (float)USECS_PER_SECOND) + ((float)sinceSync / (float)MSECS_PER_SECOND)) << "hz" << _lastDeducedNonVSyncFps; + _lastUnsynchronizedFps = 1.0f / (((usecTimestampNow() - now) / (float)USECS_PER_SECOND) + paintWaitAndQTTimerAllowance); } void Application::runTests() { diff --git a/interface/src/Application.h b/interface/src/Application.h index 197d9a7a60..17d609ee0c 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -163,11 +163,8 @@ public: float const HMD_TARGET_FRAME_RATE = 75.0f; float const DESKTOP_TARGET_FRAME_RATE = 60.0f; float getTargetFrameRate() { return isHMDMode() ? HMD_TARGET_FRAME_RATE : DESKTOP_TARGET_FRAME_RATE; } - float getTargetFramePeriod() { return isHMDMode() ? 1.0f / HMD_TARGET_FRAME_RATE : 1.0f / DESKTOP_TARGET_FRAME_RATE; } // same as 1/getTargetFrameRate, but w/compile-time division float getLastInstanteousFps() const { return _lastInstantaneousFps; } - float getLastPaintWait() const { return _lastPaintWait; }; - float getLastDeducedNonVSyncFps() const { return _lastDeducedNonVSyncFps; } - void setMarginForDeducedFramePeriod(float newValue) { _marginForDeducedFramePeriod = newValue; } + float getLastUnsynchronizedFps() const { return _lastUnsynchronizedFps; } float getFieldOfView() { return _fieldOfView.get(); } void setFieldOfView(float fov); @@ -442,10 +439,7 @@ private: QElapsedTimer _timerStart; QElapsedTimer _lastTimeUpdated; float _lastInstantaneousFps { 0.0f }; - float _lastPaintWait { 0.0f }; - uint64_t _paintWaitStart { 0 }; - float _lastDeducedNonVSyncFps { 0.0f }; - float _marginForDeducedFramePeriod{ 0.002f }; // 2ms, adjustable + float _lastUnsynchronizedFps { 0.0f }; ShapeManager _shapeManager; PhysicalEntitySimulation _entitySimulation; diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index e3b7cd5735..e2b92cc06f 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -174,14 +174,14 @@ void Avatar::simulate(float deltaTime) { _shouldSkipRender = false; _skeletonModel.setVisibleInScene(true, qApp->getMain3DScene()); if (!isControllerLogging) { // Test for isMyAvatar is prophylactic. Never occurs in current code. - //qCDebug(interfaceapp) << "Rerendering" << (isMyAvatar() ? "myself" : getSessionUUID()) << "for distance" << renderDistance; + qCDebug(interfaceapp) << "Rerendering" << (isMyAvatar() ? "myself" : getSessionUUID()) << "for distance" << renderDistance; } } } else if (distance > renderDistance * (1.0f + SKIP_HYSTERESIS_PROPORTION)) { _shouldSkipRender = true; _skeletonModel.setVisibleInScene(false, qApp->getMain3DScene()); if (!isControllerLogging) { - //qCDebug(interfaceapp) << "Unrendering" << (isMyAvatar() ? "myself" : getSessionUUID()) << "for distance" << renderDistance; + qCDebug(interfaceapp) << "Unrendering" << (isMyAvatar() ? "myself" : getSessionUUID()) << "for distance" << renderDistance; } } diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index c0485bdebb..59af6b18df 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -111,7 +111,6 @@ void AvatarManager::init() { _renderDistanceController.setKP(0.0008f); // Usually about 0.6 of largest that doesn't oscillate when other parameters 0. _renderDistanceController.setKI(0.0006f); // Big enough to bring us to target with the above KP. _renderDistanceController.setKD(0.000001f); // A touch of kd increases the speed by which we get there. - _renderDistanceController.setHistorySize("av", 240); //FIXME } void AvatarManager::updateMyAvatar(float deltaTime) { @@ -130,7 +129,7 @@ void AvatarManager::updateMyAvatar(float deltaTime) { _lastSendAvatarDataTime = now; } } -#include "InterfaceLogging.h" + void AvatarManager::updateOtherAvatars(float deltaTime) { // lock the hash for read to check the size QReadLocker lock(&_hashLock); @@ -151,24 +150,8 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { // The measured value is frame rate. When the controlled value (1 / render cutoff distance) // goes up, the render cutoff distance gets closer, the number of rendered avatars is less, and frame rate // goes up. - const float targetFps = 60.0f; - const float instantaneousFps = qApp->getLastInstanteousFps(); - const float paintWait = qApp->getLastPaintWait(); - const float deduced = qApp->getLastDeducedNonVSyncFps(); - /*const float actual = 1.0f / instantaneousFps; - const float target = 1.0f / targetFps; - const float modulus = (instantaneousFps >= targetFps) ? - (1.0f + floor(instantaneousFps / targetFps)) : - (1.0f / floor(targetFps / instantaneousFps)); - const float cap = modulus * targetFps; - const float deduced = instantaneousFps + ((cap - instantaneousFps) * (paintWait / target));*/ - /*qCDebug(interfaceapp) << "dump " << instantaneousFps << (1000.0f * paintWait) - << "(" << paintWait << actual - << "(" << firstAdjusted << machinery << secondAdjusted - << ")" << deduced << modulus << cap << capped << ")";*/ - - //const float deduced = qApp->getLastDeducedNonVSyncFps(); - const float distance = 1.0f / _renderDistanceController.update(deduced, deltaTime, false, paintWait, instantaneousFps); + const float deduced = qApp->getLastUnsynchronizedFps(); + const float distance = 1.0f / _renderDistanceController.update(deduced, deltaTime); _renderDistanceAverage.updateAverage(distance); _renderDistance = _renderDistanceAverage.getAverage(); int renderableCount = 0; diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp index b59ac24b52..9a0db0ad97 100644 --- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp @@ -168,9 +168,8 @@ OpenGLDisplayPlugin::OpenGLDisplayPlugin() { // This is likely to be mooted by further planned changes. if (_active && _sceneTextureEscrow.depth() <= 1) { #else - if (_active && _sceneTextureEscrow.depth() <= 1) { + if (_active && _sceneTextureEscrow.depth() < 1) { #endif - _sinceSync.start(); emit requestRender(); } }); diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h index 4231c8ff7f..ef78374994 100644 --- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h @@ -10,7 +10,6 @@ #include "DisplayPlugin.h" #include -#include #include #include @@ -41,7 +40,6 @@ public: } virtual QImage getScreenshot() const override; - virtual quint64 getTimeSinceSync() { return _sinceSync.elapsed(); } protected: friend class PresentThread; @@ -84,7 +82,6 @@ protected: GLTextureEscrow _sceneTextureEscrow; bool _vsyncSupported { false }; - QElapsedTimer _sinceSync; }; diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index c2e10dcc02..83afbc9402 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -58,7 +58,6 @@ public: /// By default, all HMDs are stereo virtual bool isStereo() const { return isHmd(); } virtual bool isThrottled() const { return false; } - virtual quint64 getTimeSinceSync() { return 0; } // Rendering support diff --git a/libraries/shared/src/PIDController.cpp b/libraries/shared/src/PIDController.cpp index 3607c281ec..78aa31e4c7 100644 --- a/libraries/shared/src/PIDController.cpp +++ b/libraries/shared/src/PIDController.cpp @@ -14,7 +14,7 @@ #include "SharedLogging.h" #include "PIDController.h" -float PIDController::update(float measuredValue, float dt, bool resetAccumulator, float fixme1, float fixme2) { +float PIDController::update(float measuredValue, float dt, bool resetAccumulator) { const float error = getMeasuredValueSetpoint() - measuredValue; // Sign is the direction we want measuredValue to go. Positive means go higher. const float p = getKP() * error; // term is Proportional to error @@ -32,7 +32,7 @@ float PIDController::update(float measuredValue, float dt, bool resetAccumulator getControlledValueHighLimit()); if (getIsLogging()) { // if logging/reporting - updateHistory(measuredValue, dt, error, accumulatedError, changeInError, p, i, d, computedValue, fixme1, fixme2); + updateHistory(measuredValue, dt, error, accumulatedError, changeInError, p, i, d, computedValue); } Q_ASSERT(!isnan(computedValue)); @@ -43,7 +43,7 @@ float PIDController::update(float measuredValue, float dt, bool resetAccumulator } // Just for logging/reporting. Used when picking/verifying the operational parameters. -void PIDController::updateHistory(float measuredValue, float dt, float error, float accumulatedError, float changeInError, float p, float i, float d, float computedValue, float fixme1, float fixme2) { +void PIDController::updateHistory(float measuredValue, float dt, float error, float accumulatedError, float changeInError, float p, float i, float d, float computedValue) { // Don't report each update(), as the I/O messes with the results a lot. // Instead, add to history, and then dump out at once when full. // Typically, the first few values reported in each batch should be ignored. @@ -59,7 +59,6 @@ void PIDController::updateHistory(float measuredValue, float dt, float error, fl next.i = i; next.d = d; next.computed = computedValue; - next.fixme1 = fixme1; next.fixme2 = fixme2; if (_history.size() == _history.capacity()) { // report when buffer is full reportHistory(); _history.resize(0); @@ -70,7 +69,6 @@ void PIDController::reportHistory() { for (int i = 0; i < _history.size(); i++) { Row& row = _history[i]; qCDebug(shared) << row.measured << row.dt << - (row.fixme1 * 1000.0f) << (row.fixme2) << "||" << row.error << row.accumulated << row.changed << "||" << row.p << row.i << row.d << row.computed << 1.0f/row.computed; } diff --git a/libraries/shared/src/PIDController.h b/libraries/shared/src/PIDController.h index 24e619b13b..0b2411530a 100644 --- a/libraries/shared/src/PIDController.h +++ b/libraries/shared/src/PIDController.h @@ -28,7 +28,7 @@ class PIDController { public: // These are the main interfaces: void setMeasuredValueSetpoint(float newValue) { _measuredValueSetpoint = newValue; } - float update(float measuredValue, float dt, bool resetAccumulator = false, float fixme1=0, float fixme2=0); // returns the new computedValue + float update(float measuredValue, float dt, bool resetAccumulator = false); // returns the new computedValue void setHistorySize(QString label = QString(""), int size = 0) { _history.reserve(size); _history.resize(0); _label = label; } // non-empty does logging bool getIsLogging() { return _history.capacity(); } @@ -64,11 +64,10 @@ public: float i; float d; float computed; - float fixme1; float fixme2; }; protected: void reportHistory(); - void updateHistory(float measured, float dt, float error, float accumulatedError, float changeInErro, float p, float i, float d, float computedValue, float fixme1, float fixme2); + void updateHistory(float measured, float dt, float error, float accumulatedError, float changeInErro, float p, float i, float d, float computedValue); float _measuredValueSetpoint { 0.0f }; float _controlledValueLowLimit { 0.0f }; float _controlledValueHighLimit { std::numeric_limits::max() };