Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows: Prevent high freq mice from slowing down event loops #4792

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 27 additions & 17 deletions src/events/SDL_events.c
Original file line number Diff line number Diff line change
Expand Up @@ -891,26 +891,36 @@ SDL_WaitEventTimeout(SDL_Event * event, int timeout)
}
}

for (;;) {
SDL_PumpEvents();
switch (SDL_PeepEvents(event, 1, SDL_GETEVENT, SDL_FIRSTEVENT, SDL_LASTEVENT)) {
case -1:
return 0;
case 0:
if (timeout == 0) {
/* Polling and no events, just return */
return 0;
}
if (timeout > 0 && SDL_TICKS_PASSED(SDL_GetTicks(), expiration)) {
/* Timeout expired and no events */
switch (SDL_PeepEvents(event, 1, SDL_GETEVENT, SDL_FIRSTEVENT, SDL_LASTEVENT)) {
case -1:
return 0;
case 0:
/* No old events, let's enter a pump-and-check loop. */
for (;;) {
SDL_PumpEvents();
switch (SDL_PeepEvents(event, 1, SDL_GETEVENT, SDL_FIRSTEVENT, SDL_LASTEVENT)) {
case -1:
return 0;
case 0:
if (timeout == 0) {
/* Polling and no new events, just return */
return 0;
}
if (timeout > 0 && SDL_TICKS_PASSED(SDL_GetTicks(), expiration)) {
/* Timeout expired and no new events */
return 0;
}
SDL_Delay(1);
break;
default:
/* Has new events */
return 1;
}
SDL_Delay(1);
break;
default:
/* Has events */
return 1;
}
return 0;
default:
/* Has old events */
return 1;
}
}

Expand Down
145 changes: 116 additions & 29 deletions src/video/windows/SDL_windowsevents.c
Original file line number Diff line number Diff line change
Expand Up @@ -420,14 +420,6 @@ static SDL_bool isWin10FCUorNewer = SDL_FALSE;
#define MI_WP_SIGNATURE_MASK 0xFFFFFF00
#define IsTouchEvent(dw) ((dw) & MI_WP_SIGNATURE_MASK) == MI_WP_SIGNATURE

typedef enum
{
SDL_MOUSE_EVENT_SOURCE_UNKNOWN,
SDL_MOUSE_EVENT_SOURCE_MOUSE,
SDL_MOUSE_EVENT_SOURCE_TOUCH,
SDL_MOUSE_EVENT_SOURCE_PEN,
} SDL_MOUSE_EVENT_SOURCE;

static SDL_MOUSE_EVENT_SOURCE GetMouseMessageSource()
{
LPARAM extrainfo = GetMessageExtraInfo();
Expand Down Expand Up @@ -598,6 +590,37 @@ WIN_KeyboardHookProc(int nCode, WPARAM wParam, LPARAM lParam)
return 1;
}

static void
WIN_HandleMouseMove(SDL_WindowData *data, SDL_MOUSE_EVENT_SOURCE source, LPARAM lParam)
{
/* See the huge comment in WIN_PumpEvents as to why this is handled in a special manner.
TL;DR: Windows likes to combine mouse moves and give us more when we ask for it,
but asking for it too much ends up getting the event loop stuck until the mouse stops. */
SDL_Mouse *mouse = SDL_GetMouse();
if (!mouse->relative_mode || mouse->relative_mode_warp) {
/* Only generate mouse events for real mouse */
if (source != SDL_MOUSE_EVENT_SOURCE_TOUCH &&
lParam != data->last_pointer_update) {
SDL_SendMouseMotion(data->window, 0, 0, GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam));
if (isWin10FCUorNewer && mouse->relative_mode_warp &&
(data->window->flags & SDL_WINDOW_INPUT_FOCUS) != 0) {
/* To work around #3931, Win10 bug introduced in Fall Creators Update, where
SetCursorPos() (SDL_WarpMouseInWindow()) doesn't reliably generate mouse events anymore,
after each windows mouse event generate a fake event for the middle of the window
if relative_mode_warp is used */
int center_x = 0, center_y = 0;
SDL_GetWindowSize(data->window, &center_x, &center_y);
center_x /= 2;
center_y /= 2;
SDL_SendMouseMotion(data->window, 0, 0, center_x, center_y);
}
}
} else {
/* We still need to update focus */
SDL_SetMouseFocus(data->window);
}
}

LRESULT CALLBACK
WIN_WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
{
Expand Down Expand Up @@ -740,28 +763,14 @@ WIN_WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)

case WM_MOUSEMOVE:
{
SDL_Mouse *mouse = SDL_GetMouse();
if (!mouse->relative_mode || mouse->relative_mode_warp) {
/* Only generate mouse events for real mouse */
if (GetMouseMessageSource() != SDL_MOUSE_EVENT_SOURCE_TOUCH &&
lParam != data->last_pointer_update) {
SDL_SendMouseMotion(data->window, 0, 0, GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam));
if (isWin10FCUorNewer && mouse->relative_mode_warp &&
(data->window->flags & SDL_WINDOW_INPUT_FOCUS) != 0) {
/* To work around #3931, Win10 bug introduced in Fall Creators Update, where
SetCursorPos() (SDL_WarpMouseInWindow()) doesn't reliably generate mouse events anymore,
after each windows mouse event generate a fake event for the middle of the window
if relative_mode_warp is used */
int center_x = 0, center_y = 0;
SDL_GetWindowSize(data->window, &center_x, &center_y);
center_x /= 2;
center_y /= 2;
SDL_SendMouseMotion(data->window, 0, 0, center_x, center_y);
}
}
SDL_MOUSE_EVENT_SOURCE source = GetMouseMessageSource();
if (source == SDL_MOUSE_EVENT_SOURCE_UNKNOWN ||
source == SDL_MOUSE_EVENT_SOURCE_TOUCH) {
WIN_HandleMouseMove(data, source, lParam);
} else {
/* We still need to update focus */
SDL_SetMouseFocus(data->window);
/* Don't fully handle real mouse moves immediately - check WIN_PumpEvents for more info. */
data->mousemove_source = source;
data->mousemove_position = lParam;
}
}
/* don't break here, fall through to check the wParam like the button presses */
Expand Down Expand Up @@ -1533,16 +1542,68 @@ WIN_SendWakeupEvent(_THIS, SDL_Window *window)
PostMessage(data->hwnd, data->videodata->_SDL_WAKEUP, 0, 0);
}

