Add Metal support for macOS (arm64) and iOS#88199
Conversation
There was a problem hiding this comment.
I added this to be consistent with RENDERING_DRIVER_D3D12, but I don't see how this is used anywhere.
There was a problem hiding this comment.
These don't seem used anywhere indeed, we could flag this enum as deprecated.
It used to be used in 3.x for OS::get_current_video_driver() (7a79eee), and was refactored for 4.x in f82deaa, but the API that used it no longer exists. I'm not sure what replaced it in 4.x, CC @Calinou.
We also have this piece of dead code to remove eventually:
editor/plugins/node_3d_editor_plugin.cpp
5339: //if (OS::get_singleton()->get_current_video_driver() == OS::RENDERING_DRIVER_OPENGL3) {
There was a problem hiding this comment.
I'm not sure what replaced it in 4.x, CC @Calinou.
We seem to have:
core/os/os.h:
String get_current_rendering_driver_name() const { return _current_rendering_driver_name; }
String get_current_rendering_method() const { return _current_rendering_method; }
but not exposed to scripting.
There was a problem hiding this comment.
I have a PR to expose these: #85430
It's awaiting review (specifically about moving it from OS to Engine). It's not exposed currently, so moving it doesn't break compatibility.
There was a problem hiding this comment.
Remove a series of deprecated warnings when building for newer macOS SDKs
There was a problem hiding this comment.
We have to use exceptions for this code as SPIRV-Cross uses them to communicate errors when parsing SPIRV or compiling SPIRV to Metal. The alternative is that it asserts and terminates the process.
There was a problem hiding this comment.
We usually try to patch out exceptions from the third-party code instead, but it's probably ok in this case (it's always possible to do it later, if we want to), disabling exceptions is only important for the Web.
There was a problem hiding this comment.
Sounds good – fortunately, we don't need SPIRV-Cross for Web so hopefully that is ok for the short term. I can look at how complex it might be to remove exceptions in a subsequent PR?
There was a problem hiding this comment.
How is this related to this? Should be in a separate PR if it's not specifically because of the metal changes
There was a problem hiding this comment.
It generates a number of warnings when compiling for arm64 with Metal enabled – happy to remove it, but it was a lot of noise.
There was a problem hiding this comment.
As long as it's from this specific change then it's fine :)
There was a problem hiding this comment.
My suggestion would be to split this into a separate PR and merge it. It seems like an unrelated change even if it is triggered by the Metal build.
There was a problem hiding this comment.
Yeah I agree a separate PR with that fix could be good, so we can also backport it to 4.3.x.
It's likely that this kind of deprecation warning will find its way even in non-Metal builds, I'm not sure why Metal specifically would enable it.
4fb1a60 to
fac4753
Compare
There was a problem hiding this comment.
This generates a warning, as CFBundleSignatures can only be 4 characters
There was a problem hiding this comment.
I'd suggest splitting the changes to that file to a separate PR, as it seems like things we'd need regardless of Metal.
There was a problem hiding this comment.
As noted elsewhere, SPIRV-Cross requires exceptions, otherwise any errors parsing SPIRV or generating Metal source aborts.
There was a problem hiding this comment.
Hope these are ok, as I noticed the loops weren't exiting after they found the index
There was a problem hiding this comment.
I updated these to remove some compiler warnings for newer SDKs
There was a problem hiding this comment.
I think this could also be merged (maybe together with the coreaudio change) independently of the Metal backend.
There was a problem hiding this comment.
Indeed, I'd prefer these in a separate PR. They could even have been merged for 4.3 instead of waiting for the 4.4 dev cycle for a big feature like Metal.
fac4753 to
a7a6669
Compare
bruvzg
left a comment
There was a problem hiding this comment.
The only acceptable reasons to bump min. supported OS version are:
- latest Xcode no longer support it (currently 10.12 is oldest supported one).
- it's absolutely unusable due to core changes.
8bcdb01 to
07bb489
Compare
There was a problem hiding this comment.
Isn't Vulkan SPIR-V generated via our copy of glslang (which we sync with other Vulkan SDK components)? That would still be a justification IMO to at least try to keep them in sync, as Khronos typically only supports all components being on the same known_good.json versions, which is best tracked via Vulkan SDK tags in my experience.
There was a problem hiding this comment.
That is correct - we use SPIRV-Cross to consume the SPIR-V generated from Godot's glslang and to transform it to Metal source files. Those shouldn't need to match, as it is only used as a code generator. Newer versions of the SPRIV-Cross code have fixes when producing Metal source that we want, particularly for iOS.
|
I added a second commit to do some tweaks directly, to be squashed in the main commit after review and testing. Changes:
I did these changes from Linux, please confirm that it still works as expected on macOS. |
|
I extracted the fixes to Xcode compiler warnings to a separate PR: #95854. Edit: I'm done force-pushing tweaks, 84201ef can be reviewed. |
|
@akien-mga I reviewed 84201ef and it looks great – thank you! |
|
I noticed that it's taking significantly more time to open a new project with this PR compared to the current master Master: ~5 seconds I'll test with non-empty projects too Edit: The bistro scene opens in ~30 seconds with both Vulkan and Metal, however the Metal version actually freezes after the editor opens and it's becoming completely uninteractable with the "wait" cursor for a solid 40 seconds or so. Noticed that something was off yesterday but wanted to confirm that it's not some other PR's fault first If this is to be merged in its current state I'm afraid I'll need the |
|
I confirmed that this builds fine with osxcross on our official buildsystem. Here's a test build from this PR for macOS and iOS:
You can disable it with |
ah ok then |
bruvzg
left a comment
There was a problem hiding this comment.
Seems to be working fine.
I do not get significant difference between Vulkan and Metal startup times (for a relatively small project, both are about 4-5 sec, only the first start with cleaned shader cache seems to be few seconds slower).
Give this one a shot when you have time |
|
|
@passivestar you can also switch drivers using the CLI: --rendering-driver metal|vulkanYou will need to open the project directly from the shell to do that. The easiest way is to change to the project directory and launch Godot with the |
|
I am very happy to finally merge this. The work you have been doing is excellent and is going to benefit a lot of people. Let's sneak this in before dev 1 so we can get lots of early testing this release cycle! |
|
@passivestar I've found the difference between MoltenVK and our Metal renderer in Godot, and I expect to be able to significantly reduce that initial compilation time. Going to prototype it tomorrow and push up a PR |
|
@passivestar / @bruvzg I have some very dramatic improvements to shader compilation, which are addressed in #96052 |
|
How can I enable metal support for intel based mac for testing ? I have one. |
@eimfach You can't, he already talked about this here #88199 (comment) |
|
Hey there, found a small bug in this PR when trying to build the iOS simulator templates, this line https://github.com/godotengine/godot/blob/master/drivers/metal/pixel_formats.mm#L1203 uses |
Hey @MileyHollenberg – thanks for spotting that! |
This PR introduces initial support for a Metal rendering device driver for macOS (arm64) and iOS.
I addedmetal=yesto the builds to ensure the code is compiled as part of CIEdit:
metalis enabled by default on macOS and iOS, so also included on CI.Summary
This PR adds native Metal support to Godot for macOS and iOS. Metal is Apple's native graphics API for all its platforms. DirectX is to Windows, as Metal is to macOS, iOS and tvOS.
Supported Features
This initial implementation should support all of Godot's 2D and 3D features, except multiple viewports.
Known Issues
Instruments, Xcode and profiling
Apple developers may be familiar with Instruments and Xcode's Metal debugging tools. We want to make sure that you have a good experience with these tools when developing with Godot.
Xcode frame debugging
The Metal
RenderDeviceDriverutilisesMTLCaptureScopesto create a Godot Frame for accurate frame boundaries, to simplify your experience in Xcode:Render passes are also labeled, so you can visualise and understand where your frame costs are:
Instruments
As expected, you can use Instruments to profile your games. We plan to enhance the experience using signposts, so you can get accurate statistics about different parts of the Metal render driver. Once such example is shader compilation:
This image is showing all the shaders that were compiled to run the Bistro demo under the Godot editor.
Note
The macOS Metal shader cache for Godot was cleared, so this is worst-case.
FAQ
Can I still use Vulkan for my projects?
Metal is the default rendering driver for Apple platforms, as it should perform as well or better than the current MoltenVK / Vulkan implementation. However, if you want to use Vulkan via MoltenVK, you can by following these steps:
Open Project Settings
Enable Advanced Settings
Select Rendering Device
Choose
vulkanfor your target Apple platformCan I still launch the Godot editor with Vulkan?
Yes, you can launch the editor using your terminal and launching Godot from your project directory, specifying
metalorvulkanfor therendering-driverargument. The following, with the--editorargument, will launch the Godot editor:Note
If you want to launch the game with a specific driver, drop the
--editorflag.