Metal: Add support for 2017 era iOS devices #99820
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
stuartcarnie
force-pushed
the
issue_99682
branch
from
9b3eb4c to
43cf5cc
Compare
stuartcarnie
changed the title
stuartcarnie
force-pushed
the
issue_99682
branch
3 times, most recently
from
edc5a55 to
0df4009
Compare
|
@stuartcarnie Does this PR also fall back to vulkan for iOS devices older than 2017? |
|
Or is there a separate work in progress for that? |
|
Vulkan won't work on devices prior to 2017 either, as Godot requires image cube map textures, which are only supported on 2017 devices and beyond. |
|
Gotcha. Does it fall back to compatibility from metal then? |
|
To put it another way, there should be no reason to require Vulkan / MoltenVK over Metal for Apple mobile devices, as they support the same range of devices. For desktop, Vulkan / MoltenVK is required for x86 hardware. |
Not sure – would have to ask the rendering team if that is the case. |
stuartcarnie
force-pushed
the
issue_99682
branch
from
0df4009 to
aa2562e
Compare
| @@ -61,7 +61,7 @@ class API_AVAILABLE(macos(11.0), ios(14.0)) RenderingDeviceDriverMetal : public | |||
|
|
|||
| uint32_t version_major = 2; | |||
| uint32_t version_minor = 0; | |||
| MetalDeviceProperties *metal_device_properties = nullptr; | |||
| MetalDeviceProperties *device_properties = nullptr; | |||
There was a problem hiding this comment.
I dropped the metal_ prefix, as it is obviously for metal 🤦🏻
| @@ -1058,7 +1058,7 @@ static const API_AVAILABLE(macos(11.0), ios(14.0)) MTLSamplerBorderColor SAMPLER | |||
|
|
|||
| #pragma mark - Shader | |||
|
|
|||
| const uint32_t SHADER_BINARY_VERSION = 3; | |||
| const uint32_t SHADER_BINARY_VERSION = 4; | |||
There was a problem hiding this comment.
We have to include a flag to indicate whether the shader was compiled to generate an ABI with argument buffers or classic binding model.
| uint32_t is_compute = UINT32_MAX; | ||
| uint32_t needs_view_mask_buffer = UINT32_MAX; |
There was a problem hiding this comment.
I dropped is_compute as we don't need to store it, and moved any boolean flags to a single uint32_t, so it is easier to add them in the future.
| if (String v = OS::get_singleton()->get_environment(U"GODOT_DISABLE_ARGUMENT_BUFFERS"); v == U"1") { | ||
| disable_argument_buffers = true; | ||
| } |
There was a problem hiding this comment.
The GODOT_DISABLE_ARGUMENT_BUFFERS environment variable is an undocumented feature I use for debugging purposes. I was able to test out the classic ABI binding (no argument buffers) for a number of projects and they ran without issue
stuartcarnie
force-pushed
the
issue_99682
branch
from
aa2562e to
952cd79
Compare
| std::string source; | ||
| try { | ||
| source = compiler.compile(); | ||
| } catch (CompilerError &e) { | ||
| ERR_FAIL_V_MSG(Result(), "Failed to compile stage " + String(SHADER_STAGE_NAMES[stage]) + ": " + e.what()); | ||
| } |
There was a problem hiding this comment.
It's possible that compile will throw an exception
| @@ -2088,8 +2141,8 @@ void deserialize(BufReader &p_reader) { | |||
| return res; | |||
| }; | |||
|
|
|||
| auto descriptor_bindings = [&compiler, &active, &uniform_sets, stage, &get_decoration](SmallVector<Resource> &resources, Writable writable) { | |||
| for (Resource const &res : resources) { | |||
| auto descriptor_bindings = [&compiler, &active, &uniform_sets, stage, &get_decoration](SmallVector<Resource> &p_resources, Writable p_writable) { | |||
There was a problem hiding this comment.
I renamed the arguments of the lambda to include the p_ prefix
| // We need to regenerate the shader if the cache is moved to an incompatible device. | ||
| ERR_FAIL_COND_V_MSG(device_properties->features.argument_buffers_tier < MTLArgumentBuffersTier2 && binary_data.uses_argument_buffers(), | ||
| ShaderID(), | ||
| "Shader was generated with argument buffers, but device has limited support"); | ||
|
|
There was a problem hiding this comment.
This ensures that if someone copies the Godot shader_cache to a device that supports a lower argument buffer tier, that it won't crash, but regenerate the shaders.
| @@ -249,7 +249,7 @@ | |||
| const MDSubpass &subpass = render.get_subpass(); | |||
|
|
|||
| uint32_t vertex_count = p_rects.size() * 6 * subpass.view_count; | |||
| simd::float4 vertices[vertex_count]; | |||
| simd::float4 *vertices = ALLOCA_ARRAY(simd::float4, vertex_count); | |||
There was a problem hiding this comment.
Annoying compiler warning in my IDE, so I switched to alloca
| @@ -1093,9 +1047,373 @@ | |||
| } | |||
| } | |||
|
|
|||
| BoundUniformSet &MDUniformSet::boundUniformSetForShader(MDShader *p_shader, id<MTLDevice> p_device) { | |||
| void MDUniformSet::bind_uniforms_argument_buffers(MDShader *p_shader, MDCommandBuffer::RenderState &p_state) { | |||
There was a problem hiding this comment.
This is the original argument buffers implementation moved to a new method, so we can call the right binding, depending on whether the shader uses argument buffer ABI or classic ABI
|
Thanks! |
|
@stuartcarnie I got around to testing this in my fork. It appears to fix the black screen! However, I encountered this new error: It doesn't seem to have any impact that I can yet notice. It may be something I did wrong in my fork as well. I'll test with vanilla godot master to see if anything changes. Thank you again for all of the hard work you did on this! |
|
Thanks @stuartcarnie ! I look forward to testing it out! :) |
Closes #99682
Add support for Apple4 GPUs.
Tier 2 argument buffers are required for read / write texture support, specifically for iOS, so this PR introduces support for generating shaders and binding without argument buffers for devices that only support Tier 1 argument buffers.
Important
I do not have an Apple4 GPU to test on, but I added an environment variable to allow me to force the shaders to be compiles without argument buffer support and was able to render the editor and complex scenes like the Bistro Demo.
SPIRV-Cross Update
SPIRV-Cross is also updated to the latest, which includes fixes and support for mesh shaders, which will be used for future features.