static void
WIN_HandlePendingMouseMoves(_THIS)
{
for (SDL_Window *window = _this->windows; window != NULL; window = window->next) {
SDL_WindowData *windowdata = window->driverdata;
if (windowdata != NULL && windowdata->mousemove_source != SDL_MOUSE_EVENT_SOURCE_UNKNOWN) {
WIN_HandleMouseMove(windowdata, windowdata->mousemove_source, windowdata->mousemove_position);
windowdata->mousemove_source = SDL_MOUSE_EVENT_SOURCE_UNKNOWN;
}
}
}

void
WIN_PumpEvents(_THIS)
{
const Uint8 *keystate;
MSG msg;
DWORD start_ticks = GetTickCount();
int new_messages = 0;
int mousemoves = 0;
SDL_VideoData* data = _this->driverdata;

if (g_WindowsEnableMessageLoop) {
/* There are as many WM_MOUSEMOVEs as we are asking for.
Luckily there is no such thing as a "dropped" WM_MOUSEMOVE as they all get combined.
For more info, check https://devblogs.microsoft.com/oldnewthing/20031001-00/?p=42343
Sadly non-removed WM_MOUSEMOVEs will fill the message queue, thus remove all queued ones.

Also worth noting: PeekMessage-ing an autogenerated mouse move will also make it directly
call WIN_WindowProc with WM_NCHITTEST and WM_SETCURSOR as if those were *sent* (not posted) messages.

One huge problem is that SDL_PollEvent can call SDL_PumpEvents, which then goes *through the entire msg loop*!
High frequency mice can keep generating msgs and thus overwhelm poll loops easily on some PCs, especially with the extra
handling above happening on *every poll*! (For reference, a 1kHz mouse can already tank an i7-7700K on bad days.)
We thus must make sure to not overwhelm poll loops and cut off the stream of mouse events,
instead of waiting for the user to decide where to put their mouse before leaving the event loop.

Having "start_ticks" match the loop start would also solve that, but this must keep existing poll loop setups as-is. */
const int MAX_MOUSEMOVES_PER_PUMP = 2;
const int MAX_MOUSEMOVE_PUMPS = 3;

/* Check for mouse moves left over from the last pump.
If there are any left, handle them immediately before all new msgs in the queue to avoid ending up in
an awkward spot without any new mouse moves and handling the previous pump's moves way too late. */
WIN_HandlePendingMouseMoves(_this);

while (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) {
if (msg.message == WM_MOUSEMOVE &&
GetMouseMessageSource() != SDL_MOUSE_EVENT_SOURCE_TOUCH &&
SDL_TICKS_PASSED(msg.time, start_ticks)) {
/* We should only care about the last real mouse movement, but need to
go through everything that was added to the queue before we started this loop.
Treat anything added afterwards as auto-generated, indicating the end of the normal queue.
Anything sent or posted by an overlay should've been handled by now.
Let's make sure to hit N consecutive mouse moves before dropping out too soon though... */
++mousemoves;
} else {
/* Touch mouse movement or anything that was already on the queue - let's be safe here. */
data->mousemove_only_pumps = 0;
mousemoves = 0;
}

if (g_WindowsMessageHook) {
g_WindowsMessageHook(g_WindowsMessageHookData, msg.hwnd, msg.message, msg.wParam, msg.lParam);
}
Expand All @@ -1563,6 +1624,32 @@ WIN_PumpEvents(_THIS)
break;
}
}

/* Auto-generated mouse moves at the end of the queue are nasty and make Windows send
additional events as part of PeekMessage. Let's avoid getting too many of those. */
if (mousemoves >= MAX_MOUSEMOVES_PER_PUMP) {
break;
}
}

