Browse Source

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.
pull/498/head
Vladimír Vondruš 5 years ago
parent
commit
b4d7c84ee2
  1. 2
      doc/changelog.dox
  2. 7
      src/Magnum/Platform/GlfwApplication.cpp
  3. 13
      src/Magnum/Platform/GlfwApplication.h
  4. 8
      src/Magnum/Platform/Sdl2Application.cpp
  5. 5
      src/Magnum/Platform/Sdl2Application.h

2
doc/changelog.dox

@ -308,6 +308,8 @@ See also:
- Fixed canvas size setup in @ref Platform::EmscriptenApplication to be the - Fixed canvas size setup in @ref Platform::EmscriptenApplication to be the
same in the @ref Platform::EmscriptenApplication::Configuration::WindowFlag::Contextless "Contextless" same in the @ref Platform::EmscriptenApplication::Configuration::WindowFlag::Contextless "Contextless"
case as for a WebGL-enabled context 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::BasicMatrixTransformation2D,
@ref SceneGraph::BasicMatrixTransformation3D, @ref SceneGraph::BasicMatrixTransformation3D,
@ref SceneGraph::BasicRigidMatrixTransformation2D and @ref SceneGraph::BasicRigidMatrixTransformation2D and

7
src/Magnum/Platform/GlfwApplication.cpp

@ -797,7 +797,7 @@ constexpr Int CursorMap[] {
} }
void GlfwApplication::setCursor(Cursor cursor) { void GlfwApplication::setCursor(Cursor cursor) {
CORRADE_INTERNAL_ASSERT(UnsignedInt(cursor) < Containers::arraySize(_cursors)); CORRADE_ASSERT(_window, "Platform::GlfwApplication::setCursor(): no window opened", );
_cursor = cursor; _cursor = cursor;
@ -811,6 +811,11 @@ void GlfwApplication::setCursor(Cursor cursor) {
glfwSetInputMode(_window, GLFW_CURSOR, GLFW_CURSOR_NORMAL); 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)]) if(!_cursors[UnsignedInt(cursor)])
_cursors[UnsignedInt(cursor)] = glfwCreateStandardCursor(CursorMap[UnsignedInt(cursor)]); _cursors[UnsignedInt(cursor)] = glfwCreateStandardCursor(CursorMap[UnsignedInt(cursor)]);

13
src/Magnum/Platform/GlfwApplication.h

@ -624,7 +624,8 @@ class GlfwApplication {
* @brief Set cursor type * @brief Set cursor type
* @m_since{2020,06} * @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); void setCursor(Cursor cursor);
@ -735,7 +736,15 @@ class GlfwApplication {
void setupCallbacks(); 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; Cursor _cursor = Cursor::Arrow;
/* These are saved from command-line arguments */ /* These are saved from command-line arguments */

8
src/Magnum/Platform/Sdl2Application.cpp

@ -1052,7 +1052,7 @@ constexpr const char* CursorMap[] {
void Sdl2Application::setCursor(Cursor cursor) { void Sdl2Application::setCursor(Cursor cursor) {
#ifndef CORRADE_TARGET_EMSCRIPTEN #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) { if(cursor == Cursor::Hidden) {
SDL_ShowCursor(SDL_DISABLE); SDL_ShowCursor(SDL_DISABLE);
@ -1069,11 +1069,17 @@ void Sdl2Application::setCursor(Cursor cursor) {
SDL_SetRelativeMouseMode(SDL_FALSE); 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)]) if(!_cursors[UnsignedInt(cursor)])
_cursors[UnsignedInt(cursor)] = SDL_CreateSystemCursor(CursorMap[UnsignedInt(cursor)]); _cursors[UnsignedInt(cursor)] = SDL_CreateSystemCursor(CursorMap[UnsignedInt(cursor)]);
SDL_SetCursor(_cursors[UnsignedInt(cursor)]); SDL_SetCursor(_cursors[UnsignedInt(cursor)]);
#else #else
CORRADE_ASSERT(_surface, "Platform::Sdl2Application::setCursor(): no window opened", );
_cursor = cursor; _cursor = cursor;
CORRADE_INTERNAL_ASSERT(UnsignedInt(cursor) < Containers::arraySize(CursorMap)); CORRADE_INTERNAL_ASSERT(UnsignedInt(cursor) < Containers::arraySize(CursorMap));
#pragma GCC diagnostic push #pragma GCC diagnostic push

5
src/Magnum/Platform/Sdl2Application.h

@ -968,7 +968,8 @@ class Sdl2Application {
* @brief Set cursor type * @brief Set cursor type
* @m_since{2020,06} * @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); void setCursor(Cursor cursor);
@ -1184,7 +1185,7 @@ class Sdl2Application {
CORRADE_ENUMSET_FRIEND_OPERATORS(Flags) CORRADE_ENUMSET_FRIEND_OPERATORS(Flags)
#ifndef CORRADE_TARGET_EMSCRIPTEN #ifndef CORRADE_TARGET_EMSCRIPTEN
SDL_Cursor* _cursors[14]{}; SDL_Cursor* _cursors[12]{};
#else #else
Cursor _cursor; Cursor _cursor;
#endif #endif

Loading…
Cancel
Save