Merge pull request #7816 from zzmp/perf/idle-event

Drive render loop from DisplayPlugin instead of busy wait

interface/src/Application.cpp

@@ -349,19 +349,14 @@ public:
 };
 #endif
 
-enum CustomEventTypes {
-    Lambda = QEvent::User + 1,
-    Paint = Lambda + 1,
-};
-
 class LambdaEvent : public QEvent {
     std::function<void()> _fun;
 public:
     LambdaEvent(const std::function<void()> & fun) :
-    QEvent(static_cast<QEvent::Type>(Lambda)), _fun(fun) {
+    QEvent(static_cast<QEvent::Type>(Application::Lambda)), _fun(fun) {
     }
     LambdaEvent(std::function<void()> && fun) :
-    QEvent(static_cast<QEvent::Type>(Lambda)), _fun(fun) {
+    QEvent(static_cast<QEvent::Type>(Application::Lambda)), _fun(fun) {
     }
     void call() const { _fun(); }
 };
@@ -1057,18 +1052,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) :
     connect(this, &Application::applicationStateChanged, this, &Application::activeChanged);
     qCDebug(interfaceapp, "Startup time: %4.2f seconds.", (double)startupTimer.elapsed() / 1000.0);
 
-    _idleTimer = new QTimer(this);
-    connect(_idleTimer, &QTimer::timeout, [=] {
-        idle(usecTimestampNow());
-    });
-    connect(this, &Application::beforeAboutToQuit, [=] {
-        disconnect(_idleTimer);
-    });
-    // Setting the interval to zero forces this to get called whenever there are no messages
-    // in the queue, which can be pretty damn frequent.  Hence the idle function has a bunch
-    // of logic to abort early if it's being called too often.
-    _idleTimer->start(0);
-
     // After all of the constructor is completed, then set firstRun to false.
     Setting::Handle<bool> firstRun{ Settings::firstRun, true };
     firstRun.set(false);
@@ -1441,23 +1424,15 @@ void Application::initializeUi() {
     });
 }
 
