kwhat / libuiohook

A multi-platform C library to provide global keyboard and mouse hooks from userland.
Other
505 stars 124 forks source link

Bidirectional mouse overflow issue on Windows #9

Closed munsuri closed 9 years ago

munsuri commented 10 years ago

Following the negative coordinates, the issue also occurs into the other screen direction, reporting values higher than the maximum resolution itself. For now I'm dealing with this on the application side as I wanted to avoid having static values for the resolution and/or retrieving the resolution for every event (at least not till I've done some profiling). So I thought I would let you know anyways :)

kwhat commented 10 years ago

Ohh no; that is kind of what I was afraid of. This is going to get interesting with multiple displays, especially when those displays are of mixed size. Lets start with implementing the properties methods under Windows and do a little testing. Lets create a branch with these fixes in it so I don't have to merge with the trunk so I can push back changes. It wont be the end of the world if we need to check the resolution in callback. If there is a need I will find a way to optimize it later. Thanks for the update.

munsuri commented 10 years ago

For now I'm trying to not think about the multiple displays haha Will you create such a branch or should I create the branch on my fork?

kwhat commented 10 years ago

create a branch of the master you currently have and I'll merge it back and make changes. You can call it "munsuri" or whatever else you prefer. Then hopefully we can collaborate on that branch without breaking our trunks ;)

munsuri commented 10 years ago

Alrighty, branch "munsuri_kwhat" created. I also added you to the fork, so it should be ready to go :)

kwhat commented 10 years ago

Thanks, I have pulled to code over and started testing. At this time I have only covered windows and this is going to be very, very difficult when using multiple monitors.

First, the screen edge 0,0 is only for the primary display. If you have a second monitor to the left or above your primary display, they will be in the negative coordinate range O_o

Second, Windows does not provide a way to get the total screen resolution that I know of. I have tested SM_C[X|Y]VIRTUALSCREEN and it almost works except when you have mixed resolutions. Then it does something, but I don't seem to understand what that something is nor was that something important enough to document at MSDN.

Third, I am unaware of anyway to iterate thought the monitor count on Windows to manually calculate what I expected SM_C[X|Y]VIRTUALSCREEN to actually return.

The best half-baked solution I can come up with would be to use something like MonitorFromPoint on each callback to get the current monitor we are on and use GetMonitorInfo to get its size. Then we would need to come up with some way to calculate where that monitor is in relation to the primary screen. After we have all of that information we can translate the coordinates to something a little more uniform. So we need to figure out how to calculate the monitor layout on windows.

All of this and we haven't even gotten to OS X and Linux... Welcome to cross-platform insanity.

munsuri commented 10 years ago

Haha that sounds pretty interesting. The resolution and screen distribution on Windows seems to really follow the cartesian system in more than one way :)

To iterate through the monitors on Windows and calculate the total screen resolution this snippet can be used:

#include <windows.h>
#include <stdio.h>

//http://msdn.microsoft.com/en-us/library/windows/desktop/dd162610(v=vs.85).aspx
//http://msdn.microsoft.com/en-us/library/dd162610%28VS.85%29.aspx
//http://msdn.microsoft.com/en-us/library/dd145061%28VS.85%29.aspx
//http://msdn.microsoft.com/en-us/library/dd144901(v=vs.85).aspx
// callback function called by EnumDisplayMonitors for each enabled monitor
BOOL CALLBACK MyMonitorEnumProc(HMONITOR hMonitor, HDC dcMonitor, RECT* pRECTMonitor, LPARAM lParam){
    printf( "MyMonitorEnumProc!\n");
    //Screen counter, will be passed to the next calls
    lParam++;

    //http://msdn.microsoft.com/en-us/library/windows/desktop/dd145065(v=vs.85).aspx
    MONITORINFO  info;
    info.cbSize =  sizeof(MONITORINFO);
    BOOL monitorInfo = GetMonitorInfo( hMonitor, &info );
    if( monitorInfo ) {
        //TODO: dwFlags - MONITORINFOF_PRIMARY. We can also know it due to origin must be 0,0
        printf( "From Monitor info: Resolution: %ld x %ld @ Origin: %ld,%ld\n", abs(info.rcMonitor.left - info.rcMonitor.right),
                abs(info.rcMonitor.top - info.rcMonitor.bottom), info.rcMonitor.left, info.rcMonitor.top );
    }
    //Or we can use the info from pRECTMonitor
    printf( "Monitor %p(%d)(%p): Resolution: %ld x %ld @ Origin: %ld,%ld\n", hMonitor, lParam, dcMonitor, pRECTMonitor->right - pRECTMonitor->left,
            pRECTMonitor->bottom - pRECTMonitor->top, pRECTMonitor->left, pRECTMonitor->top);
    return TRUE;
}

