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

Fixed compile errors with newer Xcode versions and issue #64 #69

Merged
merged 6 commits into from
Apr 2, 2020

Conversation

univrsal
Copy link
Contributor

@univrsal univrsal commented Mar 1, 2020

Since I've had the pleasure of working with macOS in the past weeks for my own projects, I thought I'd try to fix the compile issue with the new Xcode version.
I basically just followed this solution and tweaked the return value to void *. Some calls to this strong typed version of the function only use two arguments so I just set the fourth one to NULL.
I compiled and tested this on macOS 10.15.3 and it seemed to work without issues. I could only test the demo_hook.c though because I couldn't be bothered to figure out how to compile the asynchronous one :P

I also fixed #64 and updated the readme, since what I previously wrote there didn't really make any sense.

Copy link
Owner

@kwhat kwhat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding this fix. I found a small typo in src/demo_hook_async.c which I marked. Just missing the >. After reading the stack overflow post, I am still a bit confused with this fix for #64. I did find this which does a pretty good job explaining what happened. The solution you found is pretty good, the only complaint I have is the objc_msgSend_ definition on line 58. Would it be possible to do the typedef casting of the function signature like ((void (*)(id, SEL, CGEventRef))objc_msgSend) instead of lines 57 & 58? If I worked this out correctly, I think something like ((void (*)(id, SEL, CGEventRef))objc_msgSend)(objc_getClass("NSEvent"), sel_registerName("eventWithCGEvent:"), event_ref); sould work. We may also be able to get rid of the void (*) by replacing it with id which would get rid of the (id) cast on assignment. (Ex: ((id (*)(id, SEL, CGEventRef))objc_msgSend)) I would check this myself, but the only mac I have access to runs 10.5 which cannot produce this issue.

src/demo_hook_async.c Outdated Show resolved Hide resolved
@kwhat kwhat mentioned this pull request Mar 13, 2020
@univrsal
Copy link
Contributor Author

univrsal commented Mar 13, 2020

Well I have to say that I'm a bit confused by the function pointer stuff, if I got this right you mean that it should be replaced by something like this:

static id (*objc_msgSend_)(id, SEL, CGEventRef) = ...

and potentially

((void (*)(id, SEL, CGEventRef))objc_msgSend)(objc_getClass("NSEvent"), sel_registerName("eventWithCGEvent:"), event_ref);

I don't really know what to do with the second part though, is that just the assignment on the left?
I tried the first snippet and it seems that the compiler doesn't like CGEventRef, as well as the id:

