Fix oculus crash on switching display plugin to something else

interface/src/Application.cpp

@@ -64,6 +64,7 @@
 
 #include <EntityScriptingInterface.h>
 #include <ErrorDialog.h>
+#include <Finally.h>
 #include <FramebufferCache.h>
 #include <gpu/Batch.h>
 #include <gpu/Context.h>
@@ -1009,6 +1010,16 @@ void Application::paintGL() {
     if (nullptr == _displayPlugin) {
         return;
     }
+
+    // Some plugins process message events, potentially leading to 
+    // re-entering a paint event.  don't allow further processing if this 
+    // happens
+    if (_inPaint) {
+        return;
+    }
+    _inPaint = true;
+    Finally clearFlagLambda([this] { _inPaint = false; });
+
     auto displayPlugin = getActiveDisplayPlugin();
     displayPlugin->preRender();
     _offscreenContext->makeCurrent();
@@ -1203,7 +1214,7 @@ void Application::paintGL() {
         // Ensure all operations from the previous context are complete before we try to read the fbo
         glWaitSync(sync, 0, GL_TIMEOUT_IGNORED);
         glDeleteSync(sync);
-        
+
         {
             PROFILE_RANGE(__FUNCTION__ "/pluginDisplay");
             displayPlugin->display(finalTexture, toGlm(size));
@@ -1219,7 +1230,6 @@ void Application::paintGL() {
     _frameCount++;
     Stats::getInstance()->setRenderDetails(renderArgs._details);
 
-
     // Reset the gpu::Context Stages
     // Back to the default framebuffer;
     gpu::Batch batch;
@@ -4703,53 +4713,68 @@ void Application::updateDisplayMode() {
 
     auto offscreenUi = DependencyManager::get<OffscreenUi>();
     DisplayPluginPointer oldDisplayPlugin = _displayPlugin;
-    if (oldDisplayPlugin != newDisplayPlugin) {
+    if (newDisplayPlugin == oldDisplayPlugin) {
-        if (!_currentDisplayPluginActions.isEmpty()) {
+        return;
-            auto menu = Menu::getInstance();
-            foreach(auto itemInfo, _currentDisplayPluginActions) {
-                menu->removeMenuItem(itemInfo.first, itemInfo.second);
-            }
-            _currentDisplayPluginActions.clear();
-        }
-
-        if (newDisplayPlugin) {
-            _offscreenContext->makeCurrent();
-            _activatingDisplayPlugin = true;
-            newDisplayPlugin->activate();
-            _activatingDisplayPlugin = false;
-            _offscreenContext->makeCurrent();
-            offscreenUi->resize(fromGlm(newDisplayPlugin->getRecommendedUiSize()));
-            _offscreenContext->makeCurrent();
-        }
-
-        oldDisplayPlugin = _displayPlugin;
-        _displayPlugin = newDisplayPlugin;
-        
-        // If the displayPlugin is a screen based HMD, then it will want the HMDTools displayed
-        // Direct Mode HMDs (like windows Oculus) will be isHmd() but will have a screen of -1
-        bool newPluginWantsHMDTools = newDisplayPlugin ?
-                                        (newDisplayPlugin->isHmd() && (newDisplayPlugin->getHmdScreen() >= 0)) : false;
-        bool oldPluginWantedHMDTools = oldDisplayPlugin ? 
-                                        (oldDisplayPlugin->isHmd() && (oldDisplayPlugin->getHmdScreen() >= 0)) : false;
-                                        
-        // Only show the hmd tools after the correct plugin has
-        // been activated so that it's UI is setup correctly
-        if (newPluginWantsHMDTools) {
-            _pluginContainer->showDisplayPluginsTools();
-        }
-
-        if (oldDisplayPlugin) {
-            oldDisplayPlugin->deactivate();
-            _offscreenContext->makeCurrent();
-            
-            // if the old plugin was HMD and the new plugin is not HMD, then hide our hmdtools
-            if (oldPluginWantedHMDTools && !newPluginWantsHMDTools) {
-                DependencyManager::get<DialogsManager>()->hmdTools(false);
-            }
-        }
-        emit activeDisplayPluginChanged();
-        resetSensors();
     }
+
+    // Some plugins *cough* Oculus *cough* process message events from inside their 
+    // display function, and we don't want to change the display plugin underneath 
+    // the paintGL call, so we need to guard against that
+    if (_inPaint) {
+        qDebug() << "Deferring plugin switch until out of painting";
+        // Have the old plugin stop requesting renders
+        oldDisplayPlugin->stop();
+        QCoreApplication::postEvent(this, new LambdaEvent([this] {
+            updateDisplayMode();
+        }));
+        return;
+    }
+
+    if (!_currentDisplayPluginActions.isEmpty()) {
+        auto menu = Menu::getInstance();
+        foreach(auto itemInfo, _currentDisplayPluginActions) {
+            menu->removeMenuItem(itemInfo.first, itemInfo.second);
+        }
+        _currentDisplayPluginActions.clear();
+    }
+
+    if (newDisplayPlugin) {
+        _offscreenContext->makeCurrent();
+        _activatingDisplayPlugin = true;
+        newDisplayPlugin->activate();
+        _activatingDisplayPlugin = false;
+        _offscreenContext->makeCurrent();
+        offscreenUi->resize(fromGlm(newDisplayPlugin->getRecommendedUiSize()));
+        _offscreenContext->makeCurrent();
+    }
+
+    oldDisplayPlugin = _displayPlugin;
+    _displayPlugin = newDisplayPlugin;
+        
+    // If the displayPlugin is a screen based HMD, then it will want the HMDTools displayed
+    // Direct Mode HMDs (like windows Oculus) will be isHmd() but will have a screen of -1
+    bool newPluginWantsHMDTools = newDisplayPlugin ?
+                                    (newDisplayPlugin->isHmd() && (newDisplayPlugin->getHmdScreen() >= 0)) : false;
+    bool oldPluginWantedHMDTools = oldDisplayPlugin ? 
+                                    (oldDisplayPlugin->isHmd() && (oldDisplayPlugin->getHmdScreen() >= 0)) : false;
+                                        
+    // Only show the hmd tools after the correct plugin has
+    // been activated so that it's UI is setup correctly
+    if (newPluginWantsHMDTools) {
+        _pluginContainer->showDisplayPluginsTools();
+    }
+
+    if (oldDisplayPlugin) {
+        oldDisplayPlugin->deactivate();
+        _offscreenContext->makeCurrent();
+            
+        // if the old plugin was HMD and the new plugin is not HMD, then hide our hmdtools
+        if (oldPluginWantedHMDTools && !newPluginWantsHMDTools) {
+            DependencyManager::get<DialogsManager>()->hmdTools(false);
+        }
+    }
+    emit activeDisplayPluginChanged();
+    resetSensors();
     Q_ASSERT_X(_displayPlugin, "Application::updateDisplayMode", "could not find an activated display plugin");
 }
 
@@ -5052,3 +5077,15 @@ void Application::crashApplication() {
     
     qCDebug(interfaceapp) << "Intentionally crashed Interface";
 }
+
+void Application::setActiveDisplayPlugin(const QString& pluginName) {
+    auto menu = Menu::getInstance();
+    foreach(DisplayPluginPointer displayPlugin, PluginManager::getInstance()->getDisplayPlugins()) {
+        QString name = displayPlugin->getName();
+        QAction* action = menu->getActionForOption(name);
+        if (pluginName == name) {
+            action->setChecked(true);
+        }
+    }
+    updateDisplayMode();
+}

interface/src/Application.h

@@ -673,6 +673,7 @@ private:
     int _simsPerSecondReport = 0;
     quint64 _lastSimsPerSecondUpdate = 0;
     bool _isForeground = true; // starts out assumed to be in foreground
+    bool _inPaint = false;
 
     friend class PluginContainerProxy;
 };

interface/src/PluginContainerProxy.cpp

@@ -147,15 +147,3 @@ void PluginContainerProxy::showDisplayPluginsTools() {
 QGLWidget* PluginContainerProxy::getPrimarySurface() {
     return qApp->_glWidget;
 }
-
-void Application::setActiveDisplayPlugin(const QString& pluginName) {
-    auto menu = Menu::getInstance();
-    foreach(DisplayPluginPointer displayPlugin, PluginManager::getInstance()->getDisplayPlugins()) {
-        QString name = displayPlugin->getName();
-        QAction* action = menu->getActionForOption(name);
-        if (pluginName == name) {
-            action->setChecked(true);
-        }
-    }
-    updateDisplayMode();
-}

libraries/display-plugins/src/display-plugins/DisplayPlugin.h

@@ -57,6 +57,11 @@ public:
 
     // Rendering support
 
+    // Stop requesting renders, but don't do full deactivation
+    // needed to work around the issues caused by Oculus 
+    // processing messages in the middle of submitFrame
+    virtual void stop() = 0;
+
     /**
      *  Called by the application before the frame rendering.  Can be used for
      *  render timing related calls (for instance, the Oculus begin frame timing
@@ -120,6 +125,7 @@ public:
     virtual void resetSensors() {}
     virtual float devicePixelRatio() { return 1.0;  }
 
+
     static const QString& MENU_PATH();
 signals:
     void recommendedFramebufferSizeChanged(const QSize & size);

libraries/display-plugins/src/display-plugins/NullDisplayPlugin.cpp

@@ -30,3 +30,4 @@ void NullDisplayPlugin::finishFrame() {}
 
 void NullDisplayPlugin::activate() {}
 void NullDisplayPlugin::deactivate() {}
+void NullDisplayPlugin::stop() {}

libraries/display-plugins/src/display-plugins/NullDisplayPlugin.h

@@ -17,6 +17,7 @@ public:
 
     void activate() override;
     void deactivate() override;
+    void stop() override;
 
     virtual glm::uvec2 getRecommendedRenderSize() const override;
     virtual bool hasFocus() const override;

libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp

@@ -16,7 +16,9 @@
 
 OpenGLDisplayPlugin::OpenGLDisplayPlugin() {
     connect(&_timer, &QTimer::timeout, this, [&] {
-        emit requestRender();
+        if (_active) {
+            emit requestRender();
+        }
     });
 }
 
@@ -56,10 +58,17 @@ void OpenGLDisplayPlugin::customizeContext() {
 }
 
 void OpenGLDisplayPlugin::activate() {
+    _active = true;
     _timer.start(1);
 }
 
+void OpenGLDisplayPlugin::stop() {
+    _active = false;
+    _timer.stop();
+}
+
 void OpenGLDisplayPlugin::deactivate() {
+    _active = false;
     _timer.stop();
 
     makeCurrent();

libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h

@@ -25,7 +25,7 @@ public:
 
     virtual void activate() override;
     virtual void deactivate() override;
-
+    virtual void stop() override;
     virtual bool eventFilter(QObject* receiver, QEvent* event) override;
 
     virtual void display(GLuint sceneTexture, const glm::uvec2& sceneSize) override;
@@ -44,6 +44,7 @@ protected:
     mutable QTimer _timer;
     ProgramPtr _program;
     ShapeWrapperPtr _plane;
+    bool _active{ false };
     bool _vsyncSupported{ false };
 };
 

libraries/display-plugins/src/display-plugins/oculus/OculusDisplayPlugin.cpp

@@ -350,11 +350,6 @@ void OculusDisplayPlugin::deactivate() {
 }
 
 void OculusDisplayPlugin::display(GLuint finalTexture, const glm::uvec2& sceneSize) {
-    static bool inDisplay = false;
-    if (inDisplay) {
-        return;
-    }
-    inDisplay = true;
 #if (OVR_MAJOR_VERSION >= 6)
     using namespace oglplus;
     // Need to make sure only the display plugin is responsible for 
@@ -420,7 +415,6 @@ void OculusDisplayPlugin::display(GLuint finalTexture, const glm::uvec2& sceneSi
 
     ++_frameIndex;
 #endif
-    inDisplay = false;
 }
 
 // Pass input events on to the application

libraries/shared/src/Finally.h

@@ -0,0 +1,27 @@
+//
+//  Created by Bradley Austin Davis on 2015/09/01
+//  Copyright 2013-2105 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+// Simulates a java finally block by executing a lambda when an instance leaves
+// scope
+
+#include <functional>
+
+#pragma once
+#ifndef hifi_Finally_h
+#define hifi_Finally_h
+
+class Finally {
+public:
+    template <typename F>
+    Finally(F f) : _f(f) {}
+    ~Finally() { _f(); }
+private:
+    std::function<void()> _f;
+};
+
+#endif