int main(){
    printf( "EnumDisplayMonitors\n");
    //TODO: check those invisible monitor the msdn talks about...
    BOOL b = EnumDisplayMonitors(NULL, NULL, MyMonitorEnumProc, 0);
    printf( "End EnumDisplayMonitors\n");
    getchar(); //system("PAUSE");
    return 0;
}

Now I'm thinking about what would be the best/natural approach to represent multiple monitors.

In case of following Windows's approach, the relation to the primary or other screens is immediate. However, right now the library itself assumes positive coordinates only (and so other systems might do) For the mixed resolutions I don't really see a big issue and in case it becomes one, we can always normalize the coordinates. I'm gonna check what approach Mac uses for representing and dealing with multiple monitors, so maybe there is more approaches we can use :)

kwhat commented 10 years ago

I will check X11 for the same thing. That should give us a better idea about what the best direction is.

kwhat commented 10 years ago

It looks like X11 operates with 0,0 at the left most display regardless of it it is the primary display. So it comes down to how OS X does it.

munsuri commented 10 years ago

Mac in the other hand follows Windows approach. Negative coordinates when the second monitor is to the left or top of the main monitor. To the point that if the second monitor happen to be of higher resolution comparing to the main one, even if the second monitor is to the right, while aligned from bottom, will also have negative coordinates when moving the pointer above main top reference. Crazy :)

Now I can see why SM_C[X|Y]VIRTUALSCREEN Windows calls wouldn't work for mixed resolutions. It can happen that the whole "virtual" screen is not a just rectangle but a compound of them that doesn't necessarily form a rectangle. This "compound" with possibly irregular shape really make things complicated haha

kwhat commented 10 years ago

I think the best approach will be to fix the x11 coordinates as they are received. I did some research on a couple of options I am going to plugin tomorrow and see how they perform.

kwhat commented 10 years ago

After a lot of work and what turned out to be a time-consuming dead end, this is probably going to just result in the switch from a uint16_t to a int16_t without the coordinate translation for X11. Its not possible, and if it was, it would make the new coordinates useless for all other native functions of the platforms. Windows and OS X should get signed coordinates and X11 will just be unsigned with a different root monitor.

munsuri commented 10 years ago

That sounds good! But I'm not sure I completely understand this part:

Windows and OS X should get signed coordinates and X11 will just be unsigned with a different root monitor.

  • Does it mean that no coordinate conversion is gonna take place? I mean, I shouldn't post negative coordinates (when running on X11) even through the library abstraction, should I?
  • And what about the monitor/screen information/identifier? Will that be part of the coordinate data?
kwhat commented 10 years ago

Yah that did mean no coordinate conversion :/

I am not particularly happy with my results from this weekend. I would prefer that mouse coordinates be uniform across all platforms so you could use a dual head setup on Windows and send the same coordinates to a dual head setup using X11 over the network and get the results you expect. I was able to do some monitor detection with xinerama but I can't find a way to get the default monitor nor the current monitor that contains the mouse. There may still be some hope for XrandR but that API documentation leaves me with more questions than answers right now.

Really what we need is a way to list all monitors (completed with xinerama and probably possible with XrandR) and how to determine which of these monitors is the "default" display. I will push the test code I was hacking on this weekend to the munsuri branch in an hour in case you would like to do some hacking of your own ;) It would be really helpful if you can figure out how to use XrandR to at least duplicate what I have for xinerama. That will probably require a small patch to the configure script which you can basically cookie cutter from the diff in the munsuri branch. The pgk-config name will be [XRANDR], [xrandr] in place of [XINERAMA], [xinerama].

I am usually on irc (freenode) in the #JNativeHook channel. If you have some questions about the implementation, just stop by and ask. That way we can have some more real-time communication ;)

kwhat commented 10 years ago

Small update from XrandR:

 --primary
          Set the output as primary.  It will be sorted first in Xinerama and RANDR geometry requests.
kwhat commented 10 years ago

Well the good news is that the primary display is indeed always the first one so in theory we can calculate what we need.

xrandr --output DisplayPort-0 --primary

Xinerama Information:
    ***1 org X,Y:   0, 0
    ***1 width, height:     1680, 1050

    ***0 org X,Y:   1680, 0
    ***0 width, height:     1050, 1680

xrandr --output DVI-0 --primary

Xinerama Information:
    ***1 org X,Y:   1680, 0
    ***1 width, height:     1050, 1680

    ***0 org X,Y:   0, 0
    ***0 width, height:     1680, 1050
