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

moveMouse() is broken on Windows 10 #77

Closed
maxdeepfield opened this issue Aug 16, 2015 · 33 comments
Closed

moveMouse() is broken on Windows 10 #77

maxdeepfield opened this issue Aug 16, 2015 · 33 comments
Assignees
Labels
Milestone

Comments

@maxdeepfield
Copy link

robot.moveMouse(10, 10);
console.log(robot.getMousePos());

This code must return { x: 10, y: 10 }
It returns { x: 9, y: 9 }

Not always pos-1, for example if moveMouse(100,100) then getMousePos gives back 98,98

Windows 10 Build 10240 x64
NodeJS 0.12.7 x64
RobotJS 0.2.3 installed via npm

@octalmage
Copy link
Owner

Hi Max!

Could you try putting a sleep between the two? I believe the mouse event is just taking longer on your computer. The tests confirm that getting and setting like that should work as expected.

@maxdeepfield
Copy link
Author

\node_modules\robotjs\test>node mouse.js
TAP version 13
Get the initial mouse position.
ok 1 successfully retrieved mouse position.
ok 2 mousepos.x is a valid value.
ok 3 mousepos.y is a valid value.
Move the mouse.
ok 4 successfully moved the mouse.
not ok 5 mousepos.x is correct.


operator: ok
expected: true
actual:   false

...
not ok 6 mousepos.y is correct.


operator: ok
expected: true
actual:   false

...

1..6
tests 6
pass 4
fail 2

@octalmage
Copy link
Owner

Yeah those tests failing mean it's the mouse delay. :/

You could use RobotJS from GitHub which includes a setMouseDelay function. The default delay is currently 10ms and I knew this was going to be an issue.

@maxdeepfield
Copy link
Author

robot.moveMouse(10, 10);
setTimeout(function(){
    console.log(robot.getMousePos());
},1000);
// { x: 9, y: 9 }

It seems like mouse always try to reach 0,0 coordinates.

@octalmage
Copy link
Owner

Hmm that's super odd! Thanks for trying that.

@Deltatiger could you try the above code when you get a chance? It works in my VM, I wonder if it's a Windows 10 issue.

@maxdeepfield
Copy link
Author

I compiled robotjs from github master sources using node-gyp.
Sad, but it works same. :(

@maxdeepfield
Copy link
Author

I also will try this later on other Windows.

@maxdeepfield
Copy link
Author

Interesting: moveMouse(1,1) moves mouse precisely on very left top corner (0,0)
I think this is not getMousePos problem, but moveMouse issue...

@maxdeepfield
Copy link
Author

var pos = robot.getMousePos();
robot.moveMouse(pos.x, pos.y);

This should not do anything, but it moves cursor a bit to left and top according to current position (depends of current position distance from 0,0, very far - very large step)

@maxdeepfield maxdeepfield changed the title getMousePos() is broken moveMouse() is broken on Windows 10 Aug 16, 2015
@maxdeepfield
Copy link
Author

SetCursorPos(point.x, point.y); instead of mouse_event in mouse.c works very nice.

@octalmage
Copy link
Owner

Thanks that's good to know! I believe mouse _event is being deprecated anyway. I'll use this as an opportunity to test different mouse setting functions ( I believe there's 4 ways on Windows). If I remember correctly, SendInput works really well.

@maxdeepfield
Copy link
Author

I tested SendInput and it worked like mouse_event - weird 65535 divisions seems to be not so precise. Maybe this depends on some mouse or screen configuration.

@maxdeepfield
Copy link
Author

I found solution: #78
This will work for SendInput too.

@Deltatiger
Copy link
Collaborator

@maxdeepfield I think SendInput should be used in windows. It supports multiple input devices and is recommended by MS.
@octalmage I will test things out again in Windows 7 and 10. Just to check if things work out. Also will set up a branch on my repo to convert stuff to SendInput(Mouse and Keyboard). If that works out then it should be better.

@maxdeepfield
Copy link
Author

@Deltatiger agree, but SendInput also must use 65536.0f instead of 65536 or 0xFFFF

@maxdeepfield
Copy link
Author

Strange, but this 65536.0f correction doesnot work very fine using over remote desktop. Its is now going closer, but not so precise as SetCursorPos

@octalmage
Copy link
Owner

I was afraid of that, since the current code works for some users.

@dkrutsko
Copy link

The mouse code has some issues, first of all why is mouse_event being used instead of SendInput? According to MSDN mouse_event has been deprecated for quite some time. As for mouse movement why not use SetCursorPos here?

@maxdeepfield
Copy link
Author

As mentioned in discussion above - SendInput should be used, but this does not help in some cases, SetCursorPos working best for me.

@Deltatiger
Copy link
Collaborator

@maxdeepfield I tried tweaking stuff a bit. Try this out.

point.x = ceil(point.x * 65536.0 / GetSystemMetrics(SM_CXSCREEN));
point.y = ceil(point.y * 65536.0 / GetSystemMetrics(SM_CYSCREEN));

I still am using the mouse_event but this gives me close to accurate readings.

@Deltatiger
Copy link
Collaborator

@octalmage @maxdeepfield So the issue was mouse_event was dealing with the screen by normalizing it between values 0 to 65536. Very inaccurate if you ask me. The rounding issue fixes it a bit. But to completely fix it we would have to migrate to SendInput. That deals with pixels directly.

@DomiStyle
Copy link

Has there been any update to this?
I had issues with my mouse drifting when using moveMouse on Windows 10.

var mouse = robot.getMousePos();
robot.moveMouse(mouse.x, mouse.y);

This will make your mouse drift to the top left corner when running it in a loop.

@Deltatiger
Copy link
Collaborator

@DomiStyle This is an issue with Windows API that is being used currently. The problem has already been identified. Will send a PR soon so that it can be fixed.

@octalmage octalmage modified the milestone: First Stable Release. Sep 22, 2015
@octalmage octalmage added the bug label Sep 22, 2015
@octalmage
Copy link
Owner

Should we maybe use SetCursorPos for now? I would like to get #105 finished and this is blocking it.

@octalmage
Copy link
Owner

Actually I'm going to go ahead and merge the appveyor branch. This is broken so the tests should fail.

@octalmage
Copy link
Owner

Thanks to AutoHotkey, I got it! Looks like they went through the same thing we did:

https://github.com/Lexikos/AutoHotkey_L/blob/a0bc036d4f09825b3478f14ffcaca11c782782e0/source/keyboard_mouse.cpp#L2447

Man I LOVE open source.

Anyway, this needs more testing but I'm going to merge it soon.

@octalmage
Copy link
Owner

We can switch to SendInput later, it should be easy now that we have the conversion down. I'll open a new issue for this.

@octalmage
Copy link
Owner

@maxdeepfield could you confirm if the code in the master branch resolves this for you? In my tests moveMouse was accurate.

@DomiStyle
Copy link

@octalmage Works fine for me now.

@octalmage
Copy link
Owner

Awesome thanks @DomiStyle! I'll release a new version later today.

@octalmage
Copy link
Owner

v0.3.2 published to npm which fixes this issue.

@ChrisAntaki
Copy link

Thanks for fixing this @octalmage! 👍

@octalmage
Copy link
Owner

Of course! That's why I'm here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants