Stop a stationary pointer from pinning the menu selection
A pointer parked over a menu row re-claimed the selection on every frame's declaration, so keyboard navigation fought the mouse sixty times a second and lost -- press Up, and the hover put the selection straight back. Found by the UI demo's scripted tour: its mouse click leaves the pointer resting on the menu, and the Up keystroke that followed did nothing. Hover now moves the selection only on frames the pointer actually moved (or clicked), through a frame-latched motion edge beside the existing press edge. Parking the mouse claims nothing; moving it onto a row still selects, clicking still selects and activates. Regression test drives the exact sequence: click a row, move the selection away by keyboard, redeclare, and assert the stationary pointer stole nothing back. Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
This commit is contained in:
@@ -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().
|
||||
|
||||
34
src/ui.c
34
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;
|
||||
|
||||
17
tests/ui.c
17
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);
|
||||
|
||||
Reference in New Issue
Block a user