/Users/bigmac/git/libuiohook/src/darwin/input_hook.c:59:13: warning: type specifier missing, defaults to 'int' [-Wimplicit-int]
static id (*objc_msgSend_)(id, SEL, CGEventRef) = ((void (*)(id, SEL, CGEventRef)) objc_msgSend;
            ^
/Users/bigmac/git/libuiohook/src/darwin/input_hook.c:59:37: error: unexpected type name 'CGEventRef': expected identifier
static id (*objc_msgSend_)(id, SEL, CGEventRef) = ((void (*)(id, SEL, CGEventRef)) objc_msgSend;
                                    ^
/Users/bigmac/git/libuiohook/src/darwin/input_hook.c:59:8: warning: type specifier missing, defaults to 'int' [-Wimplicit-int]
static id (*objc_msgSend_)(id, SEL, CGEventRef) = ((void (*)(id, SEL, CGEventRef)) objc_msgSend;
~~~~~~ ^
/Users/bigmac/git/libuiohook/src/darwin/input_hook.c:59:28: error: a parameter list without types is only allowed in a function definition
static id (*objc_msgSend_)(id, SEL, CGEventRef) = ((void (*)(id, SEL, CGEventRef)) objc_msgSend;
                           ^
/Users/bigmac/git/libuiohook/src/darwin/input_hook.c:59:71: error: unexpected type name 'CGEventRef': expected identifier
static id (*objc_msgSend_)(id, SEL, CGEventRef) = ((void (*)(id, SEL, CGEventRef)) objc_msgSend;
                                                                      ^
/Users/bigmac/git/libuiohook/src/darwin/input_hook.c:59:62: error: a parameter list without types is only allowed in a function definition
static id (*objc_msgSend_)(id, SEL, CGEventRef) = ((void (*)(id, SEL, CGEventRef)) objc_msgSend;

@kwhat
Copy link
Owner

kwhat commented Mar 13, 2020

This is certainly not easy to understand and I am also have a little trouble wrapping my head around it. Lets see if I can clarify a little with line 605 as an example.

Original:
id event_data = objc_msgSend((id) objc_getClass("NSEvent"), sel_registerName("eventWithCGEvent:"), event_ref);

Proposal A:
id event_data = ((id (id, SEL, CGEventRef))objc_msgSend)(objc_getClass("NSEvent"), sel_registerName("eventWithCGEvent:"), event_ref);

Proposal B:
id event_data = ((id (*)(id, SEL, CGEventRef))objc_msgSend)(objc_getClass("NSEvent"), sel_registerName("eventWithCGEvent:"), event_ref);

Proposal C:
id event_data = (id) ((void (*)(id, SEL, CGEventRef))objc_msgSend)((id) objc_getClass("NSEvent"), sel_registerName("eventWithCGEvent:"), event_ref);

This wont work for all calls; for the calls that only use two arguments, we should be able to replace (id, SEL, CGEventRef) with (id, SEL) and remove the null 3rd argument.

I will see if I can work this out so it compiles on my ancient mac.

@univrsal
Copy link
Contributor Author

Okay now I get it. Is there any difference between having the (*) in B vs. A? Otherwise I'd go with A and maybe even make it a macro so it looks a bit cleaner.

@kwhat
Copy link
Owner

kwhat commented Mar 13, 2020

There is a difference, but I am not sure which is required. id should already be a pointer, so I don't think the (*) is needed, but the compiler should let us know and I wanted to give you all the options I had in my head.

@kwhat
Copy link
Owner

kwhat commented Mar 13, 2020

Please add yourself to the https://github.com/kwhat/libuiohook/blob/1.1/AUTHORS file and a comment in the code where you are patching. https://github.com/kwhat/libuiohook/pull/69/files#diff-01409950a25df24cd3d26c25d735c94aR604

@univrsal
Copy link
Contributor Author

Done. I used proposal B, since the compiler didn't like A. I also turned it into two macros, but if you'd rather have it without I can change it.

@wis
Copy link

wis commented Mar 30, 2020

Alex (@univrsal) it looks like Alex approved your changes and currently waiting for you to merge.

I want to ask you guys a couple questions, How can I use this library in a c++ (qt) project?
did you make any other changes to the library to be able to use it with your c++ project input-overlay? wouldn't it have been better to make fork and add it as a submodule instead of copying the code? then the git history would still be intact.

@kwhat do I have to do the keeping track of pressed keys myself to implement global shortcuts?
and how do I block input in the event handling method void dispatch_proc(uiohook_event * const event) to match XGrabKey behavior?

@univrsal
Copy link
Contributor Author

Alex (@univrsal) it looks like Alex approved your changes and currently waiting for you to merge.

Only kwhat can merge afaik.

I want to ask you guys a couple questions, How can I use this library in a c++ (qt) project?
did you make any other changes to the library to be able to use it with your c++ project input-overlay? wouldn't it have been better to make fork and add it as a submodule instead of copying the code? then the git history would still be intact.

I don't know, I just linked against it since I thought that to be the best way. I don't know if the CMakeLists file for uiohook is working correctly and even if it is I think it would need to be tweaked for it to be able to just add it as a submodule.
For now I think the best way is to just compile the library and then add it as a dependency in your qt project in the *.pro file.

@kwhat
Copy link
Owner

kwhat commented Mar 30, 2020

Hey, I am working on some cmake junk right now, but this is high on my list.

@kwhat
Copy link
Owner

kwhat commented Mar 30, 2020

@wis

do I have to do the keeping track of pressed keys myself to implement global shortcuts?

Yes and no. If you require a key combo that is not a modifier or a series of key presses, then yes. If you only need a single key or combination with modifiers (shift, ctrl, alt, etc) you can use the modifier mask on the key event.

how do I block input in the event handling method void dispatch_proc(uiohook_event * const event) to match XGrabKey behavior?

You don't really want to and its not possible on X11. You can discard events on other platforms but it is not fully supported on *nix outside of xgrab. The xgrab stuff has its own set of issues when dealing with input. Best bet would be to discard the events when they get to you on callback but you cant stop them from getting to other programs.

@kwhat
Copy link
Owner

kwhat commented Mar 30, 2020

How can I use this library in a c++ (qt) project?

You can use autotools to create a shared object or static library and link your application against it. The uiohook.h header should provide all the required api to your application via #include.

wouldn't it have been better to make fork and add it as a submodule instead of copying the code? then the git history would still be intact.

There are a couple of hacks in there that they needed. Eventually when i get the cmake stuff done and updates out the door maybe we can come up with a uniform working solution.

@ogallagher
Copy link

ogallagher commented Mar 31, 2020

@univrsal Do you know if, since jnativehook uses libuiohook, the fact that I cannot compile libuiohook version 1.1 on my Mac without your changes would also prevent jnativehook from working on my computer? Or do your changes only allow it to compile, so that the resulting library used by jnativehook and the library from your version end up working the same?

@univrsal
Copy link
Contributor Author

univrsal commented Apr 1, 2020

I don't really know, the changes here only fix compilation issues with a new XCode version so I don't see why an older compiled version shouldn't still work.

@ogallagher
Copy link

OK, that makes sense. Thanks for getting back to me

@kwhat
Copy link
Owner

kwhat commented Apr 1, 2020

Alright, I have had enough of cmake. If someone wants to use it, they can struggle though this patch work documentation and figure out how to make a platform compatible CMakeLists.txt for this thing. I honestly thought nothing could be worse than autotools, but its dead simple compared to this disaster.

@univrsal
Copy link
Contributor Author

univrsal commented Apr 1, 2020

Alright, I have had enough of cmake. If someone wants to use it, they can struggle though this patch work documentation and figure out how to make a platform compatible CMakeLists.txt for this thing. I honestly thought nothing could be worse than autotools, but its dead simple compared to this disaster.

Maybe I'll give it a shot. I've never had issues with it, but I guess working with cmake can only ever go two ways, either it's a dumpster fire or it works without issues.

@kwhat
Copy link
Owner

kwhat commented Apr 1, 2020

Alright, I have had enough of cmake. If someone wants to use it, they can struggle though this patch work documentation and figure out how to make a platform compatible CMakeLists.txt for this thing. I honestly thought nothing could be worse than autotools, but its dead simple compared to this disaster.

Maybe I'll give it a shot. I've never had issues with it, but I guess working with cmake can only ever go two ways, either it's a dumpster fire or it works without issues.

Seems easy enough until you need to support osx and windows in the same build... if you have enable/with options, just shoot yourself.

@kwhat kwhat merged commit a335265 into kwhat:1.1 Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Demo programs fails to build on macOS
4 participants