munsuri commented 10 years ago

Ok, no problem. We'll find the way to make them uniform across all platforms. At least for the same monitors setup. In fact, what you comment is what I'm already doing and what I'm planning to do too :)

Unfortunately I only have virtual linux's, so not sure if I'll be able to make real tests on those. However, I'm already able to list all monitors as well as identifying the default one. From the snippet on system_properties, there is interesting information on Display structure we can use:

typedef struct _XDisplay {
    ...
    char *display_name; /* "host:display" string used on this connect*/
    int default_screen; /* default screen for operations */
    int nscreens;       /* number of screens on this server*/
    Screen *screens;    /* pointer to list of screens */
    ...
} Display;
typedef struct {
    XExtData *ext_data; /* hook for extension to hang data */
    struct _XDisplay *display;/* back pointer to display structure */
    Window root;        /* Root window id. */
    int width, height;  /* width and height of screen */
    int mwidth, mheight;    /* width and height of  in millimeters */
    int ndepths;        /* number of depths possible */
    Depth *depths;      /* list of allowable depths on the screen */
    int root_depth;     /* bits per pixel */
    Visual *root_visual;    /* root visual */
    GC default_gc;      /* GC for the root root visual */
    ...
} Screen;

As well as function such as:

Screen *DefaultScreenOfDisplay(Display *display);
int DefaultScreen(Display *display);
int ScreenCount(Display *display);
Screen *ScreenOfDisplay(Display *display, int screen_number);
char *DisplayString(Display *display);

I'll have a look at XrandR extension (I've checked something already) in case that is not enough or doesn't work.

Oh, I'll definitely visit you on freenode then :)

kwhat commented 10 years ago

Just an FYI, don't get sucked in to the XScreen's "solution." If you have 2 monitors, you still only have 1 XScreen and 1 Default root window! I spent the better half of yesterday trudging though that. You must use either Xinerama or XrandR. That XrandR code will be very useful. The next hardest part will be coming up with an algorithm to produce the display offset. We will need to iterate all monitors and probably store 2 signed values for the difference in X,Y. This will be the inverse maximum value for monitor(s) above and to the left of the primary.

P.S. You can simulate multiple displays with VirutalBox but the usability is pretty poor.

kwhat commented 10 years ago

Alright the solution was ridiculously simple and I was clearly over thinking it. I have committed the solution for xinerama on my copy of the branch. I just need a copy of the same thing with the xrandr code you have and I need to tweak the build system.

munsuri commented 10 years ago

Just got the xrandr extension code working to list the monitors with its resolution, coordinates origin and identifier. I'll clean the code and commit the test file so you can try it out in a real scenario :) P.S. I find VirtualBox multiple monitor awesome! haha It helped to test the xrandr output better.

kwhat commented 10 years ago

Awesome! Let me know where I can merge it from.

munsuri commented 10 years ago

It is on my branch: https://github.com/munsuri/libuiohook/blob/munsuri_kwhat/test/testXrandr.c Actually I also posted it on the freenode chatroom :)

kwhat commented 10 years ago

Ok, XRandR performance is horrendous. It looks like XRRGetScreenResources is the bottle neck here. Possible solution: Cache XRRGetScreenResources *resources on hook start and refresh it when the XRR gets a XRRScreenChangeNotifyEvent event. I am not sure how to listen for that yet...

int event_base = 0;
int error_base = 0;
if (XRRQueryExtension((Display *) closeure, &event_base, &error_base)) {
    Window root = DefaultRootWindow((Display *) closeure);
    XRRScreenResources *resources = XRRGetScreenResources((Display *) closeure, root);

    if (resources->ncrtc > 0) {
        XRRCrtcInfo *crtc_info = XRRGetCrtcInfo((Display *) closeure, resources, resources->crtcs[0]);
        event.data.mouse.x -= crtc_info->x;
        event.data.mouse.y -= crtc_info->y;
        XRRFreeCrtcInfo(crtc_info);
    }

    XRRFreeScreenResources(resources);
}
kwhat commented 10 years ago

Ok the code in the trunk works reasonably with some tweaks. The issue outlined about drag events and much of the post_event code will need some work. The biggest issue is that the input_helper code is initialized with hook registration and deinitilized when the hook unloads. If the hook has not been started, or in a stopped state, post_event will not work. Moving the initialization to library load will require the application using this library restart it self (instead of restarting the hook) if something like the screen resolution changes.

