diff --git a/docs/changelog.rst b/docs/changelog.rst index 47e89fce8..19421bd0e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -64,6 +64,9 @@ To update |kitty|, :doc:`follow the instructions `. - macOS: Make the window id of OS windows available in the ``WINDOWID`` environment variable (:pull:`2877`) +- Wayland: Fix a regression in 0.18.0 that could cause crashes related to mouse + cursors in some rare circumstances (:iss:`2810`) + 0.18.1 [2020-06-23] -------------------- diff --git a/glfw/wl_cursors.c b/glfw/wl_cursors.c index d243eb175..dac6e5633 100644 --- a/glfw/wl_cursors.c +++ b/glfw/wl_cursors.c @@ -1,7 +1,5 @@ // Future devs supporting whatever Wayland protocol stabilizes for cursor selection: see _themeAdd. -#include "wl_cursors.h" - #include "internal.h" #include @@ -10,155 +8,58 @@ #include #include -typedef struct { - struct wl_cursor_theme *theme; - int px; - int refcount; -} _themeData; - -struct _wlCursorThemeManager { - size_t count; - /** Pointer to the head of an unsorted array of themes with no sentinel. - * - * The lack of sort (and thus forcing a linear search) is intentional; - * in most cases, users are likely to have 1-2 different cursor sizes loaded. - * For those cases, we get no benefit from sorting and added constant overheads. - * - * Don't change this to a flexible array member because that complicates growing/shrinking. - */ - _themeData *themes; -}; - -static void -_themeInit(_themeData *dest, const char *name, int px) { - dest->px = px; - dest->refcount = 1; - if (_glfw.wl.shm) { - dest->theme = wl_cursor_theme_load(name, px, _glfw.wl.shm); - if(!dest->theme) { - _glfwInputError(GLFW_PLATFORM_ERROR, - "Wayland: Unable to load cursor theme"); +static int +pixels_from_scale(int scale) { + static bool queried_env = false; + static int factor = 32; + if (!queried_env) { + const char *env = getenv("XCURSOR_SIZE"); + if (env) { + const int retval = atoi(env); + if (retval > 0 && retval < 2048) factor = 32; } - } else { - dest->theme = NULL; + queried_env = true; } + return factor * scale; } -static struct wl_cursor_theme* -_themeAdd(int px, _wlCursorThemeManager *manager) { - ++manager->count; - _themeData *temp = realloc(manager->themes, sizeof(_themeData)*manager->count); - if (!temp) { - _glfwInputError(GLFW_OUT_OF_MEMORY, - "OOM during cursor theme management."); - return NULL; - } else { - manager->themes = temp; - _themeInit(manager->themes + manager->count-1, - getenv("XCURSOR_THEME"), - px); - return manager->themes[manager->count-1].theme; - } -} -//WARNING: No input safety checks. -static inline void _themeInc(_themeData - *theme) { - ++(theme->refcount); -} +struct wl_cursor_theme* +glfw_wlc_theme_for_scale(int scale) { + GLFWWLCursorThemes *t = &_glfw.wl.cursor_themes; + for (size_t i = 0; i < t->count; i++) { + if (t->themes[i].scale == scale) return t->themes[i].theme; + } -// WARNING: No input safety checks. -// In particular, doesn't check if theme is actually managed by the manager. -static void -_themeDec(_themeData *theme, _wlCursorThemeManager *manager) { - if (--(theme->refcount) == 0) { - wl_cursor_theme_destroy(theme->theme); - if (--(manager->count) > 0) { - const _themeData *last_theme = (manager->themes)+(manager->count); - *theme = *last_theme; - _themeData *temp = realloc(manager->themes, (manager->count)*sizeof(_themeData)); - // We're shrinking here, so it's not catastrophic if realloc fails. - if (temp) manager->themes = temp; - } else { - free(manager->themes); - manager->themes = NULL; + if (t->count >= t->capacity) { + t->themes = realloc(t->themes, sizeof(GLFWWLCursorTheme) * (t->count + 16)); + if (!t->themes) { + _glfwInputError(GLFW_PLATFORM_ERROR, "Wayland: Out of memory allocating space for cursor themes"); + return NULL; } + t->capacity = t->count + 16; } -} - -static _wlCursorThemeManager _default = {0}; - -_wlCursorThemeManager* -_wlCursorThemeManagerDefault() { - return &_default; + struct wl_cursor_theme *ans = wl_cursor_theme_load(getenv("XCURSOR_THEME"), pixels_from_scale(scale), _glfw.wl.shm); + if (!ans) { + _glfwInputError( + GLFW_PLATFORM_ERROR, "Wayland: wl_cursor_theme_load failed at scale: %d pixels: %d", + scale, pixels_from_scale(scale) + ); + return NULL; + } + GLFWWLCursorTheme *theme = t->themes + t->count++; + theme->scale = scale; + theme->theme = ans; + return ans; } void -_wlCursorThemeManagerDestroy(_wlCursorThemeManager *manager) { - if (manager) { - for (size_t i = 0; i < manager->count; ++i) { - wl_cursor_theme_destroy(manager->themes[i].theme); - } - free(manager->themes); - } -} +glfw_wlc_destroy(void) { + GLFWWLCursorThemes *t = &_glfw.wl.cursor_themes; -static struct wl_cursor_theme* -_wlCursorThemeManagerGet(_wlCursorThemeManager *manager, int px) { - _themeData *themedata = NULL; - for (size_t i = 0; i < manager->count; ++i) { - _themeData *temp = manager->themes+i; - if (temp->px == px) { - themedata = temp; - break; - } + for (size_t i = 0; i < t->count; i++) { + wl_cursor_theme_destroy(t->themes[i].theme); } - if (themedata != NULL) { - _themeInc(themedata); - return themedata->theme; - } - return _themeAdd(px, manager); -} - -struct wl_cursor_theme* -_wlCursorThemeManage(_wlCursorThemeManager *manager, struct wl_cursor_theme *theme, int px) { - //WARNING: Multiple returns. - if (manager == NULL) { - return NULL; - } - if (theme != NULL) { - // Search for the provided theme in the manager. - _themeData *themedata = NULL; - for (size_t i = 0; i < manager->count; ++i) { - _themeData *temp = manager->themes+i; - if (temp->theme == theme) { - themedata = temp; - break; - } - } - if (themedata != NULL) { - // Search succeeded. Check if we can avoid unnecessary operations. - if (themedata->px == px) return theme; - _themeDec(themedata, manager); - } else { - _glfwInputError(GLFW_PLATFORM_ERROR, - "Wayland internal: managed theme isn't in the provided manager"); - return theme; - //^ This is probably the sanest behavior for this situation: do nothing. - } - } - return px > 0 ? _wlCursorThemeManagerGet(manager, px) : NULL; -} - -int -_wlCursorPxFromScale(int scale) { - const char *envStr = getenv("XCURSOR_SIZE"); - if(envStr != NULL) { - const int retval = atoi(envStr); - //^ atoi here is fine since 0 is an invalid value. - if(retval > 0 && retval <= INT_MAX/scale) { - return retval*scale; - } - } - return 32*scale; + free(t->themes); + t->themes = NULL; t->capacity = 0; t->count = 0; } diff --git a/glfw/wl_cursors.h b/glfw/wl_cursors.h index 3a0a8c8fc..9c1a30967 100644 --- a/glfw/wl_cursors.h +++ b/glfw/wl_cursors.h @@ -2,29 +2,17 @@ #include -typedef struct _wlCursorThemeManager _wlCursorThemeManager; +typedef struct { + struct wl_cursor_theme *theme; + int scale; +} GLFWWLCursorTheme; -/** Returns a pointer to a wlCursorThemeManagerInstance. - * Repeatedly calling this function will return the same instance. - * - * The retrieved instance must be destroyed with _wlCursorThemeManagerDestroy. - */ -_wlCursorThemeManager* _wlCursorThemeManagerDefault(void); -/** Set a wl_cursor_theme pointer variable to a pointer to a managed cursor theme. - * Pass the desired px as the third argument. - * Returns a pointer to a managed theme, or NULL if the desired px is 0 or an error occurs. - * - * The passed theme pointer must either be NULL or a pointer to a theme managed by the passed manager. - * The provided pointer may be invalidated if it's non-NULL. - */ -struct wl_cursor_theme* -_wlCursorThemeManage(_wlCursorThemeManager*, struct wl_cursor_theme*, int); +typedef struct { + GLFWWLCursorTheme *themes; + size_t count, capacity; +} GLFWWLCursorThemes; -void _wlCursorThemeManagerDestroy(_wlCursorThemeManager*); -/** Helper method to determine the appropriate size in pixels for a given scale. - * - * Reads XCURSOR_SIZE if it's set and is valid, else defaults to 32*scale. - */ -int _wlCursorPxFromScale(int); +struct wl_cursor_theme* glfw_wlc_theme_for_scale(int scale); +void glfw_wlc_destroy(void); diff --git a/glfw/wl_init.c b/glfw/wl_init.c index ed06f2d48..b336c5a3b 100644 --- a/glfw/wl_init.c +++ b/glfw/wl_init.c @@ -148,12 +148,9 @@ static void setCursor(GLFWCursorShape shape, _GLFWwindow* window) struct wl_surface* surface = _glfw.wl.cursorSurface; const int scale = window->wl.scale; - window->wl.cursorTheme = _wlCursorThemeManage( - _glfw.wl.cursorThemeManager, - window->wl.cursorTheme, - _wlCursorPxFromScale(scale) - ); - cursor = _glfwLoadCursor(shape, window->wl.cursorTheme); + struct wl_cursor_theme *theme = glfw_wlc_theme_for_scale(scale); + if (!theme) return; + cursor = _glfwLoadCursor(shape, theme); if (!cursor) return; // TODO: handle animated cursors too. image = cursor->images[0]; @@ -780,13 +777,14 @@ int _glfwPlatformInit(void) if (_glfw.wl.shm) { - _glfw.wl.cursorThemeManager = _wlCursorThemeManagerDefault(); _glfw.wl.cursorSurface = wl_compositor_create_surface(_glfw.wl.compositor); } else { - _glfw.wl.cursorThemeManager = NULL; + _glfwInputError(GLFW_PLATFORM_ERROR, + "Wayland: Failed to find Wayland SHM"); + return false; } return true; @@ -812,9 +810,7 @@ void _glfwPlatformTerminate(void) if (_glfw.wl.cursorSurface) wl_surface_destroy(_glfw.wl.cursorSurface); - if (_glfw.wl.cursorThemeManager) { - _wlCursorThemeManagerDestroy(_glfw.wl.cursorThemeManager); - } + glfw_wlc_destroy(); if (_glfw.wl.subcompositor) wl_subcompositor_destroy(_glfw.wl.subcompositor); if (_glfw.wl.compositor) diff --git a/glfw/wl_platform.h b/glfw/wl_platform.h index 7b2191bdf..aed141a3a 100644 --- a/glfw/wl_platform.h +++ b/glfw/wl_platform.h @@ -132,7 +132,6 @@ typedef struct _GLFWwindowWayland _GLFWcursor* currentCursor; double cursorPosX, cursorPosY; - struct wl_cursor_theme* cursorTheme; char* title; char appId[256]; @@ -262,8 +261,7 @@ typedef struct _GLFWlibraryWayland size_t dataOffersCounter; _GLFWWaylandDataOffer dataOffers[8]; char* primarySelectionString; - - _wlCursorThemeManager* cursorThemeManager; + GLFWWLCursorThemes cursor_themes; } _GLFWlibraryWayland; // Wayland-specific per-monitor data diff --git a/glfw/wl_window.c b/glfw/wl_window.c index 13c1ba02e..3b1fd1894 100644 --- a/glfw/wl_window.c +++ b/glfw/wl_window.c @@ -41,11 +41,6 @@ #include #include -static void -load_cursor_theme(_GLFWwindow* window) { - window->wl.cursorTheme = _wlCursorThemeManage( - _glfw.wl.cursorThemeManager, window->wl.cursorTheme, _wlCursorPxFromScale(window->wl.scale)); -} static void setCursorImage(_GLFWwindow* window) @@ -63,8 +58,9 @@ setCursorImage(_GLFWwindow* window) } else { if (cursorWayland->scale != scale) { - if (!window->wl.cursorTheme) load_cursor_theme(window); - struct wl_cursor *newCursor = _glfwLoadCursor(cursorWayland->shape, window->wl.cursorTheme); + struct wl_cursor *newCursor = NULL; + struct wl_cursor_theme *theme = glfw_wlc_theme_for_scale(scale); + if (theme) newCursor = _glfwLoadCursor(cursorWayland->shape, theme); if (newCursor != NULL) { cursorWayland->cursor = newCursor; cursorWayland->scale = scale; @@ -136,7 +132,6 @@ static bool checkScaleChange(_GLFWwindow* window) { window->wl.scale = scale; wl_surface_set_buffer_scale(window->wl.surface, scale); - load_cursor_theme(window); setCursorImage(window); return true; } @@ -945,12 +940,6 @@ int _glfwPlatformCreateWindow(_GLFWwindow* window, void _glfwPlatformDestroyWindow(_GLFWwindow* window) { - if (window->wl.cursorTheme) { - _wlCursorThemeManage(_glfw.wl.cursorThemeManager, - window->wl.cursorTheme, - 0); - window->wl.cursorTheme = NULL; - } if (window == _glfw.wl.pointerFocus) { _glfw.wl.pointerFocus = NULL;