So, 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, <[email protected]> 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=fis...@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 > [email protected] > http://www.reactos.org/mailman/listinfo/ros-dev > _______________________________________________ Ros-dev mailing list [email protected] http://www.reactos.org/mailman/listinfo/ros-dev
