diff --git a/doc/changelog.dox b/doc/changelog.dox index b0b7ef075..6badd233e 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -268,6 +268,10 @@ 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. - 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 2f7f19ba5..2cc855a6c 100644 --- a/src/Magnum/Platform/GlfwApplication.cpp +++ b/src/Magnum/Platform/GlfwApplication.cpp @@ -58,6 +58,18 @@ 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 }; @@ -582,18 +594,26 @@ void GlfwApplication::setupCallbacks() { static_cast(glfwGetWindowUserPointer(window))->drawEvent(); }); #ifdef MAGNUM_TARGET_GL - glfwSetFramebufferSizeCallback(_window, [](GLFWwindow* const window, const int w, const int h) { - auto& app = *static_cast(glfwGetWindowUserPointer(window)); - ViewportEvent e{app.windowSize(), {w, h}, app.dpiScaling()}; - app.viewportEvent(e); - }); + glfwSetFramebufferSizeCallback #else - glfwSetWindowSizeCallback(_window, [](GLFWwindow* const window, const int w, const int h) { + glfwSetWindowSizeCallback + #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 ViewportEvent e{{w, h}, app.dpiScaling()}; + #endif app.viewportEvent(e); }); - #endif glfwSetKeyCallback(_window, [](GLFWwindow* const window, const int key, int, const int action, const int mods) { auto& app = *static_cast(glfwGetWindowUserPointer(window)); diff --git a/src/Magnum/Platform/Test/GlfwApplicationTest.cpp b/src/Magnum/Platform/Test/GlfwApplicationTest.cpp index df4a65dd3..b84ef1c68 100644 --- a/src/Magnum/Platform/Test/GlfwApplicationTest.cpp +++ b/src/Magnum/Platform/Test/GlfwApplicationTest.cpp @@ -107,6 +107,10 @@ 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 */