bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
14.59k stars 1.92k forks source link

Restore support for older macOS/iOS version. Determine features at init, and use at runtime. #3284

Closed mcourteaux closed 1 month ago

mcourteaux commented 2 months ago

A lot of the warnings could be silenced by doing what Apple suggests: using the @available check at the code. However, I agree that readability increases if you name the feature you are testing for, instead of just random-looking version number checks.

Examples still work.

bkaradzic commented 2 months ago

Instead of:

CHECK_FEATURE_AVAILABLE(m_usesNewMetalAPI, macOS 13.0, iOS 16.0, tvOS 16.0, macCatalyst 16.0, VISION_OS_MINIMUM *);

Why you don't do this?

m_usesNewMetalAPI = @available(macOS 13.0, iOS 16.0, tvOS 16.0, macCatalyst 16.0, VISION_OS_MINIMUM *);

And what's this macCatalyst?

bkaradzic commented 2 months ago

I just read that @available is executed only on startup, and it's not evaluated multiple times in runtime?

bkaradzic commented 2 months ago

Also you might want to switch visionOS to #available visionOS since #available is evaluated during compile time.

mcourteaux commented 2 months ago

Also you might want to switch visionOS to #available visionOS since #available is evaluated during compile time.

I thought #available is a Swift feature.

I just read that @available is executed only on startup, and it's not evaluated multiple times in runtime? Well, given that Apple so aggressively wants us to use it everywhere, that would make sense. I haven't read anything about it's performance or how it's implemented.

And what's this macCatalyst?

It seems to be some SDK variant that works for both macOS and iPad. Like where you can write your app once against the macCatalyst SDK and you get it on both.

Why you don't do this?

 m_usesNewMetalAPI = @available(macOS 13.0, iOS 16.0, tvOS 16.0, macCatalyst 16.0, VISION_OS_MINIMUM *);

Compiler complains that this is not the correct way to use @available. Literally the only valid way seems to be: if (@available(...)) { ... } with potentially an else branch. Even putting ! in front of the @available() is not valid. Compiler will warn that it's not actually going to guard against version checks. Some folks online said that this @available is not a boolean expression :shrug:. Only valid to use in an if-condition. That would suggest they do some clever codegen regarding those branches. I wouldn't be surprised if it ends up being a single jmp [mem] instruction without any comparisons/tests. I may try to find what the compiler compiles here in the disassembly if I find some time. If performance is good or better, we could even do:

#define USES_NEW_API @available(macOS 13.0, iOS 16.0, tvOS 16.0, macCatalyst 16.0, VISION_OS_MINIMUM *)

if (USES_NEW_API) {

} else {

}
bkaradzic commented 2 months ago

I may try to find what the compiler compiles here in the disassembly if I find some time.

Yes, please do.

Because then this would be solution to have no warnings, and by looking at the code it would be clear what's going on:

#define USES_NEW_API @available(macOS 13.0, iOS 16.0, tvOS 16.0, macCatalyst 16.0, VISION_OS_MINIMUM *)

if (USES_NEW_API)
...
bkaradzic commented 2 months ago

I thought #available is a Swift feature.

From: https://developer.apple.com/documentation/swift/marking-api-availability-in-objective-c

In Swift, you use the @available attribute to control whether a declaration is available to use when building an app for a particular target platform. Similarly, you use the availability condition #available to execute code conditionally based on required platform and version conditions. Both kinds of availability specifier are also available in Objective-C.

bkaradzic commented 2 months ago

Also usesNewMetalAPI is bad feature name... You can imagine Metal will add even newer API in the future.

The best feature naming is to be specific about feature you're naming.

mcourteaux commented 2 months ago

Looking at a compiled version of a simple @available check, gives me this under -O2 (pseudocode generated by Hopper Disassembler):

int _main() {
    puts("before");
    COND = ___isPlatformVersionAtLeast(0x1, 0xd, 0x0, 0x0) == 0x0;
    rdi = "macOS 13.0";
    if (COND) {
            rdi = "NOT macOS 13.0";
    }
    puts(rdi);
    return 0x0;
}

