Add support for _MOUSEWHEEL and _MOUSEMOVEMENTx on macOS #468
Conversation
a740g
requested review from
grymmjack,
mkilgore,
SteveMcNeill,
RhoSigma-QB64 and
QB64Cobalt
| extern "C" int qb64_custom_event(int event, int v1, int v2, int v3, int v4, int v5, int v6, int v7, int v8, void *p1, void *p2); | ||
| void GLUT_MOUSEWHEEL_FUNC(int wheel, int direction, int x, int y); | ||
|
|
||
| extern int g_MouseX, g_MouseY; |
There was a problem hiding this comment.
I think it'd be cleaner to declare these static in this file and then expose a function for GLUT_MOUSE_FUNC to call that sets them. If/when we pull the basic GLUT stuff out of libqb.cpp that's the kind of change we'll make anyway.
| auto deltaX = CGEventGetIntegerValueField(event, kCGMouseEventDeltaX); | ||
| auto deltaY = CGEventGetIntegerValueField(event, kCGMouseEventDeltaY); | ||
| if (deltaX || deltaY) | ||
| qb64_custom_event(QB64_EVENT_RELATIVE_MOUSE_MOVEMENT, deltaX, deltaY, 0, 0, 0, 0, 0, 0, nullptr, nullptr); |
There was a problem hiding this comment.
I think it would be good to pull the QB64_EVENT_RELATIVE_MOUSE_MOVEMENT code out of qb64_custom_event and place it in a separate function in libqb.cpp. After that both qb64_custom_event() and your own code can call that new function.
qb64_custom_event() was designed so that there's only a single function for the GLUT code to call, but it's pretty nasty to use elsewhere with all the extra parameters and IMO worth avoiding.
There was a problem hiding this comment.
Cleaned this up as suggested.
| static CFMachPortRef g_EventTap = nullptr; | ||
| static CFRunLoopSourceRef g_RunLoopSource = nullptr; | ||
|
|
||
| static CGEventRef macMouseCallback(CGEventTapProxy proxy, CGEventType type, CGEventRef event, void *userInfo) { |
There was a problem hiding this comment.
I assume the callback gets called from the same main thread that GLUT runs on (since that should be running the event loop)?
There was a problem hiding this comment.
Yes. None of this stuff depends on GLUT as it bypasses it and goes straight to the OS.
| { | ||
| void libqb_exit(int exitcode) { | ||
| #ifdef QB64_MACOSX | ||
| macMouseDone(); |
There was a problem hiding this comment.
I think this call belongs in here instead. I suspect that you're intended to only call some of those CG* functions from the main thread.
There was a problem hiding this comment.
Moved this to glut_message_exit_program::execute().
| @@ -0,0 +1,6 @@ | |||
| #pragma once | |||
|
|
|||
| #ifdef QB64_MACOSX | |||
There was a problem hiding this comment.
It's a small thing in this case, but rather than require the caller to check QB64_MACOSX every time they want to call these functions I think it's better to declare stubs in here that don't do anything on the other OSs. That way callers don't have to check and we don't need to add #ifdefs everywhere. IE:
#ifdef QB64_MACOSX
void macMouseInit();
void macMouseDone();
#else
static inline void macMouseInit() { }
static inline void macMouseDone() { }
#endifThere was a problem hiding this comment.
Yeah. This looks a lot cleaner. Done.
|
Tested on my Mac: With this code: DO
' Keyboard handling
DIM k AS STRING
DIM AS INTEGER MMX, MMY, MW
k$ = LCASE$(INKEY$)
DO WHILE _MOUSEINPUT
MMX = _MOUSEMOVEMENTX
MMY = _MOUSEMOVEMENTY
MW = _MOUSEWHEEL
IF MMX <> 0 THEN
LOCATE 1, 1 : PRINT SPACE$(50)
LOCATE 1, 1 : PRINT "MMX:", MMX
END IF
IF MMY <> 0 THEN
LOCATE 2, 1 : PRINT SPACE$(50)
LOCATE 2, 1 : PRINT "MMY:", MMY
END IF
IF MW <> 0 THEN
LOCATE 3, 1 : PRINT SPACE$(50)
LOCATE 3, 1 : PRINT "MW:", MW
END IF
LOOP
IF k$ = CHR$(27) THEN EXIT DO
LOOP
SYSTEMWorked great! 
built from the PR branch |
Fixes #289, #465
This PR fixes some longstanding macOS issues with the mouse.
Key fixes include:
_MOUSEWHEEL._MOUSEMOVEMENTXand_MOUSEMOVEMENTYaccurately.Previously, these issues stemmed from limitations within the GLUB library on macOS. To work around these limitations, this PR leverages the Quartz Event Services API in a manner that does not disrupt the GLUT input event queue, ensuring smooth and reliable mouse functionality.