From ba3af8f2c385fdd8d1e583e2beb09a72a6d0d757 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Wed, 31 Dec 2025 23:40:34 +0530 Subject: [PATCH] Refactor GLFW scroll event callback to use a proper event struct --- glfw/cocoa_window.m | 38 +++++-------- glfw/glfw3.h | 25 +++++++- glfw/input.c | 4 +- glfw/internal.h | 2 +- glfw/wl_init.c | 28 +++++---- glfw/x11_window.c | 8 +-- kitty/data-types.h | 2 +- kitty/glfw-wrapper.h | 25 +++++++- kitty/glfw.c | 4 +- kitty/mouse.c | 133 ++++++++++++++++++++++++------------------- 10 files changed, 162 insertions(+), 107 deletions(-) diff --git a/glfw/cocoa_window.m b/glfw/cocoa_window.m index daba2e3ce..7b4e74626 100644 --- a/glfw/cocoa_window.m +++ b/glfw/cocoa_window.m @@ -1314,36 +1314,28 @@ - (void)keyUp:(NSEvent *)event - (void)scrollWheel:(NSEvent *)event { - double deltaX = [event scrollingDeltaX]; - double deltaY = [event scrollingDeltaY]; - - int flags = [event hasPreciseScrollingDeltas] ? 1 : 0; - if (flags) { + GLFWScrollEvent ev = {.keyboard_modifiers=translateFlags([event modifierFlags])}; + ev.x_offset = [event scrollingDeltaX]; + ev.y_offset = [event scrollingDeltaY]; + if ([event hasPreciseScrollingDeltas]) { + ev.offset_type = GLFW_SCROLL_OFFEST_HIGHRES; float xscale = 1, yscale = 1; _glfwPlatformGetWindowContentScale(window, &xscale, &yscale); - if (xscale > 0) deltaX *= xscale; - if (yscale > 0) deltaY *= yscale; + if (xscale > 0) ev.x_offset *= xscale; + if (yscale > 0) ev.y_offset *= yscale; } switch([event momentumPhase]) { - case NSEventPhaseBegan: - flags |= (1 << 1); break; - case NSEventPhaseStationary: - flags |= (2 << 1); break; - case NSEventPhaseChanged: - flags |= (3 << 1); break; - case NSEventPhaseEnded: - flags |= (4 << 1); break; - case NSEventPhaseCancelled: - flags |= (5 << 1); break; - case NSEventPhaseMayBegin: - flags |= (6 << 1); break; - case NSEventPhaseNone: - default: - break; + case NSEventPhaseBegan: ev.momentum_type = GLFW_MOMENTUM_PHASE_BEGAN; break; + case NSEventPhaseStationary: ev.momentum_type = GLFW_MOMENTUM_PHASE_STATIONARY; break; + case NSEventPhaseChanged: ev.momentum_type = GLFW_MOMENTUM_PHASE_ACTIVE; break; + case NSEventPhaseEnded: ev.momentum_type = GLFW_MOMENTUM_PHASE_ENDED; break; + case NSEventPhaseCancelled: ev.momentum_type = GLFW_MOMENTUM_PHASE_CANCELED; break; + case NSEventPhaseMayBegin: ev.momentum_type = GLFW_MOMENTUM_PHASE_MAY_BEGIN; break; + case NSEventPhaseNone: break; } - _glfwInputScroll(window, deltaX, deltaY, flags, translateFlags([event modifierFlags])); + _glfwInputScroll(window, &ev); } - (NSDragOperation)draggingEntered:(id )sender diff --git a/glfw/glfw3.h b/glfw/glfw3.h index d2a2f45de..7219a8209 100644 --- a/glfw/glfw3.h +++ b/glfw/glfw3.h @@ -546,6 +546,29 @@ typedef enum GLFWColorScheme { GLFW_COLOR_SCHEME_LIGHT = 2 } GLFWColorScheme; +typedef enum GLFWMomentumType { + GLFW_NO_MOMENTUM_DATA = 0, + GLFW_MOMENTUM_PHASE_BEGAN = 1, + GLFW_MOMENTUM_PHASE_STATIONARY = 2, + GLFW_MOMENTUM_PHASE_ACTIVE = 3, + GLFW_MOMENTUM_PHASE_ENDED = 4, + GLFW_MOMENTUM_PHASE_CANCELED = 5, + GLFW_MOMENTUM_PHASE_MAY_BEGIN = 6, +} GLFWMomentumType; + +typedef enum GLFWOffsetType { + GLFW_SCROLL_OFFSET_LINES = 0, + GLFW_SCROLL_OFFEST_V120 = 1, + GLFW_SCROLL_OFFEST_HIGHRES = 2, +} GLFWOffsetType; + +typedef struct GLFWScrollEvent { + double x_offset, y_offset; // offsets are scaled by the window scale + GLFWMomentumType momentum_type; + GLFWOffsetType offset_type; + int keyboard_modifiers; +} GLFWScrollEvent; + /*! @defgroup joysticks Joysticks * @brief Joystick IDs. * @@ -1698,7 +1721,7 @@ typedef void (* GLFWcursorenterfun)(GLFWwindow*,int); * * @ingroup input */ -typedef void (* GLFWscrollfun)(GLFWwindow*,double,double,int,int); +typedef void (* GLFWscrollfun)(GLFWwindow*,const GLFWScrollEvent*); /*! @brief The function pointer type for key callbacks. * diff --git a/glfw/input.c b/glfw/input.c index a6d09dda9..c2f5344fa 100644 --- a/glfw/input.c +++ b/glfw/input.c @@ -353,10 +353,10 @@ void _glfwInputKeyboard(_GLFWwindow* window, GLFWkeyevent* ev) // Notifies shared code of a scroll event // -void _glfwInputScroll(_GLFWwindow* window, double xoffset, double yoffset, int flags, int mods) +void _glfwInputScroll(_GLFWwindow* window, const GLFWScrollEvent *ev) { if (window->callbacks.scroll) - window->callbacks.scroll((GLFWwindow*) window, xoffset, yoffset, flags, mods); + window->callbacks.scroll((GLFWwindow*) window, ev); } // Notifies shared code of a mouse button click event diff --git a/glfw/internal.h b/glfw/internal.h index 0ea23e7af..b514386ad 100644 --- a/glfw/internal.h +++ b/glfw/internal.h @@ -814,7 +814,7 @@ void _glfwInputWindowMonitor(_GLFWwindow* window, _GLFWmonitor* monitor); void _glfwInputKeyboard(_GLFWwindow *window, GLFWkeyevent *ev); void _glfwInputClipboardLost(GLFWClipboardType which); -void _glfwInputScroll(_GLFWwindow* window, double xoffset, double yoffset, int flags, int mods); +void _glfwInputScroll(_GLFWwindow* window, const GLFWScrollEvent *ev); void _glfwInputMouseClick(_GLFWwindow* window, int button, int action, int mods); void _glfwInputCursorPos(_GLFWwindow* window, double xpos, double ypos); void _glfwInputCursorEnter(_GLFWwindow* window, bool entered); diff --git a/glfw/wl_init.c b/glfw/wl_init.c index f847131c2..f8a43afcd 100644 --- a/glfw/wl_init.c +++ b/glfw/wl_init.c @@ -201,33 +201,31 @@ static void pointer_handle_frame(void *data UNUSED, struct wl_pointer *pointer UNUSED) { _GLFWwindow* window = _glfw.wl.pointerFocus; if (!window) return; - float x = 0, y = 0; - static const int HIGHRES = 1; - static const int VALUE120 = 1 << 4; - int flags = 0; + GLFWScrollEvent ev = {.keyboard_modifiers=_glfw.wl.xkb.states.modifiers}; if (info.discrete.y_axis_type != AXIS_EVENT_UNKNOWN) { - y = info.discrete.y; - if (info.discrete.y_axis_type == AXIS_EVENT_VALUE120) flags |= VALUE120; + ev.y_offset = info.discrete.y; + if (info.discrete.y_axis_type == AXIS_EVENT_VALUE120) ev.offset_type = GLFW_SCROLL_OFFEST_V120; } else if (info.continuous.y_axis_type != AXIS_EVENT_UNKNOWN) { - flags |= HIGHRES; - y = info.continuous.y; + ev.offset_type = GLFW_SCROLL_OFFEST_HIGHRES; + ev.y_offset = info.continuous.y; } if (info.discrete.x_axis_type != AXIS_EVENT_UNKNOWN) { - x = info.discrete.x; - if (info.discrete.x_axis_type == AXIS_EVENT_VALUE120) flags |= VALUE120; + ev.x_offset = info.discrete.x; + if (info.discrete.x_axis_type == AXIS_EVENT_VALUE120) ev.offset_type = GLFW_SCROLL_OFFEST_V120; } else if (info.continuous.x_axis_type != AXIS_EVENT_UNKNOWN) { - flags |= HIGHRES; - x = info.continuous.x; + ev.offset_type = GLFW_SCROLL_OFFEST_HIGHRES; + ev.x_offset = info.continuous.x; } /* clear pointer_curr_axis_info for next frame */ memset(&info, 0, sizeof(info)); - if (x != 0.0f || y != 0.0f) { + if (ev.y_offset != 0.0f || ev.x_offset != 0.0f) { float scale = (float)_glfwWaylandWindowScale(window); - y *= scale; x *= scale; - _glfwInputScroll(window, -x, y, flags, _glfw.wl.xkb.states.modifiers); + ev.x_offset *= scale; ev.y_offset *= scale; + ev.x_offset *= -1; + _glfwInputScroll(window, &ev); } } diff --git a/glfw/x11_window.c b/glfw/x11_window.c index 65257674e..87df5fbb7 100644 --- a/glfw/x11_window.c +++ b/glfw/x11_window.c @@ -1445,13 +1445,13 @@ static void processEvent(XEvent *event) // Modern X provides scroll events as mouse button presses else if (event->xbutton.button == Button4) - _glfwInputScroll(window, 0.0, 1.0, 0, mods); + _glfwInputScroll(window, &(GLFWScrollEvent){.keyboard_modifiers=mods, .y_offset=1}); else if (event->xbutton.button == Button5) - _glfwInputScroll(window, 0.0, -1.0, 0, mods); + _glfwInputScroll(window, &(GLFWScrollEvent){.keyboard_modifiers=mods, .y_offset=-1}); else if (event->xbutton.button == Button6) - _glfwInputScroll(window, 1.0, 0.0, 0, mods); + _glfwInputScroll(window, &(GLFWScrollEvent){.keyboard_modifiers=mods, .x_offset=1}); else if (event->xbutton.button == Button7) - _glfwInputScroll(window, -1.0, 0.0, 0, mods); + _glfwInputScroll(window, &(GLFWScrollEvent){.keyboard_modifiers=mods, .x_offset=-1}); else { diff --git a/kitty/data-types.h b/kitty/data-types.h index 8111a2cb0..6714e49ac 100644 --- a/kitty/data-types.h +++ b/kitty/data-types.h @@ -329,7 +329,7 @@ void enter_event(int modifiers); void leave_event(int modifiers); void mouse_event(const int, int, int); void focus_in_event(void); -void scroll_event(double, double, int, int); +void scroll_event(const GLFWScrollEvent *ev); void on_key_input(const GLFWkeyevent *ev); void request_window_attention(id_type, bool); locale_t get_c_locale(void); diff --git a/kitty/glfw-wrapper.h b/kitty/glfw-wrapper.h index 74850dbf2..925ac6f6b 100644 --- a/kitty/glfw-wrapper.h +++ b/kitty/glfw-wrapper.h @@ -284,6 +284,29 @@ typedef enum GLFWColorScheme { GLFW_COLOR_SCHEME_LIGHT = 2 } GLFWColorScheme; +typedef enum GLFWMomentumType { + GLFW_NO_MOMENTUM_DATA = 0, + GLFW_MOMENTUM_PHASE_BEGAN = 1, + GLFW_MOMENTUM_PHASE_STATIONARY = 2, + GLFW_MOMENTUM_PHASE_ACTIVE = 3, + GLFW_MOMENTUM_PHASE_ENDED = 4, + GLFW_MOMENTUM_PHASE_CANCELED = 5, + GLFW_MOMENTUM_PHASE_MAY_BEGIN = 6, +} GLFWMomentumType; + +typedef enum GLFWOffsetType { + GLFW_SCROLL_OFFSET_LINES = 0, + GLFW_SCROLL_OFFEST_V120 = 1, + GLFW_SCROLL_OFFEST_HIGHRES = 2, +} GLFWOffsetType; + +typedef struct GLFWScrollEvent { + double x_offset, y_offset; // offsets are scaled by the window scale + GLFWMomentumType momentum_type; + GLFWOffsetType offset_type; + int keyboard_modifiers; +} GLFWScrollEvent; + /*! @defgroup joysticks Joysticks * @brief Joystick IDs. * @@ -1436,7 +1459,7 @@ typedef void (* GLFWcursorenterfun)(GLFWwindow*,int); * * @ingroup input */ -typedef void (* GLFWscrollfun)(GLFWwindow*,double,double,int,int); +typedef void (* GLFWscrollfun)(GLFWwindow*,const GLFWScrollEvent*); /*! @brief The function pointer type for key callbacks. * diff --git a/kitty/glfw.c b/kitty/glfw.c index ca6a032b1..a3df5d953 100644 --- a/kitty/glfw.c +++ b/kitty/glfw.c @@ -572,14 +572,14 @@ cursor_pos_callback(GLFWwindow *w, double x, double y) { } static void -scroll_callback(GLFWwindow *w, double xoffset, double yoffset, int flags, int mods) { +scroll_callback(GLFWwindow *w, const GLFWScrollEvent *ev) { if (!set_callback_window(w)) return; monotonic_t now = monotonic(); if (OPT(mouse_hide.scroll_unhide)) { cursor_active_callback(w, now); } global_state.callback_os_window->last_mouse_activity_at = now; - if (is_window_ready_for_callbacks()) scroll_event(xoffset, yoffset, flags, mods); + if (is_window_ready_for_callbacks()) scroll_event(ev); request_tick_callback(); global_state.callback_os_window = NULL; } diff --git a/kitty/mouse.c b/kitty/mouse.c index 7228c7e3b..ab562fce4 100644 --- a/kitty/mouse.c +++ b/kitty/mouse.c @@ -1162,50 +1162,79 @@ mouse_event(const int button, int modifiers, int action) { } static int -scale_scroll(MouseTrackingMode mouse_tracking_mode, double offset, bool is_high_resolution, bool is_value120, double *pending_scroll_pixels, int cell_size) { +scale_scroll(MouseTrackingMode mouse_tracking_mode, double offset, GLFWOffsetType offset_type, double *pending_scroll_pixels, int cell_size) { // scale the scroll by the multiplier unless the mouse is grabbed. If the mouse is grabbed only change direction. #define SCALE_SCROLL(which) { double scale = OPT(which); if (mouse_tracking_mode) scale /= fabs(scale); offset *= scale; } int s = 0; - if (is_high_resolution) { - SCALE_SCROLL(touch_scroll_multiplier); - double pixels = *pending_scroll_pixels + offset; - if (fabs(pixels) < cell_size) { - *pending_scroll_pixels = pixels; - return 0; - } - s = (int)round(pixels) / cell_size; - *pending_scroll_pixels = pixels - s * cell_size; - } else if (is_value120) { - SCALE_SCROLL(wheel_scroll_multiplier); - const double offset_lines = offset / 120.; - const double pixels = *pending_scroll_pixels + offset_lines * cell_size; - if (fabs(pixels) < cell_size) { - *pending_scroll_pixels = pixels; - return 0; - } - s = (int)round(pixels) / cell_size; - *pending_scroll_pixels = pixels - s * cell_size; - } else { - SCALE_SCROLL(wheel_scroll_multiplier); - s = (int) round(offset); - if (offset != 0) { - const int min_lines = mouse_tracking_mode ? 1 : OPT(wheel_scroll_min_lines); - if (min_lines > 0 && abs(s) < min_lines) s = offset > 0 ? min_lines : -min_lines; - // Always add the minimum number of lines when it is negative - else if (min_lines < 0) s = offset > 0 ? s - min_lines : s + min_lines; - // apparently on cocoa some mice generate really small yoffset values - // when scrolling slowly https://github.com/kovidgoyal/kitty/issues/1238 - if (s == 0) s = offset > 0 ? 1 : -1; - } - *pending_scroll_pixels = 0; + switch (offset_type) { + case GLFW_SCROLL_OFFEST_HIGHRES: { + SCALE_SCROLL(touch_scroll_multiplier); + double pixels = *pending_scroll_pixels + offset; + if (fabs(pixels) < cell_size) { + *pending_scroll_pixels = pixels; + return 0; + } + s = (int)round(pixels) / cell_size; + *pending_scroll_pixels = pixels - s * cell_size; + } break; + case GLFW_SCROLL_OFFEST_V120: { + SCALE_SCROLL(wheel_scroll_multiplier); + const double offset_lines = offset / 120.; + const double pixels = *pending_scroll_pixels + offset_lines * cell_size; + if (fabs(pixels) < cell_size) { + *pending_scroll_pixels = pixels; + return 0; + } + s = (int)round(pixels) / cell_size; + *pending_scroll_pixels = pixels - s * cell_size; + } break; + case GLFW_SCROLL_OFFSET_LINES: { + SCALE_SCROLL(wheel_scroll_multiplier); + s = (int) round(offset); + if (offset != 0) { + const int min_lines = mouse_tracking_mode ? 1 : OPT(wheel_scroll_min_lines); + if (min_lines > 0 && abs(s) < min_lines) s = offset > 0 ? min_lines : -min_lines; + // Always add the minimum number of lines when it is negative + else if (min_lines < 0) s = offset > 0 ? s - min_lines : s + min_lines; + // apparently on cocoa some mice generate really small yoffset values + // when scrolling slowly https://github.com/kovidgoyal/kitty/issues/1238 + if (s == 0) s = offset > 0 ? 1 : -1; + } + *pending_scroll_pixels = 0; + } break; } return s; #undef SCALE_SCROLL } +static const char* +scroll_offset_type(GLFWOffsetType t) { + switch(t) { + case GLFW_SCROLL_OFFSET_LINES: return "lines"; + case GLFW_SCROLL_OFFEST_V120: return "v120"; + case GLFW_SCROLL_OFFEST_HIGHRES: return "highres"; + } + return ""; +} + +static const char* +scroll_phase(GLFWMomentumType t) { + switch(t) { + case GLFW_NO_MOMENTUM_DATA: return "none"; + case GLFW_MOMENTUM_PHASE_MAY_BEGIN: return "may_begin"; + case GLFW_MOMENTUM_PHASE_BEGAN: return "began"; + case GLFW_MOMENTUM_PHASE_ACTIVE: return "active"; + case GLFW_MOMENTUM_PHASE_STATIONARY: return "stationary"; + case GLFW_MOMENTUM_PHASE_CANCELED: return "cancelled"; + case GLFW_MOMENTUM_PHASE_ENDED: return "ended"; + } + return ""; +} + + void -scroll_event(double xoffset, double yoffset, int flags, int modifiers) { - debug("\x1b[36mScroll\x1b[m xoffset: %f yoffset: %f flags: %x modifiers: %s\n", xoffset, yoffset, flags, format_mods(modifiers)); +scroll_event(const GLFWScrollEvent *ev) { + debug("\x1b[36mScroll\x1b[m %s x: %f y: %f momentum: %s modifiers: %s\n", scroll_offset_type(ev->offset_type), ev->x_offset, ev->y_offset, scroll_phase(ev->momentum_type), format_mods(ev->keyboard_modifiers)); bool in_tab_bar; static id_type window_for_momentum_scroll = 0; static bool main_screen_for_momentum_scroll = false; @@ -1240,38 +1269,29 @@ scroll_event(double xoffset, double yoffset, int flags, int modifiers) { } Screen *screen = w->render_data.screen; - enum MomentumData { NoMomentumData, MomentumPhaseBegan, MomentumPhaseStationary, MomentumPhaseActive, MomentumPhaseEnded, MomentumPhaseCancelled, MomentumPhaseMayBegin }; - enum MomentumData momentum_data = (flags >> 1) & 7; - - switch(momentum_data) { - case NoMomentumData: + switch(ev->momentum_type) { + case GLFW_NO_MOMENTUM_DATA: break; - case MomentumPhaseBegan: + case GLFW_MOMENTUM_PHASE_BEGAN: window_for_momentum_scroll = w->id; main_screen_for_momentum_scroll = screen->linebuf == screen->main_linebuf; break; - case MomentumPhaseStationary: - case MomentumPhaseActive: + case GLFW_MOMENTUM_PHASE_STATIONARY: case GLFW_MOMENTUM_PHASE_ACTIVE: if (window_for_momentum_scroll != w->id || main_screen_for_momentum_scroll != (screen->linebuf == screen->main_linebuf)) return; break; - case MomentumPhaseEnded: - case MomentumPhaseCancelled: + case GLFW_MOMENTUM_PHASE_ENDED: case GLFW_MOMENTUM_PHASE_CANCELED: window_for_momentum_scroll = 0; break; - case MomentumPhaseMayBegin: - default: + case GLFW_MOMENTUM_PHASE_MAY_BEGIN: break; } int s; - const bool is_high_resolution = flags & 1; - const bool is_value120 = flags & (1 << 4); - - if (yoffset != 0.0) { - s = scale_scroll(screen->modes.mouse_tracking_mode, yoffset, is_high_resolution, is_value120, &screen->pending_scroll_pixels_y, global_state.callback_os_window->fonts_data->fcm.cell_height); + if (ev->y_offset != 0.0) { + s = scale_scroll(screen->modes.mouse_tracking_mode, ev->y_offset, ev->offset_type, &screen->pending_scroll_pixels_y, global_state.callback_os_window->fonts_data->fcm.cell_height); if (s) { bool upwards = s > 0; if (screen->modes.mouse_tracking_mode) { - int sz = encode_mouse_scroll(w, upwards ? 4 : 5, modifiers); + int sz = encode_mouse_scroll(w, upwards ? 4 : 5, ev->keyboard_modifiers); if (sz > 0) { mouse_event_buf[sz] = 0; for (s = abs(s); s > 0; s--) { @@ -1287,11 +1307,11 @@ scroll_event(double xoffset, double yoffset, int flags, int modifiers) { } } } - if (xoffset != 0.0) { - s = scale_scroll(screen->modes.mouse_tracking_mode, xoffset, is_high_resolution, is_value120, &screen->pending_scroll_pixels_x, global_state.callback_os_window->fonts_data->fcm.cell_width); + if (ev->x_offset != 0.0) { + s = scale_scroll(screen->modes.mouse_tracking_mode, ev->x_offset, ev->offset_type, &screen->pending_scroll_pixels_x, global_state.callback_os_window->fonts_data->fcm.cell_width); if (s) { if (screen->modes.mouse_tracking_mode) { - int sz = encode_mouse_scroll(w, s > 0 ? 6 : 7, modifiers); + int sz = encode_mouse_scroll(w, s > 0 ? 6 : 7, ev->keyboard_modifiers); if (sz > 0) { mouse_event_buf[sz] = 0; for (s = abs(s); s > 0; s--) { @@ -1301,7 +1321,6 @@ scroll_event(double xoffset, double yoffset, int flags, int modifiers) { } } } - } static PyObject*