From d197ba42c32048a986bfd9e16635528b10bf39e2 Mon Sep 17 00:00:00 2001 From: Kp Date: Sat, 4 Apr 2020 19:30:23 +0000 Subject: [PATCH] Fix mouse-selection in listboxes Commit d355ef4030ef removed a seemingly unnecessary modification of the global variable grd_curcanv->cv_font, after eliminating all local reads of it. However, a non-local read, buried in listbox_mouse, depended on grd_curcanv->cv_font being set to a MEDIUM font. After that commit, grd_curcanv->cv_font retained its prior value, which is not a MEDIUM font. This caused listbox_mouse to compute an incorrect height of the lines in the listbox, which manifested as the game choosing the wrong line when the mouse is clicked in the listbox. Fix the problem by explicitly using MEDIUM3_FONT, since that was usually the value left in grd_curcanv->cv_font prior to that commit. In some cases, a different MEDIUM font would be left there, but all the MEDIUM fonts have the same height, so they are interchangeable for this purpose. Reported-by: Q3BFG10K Fixes: d355ef4030ef4c8e9e3a153fa2c04e56192edc26 ("Pass font to various drawing functions") --- common/include/fwd-gr.h | 1 + similar/2d/font.cpp | 11 +++++--- similar/main/newmenu.cpp | 57 ++++++++++++++++++++++++---------------- 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/common/include/fwd-gr.h b/common/include/fwd-gr.h index fd3e2c371..ea4ec8fe5 100644 --- a/common/include/fwd-gr.h +++ b/common/include/fwd-gr.h @@ -293,6 +293,7 @@ void gr_printt(grs_canvas &, const grs_font &, int x, int y, const char *format, #define gr_printf(A1,A2,A3,A4,F,...) dxx_call_printf_checked(gr_printfs,gr_string,(A1,A2,A3,A4),(F),##__VA_ARGS__) #define gr_uprintf(A1,A2,A3,A4,F,...) dxx_call_printf_checked(gr_printfus,gr_ustring,(A1,A2,A3,A4),(F),##__VA_ARGS__) std::pair gr_get_string_wrap(const grs_font &, const char *s, unsigned limit); +int gr_get_string_height(const grs_font &cv_font, unsigned lines); void gr_get_string_size(const grs_font &, const char *s, int *string_width, int *string_height, int *average_width); void gr_get_string_size(const grs_font &, const char *s, int *string_width, int *string_height, int *average_width, const unsigned max_chars_per_line); diff --git a/similar/2d/font.cpp b/similar/2d/font.cpp index e718cea0a..ca3204e50 100644 --- a/similar/2d/font.cpp +++ b/similar/2d/font.cpp @@ -771,6 +771,12 @@ void gr_ustring(grs_canvas &canvas, const grs_font &cv_font, const int x, const gr_ustring_mono(canvas, cv_font, x, y, s); } +int gr_get_string_height(const grs_font &cv_font, const unsigned lines) +{ + const auto fontscale_y = FONTSCALE_Y(cv_font.ft_h); + return static_cast(fontscale_y + (lines * (fontscale_y + FSPACY(1)))); +} + void gr_get_string_size(const grs_font &cv_font, const char *s, int *const string_width, int *const string_height, int *const average_width) { gr_get_string_size(cv_font, s, string_width, string_height, average_width, UINT_MAX); @@ -815,10 +821,7 @@ void gr_get_string_size(const grs_font &cv_font, const char *s, int *const strin if (string_width) *string_width = std::max(longest_width, string_width_f); if (string_height) - { - const auto fontscale_y = FONTSCALE_Y(cv_font.ft_h); - *string_height = static_cast(fontscale_y + (lines * (fontscale_y + FSPACY(1)))); - } + *string_height = gr_get_string_height(cv_font, lines); } std::pair gr_get_string_wrap(const grs_font &cv_font, const char *s, unsigned limit) diff --git a/similar/main/newmenu.cpp b/similar/main/newmenu.cpp index f42dfc932..102101c70 100644 --- a/similar/main/newmenu.cpp +++ b/similar/main/newmenu.cpp @@ -1753,31 +1753,41 @@ static void update_scroll_position(listbox *lb) static window_event_result listbox_mouse(window *, const d_event &event, listbox *lb, int button) { - int mx, my, mz, x1, x2, y1, y2; - switch (button) { case MBTN_LEFT: { if (lb->mouse_state) { + int mx, my, mz; mouse_get_pos(&mx, &my, &mz); - const auto &&line_spacing = LINE_SPACING(*grd_curcanv->cv_font, *GAME_FONT); - for (int i = lb->first_item; i < lb->first_item + lb->items_on_screen; ++i) - { - if (i >= lb->nitems) - break; - int h; - gr_get_string_size(*grd_curcanv->cv_font, lb->item[i], nullptr, &h, nullptr); - x1 = lb->box_x; - x2 = lb->box_x + lb->box_w; - y1 = (i - lb->first_item) * line_spacing + lb->box_y; - y2 = y1+h; - if ( ((mx > x1) && (mx < x2)) && ((my > y1) && (my < y2)) ) { - lb->citem = i; - return window_event_result::handled; - } - } + const int x1 = lb->box_x; + if (mx <= x1) + break; + const int x2 = x1 + lb->box_w; + if (mx >= x2) + break; + const int by = lb->box_y; + if (my <= by) + break; + const int my_relative_by = my - by; + const auto &&line_spacing = LINE_SPACING(*MEDIUM3_FONT, *GAME_FONT); + if (line_spacing < 1) + break; + const int idx_relative_first_visible_item = my_relative_by / line_spacing; + const auto first_visible_item = lb->first_item; + const auto nitems = lb->nitems; + const auto last_visible_item = std::min(first_visible_item + lb->items_on_screen, nitems); + if (idx_relative_first_visible_item >= last_visible_item) + break; + const int px_within_item = my_relative_by % static_cast(line_spacing); + const int h = gr_get_string_height(*MEDIUM3_FONT, 1); + if (px_within_item >= h) + break; + const int idx_absolute_item = idx_relative_first_visible_item + first_visible_item; + if (idx_absolute_item >= nitems) + break; + lb->citem = idx_absolute_item; } else if (event.type == EVENT_MOUSE_BUTTON_UP) { @@ -1786,12 +1796,13 @@ static window_event_result listbox_mouse(window *, const d_event &event, listbox if (lb->citem < 0) return window_event_result::ignored; + int mx, my, mz; mouse_get_pos(&mx, &my, &mz); - gr_get_string_size(*grd_curcanv->cv_font, lb->item[lb->citem], nullptr, &h, nullptr); - x1 = lb->box_x; - x2 = lb->box_x + lb->box_w; - y1 = (lb->citem - lb->first_item) * LINE_SPACING(*grd_curcanv->cv_font, *GAME_FONT) + lb->box_y; - y2 = y1+h; + gr_get_string_size(*MEDIUM3_FONT, lb->item[lb->citem], nullptr, &h, nullptr); + const int x1 = lb->box_x; + const int x2 = lb->box_x + lb->box_w; + const int y1 = (lb->citem - lb->first_item) * LINE_SPACING(*MEDIUM3_FONT, *GAME_FONT) + lb->box_y; + const int y2 = y1 + h; if ( ((mx > x1) && (mx < x2)) && ((my > y1) && (my < y2)) ) { // Tell callback, if it wants to close it will return window_event_result::close