Neither the current design, nor the proposed change above, is much of an improvement. What really needs to happen is the input_helper needs to be initialized on library load, and a second thread should be listening for system configuration changes in order to update the required run-time settings like screen position and offset. This is fairly simple on X11 and I have a vague idea about how to do it on Windows, but I am not sure how to do it on the Mac.

This would also be a good time to address the other issue in post_events you outlined in db172b64201e0a86b52a73817a671470ab7fce5c. There are two ways to I can think of to solve the issue. A priority for the button mask, where Mouse 1 gets a higher priority than Mouse 2 and Mouse 3, or some kind of queue implementation where Mouse Down events are stored in the order a button was pressed and removed as they are released. The drag events can then use either the oldest or newest button as the button set on the event. The best choice will depend on the answers to the questions in the comment at db172b64201e0a86b52a73817a671470ab7fce5c.

Along with the above adjustment to drag events, click count will also need to be tweaked a bit. Currently as long as you keep clicking buttons, that number will increase. We should decided if multi-click events should reset if the button being pressed doesn't match the previous button. Again this should be checked against the majority behavior.

kwhat commented 10 years ago

For Apple: http://stackoverflow.com/questions/958281/is-anything-required-to-get-a-quartz-callback-besides-registering-for-it http://lists.apple.com/archives/quartz-dev/2007/Jan/msg00026.html

kwhat commented 10 years ago

For X11:

#include <stdio.h>
#include <X11/Xlib.h>
#include <X11/extensions/Xrandr.h>

int main(int argc, char **argv) {
    Display *dpy;
    Window root;
    unsigned long event_mask = RRScreenChangeNotifyMask;

    if((dpy = XOpenDisplay(NULL)) == NULL) {
        perror(argv[0]);
        exit(1);
    }

    root = XDefaultRootWindow(dpy);

    XRRSelectInput(dpy,root,event_mask);

    XEvent ev;
    while(1) {
        XNextEvent(dpy, &ev);
        printf("Type: %d\n", ev.type);

        if (ev.type == XRRScreenChangeNotifyEvent) {
            printf("Screen Change\n");
        }
    }

    return 0;
}
kwhat commented 10 years ago

I still can figure out the Windows portion. There is a lot of .NET code to do what we are looking for but no std C/C++ code.

kwhat commented 10 years ago

It looks like the RegisterClass function will do it on Windows. Just need to listen for WM_SETTINGCHANGED. I will code it up tonight.

kwhat commented 9 years ago

Hope the travel is going well. Just a progress update, I believe most of the work is now complete on the Linux side. Coordinates are normalized with xinerama and xrandr and the system properties should handle multi-head correctly. Support for multiple displays still needs to be added to windows and darwin based on the x11 changes. I should finish up a much needed re-factoring of the callback and post event functions soon.

kwhat commented 9 years ago

@munsuri I hope your trip went well. I have a couple of questions about the remaining changes. First, what was the issue near line 358 with the scroll direction? This should be ok with the updates to normalize the coordinates on x11, correct? Second, do you have code written for getting multiple displays and their positions for the hook_get_screen_info function on Darwin and Windows? Finally, can you give me you contact info for the Authors file? I am not sure I you sent it before, but I don't have it.

Thanks again for all the help!

munsuri commented 9 years ago

Everything good, just sorting the last things out. I'll start soon checking the new coordinates values for the scrolling and porting the linux functionality (hopefully) on Windows and then Darwin (or continuing what it is there - I have some code yes but don't know whether you have started something already on Windows/Darwin) I think I sent my contact info to you but I'll send it to your email this time :) Thank you too!

kwhat commented 9 years ago

Glad to hear, and yes email is probably a better option ;) I have completed the X11 version of UIOHOOK_API screen_data* hook_get_screen_info(uint8_t *count) but I have only stubbed out the Darwin and Windows versions with the code you provided to grab only the primary screen. These functions will need to be modified to provide information on all connected screens in a similar way to the X11 version. I don't think we will need to implement the resolution change listeners with any of the threading stuff that I added for XrandR support. The reason we need that on X11 is because XrandR doesn't cache the values like Xinerama and we need that value every time the mouse is moved.

I also rewrote the post_event stuff for all platforms to try and make it easier to work with and to fix several problems. The only issue I know about there is that scroll events have not been implemented for Darwin post events.

The only other thing is the scroll wheel coordinate stuff you were tweaking in the hook callback. Is that change no longer needed with the normalized coordinates? I left some FIXME's in the code near the area. The mouse wheel should rotate in the same direction regardless of platform. See https://github.com/kwhat/libuiohook/blob/master/src/x11/hook_callback.c#L359

Try and create a new branch based on libuiohook/trunk for each item so we can merge them step by step ;)

