[ros-dev] [BUG] Utterly incorrect implementation of input device coordinates.

James Tabor jimtabor.rosdev at gmail.com
Mon Jul 27 02:28:42 CEST 2009

please, when you have time to send a patch to fix this wine code
import will be greatly appreciated!

On Thu, Jul 23, 2009 at 2:54 PM, <codeforfood at pulseforce.com> wrote:
> Here's the information required to patch the incorrect implementation.
> Abstract:
> - SendInput()/NtUserSendInput() broken:
> --- Relative hardware mouse input "working", but not well implemented due
> to lack of modifying the travel distance using cursor acceleration and
> movement speeds (I've provided info on this as well below), input-virtual
> vs screen-real coordinates (even in the relative moves), etc. The two
> coordinate spaces are not the same thing!
> --- Absolute hardware mouse input broken (used by things like
> touchscreens), by a complete misunderstanding of how the virtual input
> device coordinate space works (it's NOT in ANY way a 1:1 map that's
> translatable to the screen, but the function uses it this way everywhere!)
> read below for full info since this is the serious problem.
> - SetCursorPos() broken (should NOT be using SendInput() since that's
> pointless and ineffective and is meant for the hardware input queue, we
> already have the desired coordinates and should move the cursor directly;
> and even its use of SendInput() is incorrect since it should FIRST convert
> the desired SCREEN coordinates to absolute-INPUT-coordinate-space), BUT
> since it's using the broken SendInput() (which currently doesn't know that
> absolute input coordinates have NOTHING to do with screen coordinates) it
> works correctly and will place cursors at the right location. This will
> have to be changed when SendInput() is patched; preferrably to a direct
> cursor update (elaborated in the recap below, before the log), rather than
> the current indirect/slow approach which places a message in the serial
> (as in serial-processing, first-come-first-serve) mouse-input-translation
> queue.
> So to recap:
> - Need to improve relative movement (to handle acceleration and mouse
> speed modifiers)
> - Need to change the code everywhere where you've been thinking that input
> coordinates are the same as screen coordinates (most likely only within
> SetCursorPos(), and SendInput()/NtUserSendInput())
> - Absolute cursor placement inside SendInput() needs to be rewritten to
> convert absolute input-space coordinates to absolute screen-space
> coordinates (quite an easy change) using the algorithm I provide below,
> which is what Windows uses internally.
> - SetCursorPos() needs to COMPLETELY stop using SendInput() and instead
> manipulate the cursor X/Y struct directly, or equivalent methods, instead
> of injecting itself into the input queue. Alternatively, if it keeps using
> the input queue, it needs a reverse-algorithm to turn the desired
> screen-coordinates into virtual inputspace-coordinates, such as int
> mouseinput_abs_x_or_y_pos = (((65536 * screen_x_or_y_pos) /
> screen_width_or_height) + 1). The +1 is necessary for proper rounding.
> However, I see no reason to use the input queue within this function as
> that involves much more work than simply writing the cursor's location
> directly (which is what SendInput EVENTUALLY gets around to doing after
> LOTS of processing, and saving on cycles/jumps/calls is GOOD ;)).
> All details on how to fix these things are in the below log. Let me know
> if there's any questions.
> Log (most unrelated discussions/events snipped and long breaks between
> explaining different details have been separated with "..."):
>  19:56   Yewbacca        . I am sure you've implemented SetCursorPos()
> incorrectly. Checking your source code for it at cursor.c:280. This
> function is SUPPOSED to take a SCREEN X/Y coordinate (ie X=1400, Y=800 on
> a 1440x900 screen, and then moves the cursor location directly, without
> emulating mouse movement.
>  19:57   Yewbacca        . Instead, your version calls SendInput() with
> the x/y unmodified, that's awful. SendInput()'s absolute space is a
> virtual desktop square that stretches from 0-65535 in each direction and
> will not even be remotely close to what the intended destination is.
>  19:57   _0_             . sounds like a patch is in order Yewbacca
>  19:57   _0_             . :P
>  19:58  +encoded         . Yewbacca, file bug report!
>  19:58   Yewbacca        . _0_ Well I'd have to know how you're writing
> the cursor position inside NtSendUserInput etc.
>  19:58   Yewbacca        . Or file a bug report, I'll do that.
>  20:01   Yewbacca        . Related to this, Windows actually uses a
> slightly unexpected algorithm for its SendInput() absolute-mode
> coordinates-to-screen conversion.
>  20:01   Yewbacca        . int destx_or_y = (screen_width_or_height *
> x_or_y_abs_coord) / 65536
>  20:01   Yewbacca        . If you want pixel precise SendInput() that
> works as on windows you'd have to verify this. I'll bug report that too.
>  20:02  +encoded         . <3 Yewbacca
>  20:02   Yewbacca        . :)
>  20:02  +encoded         . you can ofcourse see the source code of
> NtSendUserInput
>  20:03   Yewbacca        . Yeah I was about to check that to make sure
> it's not already correct
>  20:04  +encoded         . its in \subsystems\win32\win32k\ntuser\input.c
>  20:04   Yewbacca        . Ah thanks, that saves a slow grep.
>  20:05  +encoded         . wait opps
>  20:05                   . garrythefish_
> [n=fisher at unaffiliated/garrythefish] has joined #reactos
>  20:05  +encoded         . oh well, nvm me
>  20:05   garrythefish_   . community choice award today. hope you guys
> win. :D
>  20:10   Yewbacca        . Okay after brief confusion I noticed that
> IntMouseInput() contains the actual logic for mouse-type input to
> SendInput(), input.c:1081. Time to check it.
> ...
>  20:14   Yewbacca        . Okay now I'm really confused. I read through
> IntMouseInput() from input.c:1081-1333 and unless I missed a call to a
> driver-coordinates-to-screen-coordinates somewhere, this function is
> miscoded as well. That'd mean that SetCursorPos() would work correctly
> since SendInput() would work incorrectly, and that normal SendInput()
> calls that expect a 0 to 65535 range would not work at all.
>  20:15   Yewbacca        . It really seems to directly write the pointers
> for cursor X and Y with the values it has received
>  20:15   Yewbacca        . with no conversion whatsoever
>  20:15   Yewbacca        . even though the incoming values are in a 0 to
> 65535 range
>  20:15   Yewbacca        . of that virtual space
> ...
>  20:16   Yewbacca        . Basically for anyone unfamiliar with it, the
> driver implements a virtual space for the cursor to either move
> relatively within, or absolutely, to allow things like touchscreens to
> function easily (they'd just have to SendInput() their current position
> as a percentage of the 65535 range).
>  20:16   Yewbacca        . So all incoming absolute-mode coordinates must
> be converted to screen coordinates, and microsoft does it with int
> destx_or_y = (screen_width_or_height * x_or_y_abs_coord) / 65536
>  20:17   Yewbacca        . I'm going to re-read it again to see if I
> missed some conversion call. If not, that'll be 2 reports. Quite easy
> fixes luckily.
>  20:18  +encoded         . <3 Yewbacca
>  20:18   Yewbacca        . I think the reason you got by this far is
> because the HARDWARE mouse sends coordinates relatively
>  20:18   Yewbacca        . The problem'd only appear with absolute input
> like touchscreens. And since SendInput()'s absolute mode is broken that
> means program's SetCursorPos() would work, so it's doubly broken and
> would only cause problems for absolute hardware-input (again,
> touchscreens mainly).
>  20:18   Yewbacca        . :P
>  20:19   Yewbacca        . Reading again
>  20:20  +encoded         . Yewbacca, we can use someone like you who has
> knowledge of windows internals
>  20:20   Yewbacca        . Maybe, right now I'm tied up with some major
> work :)
> ...
>  20:28   Yewbacca        . Alright I've finished checking everything.
> IntMouseInput() stretches from input.c:1081-1333 and is the function
> that's called when NtUserSendInput()/SendInput() receives a mouse-type
> event. At line 1140 it grabs the pointer to the cursor's X/Y struct with
> IntGetCursorLocation(WinSta, &MousePos). Then we get to the significant
> errors.
>  20:29   Yewbacca        . At lines 1146-1150 it incorrectly writes the
> virtual x/y of the INPUT coordinate system right into the memory holding
> the SCREEN x/y cursor location
>  20:30   Yewbacca        . At lines 1151-1155 it handles relative movement
> but doesn't take into account cursor acceleration and movement speed
> (those settings we all love in the control panel->mouse), which in turn
> can increase the movement distance as much as 4 times the input value.
>  20:31   Yewbacca        . Those two behaviours right there would need to
> be patched. The first should determine the destination on the screen
> through int destx_or_y = (screen_width_or_height * x_or_y_abs_coord) /
> 65536. The latter should implement handling for cursor acceleration
> (check MSDN, it describes acceleration on SendInput()).
>  20:32   Yewbacca        . And lastly, SetCursorPos() (forgot which file
> it's in) needs to *completely* skip the SendInput() baloney that it's
> doing right now (http://pastebin.com/d2db76985) and just directly grab
> the cursor X/Y pointer and update it in a simple assignment, AFTER
> checking that it's in range of course (otherwise clamping it to the
> screen).
>  20:33  +Lone_Rifle      . Yewbacca, suggest that you subscribe to ros-dev
> and retype or copy-paste your findings there, it's quite a bit of text to
> go through in one sitting
>  20:33   Yewbacca        . Oh yeah and if you wanna be 100% complete, also
> implement a check for ClipCursor() and don't allow the cursor to move
> outside that, no matter if it's from hardware input or SetCursorPos(). If
> it's outside, it should be clamped to the nearest inside value.
>  20:34   Yewbacca        . Lone_Rifle Yeah, I agree and was going to
> submit it. I'll do that now.
> ...
>  20:37   Yewbacca        . Acceleration is described at
> http://msdn.microsoft.com/en-us/library/ms646273(VS.85).aspx (info for
> the MOUSEINPUT struct). I'll submit this log to the mailing list.
> =EOF=
> _______________________________________________
> Ros-dev mailing list
> Ros-dev at reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev

More information about the Ros-dev mailing list