/* Handle the last WM_MOUSEMOVE, but only N consecutive mousemove-only pumps. */
if (mousemoves > 0) {
++data->mousemove_only_pumps;
/* Feel free to turn up your mouse poll rate, change this to #if 0 and attach Spy++
(or another slow msg handling hook) to see why this is important. */
#if 1
if (data->mousemove_only_pumps >= MAX_MOUSEMOVE_PUMPS) {
/* Process this mouse move next pump to give event loops an opportunity to leave. */
data->mousemove_only_pumps = 0;
} else
#endif
{
/* Finally handle the mouse move(s) ourselves, making it visible to our own event handling.
Auto-generated mouse move messages appear at the end of the message queue anyway,
which means that this shouldn't be too "out of order" after all, unless something decided to
put fake mouse moves into the message queue. */
WIN_HandlePendingMouseMoves(_this);
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/video/windows/SDL_windowsvideo.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@ typedef struct SDL_VideoData
TSFSink *ime_uielemsink;
TSFSink *ime_ippasink;

int mousemove_only_pumps;
MSG mousemove_last;

BYTE pre_hook_key_state[256];
UINT _SDL_WAKEUP;
} SDL_VideoData;
Expand Down
10 changes: 10 additions & 0 deletions src/video/windows/SDL_windowswindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@
#include "../SDL_egl_c.h"
#endif

typedef enum
{
SDL_MOUSE_EVENT_SOURCE_UNKNOWN,
SDL_MOUSE_EVENT_SOURCE_MOUSE,
SDL_MOUSE_EVENT_SOURCE_TOUCH,
SDL_MOUSE_EVENT_SOURCE_PEN,
} SDL_MOUSE_EVENT_SOURCE;

Comment on lines +30 to +37
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure if moving the enum here is the right thing to do, given how it's effectively only returned by GetMouseMessageSource in SDL_windowsevents.c. Also, is it safe to assume that driver data layout shouldn't be relied on by applications?

typedef struct
{
SDL_Window *window;
Expand All @@ -53,6 +61,8 @@ typedef struct
SDL_bool in_window_deactivation;
RECT cursor_clipped_rect;
SDL_Point last_raw_mouse_position;
SDL_MOUSE_EVENT_SOURCE mousemove_source;
LPARAM mousemove_position;
struct SDL_VideoData *videodata;
#if SDL_VIDEO_OPENGL_EGL
EGLSurface egl_surface;
Expand Down