int ___isPlatformVersionAtLeast(int arg0, int arg1, int arg2, int arg3) {
    stack[-16] = r15;
    stack[-24] = r14;
    stack[-32] = r12;
    stack[-40] = rbx;
    rsp = rsp - 0x30;
    r14 = arg3;
    r12 = arg2;
    rbx = arg1;
    rbp = arg0;
    if (*_DispatchOnceCounter == 0xffffffffffffffff) {
            if (*_AvailabilityVersionCheck != 0x0) {
                    rax = _AvailabilityVersionCheck(0x1, &var_38, r14 & 0xff | r12 << 0x8 & 0xffff | rbx << 0x10, r12 << 0x8 & 0xffff | rbx << 0x10);
                    r15 = rax;
            }
            else {
                    r15 = 0x1;
                    if (rbp == 0x1) {
                            if (*_CompatibilityDispatchOnceCounter == 0xffffffffffffffff) {
                                    r15 = 0x1;
                                    if (*(int32_t *)_GlobalMajor <= rbx) {
                                            if (*(int32_t *)_GlobalMajor >= rbx) {
                                                    r15 = 0x1;
                                                    if (*(int32_t *)_GlobalMinor <= r12) {
                                                            if (*(int32_t *)_GlobalMinor < r12) {
                                                                    r15 = 0x0;
                                                            }
                                                            else {
                                                                    r15 = *(int32_t *)_GlobalSubminor >= r14 ? 0x1 : 0x0;
                                                            }
                                                    }
                                            }
                                            else {
                                                    r15 = 0x0;
                                            }
                                    }
                            }
                            else {
                                    dispatch_once_f(_CompatibilityDispatchOnceCounter, 0x0, _compatibilityInitializeAvailabilityCheck);
                                    if (*(int32_t *)_GlobalMajor > rbx) {
                                            r15 = 0x1;
                                    }
                                    else {
                                            if (*(int32_t *)_GlobalMajor >= rbx) {
                                                    r15 = 0x1;
                                                    if (*(int32_t *)_GlobalMinor <= r12) {
                                                            if (*(int32_t *)_GlobalMinor < r12) {
                                                                    r15 = 0x0;
                                                            }
                                                            else {
                                                                    r15 = *(int32_t *)_GlobalSubminor >= r14 ? 0x1 : 0x0;
                                                            }
                                                    }
                                            }
                                            else {
                                                    r15 = 0x0;
                                            }
                                    }
                            }
                    }
            }
    }
    else {
            dispatch_once_f(_DispatchOnceCounter, 0x0, _initializeAvailabilityCheck);
            if (*_AvailabilityVersionCheck == 0x0) {
                    r15 = 0x1;
                    if (rbp == 0x1) {
                            if (*_CompatibilityDispatchOnceCounter == 0xffffffffffffffff) {
                                    r15 = 0x1;
                                    if (*(int32_t *)_GlobalMajor <= rbx) {
                                            if (*(int32_t *)_GlobalMajor >= rbx) {
                                                    r15 = 0x1;
                                                    if (*(int32_t *)_GlobalMinor <= r12) {
                                                            if (*(int32_t *)_GlobalMinor < r12) {
                                                                    r15 = 0x0;
                                                            }
                                                            else {
                                                                    r15 = *(int32_t *)_GlobalSubminor >= r14 ? 0x1 : 0x0;
                                                            }
                                                    }
                                            }
                                            else {
                                                    r15 = 0x0;
                                            }
                                    }
                            }
                            else {
                                    dispatch_once_f(_CompatibilityDispatchOnceCounter, 0x0, _compatibilityInitializeAvailabilityCheck);
                                    if (*(int32_t *)_GlobalMajor > rbx) {
                                            r15 = 0x1;
                                    }
                                    else {
                                            if (*(int32_t *)_GlobalMajor >= rbx) {
                                                    r15 = 0x1;
                                                    if (*(int32_t *)_GlobalMinor <= r12) {
                                                            if (*(int32_t *)_GlobalMinor < r12) {
                                                                    r15 = 0x0;
                                                            }
                                                            else {
                                                                    r15 = *(int32_t *)_GlobalSubminor >= r14 ? 0x1 : 0x0;
                                                            }
                                                    }
                                            }
                                            else {
                                                    r15 = 0x0;
                                            }
                                    }
                            }
                    }
            }
            else {
                    rax = _AvailabilityVersionCheck(0x1, &var_38, r14 & 0xff | r12 << 0x8 & 0xffff | rbx << 0x10, r12 << 0x8 & 0xffff | rbx << 0x10);
                    r15 = rax;
            }
    }
    if (**___stack_chk_guard == **___stack_chk_guard) {
            rax = r15 & 0xff;
    }
    else {
            rax = __stack_chk_fail();
    }
    return rax;
}

So it does get initialized once, but afterwards the check is still quite involved as it doesn't merge the major,minor,patch numbers into one big number, so a lot of branches are evaluated needlessly... 😞 Our approach should still be faster unfortunately. Code of the helper function does not change under different levels -O0 to -O3.

bkaradzic commented 2 months ago

Is there way to disable deprecated warnings with some define before including Apple's header files? Or we have to do it with pragma? Since you're checking features with @available, disabling deprecated warnings across whole file is option.

mcourteaux commented 1 month ago

@bkaradzic Have a look at the latest version. I silenced the warnings with specific pragmas. I am kind of bummed that the whole list of texture formats is silenced by one block, as it is informative to have the compiler yell at us if the list has a format that is not supported. This makes it easy to spot unsupported formats in the future as well. Right now we'd miss out on that information.

Other than that, I don't know if you would accept that I put the #pragmas on the indent of the actual code. In general I personally like that better for readability if it really matters on the depth of that code, but maybe you're against it in general. I can modify that if you want.

mcourteaux commented 1 month ago

From: https://developer.apple.com/documentation/swift/marking-api-availability-in-objective-c

In Swift, you use the @available attribute to control whether a declaration is available to use when building an app for a particular target platform. Similarly, you use the availability condition #available to execute code conditionally based on required platform and version conditions. Both kinds of availability specifier are also available in Objective-C.

I believe you misinterpreted that: I think what they mean is that in both Obj-C and Swift you can both specify and check for OS versions. The syntax is the languages is different, as documented below that.

Also usesNewMetalAPI is bad feature name... You can imagine Metal will add even newer API in the future.

The best feature naming is to be specific about feature you're naming.

I addressed this as well. It's now called m_usesMTLBindings.

bwrsandman commented 2 weeks ago

Hey, just want to outline that VISION_OS_MINIMUM might have to be 15.2 as 15.0 and 15.1 may not have visionOS SDK. See #3314