-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix macOS and iOS builds on older versions of Xcode #3116
Conversation
I'm not sure this is the right fix, yet, but it gets the build working for BabylonNative on older versions of Xcode. |
Better way of fixing this would be to define all those for version where they don't exist, so that code is just using logical name instead hardcoded value. |
Like this? enum GPUFamily {
Apple4 = 1004,
Apple5 = 1005,
Apple6 = 1006,
Apple7 = 1007,
Apple8 = 1008
};
...
if ([m_device respondsToSelector: @selector(supportsFamily:)])
{
if ([m_device supportsFamily: MTLGPUFamily(Apple4)])
{
g_caps.vendorId = BGFX_PCI_ID_APPLE;
if ([m_device supportsFamily: MTLGPUFamily(Apple8)])
{
g_caps.deviceId = 1008;
}
else if ([m_device supportsFamily: MTLGPUFamily(Apple7)])
{
g_caps.deviceId = 1007;
}
else if ([m_device supportsFamily: MTLGPUFamily(Apple6)])
{
g_caps.deviceId = 1006;
}
else if ([m_device supportsFamily: MTLGPUFamily(Apple5)])
{
g_caps.deviceId = 1005;
}
else
{
g_caps.deviceId = 1004;
}
}
}
|
Curiously this was how it was done before the commit that broke the build |
@bkaradzic Are you thinking this should be done like this comment suggests? |
On SDK version where |
Thanks @bkaradzic . Please take a look at the recent commits and let me know if anything else should be changed. |
I'm also having this issue. Will checkout this PR to test and report back! |
Yes! SilverNode works with this patch! I'm on macOS 12. |
Cool, thanks for testing! Just waiting on @bkaradzic to merge it in for us. |
@docEdub I'm waiting for this change: #3116 (comment) |
Define those missing defines in header file on platforms that don't have it. And then leave code that testing against defines as is. |
I would do this, but I have no ability to test on older versions. So you have to do it, make sure it works. |
I think my latest changes addressed this comment. |
As far as I can tell, the PR as it stands, does exactly that. @bkaradzic And it does work, as I tested it after the last commit to this PR. |
Build still seems to fail when new xcode is used to target older macos: |
The failing config has XCode 14 on macOS 12. |
5f564db This commit uses https://developer.apple.com/documentation/metal/MTLArgument?language=objc MTLArgument is available on iOS 8.0–16.0 https://developer.apple.com/documentation/metal/mtlbinding/3929840-isused?changes=_5 MTLBinding.isUsed is not well documented. Its behaviour is strange on my iPhone (iOS 16.6/iPhone 12 mini), it returns true only when I launch app from the XCode , and if I launch app outside of XCode, it always returns false. I doubt it is not the equivalent to the MTLArgument.isActive (https://developer.apple.com/documentation/metal/mtlargument/1461891-active?language=objc) |
This got integrated in #3284. Can be closed. |
Thank you! |
No description provided.