diff --git a/include/akgl/ui.h b/include/akgl/ui.h index 168042d..01f43da 100644 --- a/include/akgl/ui.h +++ b/include/akgl/ui.h @@ -425,8 +425,11 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_ui_label(char *id, char *text, akgl_UiAn * @brief Declare a vertical menu from caller-owned state, centred on screen. * * One row per entry, the selected row drawn inverted (ink on fill swaps to - * fill on ink). Hovering a row with the mouse moves `selected` to it; a left - * click on a row sets `activated`. Keyboard and gamepad go through + * fill on ink). Moving the mouse onto a row selects it; a left click on a + * row selects and sets `activated`. A *stationary* pointer claims nothing -- + * parking the mouse over the menu must not pin the selection against the + * keyboard, which is fighting sixty times a second and losing. Keyboard and + * gamepad go through * akgl_ui_menu_handle_event(), which the application calls for whichever * menu currently has its attention -- that routing *is* the focus model. * Legal only between akgl_ui_frame_begin() and akgl_ui_frame_end(). diff --git a/src/ui.c b/src/ui.c index 7ff8295..754bdc4 100644 --- a/src/ui.c +++ b/src/ui.c @@ -80,6 +80,18 @@ static bool ui_pointer_down; static bool ui_pointer_pending_pressed; /** @brief The frame-stable press edge: one left press happened before this frame. */ static bool ui_pointer_pressed; +/** @brief Motion arrived since the last frame_begin latched. Companion to the press edge. */ +static bool ui_pointer_pending_moved; +/** + * @brief The frame-stable motion edge: the pointer moved before this frame. + * + * What keeps a menu's hover-selection polite. A stationary pointer parked + * over a row must not re-claim the selection every frame -- the keyboard + * would fight it and lose, sixty times a second -- so hover only moves the + * selection on frames the mouse actually moved. Found the way these things + * are found: the demo script pressed Up and the selection did not move. + */ +static bool ui_pointer_moved; /** @brief Wheel notches accumulated since the last frame_begin, horizontal. */ static float ui_wheel_x; /** @brief Wheel notches accumulated since the last frame_begin, vertical. */ @@ -262,6 +274,8 @@ akerr_ErrorContext *akgl_ui_shutdown(void) ui_pointer_down = false; ui_pointer_pending_pressed = false; ui_pointer_pressed = false; + ui_pointer_pending_moved = false; + ui_pointer_moved = false; ui_wheel_x = 0.0f; ui_wheel_y = 0.0f; ui_last_frame_ns = 0; @@ -679,6 +693,7 @@ akerr_ErrorContext *akgl_ui_handle_event(void *appstate, SDL_Event *event, bool case SDL_EVENT_MOUSE_MOTION: ui_pointer_x = event->motion.x; ui_pointer_y = event->motion.y; + ui_pointer_pending_moved = true; position.x = ui_pointer_x; position.y = ui_pointer_y; // Fed per event, not per frame, which is the usage clay documents: @@ -743,10 +758,12 @@ akerr_ErrorContext *akgl_ui_frame_begin(void) ui_clay_error_text[0] = '\0'; ui_clay_error_count = 0; - // Latch the press edge for the widgets this frame declares, then let the - // event loop start accumulating the next one. + // Latch the press and motion edges for the widgets this frame declares, + // then let the event loop start accumulating the next ones. ui_pointer_pressed = ui_pointer_pending_pressed; ui_pointer_pending_pressed = false; + ui_pointer_moved = ui_pointer_pending_moved; + ui_pointer_pending_moved = false; now = SDL_GetTicksNS(); if ( ui_last_frame_ns != 0 ) { @@ -1015,11 +1032,14 @@ akerr_ErrorContext *akgl_ui_menu(akgl_UiMenu *menu) }, .backgroundColor = rowfill }) { - // Mouse selection: hovering a row makes it the selection, and - // the frame-latched press edge on a hovered row confirms it. - // The row highlight catches up next frame, which at any - // playable frame rate is invisible. - if ( Clay_Hovered() ) { + // Mouse selection: a row claims the selection when the + // pointer moves onto it or clicks it -- not merely by being + // under a stationary pointer, which would re-claim it every + // frame and the keyboard would fight it and lose. The + // frame-latched press edge on a hovered row confirms. The + // row highlight catches up next frame, which at any playable + // frame rate is invisible. + if ( Clay_Hovered() && (ui_pointer_moved || ui_pointer_pressed) ) { menu->selected = i; if ( ui_pointer_pressed ) { menu->activated = true; diff --git a/tests/ui.c b/tests/ui.c index dfa1bff..4717cc5 100644 --- a/tests/ui.c +++ b/tests/ui.c @@ -841,6 +841,23 @@ akerr_ErrorContext *test_ui_menu_widget(void) "clicking the second row moved the selection to %d, not 1", menu.selected); TEST_ASSERT(e, menu.activated == true, "clicking a row did not activate the menu"); menu.activated = false; + + // The pointer is still parked over the second row. Keyboard moves the + // selection away, and redeclaring the menu must not let the + // stationary pointer claim it back -- only motion or a click selects. + SDL_memset(&event, 0x00, sizeof(SDL_Event)); + event.type = SDL_EVENT_KEY_DOWN; + event.key.key = SDLK_DOWN; + TEST_EXPECT_OK(e, akgl_ui_menu_handle_event(&menu, &event, &consumed), + "Down after the click was refused"); + TEST_ASSERT(e, menu.selected == 2, "Down did not move the selection off the hovered row"); + TEST_EXPECT_OK(e, akgl_ui_frame_begin(), "opening the third menu frame failed"); + TEST_EXPECT_OK(e, akgl_ui_menu(&menu), "redeclaring the menu a third time failed"); + TEST_EXPECT_OK(e, akgl_ui_frame_end(akgl_renderer), "closing the third menu frame failed"); + TEST_ASSERT(e, menu.selected == 2, + "a stationary pointer stole the selection back to %d", menu.selected); + TEST_ASSERT(e, menu.activated == false, + "a stationary pointer activated the menu"); } CLEANUP { if ( shot != NULL ) { SDL_DestroySurface(shot);