munsuri commented 9 years ago

I've been checking the new implementation (very nice!) although some things (i.e. right click from Windows to Darwin) are not working for me anymore (I'll keep checking why) and create a branch for it if needed.

For the scroll events in Darwin I had that working if I'm not mistaken, so I'll check that too.

As for the scroll wheel stuff, the backward rotation now is wrong as it is missing the 2 byte cast:

process_mouse_wheel [462]: Mouse wheel type 1, rotated -1635 units at 629, 659.
dispatch_event [68]: Dispatching event type 11.
id=11,when=1418743045194,mask=0x0,type=1,amount=3,rotation=-545

It is easily fixed by adding it back:

(short)(HIWORD(mshook->mouseData))

Also, the rotation is inverted as it is being multiplied by -1, so the rotation, at least on Darwin, goes in the opposite direction.

For (at least) Windows and Darwin the specification is the same, as follows:

- A positive value indicates that the wheel was rotated forward, away from the user.
- A negative value indicates that the wheel was rotated backward, toward the user

So when multiplied by -1, Darwin will do the inverse that was done in Windows. I will also check what happens from Darwin to Windows.

Should I create a new branch for this little thing as well?

For now I'll start by creating a branch for the Windows multiple monitors and then another one for Darwin. And possibly for the Windows input_hook/post_events and Darwin respectively to not mix up things :)

kwhat commented 9 years ago

Perfect thanks for looking into this! I am sure a lot of these are typos in some of the if blocks and probably some changes I didn't understand and commented out or that got lost in the code shuffle. Please do create those pull requests, the patches and testing help tremendously ;)

kwhat commented 9 years ago

Hey I was going to start work on the wheel direction bugs. Just want to make sure you aren't currently working on it or have a pull request you hadn't made yet.

munsuri commented 9 years ago

Hei, Merry Christmas! I have some work done on the Windows and Darwin side for that but haven't pushed anything yet as I was more focused on a potential issue while retrieving screen resolutions on Windows.

Long story short, so far all methods to retrieve resolutions give logical values but not physical. That becomes a problem when there is a scale factor (higher DPI than the default), which reduces the resolution values but that doesn't affect the mouse coordinates.... example: change the DPI of your Windows from the default 96-100% to 120-125%, which will give a resolution of 1536 x 864 instead of 1920x1080. It seems that the function SetProcessDPIAware could solve that, but can't link it on mingw32 (4.8.1) (not present) so was going to try with the mingw32-64 (4.9.2). Otherwise, we could just ignore (not support) the issue for awhile haha

kwhat commented 9 years ago

Well I figured out the mouse wheel issue, it was a simple casting problem. See 92635ce2459d81a7ac6e5a6d240d9cec4f7f86c9 I haven't looked at OS X yet.

As far as the Screen coordinates go it sounds like you made substantial progress! We can solve the problem with a FIXME note for now because it works better than what is currently there ;) Have you checked SetProcessDPIAwareness instead of SetProcessDPIAware? It looks like its a replacement for that function. I would just stub it out and I will see if I can get it to link up. I had a similar problem with the Windows COM API and mingw.

munsuri commented 9 years ago

Well, no luck linking but it can be done as libsdl or glfw have done (loading the respective library and getting the pointer to it) Doing that it works as expected with both calls (SetProcessDPIAware and/or SetProcessDpiAwareness). Something to note is that SetProcessDpiAwareness is only supported so far on Windows 8.1/2012 R2 and SetProcessDPIAware from Vista/2008 onwords (so don't know how XP handles the DPI changes). So, for now I would say to try to use SetProcessDpiAwareness( Process_Per_Monitor_DPI_Aware ); call first, if not found to try to call SetProcessDPIAware(); and if not found, pray to get desired values :) I'll be creating the branches soon.

kwhat commented 9 years ago

Awesome. If its only on win8, load and call the function at runtime like you are and we'll call it good for now.

kwhat commented 9 years ago

Just check in and seeing how to progress is going. I am working on a newish library and I am really excited to play around with what you have. Is the code attached to one of your branches?

munsuri commented 9 years ago

I'm just finishing the testing before committing as I didn't have the environment. Now back to my place, so in a few days testing will be done :)

kwhat commented 9 years ago

Np, keep me posted.

munsuri commented 9 years ago

I already created the pull request to merge the first iteration of multiple screens in Windows, so it is time to close this long thread xD. I'll create smaller pull threads or issue threads individually.

kwhat commented 9 years ago

Awesome! Looking forward to it.

kwhat commented 9 years ago

Dont forget to add your self to the authros file!