-
 void Application::paintGL() {
-    updateHeartbeat();
-    // Some plugins process message events, potentially leading to
-    // re-entering a paint event.  don't allow further processing if this
-    // happens
-    if (_inPaint) {
+    // Some plugins process message events, allowing paintGL to be called reentrantly.
+    if (_inPaint || _aboutToQuit) {
         return;
     }
-    _inPaint = true;
-    Finally clearFlagLambda([this] { _inPaint = false; });
 
-    // paintGL uses a queued connection, so we can get messages from the queue even after we've quit
-    // and the plugins have shutdown
-    if (_aboutToQuit) {
-        return;
-    }
+    _inPaint = true;
+    Finally clearFlag([this] { _inPaint = false; });
+
     _frameCount++;
     _frameCounter.increment();
 
@@ -1815,13 +1790,30 @@ bool Application::event(QEvent* event) {
         return false;
     }
 
-    if ((int)event->type() == (int)Lambda) {
-        static_cast<LambdaEvent*>(event)->call();
+    static bool justPresented = false;
+    if ((int)event->type() == (int)Present) {
+        if (justPresented) {
+            justPresented = false;
+
+            // If presentation is hogging the main thread, repost as low priority to avoid hanging the GUI.
+            // This has the effect of allowing presentation to exceed the paint budget by X times and
+            // only dropping every (1/X) frames, instead of every ceil(X) frames.
+            // (e.g. at a 60FPS target, painting for 17us would fall to 58.82FPS instead of 30FPS).
+            removePostedEvents(this, Present);
+            postEvent(this, new QEvent(static_cast<QEvent::Type>(Present)), Qt::LowEventPriority);
+            return true;
+        }
+
+        idle();
+        return true;
+    } else if ((int)event->type() == (int)Paint) {
+        justPresented = true;
+        paintGL();
         return true;
     }
 
-    if ((int)event->type() == (int)Paint) {
-        paintGL();
+    if ((int)event->type() == (int)Lambda) {
+        static_cast<LambdaEvent*>(event)->call();
         return true;
     }
 
@@ -2599,72 +2591,63 @@ bool Application::acceptSnapshot(const QString& urlString) {
 
 static uint32_t _renderedFrameIndex { INVALID_FRAME };
 
-void Application::idle(uint64_t now) {
-    // NOTICE NOTICE NOTICE NOTICE
-    // Do not insert new code between here and the PROFILE_RANGE declaration 
-    // unless you know exactly what you're doing.  This idle function can be 
-    // called thousands per second or more, so any additional work that's done 
-    // here will have a serious impact on CPU usage.  Only add code after all 
-    // the thottling logic, i.e. after PROFILE_RANGE
-    // NOTICE NOTICE NOTICE NOTICE
-    updateHeartbeat();
-
-    if (_aboutToQuit || _inPaint) {
-        return; // bail early, nothing to do here.
-    }
-
-    auto displayPlugin = getActiveDisplayPlugin();
-    // depending on whether we're throttling or not.
-    // Once rendering is off on another thread we should be able to have Application::idle run at start(0) in
-    // perpetuity and not expect events to get backed up.
-    bool isThrottled = displayPlugin->isThrottled();
-    //  Only run simulation code if more than the targetFramePeriod have passed since last time we ran
-    // This attempts to lock the simulation at 60 updates per second, regardless of framerate
-    float timeSinceLastUpdateUs = (float)_lastTimeUpdated.nsecsElapsed() / NSECS_PER_USEC;
-    float secondsSinceLastUpdate = timeSinceLastUpdateUs / USECS_PER_SECOND;
-
-    if (isThrottled && (timeSinceLastUpdateUs / USECS_PER_MSEC) < THROTTLED_SIM_FRAME_PERIOD_MS) {
-        // Throttling both rendering and idle
-        return; // bail early, we're throttled and not enough time has elapsed
-    }
-
-    auto presentCount = displayPlugin->presentCount();
-    if (presentCount < _renderedFrameIndex) {
-        _renderedFrameIndex = INVALID_FRAME;
-    }
-
-    // Don't saturate the main thread with rendering and simulation,
-    // unless display plugin has increased by at least one frame
-    if (_renderedFrameIndex == INVALID_FRAME || presentCount > _renderedFrameIndex) {
-        // Record what present frame we're on
-        _renderedFrameIndex = presentCount;
-
-        // request a paint, get to it as soon as possible: high priority
-        postEvent(this, new QEvent(static_cast<QEvent::Type>(Paint)), Qt::HighEventPriority);
-    } else {
-        // there's no use in simulating or rendering faster then the present rate.
+void Application::idle() {
+    // idle is called on a queued connection, so make sure we should be here.
+    if (_inPaint || _aboutToQuit) {
         return;
     }
 
-    // NOTICE NOTICE NOTICE NOTICE
-    // do NOT add new code above this line unless you want it to be executed potentially 
-    // thousands of times per second
-    // NOTICE NOTICE NOTICE NOTICE
+    auto displayPlugin = getActiveDisplayPlugin();
 
-    PROFILE_RANGE(__FUNCTION__);
+#ifdef DEBUG_PAINT_DELAY
+    static uint64_t paintDelaySamples{ 0 };
+    static uint64_t paintDelayUsecs{ 0 };
+
+    paintDelayUsecs += displayPlugin->getPaintDelayUsecs();
+
+    static const int PAINT_DELAY_THROTTLE = 1000;
+    if (++paintDelaySamples % PAINT_DELAY_THROTTLE == 0) {
+        qCDebug(interfaceapp).nospace() <<
+            "Paint delay (" << paintDelaySamples << " samples): " <<
+            (float)paintDelaySamples / paintDelayUsecs << "us";
+    }
+#endif
+
+    float msecondsSinceLastUpdate = (float)_lastTimeUpdated.nsecsElapsed() / NSECS_PER_USEC / USECS_PER_MSEC;
+
+    // Throttle if requested
+    if (displayPlugin->isThrottled() && (msecondsSinceLastUpdate < THROTTLED_SIM_FRAME_PERIOD_MS)) {
+        return;
+    }
+
+    // Sync up the _renderedFrameIndex
+    _renderedFrameIndex = displayPlugin->presentCount();
+
+    // Request a paint ASAP
+    postEvent(this, new QEvent(static_cast<QEvent::Type>(Paint)), Qt::HighEventPriority + 1);
+
+    // Update the deadlock watchdog
+    updateHeartbeat();
+
+    auto offscreenUi = DependencyManager::get<OffscreenUi>();
 
     // These tasks need to be done on our first idle, because we don't want the showing of
     // overlay subwindows to do a showDesktop() until after the first time through
     static bool firstIdle = true;
     if (firstIdle) {
         firstIdle = false;
-        auto offscreenUi = DependencyManager::get<OffscreenUi>();
         connect(offscreenUi.data(), &OffscreenUi::showDesktop, this, &Application::showDesktop);
         _overlayConductor.setEnabled(Menu::getInstance()->isOptionChecked(MenuOption::Overlays));
+    } else {
+        // FIXME: AvatarInputs are positioned incorrectly if instantiated before the first paint
+        AvatarInputs::getInstance()->update();
     }
 
+    PROFILE_RANGE(__FUNCTION__);
+
+    float secondsSinceLastUpdate = msecondsSinceLastUpdate / MSECS_PER_SECOND;
+
     // If the offscreen Ui has something active that is NOT the root, then assume it has keyboard focus.
-    auto offscreenUi = DependencyManager::get<OffscreenUi>();
     if (_keyboardDeviceHasFocus && offscreenUi && offscreenUi->getWindow()->activeFocusItem() != offscreenUi->getRootItem()) {
         _keyboardMouseDevice->pluginFocusOutEvent();
         _keyboardDeviceHasFocus = false;
@@ -2680,7 +2663,6 @@ void Application::idle(uint64_t now) {
     checkChangeCursor();
 
     Stats::getInstance()->updateStats();
-    AvatarInputs::getInstance()->update();
 
     _simCounter.increment();
 

interface/src/Application.h

@@ -33,6 +33,7 @@
 #include <PhysicalEntitySimulation.h>
 #include <PhysicsEngine.h>
 #include <plugins/Forward.h>
+#include <plugins/DisplayPlugin.h>
 #include <ScriptEngine.h>
 #include <ShapeManager.h>
 #include <SimpleMovingAverage.h>
@@ -93,6 +94,12 @@ class Application : public QApplication, public AbstractViewStateInterface, publ
     friend class PluginContainerProxy;
 
 public:
+    enum Event {
+        Present = DisplayPlugin::Present,
+        Paint = Present + 1,
+        Lambda = Paint + 1
+    };
+
     // FIXME? Empty methods, do we still need them?
     static void initPlugins();
     static void shutdownPlugins();
@@ -281,7 +288,6 @@ public slots:
 private slots:
     void showDesktop();
     void clearDomainOctreeDetails();
-    void idle(uint64_t now);
     void aboutToQuit();
 
     void resettingDomain();
@@ -320,6 +326,7 @@ private:
 
     void cleanupBeforeQuit();
 
+    void idle();
     void update(float deltaTime);
 
     // Various helper functions called during update()
@@ -497,7 +504,6 @@ private:
     int _avatarAttachmentRequest = 0;
 
     bool _settingsLoaded { false };
-    QTimer* _idleTimer { nullptr };
 
     bool _fakedMouseEvent { false };
 

libraries/plugins/src/plugins/DisplayPlugin.cpp

@@ -1,5 +1,6 @@
 #include "DisplayPlugin.h"
 
+#include <NumericalConstants.h>
 #include <ui/Menu.h>
 
 #include "PluginContainer.h"
@@ -23,4 +24,22 @@ void DisplayPlugin::deactivate() {
     Parent::deactivate();
 }
 
+int64_t DisplayPlugin::getPaintDelayUsecs() const {
+    std::lock_guard<std::mutex> lock(_paintDelayMutex);
+    return _paintDelayTimer.isValid() ? _paintDelayTimer.nsecsElapsed() / NSECS_PER_USEC : 0;
+}
 
+void DisplayPlugin::incrementPresentCount() {
+#ifdef DEBUG_PAINT_DELAY
+    // Avoid overhead if we are not debugging
+    {
+        std::lock_guard<std::mutex> lock(_paintDelayMutex);
+        _paintDelayTimer.start();
+    }
+#endif
+
+    ++_presentedFrameIndex;
+
+    // Alert the app that it needs to paint a new presentation frame
+    qApp->postEvent(qApp, new QEvent(static_cast<QEvent::Type>(Present)), Qt::HighEventPriority);
+}

libraries/plugins/src/plugins/DisplayPlugin.h

@@ -15,6 +15,7 @@
 
 #include <QtCore/QSize>
 #include <QtCore/QPoint>
+#include <QtCore/QElapsedTimer>
 class QImage;
 
 #include <GLMHelpers.h>
@@ -59,6 +60,10 @@ class DisplayPlugin : public Plugin {
     Q_OBJECT
     using Parent = Plugin;
 public:
+    enum Event {
+        Present = QEvent::User + 1
+    };
+
     bool activate() override;
     void deactivate() override;
     virtual bool isHmd() const { return false; }
@@ -156,6 +161,8 @@ public:
     // Rate at which rendered frames are being skipped
     virtual float droppedFrameRate() const { return -1.0f; }
     uint32_t presentCount() const { return _presentedFrameIndex; }
+    // Time since last call to incrementPresentCount (only valid if DEBUG_PAINT_DELAY is defined)
+    int64_t getPaintDelayUsecs() const;
 
     virtual void cycleDebugOutput() {}
 
@@ -165,9 +172,11 @@ signals:
     void recommendedFramebufferSizeChanged(const QSize & size);
 
 protected:
-    void incrementPresentCount() { ++_presentedFrameIndex; }
+    void incrementPresentCount();
 
 private:
     std::atomic<uint32_t> _presentedFrameIndex;
+    mutable std::mutex _paintDelayMutex;
+    QElapsedTimer _paintDelayTimer;
 };