From b4d7c84ee2b9720c93e7e2650e9f5e9ae9e65b1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 17 Feb 2021 17:37:05 +0100 Subject: [PATCH] Platform: fix a crash in GlfwApplication::setCursor() on GLFW 3.4. There's four more new cursors and the _cursors array was too small. At first I got confused because I thought the assertion on top is done against the CursorMap, which didn't contain the Hidden cursors. So to avoid confusing myself again in the future, I moved the assert after the special cases and made both arrays the same size since it doesn't make sense to have always-empty fields in there. Similar change is done in Sdl2Application, and an assert is added to avoid a nondescript crash if the window is not created yet. --- doc/changelog.dox | 2 ++ src/Magnum/Platform/GlfwApplication.cpp | 7 ++++++- src/Magnum/Platform/GlfwApplication.h | 13 +++++++++++-- src/Magnum/Platform/Sdl2Application.cpp | 8 +++++++- src/Magnum/Platform/Sdl2Application.h | 5 +++-- 5 files changed, 29 insertions(+), 6 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index ad9f3c3a8..0128bf6a3 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -308,6 +308,8 @@ See also: - Fixed canvas size setup in @ref Platform::EmscriptenApplication to be the same in the @ref Platform::EmscriptenApplication::Configuration::WindowFlag::Contextless "Contextless" case as for a WebGL-enabled context +- Fixed a crash in @ref Platform::GlfwApplication::setCursor() on the + upcoming GLFW 3.4 - @ref SceneGraph::BasicMatrixTransformation2D, @ref SceneGraph::BasicMatrixTransformation3D, @ref SceneGraph::BasicRigidMatrixTransformation2D and diff --git a/src/Magnum/Platform/GlfwApplication.cpp b/src/Magnum/Platform/GlfwApplication.cpp index 6d5eae4aa..1952ed6d9 100644 --- a/src/Magnum/Platform/GlfwApplication.cpp +++ b/src/Magnum/Platform/GlfwApplication.cpp @@ -797,7 +797,7 @@ constexpr Int CursorMap[] { } void GlfwApplication::setCursor(Cursor cursor) { - CORRADE_INTERNAL_ASSERT(UnsignedInt(cursor) < Containers::arraySize(_cursors)); + CORRADE_ASSERT(_window, "Platform::GlfwApplication::setCursor(): no window opened", ); _cursor = cursor; @@ -811,6 +811,11 @@ void GlfwApplication::setCursor(Cursor cursor) { glfwSetInputMode(_window, GLFW_CURSOR, GLFW_CURSOR_NORMAL); } + /* The second condition could be a static assert but it doesn't let me + because "this pointer only accessible in a constexpr function". Thanks + for nothing, C++. */ + CORRADE_INTERNAL_ASSERT(UnsignedInt(cursor) < Containers::arraySize(_cursors) && Containers::arraySize(_cursors) == Containers::arraySize(CursorMap)); + if(!_cursors[UnsignedInt(cursor)]) _cursors[UnsignedInt(cursor)] = glfwCreateStandardCursor(CursorMap[UnsignedInt(cursor)]); diff --git a/src/Magnum/Platform/GlfwApplication.h b/src/Magnum/Platform/GlfwApplication.h index c581824a4..e32d56f40 100644 --- a/src/Magnum/Platform/GlfwApplication.h +++ b/src/Magnum/Platform/GlfwApplication.h @@ -624,7 +624,8 @@ class GlfwApplication { * @brief Set cursor type * @m_since{2020,06} * - * Default is @ref Cursor::Arrow. + * Expects that a window is already created. Default is + * @ref Cursor::Arrow. */ void setCursor(Cursor cursor); @@ -735,7 +736,15 @@ class GlfwApplication { void setupCallbacks(); - GLFWcursor* _cursors[8]{}; + /* Corresponds to size of the Cursor enum, the two Hidden cursors are + handled differently */ + GLFWcursor* _cursors[ + #ifndef GLFW_RESIZE_NWSE_CURSOR + 6 + #else + 10 + #endif + ]{}; Cursor _cursor = Cursor::Arrow; /* These are saved from command-line arguments */ diff --git a/src/Magnum/Platform/Sdl2Application.cpp b/src/Magnum/Platform/Sdl2Application.cpp index f1f99cc16..aed91cacb 100644 --- a/src/Magnum/Platform/Sdl2Application.cpp +++ b/src/Magnum/Platform/Sdl2Application.cpp @@ -1052,7 +1052,7 @@ constexpr const char* CursorMap[] { void Sdl2Application::setCursor(Cursor cursor) { #ifndef CORRADE_TARGET_EMSCRIPTEN - CORRADE_INTERNAL_ASSERT(UnsignedInt(cursor) < Containers::arraySize(_cursors)); + CORRADE_ASSERT(_window, "Platform::Sdl2Application::setCursor(): no window opened", ); if(cursor == Cursor::Hidden) { SDL_ShowCursor(SDL_DISABLE); @@ -1069,11 +1069,17 @@ void Sdl2Application::setCursor(Cursor cursor) { SDL_SetRelativeMouseMode(SDL_FALSE); } + /* The second condition could be a static assert but it doesn't let me + because "this pointer only accessible in a constexpr function". Thanks + for nothing, C++. */ + CORRADE_INTERNAL_ASSERT(UnsignedInt(cursor) < Containers::arraySize(_cursors) && Containers::arraySize(_cursors) == Containers::arraySize(CursorMap)); + if(!_cursors[UnsignedInt(cursor)]) _cursors[UnsignedInt(cursor)] = SDL_CreateSystemCursor(CursorMap[UnsignedInt(cursor)]); SDL_SetCursor(_cursors[UnsignedInt(cursor)]); #else + CORRADE_ASSERT(_surface, "Platform::Sdl2Application::setCursor(): no window opened", ); _cursor = cursor; CORRADE_INTERNAL_ASSERT(UnsignedInt(cursor) < Containers::arraySize(CursorMap)); #pragma GCC diagnostic push diff --git a/src/Magnum/Platform/Sdl2Application.h b/src/Magnum/Platform/Sdl2Application.h index 3cbd57b26..20c619c57 100644 --- a/src/Magnum/Platform/Sdl2Application.h +++ b/src/Magnum/Platform/Sdl2Application.h @@ -968,7 +968,8 @@ class Sdl2Application { * @brief Set cursor type * @m_since{2020,06} * - * Default is @ref Cursor::Arrow. + * Expects that a window is already created. Default is + * @ref Cursor::Arrow. */ void setCursor(Cursor cursor); @@ -1184,7 +1185,7 @@ class Sdl2Application { CORRADE_ENUMSET_FRIEND_OPERATORS(Flags) #ifndef CORRADE_TARGET_EMSCRIPTEN - SDL_Cursor* _cursors[14]{}; + SDL_Cursor* _cursors[12]{}; #else Cursor _cursor; #endif