diff --git a/doc/changelog.dox b/doc/changelog.dox index cd34fc600..d5c63a121 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -430,10 +430,12 @@ See also: - Fixed broken @ref Platform::EmscriptenApplication mouse event coordinates when `-s DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR=1` is enabled (see [mosra/magnum#408](https://github.com/mosra/magnum/issues/408)) -- Due to some Windows-specific issue with GLFW, - @ref Platform::GlfwApplication::viewportEvent() could get fired already - during class construction, potentially causing crashes. This is now worked - around by ignoring the very first event. +- Due to Windows- and macOS-specific issues in GLFW, + @ref Platform::GlfwApplication::viewportEvent() and/or + @ref Platform::GlfwApplication::drawEvent() could get fired already during + class construction, potentially causing crashes or + `pure virtual method call` aborts. To prevent these issues, event callback + setup is delayed to the first time the application main loop is entered. - In 2019.01 @ref Magnum/Platform/Sdl2Application.h went through an include cleanup, removing 50k lines; but unfortunately we forgot to add back @cpp #include  @ce, causing iOS builds to fail to initialize. diff --git a/src/Magnum/Platform/GlfwApplication.cpp b/src/Magnum/Platform/GlfwApplication.cpp index 863ef3bd2..66a5ec1b7 100644 --- a/src/Magnum/Platform/GlfwApplication.cpp +++ b/src/Magnum/Platform/GlfwApplication.cpp @@ -58,18 +58,6 @@ enum class GlfwApplication::Flag: UnsignedByte { TextInputActive = 1 << 1, #ifdef CORRADE_TARGET_APPLE HiDpiWarningPrinted = 1 << 2 - #elif defined(CORRADE_TARGET_WINDOWS) - /* On Windows, GLFW fires a viewport event already when creating the - window, which means viewportEvent() gets called even before the - constructor exits. That's not a problem if the window is created - implicitly (because derived class vtable is not setup yet and so the - call goes into the base class no-op viewportEvent()), but when calling - create() / tryCreate() from user constructor, this might lead to crashes - as things touched by viewportEvent() might not be initialized yet. To - fix this, we ignore the first ever viewport event. This behavior was not - observed on Linux or macOS (and thus ignoring the first viewport event - there may be harmful), so keeping this Windows-only. */ - FirstViewportEventIgnored = 1 << 2 #endif }; @@ -370,9 +358,6 @@ bool GlfwApplication::tryCreate(const Configuration& configuration) { CORRADE_IGNORE_DEPRECATED_POP #endif - /* Set callbacks */ - setupCallbacks(); - return true; } @@ -561,9 +546,6 @@ bool GlfwApplication::tryCreate(const Configuration& configuration, const GLConf CORRADE_IGNORE_DEPRECATED_POP #endif - /* Set callbacks */ - setupCallbacks(); - /* Make the final context current */ glfwMakeContextCurrent(_window); @@ -600,13 +582,6 @@ void GlfwApplication::setupCallbacks() { #endif (_window, [](GLFWwindow* const window, const int w, const int h) { auto& app = *static_cast(glfwGetWindowUserPointer(window)); - #ifdef CORRADE_TARGET_WINDOWS - /* See the flag for details */ - if(!(app._flags & Flag::FirstViewportEventIgnored)) { - app._flags |= Flag::FirstViewportEventIgnored; - return; - } - #endif #ifdef MAGNUM_TARGET_GL ViewportEvent e{app.windowSize(), {w, h}, app.dpiScaling()}; #else @@ -728,6 +703,31 @@ int GlfwApplication::exec() { bool GlfwApplication::mainLoopIteration() { CORRADE_ASSERT(_window, "Platform::GlfwApplication::mainLoopIteration(): no window opened", {}); + /* + If callbacks are not set up yet, do that. Can't be done inside + tryCreate() because: + + 1. On Windows, GLFW fires a viewport event already when creating the + window, which means viewportEvent() gets called even before the + constructor exits. That's not a problem if the window is created + implicitly (because derived class vtable is not setup yet and so + the call goes into the base class no-op viewportEvent()), but when + calling create() / tryCreate() from user constructor, this might + lead to crashes as things touched by viewportEvent() might not be + initialized yet. To fix this, we ignore the first ever viewport + event. This behavior was not observed on Linux or macOS (and thus + ignoring the first viewport event there may be harmful), so keeping + this Windows-only. + 2. On macOS, GLFW might sometimes (hard to reproduce) trigger a draw + event when creating the window. That's even worse than on Windows + because this leads to pure virtual drawEvent() getting called and + the application aborting due to a pure virtual method call in case + GL context is created implicitly by the base class constructor (at + which point the vtable pointers for the derived class are not set + up yet). + */ + if(glfwGetWindowUserPointer(_window) != this) setupCallbacks(); + if(_flags & Flag::Redraw) { _flags &= ~Flag::Redraw; drawEvent(); diff --git a/src/Magnum/Platform/Test/GlfwApplicationTest.cpp b/src/Magnum/Platform/Test/GlfwApplicationTest.cpp index ad924417b..3d6d56d38 100644 --- a/src/Magnum/Platform/Test/GlfwApplicationTest.cpp +++ b/src/Magnum/Platform/Test/GlfwApplicationTest.cpp @@ -108,10 +108,6 @@ GlfwApplicationTest::GlfwApplicationTest(const Arguments& arguments): Platform:: conf.setWindowFlags(Configuration::WindowFlag::Resizable); if(!args.value("dpi-scaling").empty()) conf.setSize({800, 600}, args.value("dpi-scaling")); - /* Creating the window in the constructor exhibits a Windows-specific GLFW - issue where viewportEvent() gets called even before constructor exits; - it's now worked around but the same problem might start occuring on - other platforms as well in the future */ create(conf); /* For testing resize events */