From 82081d7d57f7e27fa4b37ecb6d277218e6c9ce30 Mon Sep 17 00:00:00 2001
From: Brad Davis <bdavis@saintandreas.org>
Date: Mon, 8 Aug 2016 20:03:58 -0700
Subject: [PATCH] Address vsync weirdness
---
.../Basic2DWindowOpenGLDisplayPlugin.cpp | 12 ----
.../Basic2DWindowOpenGLDisplayPlugin.h | 5 --
.../src/display-plugins/NullDisplayPlugin.cpp | 4 ++
.../display-plugins/OpenGLDisplayPlugin.cpp | 58 +++++++------------
.../src/display-plugins/OpenGLDisplayPlugin.h | 15 +++--
.../display-plugins/hmd/HmdDisplayPlugin.cpp | 44 +++++++++-----
.../display-plugins/hmd/HmdDisplayPlugin.h | 8 ++-
libraries/gpu/src/gpu/Buffer.cpp | 1 -
plugins/oculus/src/OculusDisplayPlugin.cpp | 5 --
plugins/oculus/src/OculusDisplayPlugin.h | 2 -
plugins/openvr/src/OpenVrDisplayPlugin.cpp | 13 +++--
11 files changed, 75 insertions(+), 92 deletions(-)
diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp
index eb8c275123..588c43d534 100644
--- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp
+++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp
@@ -33,18 +33,6 @@ bool Basic2DWindowOpenGLDisplayPlugin::internalActivate() {
return Parent::internalActivate();
}
-void Basic2DWindowOpenGLDisplayPlugin::submitFrame(const gpu::FramePointer& newFrame) {
- _wantVsync = true; // always
- Parent::submitFrame(newFrame);
-}
-
-void Basic2DWindowOpenGLDisplayPlugin::internalPresent() {
- if (_wantVsync != isVsyncEnabled()) {
- enableVsync(_wantVsync);
- }
- Parent::internalPresent();
-}
-
static const uint32_t MIN_THROTTLE_CHECK_FRAMES = 60;
bool Basic2DWindowOpenGLDisplayPlugin::isThrottled() const {
diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h
index 6321bb6d79..2e4e57e15a 100644
--- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h
+++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h
@@ -24,10 +24,6 @@ public:
virtual bool internalActivate() override;
- void submitFrame(const gpu::FramePointer& newFrame) override;
-
- virtual void internalPresent() override;
-
virtual bool isThrottled() const override;
protected:
@@ -40,5 +36,4 @@ private:
QAction* _vsyncAction { nullptr };
uint32_t _framerateTarget { 0 };
int _fullscreenTarget{ -1 };
- bool _wantVsync { true };
};
diff --git a/libraries/display-plugins/src/display-plugins/NullDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/NullDisplayPlugin.cpp
index 05dacea385..23e4a9dd6a 100644
--- a/libraries/display-plugins/src/display-plugins/NullDisplayPlugin.cpp
+++ b/libraries/display-plugins/src/display-plugins/NullDisplayPlugin.cpp
@@ -12,6 +12,7 @@
#include <QtGui/QImage>
#include <ui-plugins/PluginContainer.h>
#include <FramebufferCache.h>
+#include <gpu/Frame.h>
const QString NullDisplayPlugin::NAME("NullDisplayPlugin");
@@ -24,6 +25,9 @@ bool NullDisplayPlugin::hasFocus() const {
}
void NullDisplayPlugin::submitFrame(const gpu::FramePointer& resultFramebuffer) {
+ if (resultFramebuffer) {
+ resultFramebuffer->preRender();
+ }
}
QImage NullDisplayPlugin::getScreenshot() const {
diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp
index 6dadcb2466..d969daf231 100644
--- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp
+++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp
@@ -133,8 +133,6 @@ public:
// Main thread does it's thing while we wait on the lock to release
Lock lock(_mutex);
_condition.wait(lock, [&] { return _finishedMainThreadOperation; });
- _context->makeCurrent();
- Q_ASSERT(isCurrentContext(_context->contextHandle()));
}
}
@@ -147,13 +145,34 @@ public:
if (newPlugin != currentPlugin) {
// Deactivate the old plugin
if (currentPlugin != nullptr) {
+ _context->makeCurrent();
currentPlugin->uncustomizeContext();
CHECK_GL_ERROR();
+ _context->doneCurrent();
}
if (newPlugin) {
+ bool hasVsync = true;
+ bool wantVsync = newPlugin->wantVsync();
+ _context->makeCurrent();
+#if defined(Q_OS_WIN)
+ wglSwapIntervalEXT(wantVsync ? 1 : 0);
+ hasVsync = wglGetSwapIntervalEXT() != 0;
+#elif defined(Q_OS_MAC)
+ GLint interval = wantVsync ? 1 : 0;
+ newPlugin->swapBuffers();
+ CGLSetParameter(CGLGetCurrentContext(), kCGLCPSwapInterval, &interval);
+ newPlugin->swapBuffers();
+ CGLGetParameter(CGLGetCurrentContext(), kCGLCPSwapInterval, &interval);
+ hasVsync = interval != 0;
+#else
+ // TODO: Fill in for linux
+ Q_UNUSED(wantVsync);
+#endif
+ newPlugin->setVsyncEnabled(hasVsync);
newPlugin->customizeContext();
CHECK_GL_ERROR();
+ _context->doneCurrent();
}
currentPlugin = newPlugin;
_newPluginQueue.pop();
@@ -176,8 +195,8 @@ public:
currentPlugin->present();
CHECK_GL_ERROR();
}
+ _context->doneCurrent();
}
- _context->doneCurrent();
Lock lock(_mutex);
_context->moveToThread(qApp->thread());
@@ -239,7 +258,6 @@ bool OpenGLDisplayPlugin::activate() {
if (!_container) {
return false;
}
- _vsyncSupported = _container->getPrimaryWidget()->isVsyncSupported();
// Start the present thread if necessary
QSharedPointer<PresentThread> presentThread;
@@ -309,7 +327,6 @@ void OpenGLDisplayPlugin::deactivate() {
void OpenGLDisplayPlugin::customizeContext() {
auto presentThread = DependencyManager::get<PresentThread>();
Q_ASSERT(thread() == presentThread->thread());
- enableVsync();
getGLBackend()->setCameraCorrection(mat4());
@@ -619,37 +636,6 @@ float OpenGLDisplayPlugin::presentRate() const {
return _presentRate.rate();
}
-void OpenGLDisplayPlugin::enableVsync(bool enable) {
- if (!_vsyncSupported) {
- return;
- }
-#if defined(Q_OS_WIN)
- wglSwapIntervalEXT(enable ? 1 : 0);
-#elif defined(Q_OS_MAC)
- GLint interval = enable ? 1 : 0;
- CGLSetParameter(CGLGetCurrentContext(), kCGLCPSwapInterval, &interval);
-#else
- // TODO: Fill in for linux
- return;
-#endif
-}
-
-bool OpenGLDisplayPlugin::isVsyncEnabled() {
- if (!_vsyncSupported) {
- return true;
- }
-#if defined(Q_OS_WIN)
- return wglGetSwapIntervalEXT() != 0;
-#elif defined(Q_OS_MAC)
- GLint interval;
- CGLGetParameter(CGLGetCurrentContext(), kCGLCPSwapInterval, &interval);
- return interval != 0;
-#else
- // TODO: Fill in for linux
- return true;
-#endif
-}
-
void OpenGLDisplayPlugin::swapBuffers() {
static auto widget = _container->getPrimaryWidget();
widget->swapBuffers();
diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h
index 1ab7be2511..48f9a78eda 100644
--- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h
+++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h
@@ -63,6 +63,11 @@ public:
float droppedFrameRate() const override;
bool beginFrameRender(uint32_t frameIndex) override;
+
+ virtual bool wantVsync() const { return true; }
+ void setVsyncEnabled(bool vsyncEnabled) { _vsyncEnabled = vsyncEnabled; }
+ bool isVsyncEnabled() const { return _vsyncEnabled; }
+
protected:
friend class PresentThread;
@@ -77,10 +82,6 @@ protected:
virtual bool hasFocus() const override;
- // FIXME make thread safe?
- virtual bool isVsyncEnabled();
- virtual void enableVsync(bool enable = true);
-
// These functions must only be called on the presentation thread
virtual void customizeContext();
virtual void uncustomizeContext();
@@ -97,11 +98,12 @@ protected:
void withMainThreadContext(std::function<void()> f) const;
void present();
- void swapBuffers();
+ virtual void swapBuffers();
ivec4 eyeViewport(Eye eye) const;
void render(std::function<void(gpu::Batch& batch)> f);
+ bool _vsyncEnabled { true };
QThread* _presentThread{ nullptr };
std::queue<gpu::FramePointer> _newFrameQueue;
RateCounter<> _droppedFrameRate;
@@ -116,9 +118,6 @@ protected:
gpu::PipelinePointer _cursorPipeline;
float _compositeOverlayAlpha { 1.0f };
-
- bool _vsyncSupported { false };
-
struct CursorData {
QImage image;
vec2 hotSpot;
diff --git a/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp
index 2f999965f8..82cd08db39 100644
--- a/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp
+++ b/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp
@@ -63,7 +63,24 @@ QRect HmdDisplayPlugin::getRecommendedOverlayRect() const {
return CompositorHelper::VIRTUAL_SCREEN_RECOMMENDED_OVERLAY_RECT;
}
+bool HmdDisplayPlugin::beginFrameRender(uint32_t frameIndex) {
+ if (!_vsyncEnabled && !_disablePreviewItemAdded) {
+ _container->addMenuItem(PluginType::DISPLAY_PLUGIN, MENU_PATH(), DISABLE_PREVIEW,
+ [this](bool clicked) {
+ _disablePreview = clicked;
+ _container->setBoolSetting("disableHmdPreview", _disablePreview);
+ if (_disablePreview) {
+ _clearPreviewFlag = true;
+ }
+ }, true, _disablePreview);
+ _disablePreviewItemAdded = true;
+ }
+ return Parent::beginFrameRender(frameIndex);
+}
+
+
bool HmdDisplayPlugin::internalActivate() {
+ _disablePreviewItemAdded = false;
_monoPreview = _container->getBoolSetting("monoPreview", DEFAULT_MONO_VIEW);
_clearPreviewFlag = true;
_container->addMenuItem(PluginType::DISPLAY_PLUGIN, MENU_PATH(), MONO_PREVIEW,
@@ -74,17 +91,7 @@ bool HmdDisplayPlugin::internalActivate() {
#if defined(Q_OS_MAC)
_disablePreview = true;
#else
- _disablePreview = _container->getBoolSetting("disableHmdPreview", DEFAULT_DISABLE_PREVIEW || !_vsyncSupported);
- if (_vsyncSupported) {
- _container->addMenuItem(PluginType::DISPLAY_PLUGIN, MENU_PATH(), DISABLE_PREVIEW,
- [this](bool clicked) {
- _disablePreview = clicked;
- _container->setBoolSetting("disableHmdPreview", _disablePreview);
- if (_disablePreview) {
- _clearPreviewFlag = true;
- }
- }, true, _disablePreview);
- }
+ _disablePreview = _container->getBoolSetting("disableHmdPreview", DEFAULT_DISABLE_PREVIEW || _vsyncEnabled);
#endif
_container->removeMenu(FRAMERATE);
@@ -103,15 +110,20 @@ void HmdDisplayPlugin::internalDeactivate() {
void HmdDisplayPlugin::customizeContext() {
Parent::customizeContext();
- // Only enable mirroring if we know vsync is disabled
- // On Mac, this won't work due to how the contexts are handled, so don't try
-#if !defined(Q_OS_MAC)
- enableVsync(false);
-#endif
_overlayRenderer.build();
}
void HmdDisplayPlugin::uncustomizeContext() {
+ // This stops the weirdness where if the preview was disabled, on switching back to 2D,
+ // the vsync was stuck in the disabled state. No idea why that happens though.
+ _disablePreview = false;
+ render([&](gpu::Batch& batch) {
+ batch.enableStereo(false);
+ batch.clearViewTransform();
+ batch.setFramebuffer(_compositeFramebuffer);
+ batch.clearColorFramebuffer(gpu::Framebuffer::BUFFER_COLOR0, vec4(0));
+ });
+ internalPresent();
_overlayRenderer = OverlayRenderer();
Parent::uncustomizeContext();
}
diff --git a/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.h
index fc3b40670a..a5710b6077 100644
--- a/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.h
+++ b/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.h
@@ -38,12 +38,17 @@ public:
bool setHandLaser(uint32_t hands, HandLaserMode mode, const vec4& color, const vec3& direction) override;
+ bool wantVsync() const override {
+ return false;
+ }
+
protected:
virtual void hmdPresent() = 0;
virtual bool isHmdMounted() const = 0;
virtual void postPreview() {};
virtual void updatePresentPose();
+ bool beginFrameRender(uint32_t frameIndex) override;
bool internalActivate() override;
void internalDeactivate() override;
void compositeOverlay() override;
@@ -94,10 +99,11 @@ protected:
FrameInfo _currentPresentFrameInfo;
FrameInfo _currentRenderFrameInfo;
+ bool _disablePreview{ true };
private:
ivec4 getViewportForSourceSize(const uvec2& size) const;
- bool _disablePreview{ true };
+ bool _disablePreviewItemAdded { false };
bool _monoPreview { true };
bool _clearPreviewFlag { false };
gpu::TexturePointer _previewTexture;
diff --git a/libraries/gpu/src/gpu/Buffer.cpp b/libraries/gpu/src/gpu/Buffer.cpp
index 0604980ddb..bed02035bb 100644
--- a/libraries/gpu/src/gpu/Buffer.cpp
+++ b/libraries/gpu/src/gpu/Buffer.cpp
@@ -118,7 +118,6 @@ Buffer::Update::Update(const Buffer& parent) : buffer(parent) {
void Buffer::Update::apply() const {
// Make sure we're loaded in order
++buffer._applyUpdateCount;
- assert(isRenderThread());
assert(buffer._applyUpdateCount.load() == updateNumber);
const auto pageSize = buffer._pages._pageSize;
buffer._renderSysmem.resize(size);
diff --git a/plugins/oculus/src/OculusDisplayPlugin.cpp b/plugins/oculus/src/OculusDisplayPlugin.cpp
index b5d39b8589..0c19a62585 100644
--- a/plugins/oculus/src/OculusDisplayPlugin.cpp
+++ b/plugins/oculus/src/OculusDisplayPlugin.cpp
@@ -83,11 +83,6 @@ void OculusDisplayPlugin::customizeContext() {
_sceneLayer.ColorTexture[0] = _textureSwapChain;
// not needed since the structure was zeroed on init, but explicit
_sceneLayer.ColorTexture[1] = nullptr;
-
- enableVsync(false);
- // Only enable mirroring if we know vsync is disabled
- _enablePreview = !isVsyncEnabled();
-
}
void OculusDisplayPlugin::uncustomizeContext() {
diff --git a/plugins/oculus/src/OculusDisplayPlugin.h b/plugins/oculus/src/OculusDisplayPlugin.h
index bcd8f5d8ab..80705319c6 100644
--- a/plugins/oculus/src/OculusDisplayPlugin.h
+++ b/plugins/oculus/src/OculusDisplayPlugin.h
@@ -29,8 +29,6 @@ protected:
private:
static const QString NAME;
- bool _enablePreview { false };
- bool _monoPreview { true };
ovrTextureSwapChain _textureSwapChain;
gpu::FramebufferPointer _outputFramebuffer;
};
diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.cpp b/plugins/openvr/src/OpenVrDisplayPlugin.cpp
index 950f01496e..41f8df6d1b 100644
--- a/plugins/openvr/src/OpenVrDisplayPlugin.cpp
+++ b/plugins/openvr/src/OpenVrDisplayPlugin.cpp
@@ -297,7 +297,6 @@ bool OpenVrDisplayPlugin::internalActivate() {
_submitThread = std::make_shared<OpenVrSubmitThread>(*this);
});
_submitThread->setObjectName("OpenVR Submit Thread");
- _submitThread->start(QThread::TimeCriticalPriority);
#endif
return Parent::internalActivate();
@@ -306,11 +305,6 @@ bool OpenVrDisplayPlugin::internalActivate() {
void OpenVrDisplayPlugin::internalDeactivate() {
Parent::internalDeactivate();
-#if OPENVR_THREADED_SUBMIT
- _submitThread->_quit = true;
- _submitThread->wait();
-#endif
-
_openVrDisplayActive = false;
_container->setIsOptionChecked(StandingHMDSensorMode, false);
if (_system) {
@@ -339,10 +333,17 @@ void OpenVrDisplayPlugin::customizeContext() {
}
_compositeInfos[i].textureID = getGLBackend()->getTextureID(_compositeInfos[i].texture, false);
}
+
+ _submitThread->start(QThread::HighPriority);
}
void OpenVrDisplayPlugin::uncustomizeContext() {
Parent::uncustomizeContext();
+
+#if OPENVR_THREADED_SUBMIT
+ _submitThread->_quit = true;
+ _submitThread->wait();
+#endif
}
void OpenVrDisplayPlugin::resetSensors() {