This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• patch - apply to stated revision (SDL_metal_issue.patch, text/plain, 2020-11-25 23:00:36 +0000, 1107 bytes)
• Patch 2 - Apply to 4eb049c9bb1ca94efe3c40b57beda3169984d0cb (patch 2 - apply to 4eb049c9bb1ca94efe3c40b57beda3169984d0cb, text/plain, 2020-12-09 14:01:22 +0000, 1116 bytes)

Reported in version: don't know
Reported for operating system, platform: macOS 10.14, x86_64

Comments on the original bug report:

On 2020-11-25 23:00:36 +0000, Tom Seddon wrote:

Created attachment 4543
patch - apply to stated revision

This is as of commit 50d804e from https://github.com/SDL-mirror/SDL, which is HEAD as I write (apologies, not confident with Mercurial)

Config

macOS: 10.14.6 (18G6042)
cmake --version: cmake version 3.16.20200101-g23e782c
clang --version: Apple clang version 11.0.0 (clang-1100.0.33.17)
Xcode version: Version 11.3.1 (11C504)

Repro steps

Run the following commands in the shell.

cd /tmp/
git clone https://github.com/SDL-mirror/SDL
mkdir build.SDL
cd build.SDL
cmake -G ../SDL/

Examine cmake output.

Expected result

Metal is detected.

Actual result

It appears that Metal is not detected! Note this line in the summary:

-- RENDER_METAL (Wanted: 0): OFF

Fix

Change check_c_source_compiles to check_objc_source_compiles. The cmake script tries to add -ObjC to the clang command line, but, for whatever reason, this doesn't seem to work.

Change the test source to have an empty main. The "return 0;" line seems to confuse cmake somehow, causing it to crap out with an error about HAVE_FRAMEWORK_METAL being an unknown argument. (Maybe I'm just dense, but it's not obvious to me what the problem is here.)

With these two changes:

-- RENDER_METAL (Wanted: ON): ON

Patch attached.

On 2020-11-26 00:37:53 +0000, Tom Seddon wrote:

I've now updated to cmake version 3.19.20201125-g7137dff, and I get the same result.

On 2020-12-01 21:51:51 +0000, Sam Lantinga wrote:

Patch added, thanks!
https://hg.libsdl.org/SDL/rev/face37fbd0e6

On 2020-12-08 20:12:20 +0000, Ozkan Sezer wrote:

This change bumps cmake minimum required version from 3.0
to 3.16 which is pretty insane.

If we revert this patch, but just remove the "return 0;"
line to make main() empty, does it still fail?

On 2020-12-08 22:28:55 +0000, Sam Lantinga wrote:

It seems like we shouldn't remove "return 0;" either. main() is defined as returning an int, and it should return one.

On 2020-12-08 22:31:02 +0000, Sam Lantinga wrote:

Tom, can you revisit this?

We'll have to revert this patch for the upcoming release if we can't find a better solution.

Maybe we can conditionally use include(CheckOBJCSourceCompiles), just on Mac?

On 2020-12-08 23:13:49 +0000, Ozkan Sezer wrote:

(In reply to Sam Lantinga from comment # 4)

It seems like we shouldn't remove "return 0;" either. main() is
defined as returning an int, and it should return one.

That's correct, but there are many other check_c_source_compiles()
calls there with a main() without return. Does cmake add a return
by itself -- don't know, but those ones work. (I never was good
with cmake, though.)

On 2020-12-08 23:27:29 +0000, Tom Seddon wrote:

Sure, no problem - sorry for the bother. I can have a quick look now.

--Tom

On 2020-12-09 01:57:48 +0000, Tom Seddon wrote:

