From 7a6dcdcbfd92403b988d4708fbf4ce19f77b5de4 Mon Sep 17 00:00:00 2001
From: Nav <nav@navnav.me>
Date: Wed, 11 Aug 2021 00:07:12 +0100
Subject: [PATCH] Fixed bug with Insight QApplication object being created too
 soon, which in turn caused issues with signal capturing. Also ensured that we
 only initialise the Insight object when the user has enabled Insight in their
 config.

---
 src/Application.cpp     |  9 +++++----
 src/Application.hpp     |  9 ++++++++-
 src/Insight/Insight.cpp |  5 ++---
 src/Insight/Insight.hpp | 17 ++++++++++++++++-
 4 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/src/Application.cpp b/src/Application.cpp
index 597c58b1..86c1b05d 100644
--- a/src/Application.cpp
+++ b/src/Application.cpp
@@ -44,10 +44,11 @@ int Application::run(const std::vector<std::string>& arguments) {
         this->startup();
 
         if (this->insightConfig.insightEnabled) {
-            this->insight.setApplicationConfig(this->applicationConfig);
-            this->insight.setEnvironmentConfig(this->environmentConfig);
-            this->insight.setInsightConfig(this->insightConfig);
-            this->insight.run();
+            this->insight = std::make_unique<Insight>(this->eventManager);
+            this->insight->setApplicationConfig(this->applicationConfig);
+            this->insight->setEnvironmentConfig(this->environmentConfig);
+            this->insight->setInsightConfig(this->insightConfig);
+            this->insight->run();
             Logger::debug("Insight closed");
             this->shutdown();
             return EXIT_SUCCESS;
diff --git a/src/Application.hpp b/src/Application.hpp
index 2862b697..12edf06f 100644
--- a/src/Application.hpp
+++ b/src/Application.hpp
@@ -73,12 +73,19 @@ namespace Bloom
          * Insight is, effectively, a small Qt application that serves a GUI to the user. It occupies the main thread,
          * as well as a single worker thread, and possibly other threads created by Qt.
          *
+         * Insight is optional - users can disable it via their project configuration.
+         *
          * When the user closes the Insight GUI, control of the main thread is returned to Application::run(). How we
          * deal with the GUI being closed at this point depends on user configuration.
          *
          * See the Insight class for more on this.
+         *
+         * Because Insight is optional, we only construct the Insight object when we need it. We can't use
+         * std::optional here because the Insight class extends QObject, which disables the copy constructor and
+         * the assignment operator. So we use an std::unique_ptr instead, which is perfectly fine for this use case,
+         * as we want to manage the lifetime of the object here.
          */
-        Insight insight = Insight(this->eventManager);
+        std::unique_ptr<Insight> insight = nullptr;
 
         /**
          * Configuration extracted from the user's project configuration file.
diff --git a/src/Insight/Insight.cpp b/src/Insight/Insight.cpp
index cea7ed84..d0b78297 100644
--- a/src/Insight/Insight.cpp
+++ b/src/Insight/Insight.cpp
@@ -1,7 +1,8 @@
+#include "Insight.hpp"
+
 #include <typeindex>
 #include <QTimer>
 
-#include "Insight.hpp"
 #include "InsightWorker.hpp"
 #include "src/Helpers/Paths.hpp"
 #include "src/Logger/Logger.hpp"
@@ -54,8 +55,6 @@ void Insight::startup() {
 #ifndef BLOOM_DEBUG_BUILD
     QCoreApplication::addLibraryPath(QString::fromStdString(Paths::applicationDirPath() + "/plugins"));
 #endif
-    QCoreApplication::setAttribute(Qt::AA_ShareOpenGLContexts, true);
-
     this->application.setQuitOnLastWindowClosed(true);
     qRegisterMetaType<Bloom::Targets::TargetDescriptor>();
     qRegisterMetaType<Bloom::Targets::TargetPinDescriptor>();
diff --git a/src/Insight/Insight.hpp b/src/Insight/Insight.hpp
index 169f620f..fdf958f1 100644
--- a/src/Insight/Insight.hpp
+++ b/src/Insight/Insight.hpp
@@ -52,7 +52,22 @@ namespace Bloom
         void startup();
 
     public:
-        explicit Insight(EventManager& eventManager): eventManager(eventManager) {};
+        /**
+         * Insight constructor.
+         *
+         * Note: We use the comma operator in the application() initializer to set the Qt::AA_ShareOpenGLContexts
+         * attribute, as this is required by Qt before creating a QCoreApplication instance.
+         *
+         * @param eventManager
+         */
+        explicit Insight(EventManager& eventManager):
+        eventManager(eventManager),
+        application(
+            (
+                QCoreApplication::setAttribute(Qt::AA_ShareOpenGLContexts, true),
+                QApplication(this->qtApplicationArgc, this->qtApplicationArgv.data())
+            )
+        ) {};
 
         void setApplicationConfig(const ApplicationConfig& applicationConfig) {
             this->applicationConfig = applicationConfig;