[PATCH v4 0/2] MR5634: win32u: Ignore WM_MOUSEMOVE when tracking scrollbar and arrow/track is clicked
This fixes a regression from bb496ea8. Also closer mimics windows behavior, moving the mouse should not reset the scrolling timer. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56582 -- v4: win32u: Only set scroll timer if it's not running https://gitlab.winehq.org/wine/wine/-/merge_requests/5634
From: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/win32u/scroll.c | 65 +++++++++++++------------------------------- 1 file changed, 19 insertions(+), 46 deletions(-) diff --git a/dlls/win32u/scroll.c b/dlls/win32u/scroll.c index f72ef0c2929..5ea35f1d908 100644 --- a/dlls/win32u/scroll.c +++ b/dlls/win32u/scroll.c @@ -512,6 +512,21 @@ static POINT clip_scroll_pos( RECT *rect, POINT pt ) return pt; } +void update_scroll_timer(HWND hwnd, HWND owner_hwnd, HWND ctl_hwnd, enum SCROLL_HITTEST hittest, UINT msg, UINT msg_send, BOOL vertical ) +{ + if (hittest == g_tracking_info.hit_test) + { + if (msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER) + { + send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL, msg_send, (LPARAM)ctl_hwnd ); + } + + NtUserSetSystemTimer( hwnd, SCROLL_TIMER, + msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY ); + } + else NtUserKillSystemTimer( hwnd, SCROLL_TIMER ); +} + /*********************************************************************** * handle_scroll_event * @@ -681,33 +696,12 @@ void handle_scroll_event( HWND hwnd, int bar, UINT msg, POINT pt ) case SCROLL_TOP_ARROW: draw_scroll_bar( hwnd, hdc, bar, hittest, &g_tracking_info, TRUE, FALSE ); - if (hittest == g_tracking_info.hit_test) - { - if ((msg == WM_LBUTTONDOWN) || (msg == WM_SYSTIMER)) - { - send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL, - SB_LINEUP, (LPARAM)ctl_hwnd ); - } - - NtUserSetSystemTimer( hwnd, SCROLL_TIMER, - msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY ); - } - else NtUserKillSystemTimer( hwnd, SCROLL_TIMER ); + update_scroll_timer( hwnd, owner_hwnd, ctl_hwnd, hittest, msg, SB_LINEUP, vertical ); break; case SCROLL_TOP_RECT: draw_scroll_bar( hwnd, hdc, bar, hittest, &g_tracking_info, FALSE, TRUE ); - if (hittest == g_tracking_info.hit_test) - { - if (msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER) - { - send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL, - SB_PAGEUP, (LPARAM)ctl_hwnd ); - } - NtUserSetSystemTimer( hwnd, SCROLL_TIMER, - msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY ); - } - else NtUserKillSystemTimer( hwnd, SCROLL_TIMER ); + update_scroll_timer( hwnd, owner_hwnd, ctl_hwnd, hittest, msg, SB_PAGEUP, vertical ); break; case SCROLL_THUMB: @@ -755,33 +749,12 @@ void handle_scroll_event( HWND hwnd, int bar, UINT msg, POINT pt ) case SCROLL_BOTTOM_RECT: draw_scroll_bar( hwnd, hdc, bar, hittest, &g_tracking_info, FALSE, TRUE ); - if (hittest == g_tracking_info.hit_test) - { - if (msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER) - { - send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL, - SB_PAGEDOWN, (LPARAM)ctl_hwnd ); - } - NtUserSetSystemTimer( hwnd, SCROLL_TIMER, - msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY ); - } - else NtUserKillSystemTimer( hwnd, SCROLL_TIMER ); + update_scroll_timer( hwnd, owner_hwnd, ctl_hwnd, hittest, msg, SB_PAGEDOWN, vertical ); break; case SCROLL_BOTTOM_ARROW: draw_scroll_bar( hwnd, hdc, bar, hittest, &g_tracking_info, TRUE, FALSE ); - if (hittest == g_tracking_info.hit_test) - { - if (msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER) - { - send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL, - SB_LINEDOWN, (LPARAM)ctl_hwnd ); - } - - NtUserSetSystemTimer( hwnd, SCROLL_TIMER, - msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY ); - } - else NtUserKillSystemTimer( hwnd, SCROLL_TIMER ); + update_scroll_timer( hwnd, owner_hwnd, ctl_hwnd, hittest, msg, SB_LINEDOWN, vertical ); break; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5634
From: Fabian Maurer <dark.shadow4(a)web.de> This fixes a regression from bb496ea8 where we got a WM_MOUSEMOVE and this leads to skipping the initial scroll delay. Also closer mimics windows behavior, moving the mouse should not reset the scrolling timer. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56582 --- dlls/win32u/scroll.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/dlls/win32u/scroll.c b/dlls/win32u/scroll.c index 5ea35f1d908..f34fe80c7e6 100644 --- a/dlls/win32u/scroll.c +++ b/dlls/win32u/scroll.c @@ -62,6 +62,9 @@ static struct SCROLL_TRACKING_INFO g_tracking_info; /* Is the moving thumb being displayed? */ static BOOL scroll_moving_thumb = FALSE; +/* is there currently a running timer for scrolling delay ?*/ +static BOOL scroll_timer_running = FALSE; + /* data for window that has (one or two) scroll bars */ struct win_scroll_bar_info { @@ -516,15 +519,19 @@ void update_scroll_timer(HWND hwnd, HWND owner_hwnd, HWND ctl_hwnd, enum SCROLL_ { if (hittest == g_tracking_info.hit_test) { - if (msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER) + if (!scroll_timer_running || msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER) { send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL, msg_send, (LPARAM)ctl_hwnd ); + scroll_timer_running = TRUE; + NtUserSetSystemTimer( hwnd, SCROLL_TIMER, + msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY ); } - - NtUserSetSystemTimer( hwnd, SCROLL_TIMER, - msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY ); } - else NtUserKillSystemTimer( hwnd, SCROLL_TIMER ); + else + { + scroll_timer_running = FALSE; + NtUserKillSystemTimer( hwnd, SCROLL_TIMER ); + } } /*********************************************************************** @@ -789,6 +796,7 @@ void handle_scroll_event( HWND hwnd, int bar, UINT msg, POINT pt ) /* Terminate tracking */ g_tracking_info.win = 0; scroll_moving_thumb = FALSE; + scroll_timer_running = FALSE; hittest = SCROLL_NOWHERE; draw_scroll_bar( hwnd, hdc, bar, hittest, &g_tracking_info, TRUE, TRUE ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5634
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=145628 Your paranoid android. === build (build log) === error: patch failed: dlls/win32u/scroll.c:516 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/win32u/scroll.c:516 Task: Patch failed to apply === debian11b (build log) === error: patch failed: dlls/win32u/scroll.c:516 Task: Patch failed to apply
On Thu May 16 15:28:00 2024 +0000, Fabian Maurer wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/5634/diffs?diff_id=114038&start_sha=2065757651acb69e39e395b00171aeede3a3c7f4#1309fd48153be56cdd4228d6e4bc8022a837b2de_529_525) I pushed a new version merging the two ifs as well, the only change is that now the "send_message" is also called when entering the scrollbar again. Seems reasonable to me, unless you want tests for that?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5634#note_70683
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5634
participants (4)
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Marvin -
Rémi Bernon