I've drawn a bit of a blank here I'm afraid, so if nobody else has complained, and cmake 3.16 is a step too far, then maybe best just revert it and hope nobody else noticed :(

1. Regarding check_objc_source_compiles: the rationale behind changing this was that the check_c_source_compiles invocation just didn't seem to be compiling the code as Objective C. The CMakeError.log file contained an error "clang: warning: argument unused during compilation: '-ObjC' [-Wunused-command-line-argument]", followed by a bunch of the sort of errors you get when trying to compile Objective C (such as the code in the Apple headers) as C. Removing the return 0 doesn't help with this issue, as the code still doesn't compile as Objective C.

Fiddling around on the command line interactively, it seems the problem is that cmake adds "-x c" to the clang command line, which takes priority over "-ObjC". You can see this with something like this:

$ touch test.c
$ clang -ObjC -x c -c test.c
clang: warning: argument unused during compilation: '-ObjC' [-Wunused-command-line-argument]
$ clang -ObjC -c test.c
$ 

I don't know enough about cmake to fix this. I couldn't follow the implementation of check_objc_source_compiles.

2. Regarding "return 0;": I initially determined the need for this change by experimentation. I don't know what's going on, but if you put even a single semicolon in main here then the compilation fails with an error from CMake. And even the following fails with a ``Unknown argument: HAVE_FRAMEWORK_METAL'' error:

---8<---
check_objc_source_compiles("
#define RETURN0 return 0;
#include <AvailabilityMacros.h>
#import <Metal/Metal.h>
#import <QuartzCore/CAMetalLayer.h>

    #if TARGET_OS_SIMULATOR || (!TARGET_CPU_X86_64 && !TARGET_CPU_ARM64)
    #error Metal doesn't work on this configuration
    #endif
    int main(int argc, char **argv)
    {
        RETURN0
    }
    " HAVE_FRAMEWORK_METAL)

---8<---

This is a complete mystery to me, I'm afraid.

If Objective C always implies C99, then removing the return 0 is valid, as an explicit return from main is optional in C99 for some reason: https://port70.net/~nsz/c/c99/n1256.html# 5.1.2.2.3 - this doesn't eliminate the need for check_objc_source_compiles though :(

--Tom

On 2020-12-09 02:53:45 +0000, Sam Lantinga wrote:

Is there a way to only include CheckOBJCSourceCompiles on Mac? Would that take care of the CMake version requirement?

On 2020-12-09 06:16:16 +0000, Vitaly Novichkov wrote:

Unrelated, but the inclusion of CheckOBJCSourceCompiles will break the build on Linux.

CMake Error at CMakeLists.txt:35 (include):
  include could not find load file:

    CheckOBJCSourceCompiles
``

https://semaphoreci.com/wohlstand/sdl-mixer-x/branches/wip-win32-alt-native-midi/builds/2

On 2020-12-09 08:42:49 +0000, Ozkan Sezer wrote:

(In reply to Vitaly Novichkov from comment # 10)

Unrelated, but the inclusion of CheckOBJCSourceCompiles will break the build
on Linux.

You probably have cmake older than 3.16

On 2020-12-09 08:45:44 +0000, Vitaly Novichkov wrote:

You probably have cmake older than 3.16

Seems right, that was the default CMake at Ubuntu 18.04 packages. I guess it needs to add the condition to don't try to include this on CMake older than 3.16.

On 2020-12-09 08:47:19 +0000, Ozkan Sezer wrote:

(In reply to Tom Seddon from comment # 8)

1. Regarding check_objc_source_compiles: the rationale behind changing this
   was that the check_c_source_compiles invocation just didn't seem to be
   compiling the code as Objective C. The CMakeError.log file contained an
   error "clang: warning: argument unused during compilation: '-ObjC'
   [-Wunused-command-line-argument]", followed by a bunch of the sort of errors
   you get when trying to compile Objective C (such as the code in the Apple
   headers) as C. Removing the return 0 doesn't help with this issue, as the
   code still doesn't compile as Objective C.

Fiddling around on the command line interactively, it seems the problem is
that cmake adds "-x c" to the clang command line, which takes priority over
"-ObjC". You can see this with something like this:

Just an idea to test: Revert the change, and change the line
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -ObjC")
to:
set(CMAKE_REQUIRED_FLAGS "-ObjC")

And remove the return 0; line, too if you want.
Does it help?

On 2020-12-09 09:19:04 +0000, Vitaly Novichkov wrote:

(In reply to Vitaly Novichkov from comment # 10)

Unrelated, but the inclusion of CheckOBJCSourceCompiles will break the build
on Linux.

CMake Error at CMakeLists.txt:35 (include):
  include could not find load file:

    CheckOBJCSourceCompiles
``

https://semaphoreci.com/wohlstand/sdl-mixer-x/branches/wip-win32-alt-native-
midi/builds/2

For this, I made my own issue and the patch:
https://bugzilla.libsdl.org/show_bug.cgi?id=5385

On 2020-12-09 14:00:31 +0000, Tom Seddon wrote:

2nd time lucky, perhaps. patch 2 applies to current HEAD at time of writing - 4eb049c from https://github.com/SDL-mirror/SDL.

This basically goes back to what was there originally, but now manually adding "-x objective-c" to the clang command line rather than "-ObjC". clang is then invoked without the "-x c" that was causing the problem, the snippet builds, and Metal is detected. (I had a quick trawl through the cmake code, but I couldn't see where this is handled.)

I was moved to try this after finding SDL's own CHECK_OBJC_SOURCE_COMPILES macro, and noting what it does: https://github.com/SDL-mirror/SDL/blob/4eb049c9bb1ca94efe3c40b57beda3169984d0cb/cmake/macros.cmake#L67

An alternative fix of course would be to use CHECK_OBJC_SOURCE_COMPILES instead of cmake's check_objc_source_compiles - but that had the same problem of getting confused by "return 0;". (Maybe that's because it's a macro? I'll defer to a cmake expert on this one.)

I decided in the end to err on the side of leaving things looking basically the same as they were before my first patch.

--Tom

On 2020-12-09 14:01:22 +0000, Tom Seddon wrote:

Created attachment 4558
Patch 2 - Apply to 4eb049c

On 2020-12-09 14:10:22 +0000, Ozkan Sezer wrote:

(In reply to Tom Seddon from comment # 15)

2nd time lucky, perhaps. patch 2 applies to current HEAD at time of writing

• 4eb049c from
  https://github.com/SDL-mirror/SDL.

This basically goes back to what was there originally, but now manually
adding "-x objective-c" to the clang command line rather than "-ObjC". clang
is then invoked without the "-x c" that was causing the problem, the snippet
builds, and Metal is detected. (I had a quick trawl through the cmake code,
but I couldn't see where this is handled.)

I was moved to try this after finding SDL's own CHECK_OBJC_SOURCE_COMPILES
macro, and noting what it does:
https://github.com/SDL-mirror/SDL/blob/
4eb049c/cmake/macros.cmake#L67

An alternative fix of course would be to use CHECK_OBJC_SOURCE_COMPILES
instead of cmake's check_objc_source_compiles - but that had the same
problem of getting confused by "return 0;". (Maybe that's because it's a
macro? I'll defer to a cmake expert on this one.)

I decided in the end to err on the side of leaving things looking basically
the same as they were before my first patch.

--Tom

(In reply to Tom Seddon from comment # 16)

Created attachment 4558 [details]
Patch 2 - Apply to 4eb049c

Would this not bring back bug # 4988 and bug # 5204 ?

On 2020-12-09 14:18:33 +0000, Sam Lantinga wrote:

Awesome, this is fixed, thanks!
https://hg.libsdl.org/SDL/rev/009f21e50409

On 2020-12-09 14:32:18 +0000, Ozkan Sezer wrote:

(In reply to Sam Lantinga from comment # 18)

Awesome, this is fixed, thanks!
https://hg.libsdl.org/SDL/rev/009f21e50409

Did this bring back bug # 4988 and bug # 5204?

On 2020-12-09 14:49:03 +0000, Tom Seddon wrote:

(In reply to Ozkan Sezer from comment # 17)

Would this not bring back bug # 4988 and bug # 5204 ?

Ugh, yes, sorry about that... looks like it might.

If none of -ObjC/-x objective-c/check_objc_source_compiles is going to work for everybody, I suggest reverting my original patch, going with what was there before, and accepting that it won't work for me ;) - it's probably not a big problem, if I'm the first to report it.

This bug isn't actually terribly important to me, I just noticed it while looking at something else. I can't really justify spending any more time on it.

Thanks,

--Tom

On 2020-12-09 15:30:50 +0000, Sam Lantinga wrote:

Thanks for catching that, Ozkan.

On 2020-12-09 15:33:16 +0000, Sam Lantinga wrote:

Okay, I backed out the changes until we can sort this out:
https://hg.libsdl.org/SDL/rev/450cb7d8a67f

Thanks everyone!