Platform: make *Application::exit() exit right after constructor ends.

Instead of first entering the main loop, processing events etc. This also makes it finally possible to exit the application cleanly, with all non-global destructors executed as well.
mosra · Apr 9, 2020 · 2149e78 · 2149e78
1 parent 3de5075
commit 2149e78
Show file tree

Hide file tree

Showing 15 changed files with 201 additions and 36 deletions.
diff --git a/doc/changelog.dox b/doc/changelog.dox
@@ -371,6 +371,9 @@ See also:
     @ref Platform::Sdl2Application (which was behaving like virtual before)
     (see [mosra/magnum#243](https://github.com/mosra/magnum/issues/243))
 -   Undefining more noise from `Xlib.h` (see [mosra/magnum#430](https://github.com/mosra/magnum/pull/430))
+-   Calling @ref Platform::Sdl2Application::exit() "Platform::*Application::exit()"
+    directly in the application constructor will now make it exit right after
+    constructor finished, without any event processing (see [mosra/magnum#429](https://github.com/mosra/magnum/issues/429))
 
 @subsubsection changelog-latest-changes-trade Trade library
 

diff --git a/doc/snippets/MagnumPlatform.cpp b/doc/snippets/MagnumPlatform.cpp
@@ -204,3 +204,27 @@ for(Platform::Screen* s = app.screens().first(); s; s = s->nextFartherScreen())
 }
 
 }
+
+namespace G {
+
+struct MyApplication: Platform::Application {
+    MyApplication(const Arguments& arguments);
+    bool everythingGoingAsExpected = false;
+};
+
+/* [exit-from-constructor] */
+MyApplication::MyApplication(const Arguments& arguments):
+    Platform::Application{arguments, NoCreate}
+{
+    // …
+
+    if(!everythingGoingAsExpected) {
+        exit(1);
+        return;
+    }
+
+    // …
+}
+/* [exit-from-constructor] */
+
+}
diff --git a/src/Magnum/Platform/AbstractXApplication.cpp b/src/Magnum/Platform/AbstractXApplication.cpp
@@ -123,6 +123,10 @@ void AbstractXApplication::swapBuffers() {
 }
 
 int AbstractXApplication::exec() {
+    /* If exit was requested directly in the constructor, exit immediately
+       without calling anything else */
+    if(_flags & Flag::Exit) return _exitCode;
+
     /* Show window */
     XMapWindow(_display, _window);
 
@@ -132,6 +136,10 @@ int AbstractXApplication::exec() {
 }
 
 bool AbstractXApplication::mainLoopIteration() {
+    /* If exit was requested directly in the constructor, exit immediately
+       without calling anything else */
+    if(_flags & Flag::Exit) return false;
+
     XEvent event;
 
     /* Closed window */

diff --git a/src/Magnum/Platform/AbstractXApplication.h b/src/Magnum/Platform/AbstractXApplication.h
@@ -131,8 +131,21 @@ class AbstractXApplication {
         bool mainLoopIteration();
 
         /**
-         * @brief Exit application main loop
+         * @brief Exit application
          * @param exitCode  The exit code the application should return
+         *
+         * When called from application constructor, it will cause the
+         * application to exit immediately after constructor ends, without any
+         * events being processed. Calling this function is recommended over
+         * @ref std::exit() or @ref Corrade::Utility::Fatal "Fatal", which exit
+         * without calling destructors on local scope. Note that, however, you
+         * need to explicitly @cpp return @ce after calling it, as it can't
+         * exit the constructor on its own:
+         *
+         * @snippet MagnumPlatform.cpp exit-from-constructor
+         *
+         * When called from the main loop, the application exits cleanly
+         * before next main loop iteration is executed.
          */
         void exit(int exitCode = 0) {
             _flags |= Flag::Exit;

diff --git a/src/Magnum/Platform/EmscriptenApplication.cpp b/src/Magnum/Platform/EmscriptenApplication.cpp
@@ -696,6 +696,10 @@ EmscriptenApplication::GLConfiguration::GLConfiguration():
 #endif
 
 int EmscriptenApplication::exec() {
+    /* If exit was requested directly in the constructor, exit immediately
+       without calling anything else */
+    if(_flags & Flag::ExitRequested) return 0;
+
     redraw();
     return 0;
 }

diff --git a/src/Magnum/Platform/EmscriptenApplication.h b/src/Magnum/Platform/EmscriptenApplication.h
@@ -347,10 +347,21 @@ class EmscriptenApplication {
 
         /**
          * @brief Exit application main loop
+         * @param exitCode  Ignored, present only for API compatibility with
+         *      other app implementations.
          *
-         * Stops execution started by @ref exec(). The @p exitCode is ignored
-         * and present only for API compatibility with other app
-         * implementations.
+         * When called from application constructor, it will cause the
+         * application to exit immediately after constructor ends, without any
+         * events being processed. Calling this function is recommended over
+         * @ref std::exit() or @ref Corrade::Utility::Fatal "Fatal", which exit
+         * immediately and without calling destructors on local scope. Note
+         * that, however, you need to explicitly @cpp return @ce after calling
+         * it, as it can't exit the constructor on its own:
+         *
+         * @snippet MagnumPlatform.cpp exit-from-constructor
+         *
+         * When called from the main loop, the application exits cleanly
+         * before next main loop iteration is executed.
          */
         void exit(int exitCode = 0);
 

diff --git a/src/Magnum/Platform/GlfwApplication.cpp b/src/Magnum/Platform/GlfwApplication.cpp
@@ -56,8 +56,9 @@ static_assert(GLFW_TRUE == true && GLFW_FALSE == false, "GLFW does not have sane
 enum class GlfwApplication::Flag: UnsignedByte {
     Redraw = 1 << 0,
     TextInputActive = 1 << 1,
+    Exit = 1 << 2,
     #ifdef CORRADE_TARGET_APPLE
-    HiDpiWarningPrinted = 1 << 2
+    HiDpiWarningPrinted = 1 << 3
     #endif
 };
 
@@ -546,6 +547,10 @@ bool GlfwApplication::tryCreate(const Configuration& configuration, const GLConf
     CORRADE_IGNORE_DEPRECATED_POP
     #endif
 
+    /* If exit() was called before the window got created, be sure to propagate
+       it */
+    glfwSetWindowShouldClose(_window, !!(_flags & Flag::Exit));
+
     /* Make the final context current */
     glfwMakeContextCurrent(_window);
 
@@ -701,6 +706,10 @@ int GlfwApplication::exec() {
 }
 
 bool GlfwApplication::mainLoopIteration() {
+    /* If exit was requested directly in the constructor, exit immediately
+       without calling anything else */
+    if(_flags & Flag::Exit || glfwWindowShouldClose(_window)) return false;
+
     CORRADE_ASSERT(_window, "Platform::GlfwApplication::mainLoopIteration(): no window opened", {});
 
     /*
@@ -737,6 +746,15 @@ bool GlfwApplication::mainLoopIteration() {
     return !glfwWindowShouldClose(_window);
 }
 
+void GlfwApplication::exit(int exitCode) {
+    _flags |= Flag::Exit;
+    _exitCode = exitCode;
+
+    /* If the window is already created, tell GLFW that it should close. If
+       not, this is done in tryCreate() once the window is created */
+    if(_window) glfwSetWindowShouldClose(_window, true);
+}
+
 namespace {
 
 constexpr Int CursorMap[] {

diff --git a/src/Magnum/Platform/GlfwApplication.h b/src/Magnum/Platform/GlfwApplication.h
@@ -263,13 +263,24 @@ class GlfwApplication {
         bool mainLoopIteration();
 
         /**
-         * @brief Exit application main loop
+         * @brief Exit application
          * @param exitCode  The exit code the application should return
+         *
+         * When called from application constructor, it will cause the
+         * application to exit immediately after constructor ends, without any
+         * events being processed (thus not even @ref exitEvent()). Calling
+         * this function is recommended over @ref std::exit() or
+         * @ref Corrade::Utility::Fatal "Fatal", which exit without calling
+         * destructors on local scope. Note that, however, you need to
+         * explicitly @cpp return @ce after calling it, as it can't exit the
+         * constructor on its own:
+         *
+         * @snippet MagnumPlatform.cpp exit-from-constructor
+         *
+         * When called from the main loop, the application exits cleanly
+         * before next main loop iteration is executed.
          */
-        void exit(int exitCode = 0) {
-            glfwSetWindowShouldClose(_window, true);
-            _exitCode = exitCode;
-        }
+        void exit(int exitCode = 0);
 
         /**
          * @brief Underlying window handle

diff --git a/src/Magnum/Platform/Sdl2Application.cpp b/src/Magnum/Platform/Sdl2Application.cpp
@@ -812,6 +812,10 @@ void Sdl2Application::exit(const int exitCode) {
 }
 
 bool Sdl2Application::mainLoopIteration() {
+    /* If exit was requested directly in the constructor, exit immediately
+       without calling anything else */
+    if(_flags & Flag::Exit) return false;
+
     #ifndef CORRADE_TARGET_EMSCRIPTEN
     CORRADE_ASSERT(_window, "Platform::Sdl2Application::mainLoopIteration(): no window opened", {});
     #else

diff --git a/src/Magnum/Platform/Sdl2Application.h b/src/Magnum/Platform/Sdl2Application.h
@@ -529,14 +529,6 @@ class Sdl2Application {
          */
         int exec();
 
-        /**
-         * @brief Exit application main loop
-         * @param exitCode  The exit code the application should return
-         *
-         * Stops main loop started by @ref exec().
-         */
-        void exit(int exitCode = 0);
-
         /**
          * @brief Run one iteration of application main loop
          * @return @cpp false @ce if @ref exit() was called and the application
@@ -549,6 +541,26 @@ class Sdl2Application {
          */
         bool mainLoopIteration();
 
+        /**
+         * @brief Exit application
+         * @param exitCode  The exit code the application should return
+         *
+         * When called from application constructor, it will cause the
+         * application to exit immediately after constructor ends, without any
+         * events being processed (thus not even @ref exitEvent()). Calling
+         * this function is recommended over @ref std::exit() or
+         * @ref Corrade::Utility::Fatal "Fatal", which exit without calling
+         * destructors on local scope. Note that, however, you need to
+         * explicitly @cpp return @ce after calling it, as it can't exit the
+         * constructor on its own:
+         *
+         * @snippet MagnumPlatform.cpp exit-from-constructor
+         *
+         * When called from the main loop, the application exits cleanly
+         * before next main loop iteration is executed.
+         */
+        void exit(int exitCode = 0);
+
         #ifndef CORRADE_TARGET_EMSCRIPTEN
         /**
          * @brief Underlying window handle

diff --git a/src/Magnum/Platform/Test/EmscriptenApplicationTest.cpp b/src/Magnum/Platform/Test/EmscriptenApplicationTest.cpp
@@ -24,6 +24,7 @@
     DEALINGS IN THE SOFTWARE.
 */
 
+#include <Corrade/Utility/Arguments.h>
 #include <Corrade/Utility/DebugStl.h>
 
 #include "Magnum/Platform/EmscriptenApplication.h"
@@ -35,22 +36,7 @@ namespace Magnum { namespace Platform { namespace Test {
 
 struct EmscriptenApplicationTest: Platform::Application {
     /* For testing resize events */
-    explicit EmscriptenApplicationTest(const Arguments& arguments):
-        Platform::Application{arguments,
-            Configuration{}.setWindowFlags(Configuration::WindowFlag::Resizable)
-            //, GLConfiguration{}.setFlags({})
-        } {
-
-        Debug{} << "window size" << windowSize()
-            #ifdef MAGNUM_TARGET_GL
-            << framebufferSize()
-            #endif
-            << dpiScaling() << devicePixelRatio();
-
-        /* This uses a VAO on WebGL 1, so it will crash in case GL flags are
-           missing EnableExtensionsByDefault (uncomment above) */
-        GL::Mesh mesh;
-    }
+    explicit EmscriptenApplicationTest(const Arguments& arguments);
 
     virtual void drawEvent() override {
         Debug() << "draw event";
@@ -150,6 +136,32 @@ struct EmscriptenApplicationTest: Platform::Application {
         bool _redraw = false;
 };
 
+EmscriptenApplicationTest::EmscriptenApplicationTest(const Arguments& arguments): Platform::Application{arguments, NoCreate} {
+    Utility::Arguments args;
+    args.addSkippedPrefix("magnum", "engine-specific options")
+        .addBooleanOption("exit-immediately").setHelp("exit-immediately", "exit the application immediately from the constructor, to test that the app doesn't run any event handlers after")
+        .parse(arguments.argc, arguments.argv);
+
+    if(args.isSet("exit-immediately")) {
+        exit();
+        return;
+    }
+
+    create(Configuration{}.setWindowFlags(Configuration::WindowFlag::Resizable)
+        //, GLConfiguration{}.setFlags({})
+    );
+
+    Debug{} << "window size" << windowSize()
+        #ifdef MAGNUM_TARGET_GL
+        << framebufferSize()
+        #endif
+        << dpiScaling() << devicePixelRatio();
+
+    /* This uses a VAO on WebGL 1, so it will crash in case GL flags are
+       missing EnableExtensionsByDefault (uncomment above) */
+    GL::Mesh mesh;
+}
+
 }}}
 
 MAGNUM_APPLICATION_MAIN(Magnum::Platform::Test::EmscriptenApplicationTest)
diff --git a/src/Magnum/Platform/Test/GlfwApplicationTest.cpp b/src/Magnum/Platform/Test/GlfwApplicationTest.cpp
@@ -105,8 +105,14 @@ GlfwApplicationTest::GlfwApplicationTest(const Arguments& arguments): Platform::
     Utility::Arguments args;
     args.addOption("dpi-scaling").setHelp("dpi-scaling", "DPI scaled passed via Configuration instead of --magnum-dpi-scaling, to test app overrides")
         .addSkippedPrefix("magnum", "engine-specific options")
+        .addBooleanOption("exit-immediately").setHelp("exit-immediately", "exit the application immediately from the constructor, to test that the app doesn't run any event handlers after")
         .parse(arguments.argc, arguments.argv);
 
+    if(args.isSet("exit-immediately")) {
+        exit();
+        return;
+    }
+
     Configuration conf;
     conf.setWindowFlags(Configuration::WindowFlag::Resizable);
     if(!args.value("dpi-scaling").empty())

diff --git a/src/Magnum/Platform/Test/GlxApplicationTest.cpp b/src/Magnum/Platform/Test/GlxApplicationTest.cpp
@@ -23,19 +23,35 @@
     DEALINGS IN THE SOFTWARE.
 */
 
+#include <Corrade/Utility/Arguments.h>
+
 #include "Magnum/Platform/GlxApplication.h"
 
 namespace Magnum { namespace Platform { namespace Test { namespace {
 
 struct GlxApplicationTest: Platform::Application {
-    explicit GlxApplicationTest(const Arguments& arguments): Platform::Application{arguments} {}
+    explicit GlxApplicationTest(const Arguments& arguments);
 
     void drawEvent() override {
         Debug{} << "draw event";
         swapBuffers();
     }
 };
 
+GlxApplicationTest::GlxApplicationTest(const Arguments& arguments): Platform::Application{arguments, NoCreate} {
+    Utility::Arguments args;
+    args.addSkippedPrefix("magnum", "engine-specific options")
+        .addBooleanOption("exit-immediately").setHelp("exit-immediately", "exit the application immediately from the constructor, to test that the app doesn't run any event handlers after")
+        .parse(arguments.argc, arguments.argv);
+
+    if(args.isSet("exit-immediately")) {
+        exit();
+        return;
+    }
+
+    create(Configuration{});
+}
+
 }}}}
 
 MAGNUM_APPLICATION_MAIN(Magnum::Platform::Test::GlxApplicationTest)
diff --git a/src/Magnum/Platform/Test/Sdl2ApplicationTest.cpp b/src/Magnum/Platform/Test/Sdl2ApplicationTest.cpp
@@ -150,8 +150,14 @@ Sdl2ApplicationTest::Sdl2ApplicationTest(const Arguments& arguments): Platform::
     Utility::Arguments args;
     args.addOption("dpi-scaling").setHelp("dpi-scaling", "DPI scaled passed via Configuration instead of --magnum-dpi-scaling, to test app overrides")
         .addSkippedPrefix("magnum", "engine-specific options")
+        .addBooleanOption("exit-immediately").setHelp("exit-immediately", "exit the application immediately from the constructor, to test that the app doesn't run any event handlers after")
         .parse(arguments.argc, arguments.argv);
 
+    if(args.isSet("exit-immediately")) {
+        exit();
+        return;
+    }
+
     Configuration conf;
     conf.setWindowFlags(Configuration::WindowFlag::Resizable);
     if(!args.value("dpi-scaling").empty())