From 158722b642dd1267e85f70c68f520b3a30a74877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 7 Aug 2020 19:51:34 +0200 Subject: [PATCH 1/9] input: fix col/row calculation when cursor is in the top or left margin This fixes an issue where the col/row were incorrectly set to 0 instead of -1, when the mouse cursor was inside the grid margins. This resulted in e.g. the mouse cursor having the wrong shape, and foot incorrectly handling mouse events as if they were inside the grid. --- CHANGELOG.md | 1 + input.c | 12 ++++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62aa0906..b9edbf1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ * Incorrect multi-column character spacer insertion when reflowing text. * Compilation errors in 32-bit builds. +* Mouse cursor style of top and left margins. ### Security diff --git a/input.c b/input.c index 645fa4b3..161af06b 100644 --- a/input.c +++ b/input.c @@ -1048,8 +1048,12 @@ wl_pointer_enter(void *data, struct wl_pointer *wl_pointer, switch ((term->active_surface = term_surface_kind(term, surface))) { case TERM_SURF_GRID: { - int col = (x - term->margins.left) / term->cell_width; - int row = (y - term->margins.top) / term->cell_height; + int col = x >= term->margins.left + ? (x - term->margins.left) / term->cell_width + : -1; + int row = y >= term->margins.top + ? (y - term->margins.top) / term->cell_height + : -1; seat->mouse.col = col >= 0 && col < term->cols ? col : -1; seat->mouse.row = row >= 0 && row < term->rows ? row : -1; @@ -1202,8 +1206,8 @@ wl_pointer_motion(void *data, struct wl_pointer *wl_pointer, case TERM_SURF_GRID: { term_xcursor_update_for_seat(term, seat); - int col = (x - term->margins.left) / term->cell_width; - int row = (y - term->margins.top) / term->cell_height; + int col = x >= term->margins.left ? (x - term->margins.left) / term->cell_width : -1; + int row = y >= term->margins.top ? (y - term->margins.top) / term->cell_height : -1; int old_col = seat->mouse.col; int old_row = seat->mouse.row; From 721ca80abe13d47a232a41452780a3c1bb03b862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 7 Aug 2020 19:55:02 +0200 Subject: [PATCH 2/9] input: motion: do selection update even if cursor is outside the grid Previously, the selection was only updated when the mouse cursor was inside the grid. This makes it difficult to e.g. do large selections fast, since you often end up moving the cursor outside the grid, or outside the terminal window even. Now, we update the selection regardless of *where* the cursor is. This is done by bounding the row/col we pass to 'selection_update()' to the grid, while still setting the seat's row/col to -1 when the cursor is outside the grid, to ensure the xcursor etc are set correctly. Care must also be taken to *not* pass any motion events to a mouse grabbing client, when the cursor is outside the grid. Closes #70. --- CHANGELOG.md | 2 ++ input.c | 29 ++++++++++++++++++++--------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9edbf1b..d6408bf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,8 @@ text. * Compilation errors in 32-bit builds. * Mouse cursor style of top and left margins. +* Selection is now **updated** when the cursor moves outside the grid + (https://codeberg.org/dnkl/foot/issues/70). ### Security diff --git a/input.c b/input.c index 161af06b..cd23fd80 100644 --- a/input.c +++ b/input.c @@ -1204,8 +1204,6 @@ wl_pointer_motion(void *data, struct wl_pointer *wl_pointer, break; case TERM_SURF_GRID: { - term_xcursor_update_for_seat(term, seat); - int col = x >= term->margins.left ? (x - term->margins.left) / term->cell_width : -1; int row = y >= term->margins.top ? (y - term->margins.top) / term->cell_height : -1; @@ -1215,22 +1213,35 @@ wl_pointer_motion(void *data, struct wl_pointer *wl_pointer, seat->mouse.col = col >= 0 && col < term->cols ? col : -1; seat->mouse.row = row >= 0 && row < term->rows ? row : -1; - if (seat->mouse.col < 0 || seat->mouse.row < 0) - break; + term_xcursor_update_for_seat(term, seat); - bool update_selection = seat->mouse.button == BTN_LEFT || seat->mouse.button == BTN_RIGHT; + /* Update selection even if cursor is outside the grid, + * including outside the terminal bounds */ + int selection_col = max(0, min(col, term->cols - 1)); + int selection_row = max(0, min(row, term->rows - 1)); + + bool update_selection = + seat->mouse.button == BTN_LEFT || seat->mouse.button == BTN_RIGHT; bool update_selection_early = term->selection.end.row == -1; if (update_selection && update_selection_early) - selection_update(term, seat->mouse.col, seat->mouse.row); + selection_update(term, selection_col, selection_row); - if (old_col == seat->mouse.col && old_row == seat->mouse.row) + if (old_col == seat->mouse.col && old_row == seat->mouse.row) { + /* Cursor hasn't moved to a new cell since last motion event */ break; + } if (update_selection && !update_selection_early) - selection_update(term, seat->mouse.col, seat->mouse.row); + selection_update(term, selection_col, selection_row); + + if (!term_mouse_grabbed(term, seat) && + seat->mouse.col >= 0 && + seat->mouse.row >= 0) + { + assert(seat->mouse.col < term->cols); + assert(seat->mouse.row < term->rows); - if (!term_mouse_grabbed(term, seat)) { term_mouse_motion( term, seat->mouse.button, seat->mouse.row, seat->mouse.col, seat->kbd.shift, seat->kbd.alt, seat->kbd.ctrl); From 4178418010f571c3744129a3c294660e9019da0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 7 Aug 2020 19:59:03 +0200 Subject: [PATCH 3/9] input: button: button events aren't passed to clients, when cursor is outside grid Don't send mouse button escapes to a mouse grabbing client when the cursor is outside the grid (i.e. when it is inside the margins). Also don't do *any* kind of selection when the cursor is outside the grid. Previously, we only checked this for selection_start(), but e.g. double clicking would run selection_extend(), or selection_mark_word() etc. --- input.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/input.c b/input.c index cd23fd80..0823f716 100644 --- a/input.c +++ b/input.c @@ -1410,19 +1410,19 @@ wl_pointer_button(void *data, struct wl_pointer *wl_pointer, case TERM_SURF_GRID: { search_cancel(term); + bool cursor_is_on_grid = seat->mouse.col >= 0 && seat->mouse.row >= 0; + switch (state) { case WL_POINTER_BUTTON_STATE_PRESSED: { if (button == BTN_LEFT && seat->mouse.count <= 3) { selection_cancel(term); - if (selection_enabled(term, seat)) { + if (selection_enabled(term, seat) && cursor_is_on_grid) { switch (seat->mouse.count) { case 1: - if (seat->mouse.col >= 0 && seat->mouse.row >= 0) { - selection_start( - term, seat->mouse.col, seat->mouse.row, - seat->kbd.ctrl ? SELECTION_BLOCK : SELECTION_NORMAL); - } + selection_start( + term, seat->mouse.col, seat->mouse.row, + seat->kbd.ctrl ? SELECTION_BLOCK : SELECTION_NORMAL); break; case 2: @@ -1439,7 +1439,7 @@ wl_pointer_button(void *data, struct wl_pointer *wl_pointer, } else if (button == BTN_RIGHT && seat->mouse.count == 1) { - if (selection_enabled(term, seat)) { + if (selection_enabled(term, seat) && cursor_is_on_grid) { selection_extend( seat, term, seat->mouse.col, seat->mouse.row, serial); } @@ -1464,7 +1464,7 @@ wl_pointer_button(void *data, struct wl_pointer *wl_pointer, } } - if (!term_mouse_grabbed(term, seat)) { + if (!term_mouse_grabbed(term, seat) && cursor_is_on_grid) { term_mouse_down( term, button, seat->mouse.row, seat->mouse.col, seat->kbd.shift, seat->kbd.alt, seat->kbd.ctrl); @@ -1476,7 +1476,7 @@ wl_pointer_button(void *data, struct wl_pointer *wl_pointer, if (button == BTN_LEFT && term->selection.end.col != -1) selection_finalize(seat, term, serial); - if (!term_mouse_grabbed(term, seat)) { + if (!term_mouse_grabbed(term, seat) && cursor_is_on_grid) { term_mouse_up( term, button, seat->mouse.row, seat->mouse.col, seat->kbd.shift, seat->kbd.alt, seat->kbd.ctrl); From 0a9aa2dd3202125cfd5b22faf626090c78a33bb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 7 Aug 2020 20:08:30 +0200 Subject: [PATCH 4/9] changelog: 'of' -> 'in' --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6408bf2..48ae1331 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,7 @@ * Incorrect multi-column character spacer insertion when reflowing text. * Compilation errors in 32-bit builds. -* Mouse cursor style of top and left margins. +* Mouse cursor style in top and left margins. * Selection is now **updated** when the cursor moves outside the grid (https://codeberg.org/dnkl/foot/issues/70). From 76c3629cbf405845d0866b0372c27984c6a21d3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 7 Aug 2020 22:04:37 +0200 Subject: [PATCH 5/9] input: mouse: motion/enter: cleanup inside/outside grid handling --- input.c | 80 +++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 30 deletions(-) diff --git a/input.c b/input.c index 0823f716..8db6a1f6 100644 --- a/input.c +++ b/input.c @@ -1048,15 +1048,15 @@ wl_pointer_enter(void *data, struct wl_pointer *wl_pointer, switch ((term->active_surface = term_surface_kind(term, surface))) { case TERM_SURF_GRID: { - int col = x >= term->margins.left - ? (x - term->margins.left) / term->cell_width - : -1; - int row = y >= term->margins.top - ? (y - term->margins.top) / term->cell_height - : -1; + if (x < term->margins.left || x >= term->width - term->margins.right) + seat->mouse.col = -1; + else + seat->mouse.col = (x - term->margins.left) / term->cell_width; - seat->mouse.col = col >= 0 && col < term->cols ? col : -1; - seat->mouse.row = row >= 0 && row < term->rows ? row : -1; + if (y < term->margins.top || y >= term->height - term->margins.bottom) + seat->mouse.row = -1; + else + seat->mouse.row = (y - term->margins.top) / term->cell_height; term_xcursor_update_for_seat(term, seat); break; @@ -1204,38 +1204,58 @@ wl_pointer_motion(void *data, struct wl_pointer *wl_pointer, break; case TERM_SURF_GRID: { - int col = x >= term->margins.left ? (x - term->margins.left) / term->cell_width : -1; - int row = y >= term->margins.top ? (y - term->margins.top) / term->cell_height : -1; - int old_col = seat->mouse.col; int old_row = seat->mouse.row; - seat->mouse.col = col >= 0 && col < term->cols ? col : -1; - seat->mouse.row = row >= 0 && row < term->rows ? row : -1; + /* + * While the seat's mouse coordinates must always be on the + * grid, or -1, we allow updating the selection even when the + * mouse is outside the grid (could also be outside the + * terminal window). + */ + int selection_col; + int selection_row; + + if (x < term->margins.left) { + seat->mouse.col = -1; + selection_col = 0; + } else if (x >= term->width - term->margins.right) { + seat->mouse.col = -1; + selection_col = term->cols - 1; + } else { + seat->mouse.col = (x - term->margins.left) / term->cell_width; + selection_col = seat->mouse.col; + } + + if (y < term->margins.top) { + seat->mouse.row = -1; + selection_row = 0; + } else if (y >= term->height - term->margins.bottom) { + seat->mouse.row = -1; + selection_row = term->rows - 1; + } else { + seat->mouse.row = (y - term->margins.top) / term->cell_height; + selection_row = seat->mouse.row; + } + + assert(seat->mouse.col == -1 || (seat->mouse.col >= 0 && seat->mouse.col < term->cols)); + assert(seat->mouse.row == -1 || (seat->mouse.row >= 0 && seat->mouse.row < term->rows)); term_xcursor_update_for_seat(term, seat); - /* Update selection even if cursor is outside the grid, - * including outside the terminal bounds */ - int selection_col = max(0, min(col, term->cols - 1)); - int selection_row = max(0, min(row, term->rows - 1)); + /* Cursor has moved to a different cell since last time */ + bool cursor_is_on_new_cell + = old_col != seat->mouse.col || old_row != seat->mouse.row; - bool update_selection = - seat->mouse.button == BTN_LEFT || seat->mouse.button == BTN_RIGHT; - bool update_selection_early = term->selection.end.row == -1; - - if (update_selection && update_selection_early) - selection_update(term, selection_col, selection_row); - - if (old_col == seat->mouse.col && old_row == seat->mouse.row) { - /* Cursor hasn't moved to a new cell since last motion event */ - break; + /* Update selection */ + if (seat->mouse.button == BTN_LEFT || seat->mouse.button) { + if (cursor_is_on_new_cell || term->selection.end.row < 0) + selection_update(term, selection_col, selection_row); } - if (update_selection && !update_selection_early) - selection_update(term, selection_col, selection_row); - + /* Send mouse event to client application */ if (!term_mouse_grabbed(term, seat) && + cursor_is_on_new_cell && seat->mouse.col >= 0 && seat->mouse.row >= 0) { From d49d1fd59dc7d67ee60e8ec7e95aa8d7200e2264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 7 Aug 2020 22:07:16 +0200 Subject: [PATCH 6/9] input: scrolling: don't send events to client if cursor is outside grid --- input.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/input.c b/input.c index 8db6a1f6..8eca83bd 100644 --- a/input.c +++ b/input.c @@ -1548,16 +1548,23 @@ mouse_scroll(struct seat *seat, int amount) keyboard_key(seat, NULL, seat->kbd.serial, 0, key - 8, XKB_KEY_DOWN); keyboard_key(seat, NULL, seat->kbd.serial, 0, key - 8, XKB_KEY_UP); } else { - if (!term_mouse_grabbed(term, seat)) { + if (!term_mouse_grabbed(term, seat) && + seat->mouse.col >= 0 && seat->mouse.row >= 0) + { + assert(seat->mouse.col < term->cols); + assert(seat->mouse.row < term->rows); + for (int i = 0; i < amount; i++) { term_mouse_down( term, button, seat->mouse.row, seat->mouse.col, seat->kbd.shift, seat->kbd.alt, seat->kbd.ctrl); } + term_mouse_up( term, button, seat->mouse.row, seat->mouse.col, seat->kbd.shift, seat->kbd.alt, seat->kbd.ctrl); } + scrollback(term, amount); } } From 7497e448265c2924031f7e2feca7373a34578994 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 8 Aug 2020 10:00:18 +0200 Subject: [PATCH 7/9] input: motion: regression: check for RMB when updating selection While refactoring this code, the check for LMB or RMB got changed to LMB or "any button". This fixes that. It also adds a boolean 'cursor_is_on_grid', which should make the code easier to read and understand. --- input.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/input.c b/input.c index 8eca83bd..38e8e8dd 100644 --- a/input.c +++ b/input.c @@ -1247,17 +1247,19 @@ wl_pointer_motion(void *data, struct wl_pointer *wl_pointer, bool cursor_is_on_new_cell = old_col != seat->mouse.col || old_row != seat->mouse.row; + /* Cursor is inside the grid, or in the margins (or even + * outside the terminal window) */ + bool cursor_is_on_grid = seat->mouse.col >= 0 && seat->mouse.row >= 0; + /* Update selection */ - if (seat->mouse.button == BTN_LEFT || seat->mouse.button) { + if (seat->mouse.button == BTN_LEFT || seat->mouse.button == BTN_RIGHT) { if (cursor_is_on_new_cell || term->selection.end.row < 0) selection_update(term, selection_col, selection_row); } /* Send mouse event to client application */ if (!term_mouse_grabbed(term, seat) && - cursor_is_on_new_cell && - seat->mouse.col >= 0 && - seat->mouse.row >= 0) + cursor_is_on_new_cell && cursor_is_on_grid) { assert(seat->mouse.col < term->cols); assert(seat->mouse.row < term->rows); From a3d56262564955b803995ddb58f7c64d056b9984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 8 Aug 2020 10:07:25 +0200 Subject: [PATCH 8/9] input: pointer-enter: add comment describing what we're doing --- input.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/input.c b/input.c index 38e8e8dd..eb4e394b 100644 --- a/input.c +++ b/input.c @@ -1048,6 +1048,12 @@ wl_pointer_enter(void *data, struct wl_pointer *wl_pointer, switch ((term->active_surface = term_surface_kind(term, surface))) { case TERM_SURF_GRID: { + /* + * Translate x,y pixel coordinate to a cell coordinate, or -1 + * if the cursor is outside the grid. I.e. if it is inside the + * margins. + */ + if (x < term->margins.left || x >= term->width - term->margins.right) seat->mouse.col = -1; else From 4156a640446ebb0ea2ee00b950cc0fe42ccb9abd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 8 Aug 2020 10:08:47 +0200 Subject: [PATCH 9/9] input: motion: improve comment --- input.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/input.c b/input.c index eb4e394b..345fe417 100644 --- a/input.c +++ b/input.c @@ -1253,8 +1253,7 @@ wl_pointer_motion(void *data, struct wl_pointer *wl_pointer, bool cursor_is_on_new_cell = old_col != seat->mouse.col || old_row != seat->mouse.row; - /* Cursor is inside the grid, or in the margins (or even - * outside the terminal window) */ + /* Cursor is inside the grid, i.e. *not* in the margins */ bool cursor_is_on_grid = seat->mouse.col >= 0 && seat->mouse.row >= 0; /* Update selection */