floooh / sokol

minimal cross-platform standalone C headers
https://floooh.github.io/sokol-html5
zlib License
6.83k stars 480 forks source link

Win32 Raw mouse input seems wrong #806

Closed dtrebilco closed 1 year ago

dtrebilco commented 1 year ago

I have not tested this - this is just me doing a code review - but it seems that the raw mouse input in win32 seems off

https://github.com/floooh/sokol/blob/3ebd63e8d13e3d5d4809b5f3b91d31933ba37d5e/sokol_app.h#L7230

The variable raw_input_mousepos_valid is only ever set to true when gated behind an if check of itself?

I suspect the code should do:

  LONG new_x = raw_mouse_data->data.mouse.lLastX;
  LONG new_y = raw_mouse_data->data.mouse.lLastY;

  if (_sapp.win32.raw_input_mousepos_valid) {
      _sapp.mouse.dx = (float) (new_x - _sapp.win32.raw_input_mousepos_x);
      _sapp.mouse.dy = (float) (new_y - _sapp.win32.raw_input_mousepos_y);
   }  // <----- End brace moved here
  _sapp.win32.raw_input_mousepos_x = new_x;
  _sapp.win32.raw_input_mousepos_y = new_y;
  _sapp.win32.raw_input_mousepos_valid = true;

Not really a bug, but is there a reason when calling GetRawInputData() you pass in a huge buffer and then cast it? Rather than simply a local RAWINPUT that has the address taken of. The doc linked in the comment https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getrawinputdata does state:

GetRawInputData gets the raw input one RAWINPUT structure at a time. In contrast, GetRawInputBuffer gets an array of RAWINPUT structures.

Perhaps at one point you were calling GetRawInputBuffer()? I am not an expert in this area - just something I noticed.

floooh commented 1 year ago

Right, that looks completely wrong. Thanks for catching that. From the comment there I guess that I was never able to properly test the code block inside the MOUSE_MOVE_ABSOLUTE block, because I didn't get back such messages on my machine. I'll try to look into it tomorrow.

floooh commented 1 year ago

PS: The GetRawInput() call is actually as intended. The array that's passed in is a byte array, not an array of RAWINPUT structs. It's not very clear from the documentation (https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getrawinputdata) what data the function actually returns, one is supposed to first query the number of required bytes (e.g. GLFW calls GetRawInput() twice, first with a null pointer which returns the number of bytes required, then it on-demand allocates this number of bytes and calls GetRawInput() again with the allocated pointer. I just changed this procedure by passing in a pointer to buffer that should be big enough in the first place.

I'll fix the buggy part in the MOUSE_MOVE_ABSOLUTE block though. Problem is that I cannot actually debug/test this because I never get such absolute position data back.

PS: this GLFW issue is enlightening: https://github.com/glfw/glfw/issues/1276

...along with this DirectXTK patch: https://github.com/microsoft/DirectXTK/commit/ef56b63f3739381e451f7a5a5bd2c9779d2a7555

...looks like absolute mouse positions are sent in a Remote Desktop session... I'll put those link into code comments in that code block, but will only fix the 'obvious' problem that you found so far (but this won't be enough to make raw mouse input in Remote Desktop sessions work).

dtrebilco commented 1 year ago

You are correct, GetRawInputData() does only return one struct -but the struct is not a fixed size one.