From 86e348916724fc8dd290569acd9fd44ee0c37e02 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 25 Jul 2017 09:42:42 -0700 Subject: [PATCH] Updates from Threaded Rendering project --- interface/src/Application.cpp | 49 +++++------- interface/src/Application.h | 9 ++- interface/src/scripting/AudioDevices.cpp | 1 + interface/src/ui/ResourceImageItem.cpp | 3 +- interface/src/ui/SnapshotAnimated.cpp | 3 +- .../src/RenderableWebEntityItem.cpp | 7 +- .../src/RenderableWebEntityItem.h | 2 +- .../plugins/src/plugins/DisplayPlugin.cpp | 17 +++- libraries/plugins/src/plugins/DisplayPlugin.h | 11 ++- libraries/render/src/render/Args.h | 3 - libraries/shared/src/ThreadHelpers.cpp | 77 ++++++++++++++----- libraries/shared/src/ThreadHelpers.h | 13 +++- libraries/shared/src/shared/QtHelpers.cpp | 29 ++++++- libraries/shared/src/shared/QtHelpers.h | 1 + tests/render-perf/src/main.cpp | 2 +- 15 files changed, 155 insertions(+), 72 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index d016c9d020..549e5338a0 100644 --- a/interface/src/Application.cpp +++ b/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(Idle)), Qt::HighEventPriority); + postEvent(this, new QEvent(static_cast(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(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); } diff --git a/interface/src/Application.h b/interface/src/Application.h index 123aa85e2e..300bd4ac02 100644 --- a/interface/src/Application.h +++ b/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; diff --git a/interface/src/scripting/AudioDevices.cpp b/interface/src/scripting/AudioDevices.cpp index d02f4d8fcf..f2e6dbf4d7 100644 --- a/interface/src/scripting/AudioDevices.cpp +++ b/interface/src/scripting/AudioDevices.cpp @@ -12,6 +12,7 @@ #include #include +#include #include "AudioDevices.h" diff --git a/interface/src/ui/ResourceImageItem.cpp b/interface/src/ui/ResourceImageItem.cpp index 7b9592fa4c..5b7c1896fe 100644 --- a/interface/src/ui/ResourceImageItem.cpp +++ b/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 @@ -16,6 +15,8 @@ #include #include +#include + ResourceImageItem::ResourceImageItem() : QQuickFramebufferObject() { auto textureCache = DependencyManager::get(); connect(textureCache.data(), SIGNAL(spectatorCameraFramebufferReset()), this, SLOT(update())); diff --git a/interface/src/ui/SnapshotAnimated.cpp b/interface/src/ui/SnapshotAnimated.cpp index 70767b007d..3c00be8358 100644 --- a/interface/src/ui/SnapshotAnimated.cpp +++ b/interface/src/ui/SnapshotAnimated.cpp @@ -13,8 +13,9 @@ #include #include #include +#include -#include +#include #include "SnapshotAnimated.h" QTimer* SnapshotAnimated::snapshotAnimatedTimer = NULL; diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index 9a898b4071..ba8e0c18e7 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -69,7 +69,7 @@ RenderableWebEntityItem::~RenderableWebEntityItem() { } } -bool RenderableWebEntityItem::buildWebSurface(QSharedPointer 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 handlePointerEvent(event); } }; + + auto renderer = DependencyManager::get(); _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(args->_renderData); - if (!buildWebSurface(renderer)) { + if (!buildWebSurface()) { return; } _fadeStartTime = usecTimestampNow(); diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.h b/libraries/entities-renderer/src/RenderableWebEntityItem.h index 0f5d307766..a2318081b6 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.h +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.h @@ -57,7 +57,7 @@ public: virtual QObject* getRootItem() override; private: - bool buildWebSurface(QSharedPointer renderer); + bool buildWebSurface(); void destroyWebSurface(); glm::vec2 getWindowSize() const; diff --git a/libraries/plugins/src/plugins/DisplayPlugin.cpp b/libraries/plugins/src/plugins/DisplayPlugin.cpp index 747c72c08e..20c72159c4 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.cpp +++ b/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(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; + } + } +} \ No newline at end of file diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index 481a2609fc..9e18ee534d 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -17,6 +17,8 @@ #include #include #include +#include +#include #include #include @@ -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 _presentedFrameIndex; mutable std::mutex _paintDelayMutex; QElapsedTimer _paintDelayTimer; diff --git a/libraries/render/src/render/Args.h b/libraries/render/src/render/Args.h index d5b5440c32..6a91081c95 100644 --- a/libraries/render/src/render/Args.h +++ b/libraries/render/src/render/Args.h @@ -77,7 +77,6 @@ namespace render { Args() {} Args(const gpu::ContextPointer& context, - QSharedPointer renderData = QSharedPointer(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 _context; std::shared_ptr _blitFramebuffer; std::shared_ptr _shapePipeline; - QSharedPointer _renderData; std::stack _viewFrustums; glm::ivec4 _viewport { 0.0f, 0.0f, 1.0f, 1.0f }; glm::vec3 _boomOffset { 0.0f, 0.0f, 1.0f }; diff --git a/libraries/shared/src/ThreadHelpers.cpp b/libraries/shared/src/ThreadHelpers.cpp index 8f3d16a577..8cf8b85284 100644 --- a/libraries/shared/src/ThreadHelpers.cpp +++ b/libraries/shared/src/ThreadHelpers.cpp @@ -10,29 +10,66 @@ #include +// 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 +#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 preStartCallback, std::function 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 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); } diff --git a/libraries/shared/src/ThreadHelpers.h b/libraries/shared/src/ThreadHelpers.h index 6e024f787a..d236344dc5 100644 --- a/libraries/shared/src/ThreadHelpers.h +++ b/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 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 preStartCallback, + std::function startCallback, + QThread::Priority priority = QThread::InheritPriority); + +void moveToNewNamedThread(QObject* object, const QString& name, + std::function startCallback, + QThread::Priority priority = QThread::InheritPriority); + +void moveToNewNamedThread(QObject* object, const QString& name, + QThread::Priority priority = QThread::InheritPriority); class ConditionalGuard { public: diff --git a/libraries/shared/src/shared/QtHelpers.cpp b/libraries/shared/src/shared/QtHelpers.cpp index 1ce1c3e07c..3e8c6d57ed 100644 --- a/libraries/shared/src/shared/QtHelpers.cpp +++ b/libraries/shared/src/shared/QtHelpers.cpp @@ -11,11 +11,24 @@ #include #include #include +#include +#include "../Profile.h" Q_LOGGING_CATEGORY(thread_safety, "hifi.thread_safety") namespace hifi { namespace qt { +static QHash 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); } diff --git a/libraries/shared/src/shared/QtHelpers.h b/libraries/shared/src/shared/QtHelpers.h index 5da65a378f..2133119324 100644 --- a/libraries/shared/src/shared/QtHelpers.h +++ b/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, diff --git a/tests/render-perf/src/main.cpp b/tests/render-perf/src/main.cpp index ce47a896aa..dbb315a9ae 100644 --- a/tests/render-perf/src/main.cpp +++ b/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);