We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 5355

Summary: Add GameController Framework support to macOS
Product: SDL Reporter: C.W. Betts <computers57>
Component: joystickAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: enhancement    
Priority: P2 CC: amaranth72, sezeroz
Version: HG 2.1   
Hardware: x86   
OS: macOS 10.15   
Attachments: GameController macOS Patch
Temporary fix for building SDL_mfijoystick without ARC
Better wrapping around MFI Mac code
001.patch
002.patch
003.patch
004.patch

Description C.W. Betts 2020-11-18 04:35:10 UTC
Created attachment 4524 [details]
GameController macOS Patch

This patch adds support to the GameController framework on macOS Big Sur and later, adding support for MFi controllers as well as rumble support for PS4 and Xbox One. There is some code to make sure that the IOKit joystick handler doesn't include two controllers at once.

While the GameController framework is present in earlier versions of macOS, there was no public, approved way of checking if a specific IOHIDDevice is a controller that GameController could handle. This was changed in Big Sur.
Comment 1 Sam Lantinga 2020-11-21 21:16:10 UTC
Patch added, thanks!
https://hg.libsdl.org/SDL/rev/abdb4dfcfc27
Comment 2 Alex Szpakowski 2020-11-22 01:46:47 UTC
 seems to be breaking Xcode builds on macOS. The MFi joystick code was written to use automatic reference counting (ARC), whereas SDL on macOS uses manual reference counting rather than ARC.

There's at least one line that causes an actual compile error for me, but aside from that, the file should be audited to make sure there's no memory leaks when it's compiled for manual reference counting instead of ARC.
Comment 3 Alex Szpakowski 2020-11-22 01:49:47 UTC
I guess it's also worth mentioning SDL for macOS doesn't use ARC because of 32 bit compatibility (from what I remember). Using ARC would mean reevaluating the architectures SDL supports on macOS - personally I'm in favour of updating the macOS backend to use ARC everywhere, but I don't know whether Sam/Ryan have other thoughts.
Comment 4 C.W. Betts 2020-11-22 02:01:05 UTC
I know that on Xcode 12, setting the deployment target to 10.7 or below will cause the build to fail due to changes with how blocks are handled. In 10.8 and later, they could be handled as Objective-C objects. Older SDKs has a preprocessor macro around property declarations to make code targeting older OSes work. This was changed with the 11.0 SDK: the preprocessor macro was removed, and Apple wants people to target 10.9 as the oldest release.
Comment 5 Ozkan Sezer 2020-11-22 07:55:37 UTC
Tried building against 10.8 SDK targeting 10.6, it fails:

In file included from src/joystick/iphoneos/SDL_mfijoystick.m:28:
src/joystick/iphoneos/../../../include/SDL_config_iphoneos.h:88:9: warning: 'HAVE_M_PI' macro redefined
#define HAVE_M_PI   1
        ^
include/SDL_config.h:143:9: note: previous definition is here
#define HAVE_M_PI /**/
        ^
src/joystick/iphoneos/SDL_mfijoystick.m:46:9: fatal error: 'CoreMotion/CoreMotion.h' file not found
#import <CoreMotion/CoreMotion.h>
        ^
