Updates from Threaded Rendering project

interface/src/Application.cpp

@@ -2256,7 +2256,7 @@ void Application::paintGL() {
             QMutexLocker viewLocker(&_viewMutex);
             _viewFrustum.calculate();
         }
-        renderArgs = RenderArgs(_gpuContext, getEntities(), lodManager->getOctreeSizeScale(),
+        renderArgs = RenderArgs(_gpuContext, lodManager->getOctreeSizeScale(),
             lodManager->getBoundaryLevelAdjust(), RenderArgs::DEFAULT_RENDER_MODE,
             RenderArgs::MONO, RenderArgs::RENDER_DEBUG_NONE);
         {
@@ -2772,7 +2772,12 @@ bool Application::importSVOFromURL(const QString& urlString) {
     return true;
 }
 
-bool _renderRequested { false };
+void Application::onPresent(quint32 frameCount) {
+    if (shouldPaint()) {
+        postEvent(this, new QEvent(static_cast<QEvent::Type>(Idle)), Qt::HighEventPriority);
+        postEvent(this, new QEvent(static_cast<QEvent::Type>(Paint)), Qt::HighEventPriority);
+    }
+}
 
 bool Application::event(QEvent* event) {
     if (!Menu::getInstance()) {
@@ -2788,23 +2793,9 @@ bool Application::event(QEvent* event) {
         // Explicit idle keeps the idle running at a lower interval, but without any rendering
         // see (windowMinimizedChanged)
         case Event::Idle:
-            {
-                float nsecsElapsed = (float)_lastTimeUpdated.nsecsElapsed();
-                _lastTimeUpdated.start();
-                idle(nsecsElapsed);
-            }
-            return true;
-
-        case Event::Present:
-            if (!_renderRequested) {
-                float nsecsElapsed = (float)_lastTimeUpdated.nsecsElapsed();
-                if (shouldPaint(nsecsElapsed)) {
-                    _renderRequested = true;
-                    _lastTimeUpdated.start();
-                    idle(nsecsElapsed);
-                    postEvent(this, new QEvent(static_cast<QEvent::Type>(Paint)), Qt::HighEventPriority);
-                }
-            }
+            idle();
+            // Clear the event queue of pending idle calls
+            removePostedEvents(this, Idle);
             return true;
 
         case Event::Paint:
@@ -2812,9 +2803,8 @@ bool Application::event(QEvent* event) {
             //       or AvatarInputs will mysteriously move to the bottom-right
             AvatarInputs::getInstance()->update();
             paintGL();
-            // wait for the next present event before starting idle / paint again
-            removePostedEvents(this, Present);
-            _renderRequested = false;
+            // Clear the event queue of pending paint calls
+            removePostedEvents(this, Paint);
             return true;
 
         default:
@@ -3633,7 +3623,7 @@ bool Application::acceptSnapshot(const QString& urlString) {
 
 static uint32_t _renderedFrameIndex { INVALID_FRAME };
 
-bool Application::shouldPaint(float nsecsElapsed) {
+bool Application::shouldPaint() {
     if (_aboutToQuit) {
         return false;
     }
@@ -3653,11 +3643,9 @@ bool Application::shouldPaint(float nsecsElapsed) {
             (float)paintDelaySamples / paintDelayUsecs << "us";
     }
 #endif
-
-    float msecondsSinceLastUpdate = nsecsElapsed / NSECS_PER_USEC / USECS_PER_MSEC;
-
+    
     // Throttle if requested
-    if (displayPlugin->isThrottled() && (msecondsSinceLastUpdate < THROTTLED_SIM_FRAME_PERIOD_MS)) {
+    if (displayPlugin->isThrottled() && (_lastTimeUpdated.elapsed() < THROTTLED_SIM_FRAME_PERIOD_MS)) {
         return false;
     }
 
@@ -3874,7 +3862,7 @@ void setupCpuMonitorThread() {
 #endif
 
 
-void Application::idle(float nsecsElapsed) {
+void Application::idle() {
     PerformanceTimer perfTimer("idle");
 
     // Update the deadlock watchdog
@@ -3931,7 +3919,8 @@ void Application::idle(float nsecsElapsed) {
         steamClient->runCallbacks();
     }
 
-    float secondsSinceLastUpdate = nsecsElapsed / NSECS_PER_MSEC / MSECS_PER_SECOND;
+    float secondsSinceLastUpdate = (float)_lastTimeUpdated.nsecsElapsed() / NSECS_PER_MSEC / MSECS_PER_SECOND;
+    _lastTimeUpdated.start();
 
     // If the offscreen Ui has something active that is NOT the root, then assume it has keyboard focus.
     if (_keyboardDeviceHasFocus && offscreenUi && offscreenUi->getWindow()->activeFocusItem() != offscreenUi->getRootItem()) {
@@ -7106,6 +7095,7 @@ void Application::updateDisplayMode() {
 
         auto oldDisplayPlugin = _displayPlugin;
         if (_displayPlugin) {
+            disconnect(_displayPluginPresentConnection);
             _displayPlugin->deactivate();
         }
 
@@ -7146,6 +7136,7 @@ void Application::updateDisplayMode() {
         _offscreenContext->makeCurrent();
         getApplicationCompositor().setDisplayPlugin(newDisplayPlugin);
         _displayPlugin = newDisplayPlugin;
+        _displayPluginPresentConnection = connect(_displayPlugin.get(), &DisplayPlugin::presented, this, &Application::onPresent);
         offscreenUi->getDesktop()->setProperty("repositionLocked", wasRepositionLocked);
     }
 

interface/src/Application.h

@@ -129,8 +129,7 @@ public:
     virtual DisplayPluginPointer getActiveDisplayPlugin() const override;
 
     enum Event {
-        Present = DisplayPlugin::Present,
-        Paint,
+        Paint = QEvent::User + 1,
         Idle,
         Lambda
     };
@@ -409,6 +408,7 @@ private slots:
     void clearDomainOctreeDetails();
     void clearDomainAvatars();
     void onAboutToQuit();
+    void onPresent(quint32 frameCount);
 
     void resettingDomain();
 
@@ -455,8 +455,8 @@ private:
 
     void cleanupBeforeQuit();
 
-    bool shouldPaint(float nsecsElapsed);
-    void idle(float nsecsElapsed);
+    bool shouldPaint();
+    void idle();
     void update(float deltaTime);
 
     // Various helper functions called during update()
@@ -518,6 +518,7 @@ private:
 
     OffscreenGLCanvas* _offscreenContext { nullptr };
     DisplayPluginPointer _displayPlugin;
+    QMetaObject::Connection _displayPluginPresentConnection;
     mutable std::mutex _displayPluginLock;
     InputPluginList _activeInputPlugins;
 

interface/src/scripting/AudioDevices.cpp

@@ -12,6 +12,7 @@
 #include <map>
 
 #include <shared/QtHelpers.h>
+#include <plugins/DisplayPlugin.h>
 
 #include "AudioDevices.h"
 

interface/src/ui/ResourceImageItem.cpp

@@ -8,7 +8,6 @@
 // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
-//#include "Application.h"
 #include "ResourceImageItem.h"
 
 #include <QOpenGLFramebufferObjectFormat>
@@ -16,6 +15,8 @@
 #include <QOpenGLExtraFunctions>
 #include <QOpenGLContext>
 
+#include <plugins/DisplayPlugin.h>
+
 ResourceImageItem::ResourceImageItem() : QQuickFramebufferObject() {
     auto textureCache = DependencyManager::get<TextureCache>();
     connect(textureCache.data(), SIGNAL(spectatorCameraFramebufferReset()), this, SLOT(update()));

interface/src/ui/SnapshotAnimated.cpp

@@ -13,8 +13,9 @@
 #include <QtCore/QObject>
 #include <QtCore/QString>
 #include <QtGui/QImage>
+#include <QtConcurrent/QtConcurrentRun>
 
-#include <QtConcurrent/qtconcurrentrun.h>
+#include <plugins/DisplayPlugin.h>
 #include "SnapshotAnimated.h"
 
 QTimer* SnapshotAnimated::snapshotAnimatedTimer = NULL;

libraries/entities-renderer/src/RenderableWebEntityItem.cpp

@@ -69,7 +69,7 @@ RenderableWebEntityItem::~RenderableWebEntityItem() {
     }
 }
 
-bool RenderableWebEntityItem::buildWebSurface(QSharedPointer<EntityTreeRenderer> renderer) {
+bool RenderableWebEntityItem::buildWebSurface() {
     if (_currentWebCount >= MAX_CONCURRENT_WEB_VIEWS) {
         qWarning() << "Too many concurrent web views to create new view";
         return false;
@@ -132,6 +132,8 @@ bool RenderableWebEntityItem::buildWebSurface(QSharedPointer<EntityTreeRenderer>
             handlePointerEvent(event);
         }
     };
+
+    auto renderer = DependencyManager::get<EntityTreeRenderer>();
     _mousePressConnection = QObject::connect(renderer.data(), &EntityTreeRenderer::mousePressOnEntity, forwardPointerEvent);
     _mouseReleaseConnection = QObject::connect(renderer.data(), &EntityTreeRenderer::mouseReleaseOnEntity, forwardPointerEvent);
     _mouseMoveConnection = QObject::connect(renderer.data(), &EntityTreeRenderer::mouseMoveOnEntity, forwardPointerEvent);
@@ -185,8 +187,7 @@ void RenderableWebEntityItem::render(RenderArgs* args) {
     #endif
 
     if (!_webSurface) {
-        auto renderer = qSharedPointerCast<EntityTreeRenderer>(args->_renderData);
-        if (!buildWebSurface(renderer)) {
+        if (!buildWebSurface()) {
             return;
         }
         _fadeStartTime = usecTimestampNow();

libraries/entities-renderer/src/RenderableWebEntityItem.h

@@ -57,7 +57,7 @@ public:
     virtual QObject* getRootItem() override;
 
 private:
-    bool buildWebSurface(QSharedPointer<EntityTreeRenderer> renderer);
+    bool buildWebSurface();
     void destroyWebSurface();
     glm::vec2 getWindowSize() const;
 

libraries/plugins/src/plugins/DisplayPlugin.cpp

@@ -18,6 +18,19 @@ void DisplayPlugin::incrementPresentCount() {
 
     ++_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);
+    {
+        QMutexLocker locker(&_presentMutex);
+        _presentCondition.wakeAll();
+    }
+
+    emit presented(_presentedFrameIndex);
 }
+
+void DisplayPlugin::waitForPresent() {
+    QMutexLocker locker(&_presentMutex);
+    while (isActive()) {
+        if (_presentCondition.wait(&_presentMutex, MSECS_PER_SECOND)) {
+            break;
+        }
+    }
+}

libraries/plugins/src/plugins/DisplayPlugin.h

@@ -17,6 +17,8 @@
 #include <QtCore/QPoint>
 #include <QtCore/QElapsedTimer>
 #include <QtCore/QJsonObject>
+#include <QtCore/QMutex>
+#include <QtCore/QWaitCondition>
 
 #include <GLMHelpers.h>
 #include <RegisteredMetaTypes.h>
@@ -134,10 +136,6 @@ class DisplayPlugin : public Plugin, public HmdDisplay {
     Q_OBJECT
     using Parent = Plugin;
 public:
-    enum Event {
-        Present = QEvent::User + 1
-    };
-
     virtual int getRequiredThreadCount() const { return 0; }
     virtual bool isHmd() const { return false; }
     virtual int getHmdScreen() const { return -1; }
@@ -221,12 +219,15 @@ public:
 
     virtual void cycleDebugOutput() {}
 
+    void waitForPresent();
+
     static const QString& MENU_PATH();
 
 
 signals:
     void recommendedFramebufferSizeChanged(const QSize& size);
     void resetSensorsRequested();
+    void presented(quint32 frame);
 
 protected:
     void incrementPresentCount();
@@ -234,6 +235,8 @@ protected:
     gpu::ContextPointer _gpuContext;
 
 private:
+    QMutex _presentMutex;
+    QWaitCondition _presentCondition;
     std::atomic<uint32_t> _presentedFrameIndex;
     mutable std::mutex _paintDelayMutex;
     QElapsedTimer _paintDelayTimer;

libraries/render/src/render/Args.h

@@ -77,7 +77,6 @@ namespace render {
         Args() {}
 
         Args(const gpu::ContextPointer& context,
-            QSharedPointer<QObject> renderData = QSharedPointer<QObject>(nullptr),
             float sizeScale = 1.0f,
             int boundaryLevelAdjust = 0,
             RenderMode renderMode = DEFAULT_RENDER_MODE,
@@ -85,7 +84,6 @@ namespace render {
             DebugFlags debugFlags = RENDER_DEBUG_NONE,
             gpu::Batch* batch = nullptr) :
             _context(context),
-            _renderData(renderData),
             _sizeScale(sizeScale),
             _boundaryLevelAdjust(boundaryLevelAdjust),
             _renderMode(renderMode),
@@ -110,7 +108,6 @@ namespace render {
         std::shared_ptr<gpu::Context> _context;
         std::shared_ptr<gpu::Framebuffer> _blitFramebuffer;
         std::shared_ptr<render::ShapePipeline> _shapePipeline;
-        QSharedPointer<QObject> _renderData;
         std::stack<ViewFrustum> _viewFrustums;
         glm::ivec4 _viewport { 0.0f, 0.0f, 1.0f, 1.0f };
         glm::vec3 _boomOffset { 0.0f, 0.0f, 1.0f };

libraries/shared/src/ThreadHelpers.cpp

@@ -10,29 +10,66 @@
 
 #include <QtCore/QDebug>
 
+// Support for viewing the thread name in the debugger.  
+// Note, Qt actually does this for you but only in debug builds
+// Code from https://msdn.microsoft.com/en-us/library/xcb2z8hs.aspx
+// and matches logic in `qt_set_thread_name` in qthread_win.cpp
+#ifdef Q_OS_WIN
+#include <qt_windows.h>
+#pragma pack(push,8)  
+struct THREADNAME_INFO {
+    DWORD dwType; // Must be 0x1000.  
+    LPCSTR szName; // Pointer to name (in user addr space).  
+    DWORD dwThreadID; // Thread ID (-1=caller thread).  
+    DWORD dwFlags; // Reserved for future use, must be zero.  
+};
+#pragma pack(pop)  
+#endif
+
+void setThreadName(const std::string& name) {
+#ifdef Q_OS_WIN
+    static const DWORD MS_VC_EXCEPTION = 0x406D1388;
+    THREADNAME_INFO info{ 0x1000, name.c_str(), (DWORD)-1, 0 };
+    __try {
+        RaiseException(MS_VC_EXCEPTION, 0, sizeof(info) / sizeof(ULONG_PTR), (ULONG_PTR*)&info);
+    } __except (EXCEPTION_EXECUTE_HANDLER) { }
+#endif
+}
+
+void moveToNewNamedThread(QObject* object, const QString& name, std::function<void(QThread*)> preStartCallback, std::function<void()> startCallback, QThread::Priority priority) {
+    Q_ASSERT(QThread::currentThread() == object->thread());
+    // setup a thread for the NodeList and its PacketReceiver
+    QThread* thread = new QThread();
+    thread->setObjectName(name);
+
+    // Execute any additional work to do before the thread starts (like moving members to the target thread
+    preStartCallback(thread);
+
+    // Link the in-thread initialization code
+    QObject::connect(thread, &QThread::started, [name, startCallback] { 
+        if (!name.isEmpty()) {
+            // Make it easy to spot our thread processes inside the debugger
+            setThreadName("Hifi_" + name.toStdString());
+        }
+        startCallback();
+    });
+
+    // Make sure the thread will be destroyed and cleaned up
+    QObject::connect(object, &QObject::destroyed, thread, &QThread::quit);
+    QObject::connect(thread, &QThread::finished, thread, &QThread::deleteLater);
+
+    // put the object on the thread
+    object->moveToThread(thread);
+    thread->start();
+    if (priority != QThread::InheritPriority) {
+        thread->setPriority(priority);
+    }
+}
 
 void moveToNewNamedThread(QObject* object, const QString& name, std::function<void()> startCallback, QThread::Priority priority) {
-     Q_ASSERT(QThread::currentThread() == object->thread());
-     // setup a thread for the NodeList and its PacketReceiver
-     QThread* thread = new QThread();
-     thread->setObjectName(name);
-
-     QString tempName = name;
-     QObject::connect(thread, &QThread::started, [startCallback] {
-         startCallback();
-     });
-     // Make sure the thread will be destroyed and cleaned up
-     QObject::connect(object, &QObject::destroyed, thread, &QThread::quit);
-     QObject::connect(thread, &QThread::finished, thread, &QThread::deleteLater);
-
-     // put the object on the thread
-     object->moveToThread(thread);
-     thread->start();
-     if (priority != QThread::InheritPriority) {
-         thread->setPriority(priority);
-     }
+    moveToNewNamedThread(object, name, [](QThread*){}, startCallback, priority);
 }
 
 void moveToNewNamedThread(QObject* object, const QString& name, QThread::Priority priority) {
-    moveToNewNamedThread(object, name, [] {}, priority);
+    moveToNewNamedThread(object, name, [](QThread*){}, []{}, priority);
 }

libraries/shared/src/ThreadHelpers.h

@@ -32,8 +32,17 @@ void withLock(QMutex& lock, F function) {
     function();
 }
 
-void moveToNewNamedThread(QObject* object, const QString& name, std::function<void()> startCallback, QThread::Priority priority = QThread::InheritPriority);
-void moveToNewNamedThread(QObject* object, const QString& name, QThread::Priority priority = QThread::InheritPriority);
+void moveToNewNamedThread(QObject* object, const QString& name, 
+    std::function<void(QThread*)> preStartCallback, 
+    std::function<void()> startCallback, 
+    QThread::Priority priority = QThread::InheritPriority);
+
+void moveToNewNamedThread(QObject* object, const QString& name, 
+    std::function<void()> startCallback, 
+    QThread::Priority priority = QThread::InheritPriority);
+
+void moveToNewNamedThread(QObject* object, const QString& name, 
+    QThread::Priority priority = QThread::InheritPriority);
 
 class ConditionalGuard {
 public:

libraries/shared/src/shared/QtHelpers.cpp

@@ -11,11 +11,24 @@
 #include <QtCore/QThread>
 #include <QtCore/QCoreApplication>
 #include <QtCore/QLoggingCategory>
+#include <QtCore/QReadWriteLock>
 
+#include "../Profile.h"
 Q_LOGGING_CATEGORY(thread_safety, "hifi.thread_safety")
 
 namespace hifi { namespace qt {
 
+static QHash<QThread*, QString> threadHash;
+static QReadWriteLock threadHashLock;
+
+void addBlockingForbiddenThread(const QString& name, QThread* thread) {
+    if (!thread) {
+        thread = QThread::currentThread();
+    }
+    QWriteLocker locker(&threadHashLock);
+    threadHash[thread] = name;
+}
+
 bool blockingInvokeMethod(
     const char* function,
     QObject *obj, const char *member,
@@ -30,9 +43,23 @@ bool blockingInvokeMethod(
     QGenericArgument val7,
     QGenericArgument val8,
     QGenericArgument val9) {
-    if (QThread::currentThread() == qApp->thread()) {
+    auto currentThread = QThread::currentThread();
+    if (currentThread == qApp->thread()) {
         qCWarning(thread_safety) << "BlockingQueuedConnection invoked on main thread from " << function;
+        return QMetaObject::invokeMethod(obj, member,
+            Qt::BlockingQueuedConnection, ret, val0, val1, val2, val3, val4, val5, val6, val7, val8, val9);
+    } 
+
+    {
+        QReadLocker locker(&threadHashLock);
+        for (const auto& thread : threadHash.keys()) {
+            if (currentThread == thread) {
+                qCWarning(thread_safety) << "BlockingQueuedConnection invoked on forbidden thread " << threadHash[thread];
+            }
+        }
     }
+
+    PROFILE_RANGE(app, function);
     return QMetaObject::invokeMethod(obj, member,
             Qt::BlockingQueuedConnection, ret, val0, val1, val2, val3, val4, val5, val6, val7, val8, val9);
 }

libraries/shared/src/shared/QtHelpers.h

@@ -14,6 +14,7 @@
 
 
 namespace hifi { namespace qt {
+void addBlockingForbiddenThread(const QString& name, QThread* thread = nullptr);
 
 bool blockingInvokeMethod(
     const char* function,

tests/render-perf/src/main.cpp

@@ -681,7 +681,7 @@ private:
         _renderCount = _renderThread._presentCount.load();
         update();
 
-        RenderArgs renderArgs(_renderThread._gpuContext, _octree, DEFAULT_OCTREE_SIZE_SCALE,
+        RenderArgs renderArgs(_renderThread._gpuContext, DEFAULT_OCTREE_SIZE_SCALE,
             0, RenderArgs::DEFAULT_RENDER_MODE,
             RenderArgs::MONO, RenderArgs::RENDER_DEBUG_NONE);