1 warning and 1 error generated.
Comment 6 Ozkan Sezer 2020-11-22 08:06:08 UTC
(In reply to Ozkan Sezer from comment #5)
> Tried building against 10.8 SDK targeting 10.6, it fails:

That was with x86 (32 bit) as CPU target (not that it matters.)
Comment 7 C.W. Betts 2020-11-22 08:08:25 UTC
I'll look into fixing compiling on older SDKs tomorrow.
Comment 8 C.W. Betts 2020-11-22 20:29:49 UTC
Created attachment 4533 [details]
Temporary fix for building SDL_mfijoystick without ARC

Attaching a temporary patch for SDL_mfijoystick.m that makes it build when targeting manual retain counting.
Something needs to be done when building against Big Sur's SDK. My guess is targeting at minimum 10.8 for the x86_64/arm64 side, and not building the MFI code for i386. Thoughts?
Comment 9 C.W. Betts 2020-11-23 06:11:18 UTC
Created attachment 4535 [details]
Better wrapping around MFI Mac code

This patch ups the minimum deployment target on 64-bit architectures to 10.9 (10.8 should just be fine, other than Xcode complaining about it), while leaving the deployment target for i386 to 10.6.
Don't attempt to build MFI code on 32-bit macOS, or if the SDK isn't 11.0 or later.
Comment 10 Ozkan Sezer 2020-11-23 06:39:30 UTC
(In reply to C.W. Betts from comment #9)
> Created attachment 4535 [details]
> Better wrapping around MFI Mac code
> 
> This patch ups the minimum deployment target on 64-bit architectures to 10.9

Why?

> Don't attempt to build MFI code on 32-bit macOS, or if the SDK isn't 11.0 or
> later.

The configury (and possibly CMake'ry) must be adjusted too.
Comment 11 C.W. Betts 2020-11-23 07:10:30 UTC
(In reply to Ozkan Sezer from comment #10)
> (In reply to C.W. Betts from comment #9)
> > This patch ups the minimum deployment target on 64-bit architectures to 10.9
> 
> Why?

The GameController framework changed the following code which worked fine with 10.7 and earlier OSes:

    #if defined(OS_OBJECT_USE_OBJC) && OS_OBJECT_USE_OBJC==1
    @property (retain) dispatch_queue_t handlerQueue;
    #else
    @property (assign) dispatch_queue_t handlerQueue;
    #endif

To this, in the 11.0 SDK:

    @property (assign) dispatch_queue_t handlerQueue;

In 10.8, OS Objects were treated similarly to Objective C classes. Targeting an OS version below that tells the compiler to use the older way of handling OS Objects. Code that works with the older way has been removed in the 11.0 SDK, and the 10.15 SDK lacks many of the needed methods.

... I had an epiphany on how to maybe make it work with older SDKs.
Comment 12 Ozkan Sezer 2020-11-23 07:28:22 UTC
Well, since that 10.9+ restriction is in Xcode project files only,
I have no objections.
Comment 13 C.W. Betts 2020-11-23 07:40:16 UTC
The 10.9+ restriction is due to changes between the SDK versions. Creating a release version of SDL2.

If needed, perhaps a sperate Xcode project could be used for older OSes.
Comment 14 Ozkan Sezer 2020-11-23 08:10:35 UTC
(In reply to C.W. Betts from comment #13)
> The 10.9+ restriction is due to changes between the SDK versions. Creating a
> release version of SDL2.

As I understand it, that SDK change affects only GameController framework:
So, if SDL_JOYSTICK_MFI is disabled, I can happily target 64 bits down to
10.6, yes? If yes, we need an option+check in autotools (and cmake) for it.

> If needed, perhaps a sperate Xcode project could be used for older OSes.

Autotools (and cmake) should be enough for older OS support.
Comment 15 C.W. Betts 2020-11-23 17:24:28 UTC
> So, if SDL_JOYSTICK_MFI is disabled, I can happily target 64 bits down to 10.6, yes?

Just tested it, and yes.
Comment 16 Sam Lantinga 2020-11-24 05:00:31 UTC
Hmm, I went through and cleaned up the MFI code a bit. Can you see if this works?
https://hg.libsdl.org/SDL/rev/bb854f215b2a
Comment 17 C.W. Betts 2020-11-24 05:06:39 UTC
SDL_mfijoystick.m needs to be set to compile as ARC on macOS.
Problem is, I can't see a way to do this that makes Intel Mac 32-bit happy.
Comment 18 Ozkan Sezer 2020-11-24 11:13:40 UTC
Attaching four patches below for targeting older macOS and
building against older SDKs. Please review.
Comment 19 Ozkan Sezer 2020-11-24 11:14:13 UTC
Created attachment 4538 [details]
001.patch

SDL_mfijoystick.m: change TARGET_OS_OSX conditions to !TARGET_OS_IOS

TARGET_OS_OSX is in 10.12 SDK and newer.  Therefore, if building for
Mac OS X against an older SDK (e.g. 10.8), unconditionally including
SDL_config_iphoneos.h unconditionally defines SDL_JOYSTICK_MFI and
the build fails.

Also change the macOS version 11.0 checks to be more compatible with
older OS targets.
Comment 20 Ozkan Sezer 2020-11-24 11:14:49 UTC
Created attachment 4539 [details]
002.patch

SDL_config_macosx.h: bump SDL_JOYSTICK_MFI requirement from 10.8 to 10.9

as per comments at https://bugzilla.libsdl.org/show_bug.cgi?id=5355#c11
Comment 21 Ozkan Sezer 2020-11-24 11:15:45 UTC
Created attachment 4540 [details]
003.patch

CMakeLists.txt: add src/joystick/iphoneos/*.m to Darwin joystick sources
so that there won't be missing symbols.
TODO: add checks for SDL_JOYSTICK_MFI ???
Comment 22 Ozkan Sezer 2020-11-24 11:16:37 UTC
Created attachment 4541 [details]
004.patch

configure.ac: check GameController framework support when targeting Darwin

disables SDL_JOYSTICK_MFI for i386 or if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
--disable-joystick-mfi disables it unconditionally.
Comment 23 Ozkan Sezer 2020-11-24 11:20:09 UTC
Build-tested by cross-compiling on linux:
- clang-5.0.2, 10.12 SDK, targeting x86_64
- clang-5.0.2, 10.8 SDK, targeting i686
- clang-3.4.2, 10.8 SDK, targeting i686
Comment 24 Sam Lantinga 2020-11-24 14:51:47 UTC
I applied patches 3 and 4:
https://hg.libsdl.org/SDL/rev/ea271694a386
https://hg.libsdl.org/SDL/rev/dff6932ed52d

I'm still working through build issues on my side which might conflict with patches 1 and 2. Let's revisit them shortly.
Comment 25 Sam Lantinga 2020-11-24 14:55:07 UTC
It looks like the Objective C support needed to build MFI controller code is in 10.8

Does this build in your configuration?
https://hg.libsdl.org/SDL/rev/a58cfff9a09a
Comment 26 Ozkan Sezer 2020-11-24 15:23:17 UTC
(In reply to Sam Lantinga from comment #25)
> It looks like the Objective C support needed to build MFI controller
> code is in 10.8
> 
> Does this build in your configuration?
> https://hg.libsdl.org/SDL/rev/a58cfff9a09a

Nope.

With older versions of clang (e.g. clang-3.4.2):
src/joystick/iphoneos/SDL_mfijoystick.m:470:9: error: unexpected '@' in program
    if (@available(macos 11.0, *)) @autoreleasepool {
        ^
1 error generated.
make: *** [build/SDL_mfijoystick.lo] Error 1
make: *** Waiting for unfinished jobs....

With newer versionhs of clang (clang-5.0.2):
Undefined symbols for architecture i386:
  "_IOS_SupportedHIDDevice", referenced from:
      _JoystickDeviceWasAddedCallback in SDL_iokitjoystick.o
  "___isOSVersionAtLeast", referenced from:
      _IOS_JoystickInit in SDL_mfijoystick.o
ld: symbol(s) not found for architecture i386
clang-5.0: error: linker command failed with exit code 1 (use -v to see invocation)


This is part of what patch-1 fixes.

The other part is the TARGET_OS_OSX macro not being available on older SDKs.
(my solution to that is ugly/hairy, I know.)
Comment 27 Sam Lantinga 2020-11-24 15:23:34 UTC
Okay, I've gotten things building in my environment. How is it looking for you guys?
Comment 28 Ozkan Sezer 2020-11-24 15:31:02 UTC
(In reply to Sam Lantinga from comment #27)
> Okay, I've gotten things building in my environment. How is it looking for
> you guys?

Got https://hg.libsdl.org/SDL/rev/ba3af39f96b1
same failure for me as I noted in comment #26.
Comment 29 Sam Lantinga 2020-11-24 15:32:18 UTC
Okay, I added that part of your patch, and I'm done playing with things for now. Does it work for you?
Comment 30 Ozkan Sezer 2020-11-24 15:44:07 UTC
(In reply to Sam Lantinga from comment #29)
> Okay, I added that part of your patch, and I'm done playing with things for
> now. Does it work for you?

Got https://hg.libsdl.org/SDL/rev/16bae903b2de,
it does build and link, yes.

Note, however:

- Both darwin/SDL_iokitjoystick.c (see JoystickAlreadyKnown())
  and iphoneos/SDL_mfijoystick.m rely on TARGET_OS_OSX macro
  which is available 10.12+ SDKs. Building against older SDKs,
  those parts of the guarded code are omitted.

- Patch 002 still applies (and seems to be needed (see comment #11)
Comment 31 Sam Lantinga 2020-11-24 15:57:52 UTC
(In reply to Ozkan Sezer from comment #30)
> - Both darwin/SDL_iokitjoystick.c (see JoystickAlreadyKnown())
>   and iphoneos/SDL_mfijoystick.m rely on TARGET_OS_OSX macro
>   which is available 10.12+ SDKs. Building against older SDKs,
>   those parts of the guarded code are omitted.
> 
> - Patch 002 still applies (and seems to be needed (see comment #11)

Comment 11 says:
"In 10.8, OS Objects were treated similarly to Objective C classes. Targeting an OS version below that tells the compiler to use the older way of handling OS Objects."

10.8 uses the new way of handling OS objects, which matches my own research. There's nothing about 10.9 being a runtime requirement here.

Usage of TARGET_OS_OSX should be fixed now:
https://hg.libsdl.org/SDL/rev/0bed6ba4e2bc

Thanks!
Comment 32 Ozkan Sezer 2020-11-24 16:22:50 UTC
(In reply to Sam Lantinga from comment #31)
> There's nothing about 10.9 being a runtime requirement here.

Apologies, I must have mixed things in my mind.

> Usage of TARGET_OS_OSX should be fixed now:
> https://hg.libsdl.org/SDL/rev/0bed6ba4e2bc

Thanks.

One issue, though:  Looking at my config.log, I see this:

configure:23163: checking for GameController framework
configure:23189: clang -target x86_64-apple-darwin9 -arch x86_64 -mlinker-version=274.2 -isysroot /opt/MacOSX10.12.sdk -B /opt/cctools-port/bin -mmacosx-version-min=10.6 -c  -O2 -Iinclude -I/home/ozzie/2s/include -idirafter /home/ozzie/2s/src/video/khronos  -x objective-c -fobjc-weak  -Iinclude -I/home/ozzie/2s/include -idirafter /home/ozzie/2s/src/video/khronos  conftest.c >&5
clang-5.0: error: -fobjc-weak is not supported on the current deployment target
configure:23189: $? = 1

So, CheckJoystickMFI() of configure fails not because the framework is
unavailable or because I'm targeting i686, but because -fobjc-weak is
'not supported on the current deployment target'. I'm not familiar with
that compiler switch: is it truly needed? 
(also that  -Wno-unused-command-line-argument thing?)
Comment 33 Ozkan Sezer 2020-11-24 16:37:44 UTC
(In reply to Ozkan Sezer from comment #32)
>  but because -fobjc-weak is 'not supported on the current deployment target'

OK, searching for it though github shows that it's supported
when targeting 10.7 and newer.  Since I'm targeting 10.6, it
fails. The requirement itself is 10.8+ so things should be good.

Sorry for the noise.
Comment 34 Sam Lantinga 2020-11-24 17:46:55 UTC
(In reply to Ozkan Sezer from comment #33)
> (In reply to Ozkan Sezer from comment #32)
> >  but because -fobjc-weak is 'not supported on the current deployment target'
> 
> OK, searching for it though github shows that it's supported
> when targeting 10.7 and newer.  Since I'm targeting 10.6, it
> fails. The requirement itself is 10.8+ so things should be good.

Yep, that's correct behavior. It should fail when targeting 10.6.