From 82f2fa1180e216320d7dbb9c3133bb4e26324c7e Mon Sep 17 00:00:00 2001 From: Stuart Carnie Date: Tue, 14 Apr 2026 07:00:03 +1000 Subject: [PATCH] Metal: Reduce warnings and prevent incompatible library code when baking - Remove compiler warnings whilst baking Metal source to binary libraries, so true errors are easier to spot. Speeds up compilation, with reduced STDOUT. - Prevent invalid Metal libraries when baking to older target OSs, which earlier MSL language support. In these cases, SPIRV-Cross generates emulated atomic code via buffers (rather than images), that isn't supported by Godot's Metal driver. These shaders are marked with a new INVALID flag that triggers a recompilation on the device, by signaling that the cache is invalid. --- drivers/metal/metal_device_profile.h | 13 ++ .../metal/rendering_device_driver_metal.cpp | 5 + .../rendering_shader_container_metal.cpp | 181 +++++++++++------- .../metal/rendering_shader_container_metal.h | 20 +- 4 files changed, 149 insertions(+), 70 deletions(-) diff --git a/drivers/metal/metal_device_profile.h b/drivers/metal/metal_device_profile.h index 1521a582b3..3ba4168eaa 100644 --- a/drivers/metal/metal_device_profile.h +++ b/drivers/metal/metal_device_profile.h @@ -122,6 +122,19 @@ struct MetalDeviceProfile { MinOsVersion min_os_version; Features features; + struct MinimumRequirements { + GPU gpu = GPU::Apple1; + uint32_t msl_version = 0; + + bool operator>(const MinimumRequirements &p_other) const { + return gpu > p_other.gpu || msl_version > p_other.msl_version; + } + }; + + MinimumRequirements get_minimum_requirements() const { + return { gpu, features.msl_version }; + } + static const MetalDeviceProfile *get_profile(Platform p_platform, GPU p_gpu, MinOsVersion p_min_os_version); MetalDeviceProfile() = default; diff --git a/drivers/metal/rendering_device_driver_metal.cpp b/drivers/metal/rendering_device_driver_metal.cpp index a914664f3c..ff9856df1e 100644 --- a/drivers/metal/rendering_device_driver_metal.cpp +++ b/drivers/metal/rendering_device_driver_metal.cpp @@ -1074,6 +1074,11 @@ RDD::ShaderID RenderingDeviceDriverMetal::shader_create_from_container(const Ref Ref shader_container = p_shader_container; using RSCM = RenderingShaderContainerMetal; + if (shader_container->is_invalid()) { + WARN_PRINT("Metal shader container is invalid and will be recompiled."); + return RDD::ShaderID(); + } + CharString shader_name = shader_container->shader_name; RSCM::HeaderData &mtl_reflection_data = shader_container->mtl_reflection_data; Vector &shaders = shader_container->shaders; diff --git a/drivers/metal/rendering_shader_container_metal.cpp b/drivers/metal/rendering_shader_container_metal.cpp index e0bc77e410..ac0bdc42c8 100644 --- a/drivers/metal/rendering_shader_container_metal.cpp +++ b/drivers/metal/rendering_shader_container_metal.cpp @@ -146,7 +146,15 @@ Error RenderingShaderContainerMetal::compile_metal_source(const char *p_source, // Build the .metallib binary. { - List args{ "-sdk", sdk, "metal", "-O3" }; + List args{ + "-sdk", + sdk, + "metal", + "-O3", + "-Wno-unused-variable", + "-Wno-uninitialized", + "-Wno-unused-function", + }; // Compile metal shaders for the minimum supported target instead of the host machine. switch (device_profile->platform) { @@ -188,8 +196,10 @@ Error RenderingShaderContainerMetal::compile_metal_source(const char *p_source, ERR_FAIL_COND_V_MSG(len == 0, ERR_CANT_CREATE, "Metal compiler created empty library"); } - // Strip the source from the binary. - { + // Strip the source from the binary. AIR versions before 2.4 (MSL 2.4) don't support + // companion MetalLib, so metal-dsymutil copies verbatim and emits a warning: + // "architecture air64_v23 does not support a companion MetalLib; copying verbatim" + if (device_profile->features.msl_version >= MSL_VERSION_24) { List args{ "-sdk", sdk, "metal-dsymutil", "--remove-source", result_file->get_path_absolute() }; String r_pipe; int exit_code; @@ -235,72 +245,79 @@ spv::ExecutionModel map_stage(RDD::ShaderStage p_stage) { return SHADER_STAGE_REMAP[p_stage]; } -Error RenderingShaderContainerMetal::reflect_spirv(const ReflectShader &p_shader) { - // const LocalVector &p_spirv = p_shader.shader_stages; - // - // using ShaderStage = RenderingDeviceCommons::ShaderStage; - // - // const uint32_t spirv_size = p_spirv.size(); - // - // HashSet atomic_spirv_ids; - // bool atomics_scanned = false; - // auto scan_atomic_accesses = [&atomic_spirv_ids, &p_spirv, spirv_size, &atomics_scanned]() { - // if (atomics_scanned) { - // return; - // } - // - // for (uint32_t i = 0; i < spirv_size + 0; i++) { - // const uint32_t STARTING_WORD_INDEX = 5; - // Span spirv = p_spirv[i].spirv(); - // const uint32_t *words = spirv.ptr() + STARTING_WORD_INDEX; - // while (words < spirv.end()) { - // uint32_t instruction = *words; - // uint16_t word_count = instruction >> 16; - // SpvOp opcode = (SpvOp)(instruction & 0xFFFF); - // if (opcode == SpvOpImageTexelPointer) { - // uint32_t image_var_id = words[3]; - // atomic_spirv_ids.insert(image_var_id); - // } - // words += word_count; - // } - // } - // - // atomics_scanned = true; - // }; - // - // for (uint32_t i = 0; i < spirv_size + 0; i++) { - // ShaderStage stage = p_spirv[i].shader_stage; - // ShaderStage stage_flag = (ShaderStage)(1 << p_spirv[i].shader_stage); - // SpvReflectResult result; - // - // const SpvReflectShaderModule &module = p_spirv[i].module(); - // - // uint32_t binding_count = 0; - // result = spvReflectEnumerateDescriptorBindings(&module, &binding_count, nullptr); - // CRASH_COND(result != SPV_REFLECT_RESULT_SUCCESS); - // - // if (binding_count > 0) { - // LocalVector bindings; - // bindings.resize_uninitialized(binding_count); - // result = spvReflectEnumerateDescriptorBindings(&module, &binding_count, bindings.ptr()); - // - // for (uint32_t j = 0; j < binding_count; j++) { - // const SpvReflectDescriptorBinding &binding = *bindings[j]; - // - // switch (binding.descriptor_type) { - // case SPV_REFLECT_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: - // case SPV_REFLECT_DESCRIPTOR_TYPE_SAMPLED_IMAGE: - // case SPV_REFLECT_DESCRIPTOR_TYPE_STORAGE_IMAGE: - // case SPV_REFLECT_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER: - // break; - // default: - // break; - // } - // } - // } - // } - // - return OK; +MetalDeviceProfile::MinimumRequirements RenderingShaderContainerMetal::inspect_spirv(const ReflectShader &p_shader) { + // Scan SPIR-V for OpImageTexelPointer to detect image atomic usage and determine + // the minimum GPU family and MSL version required. + // 32-bit atomics (R32ui/R32i) require Apple6+ and MSL 3.1+. + // 64-bit atomics (Rg32ui/Rg32i) require Apple8+ and MSL 3.1+. + + MetalDeviceProfile::MinimumRequirements reqs; + + for (const ReflectShaderStage &stage : p_shader.shader_stages) { + Span spirv = stage.spirv(); + + // SPIR-V header is 5 words (magic, version, generator, bound, schema). + HashSet atomic_image_ids; + const uint32_t *words = spirv.ptr() + 5; + while (words < spirv.end()) { + const uint32_t instruction = *words; + const uint16_t word_count = instruction >> 16; + spv::Op opcode = static_cast(instruction & 0xFFFF); + if (opcode == spv::OpImageTexelPointer) { + atomic_image_ids.insert(words[3]); + } + words += word_count; + } + + if (atomic_image_ids.is_empty()) { + continue; + } + + // Look up image formats via spv-reflect bindings. + const SpvReflectShaderModule &module = stage.module(); + uint32_t binding_count = 0; + spvReflectEnumerateDescriptorBindings(&module, &binding_count, nullptr); + + LocalVector bindings_heap; + constexpr uint32_t MAX_STACK_BINDINGS = 256; + + SpvReflectDescriptorBinding **bindings; + if (binding_count > MAX_STACK_BINDINGS) { + bindings_heap.resize(binding_count); + bindings = bindings_heap.ptr(); + } else { + bindings = ALLOCA_ARRAY(SpvReflectDescriptorBinding *, binding_count); + } + spvReflectEnumerateDescriptorBindings(&module, &binding_count, bindings); + + for (uint32_t i = 0; i < binding_count; i++) { + const SpvReflectDescriptorBinding &binding = *bindings[i]; + if (!atomic_image_ids.has(binding.spirv_id)) { + continue; + } + + switch (binding.image.image_format) { + case SpvImageFormatR32ui: + case SpvImageFormatR32i: + if (reqs.gpu < MetalDeviceProfile::GPU::Apple6) { + reqs.gpu = MetalDeviceProfile::GPU::Apple6; + } + reqs.msl_version = MAX(reqs.msl_version, MSL_VERSION_31); + break; + case SpvImageFormatRg32ui: + case SpvImageFormatRg32i: + if (reqs.gpu < MetalDeviceProfile::GPU::Apple8) { + reqs.gpu = MetalDeviceProfile::GPU::Apple8; + } + reqs.msl_version = MAX(reqs.msl_version, MSL_VERSION_31); + break; + default: + break; + } + } + } + + return reqs; } bool RenderingShaderContainerMetal::_set_code_from_spirv(const ReflectShader &p_shader) { @@ -312,6 +329,24 @@ bool RenderingShaderContainerMetal::_set_code_from_spirv(const ReflectShader &p_ if (export_mode) { _initialize_toolchain_properties(); + + // When baking shaders for export, check if the SPIR-V requires capabilities + // that the target profile can't support natively. SPIRV-Cross would emulate + // image atomics with auxiliary buffer bindings incompatible with Godot's binding + // layout. Return an empty baked shader so the runtime recompiles for the actual device. + MetalDeviceProfile::MinimumRequirements reqs = inspect_spirv(p_shader); + MetalDeviceProfile::MinimumRequirements target = device_profile->get_minimum_requirements(); + if (reqs > target) { + uint32_t req_maj, req_min, tgt_maj, tgt_min; + parse_msl_version(reqs.msl_version, req_maj, req_min); + parse_msl_version(target.msl_version, tgt_maj, tgt_min); + WARN_PRINT(vformat("Shader '%s' requires Apple%d / MSL %d.%d but target is Apple%d / MSL %d.%d. Shader will be compiled at runtime on the device.", + String(shader_name.ptr()), + static_cast(reqs.gpu) - 1000, req_maj, req_min, + static_cast(target.gpu) - 1000, tgt_maj, tgt_min)); + mtl_reflection_data.mark_invalid(); + return true; + } } // initialize Metal-specific reflection data @@ -726,6 +761,10 @@ uint32_t RenderingShaderContainerMetal::_to_bytes_reflection_extra_data(uint8_t } uint32_t RenderingShaderContainerMetal::_to_bytes_reflection_binding_uniform_extra_data(uint8_t *p_bytes, uint32_t p_index) const { + if (is_invalid()) { + return 0; + } + if (p_bytes != nullptr) { *(UniformData *)p_bytes = mtl_reflection_binding_set_uniforms_data[p_index]; } @@ -733,6 +772,10 @@ uint32_t RenderingShaderContainerMetal::_to_bytes_reflection_binding_uniform_ext } uint32_t RenderingShaderContainerMetal::_to_bytes_shader_extra_data(uint8_t *p_bytes, uint32_t p_index) const { + if (is_invalid()) { + return 0; + } + if (p_bytes != nullptr) { *(StageData *)p_bytes = mtl_shaders[p_index]; } diff --git a/drivers/metal/rendering_shader_container_metal.h b/drivers/metal/rendering_shader_container_metal.h index 9a3aad83cf..6de0e62e40 100644 --- a/drivers/metal/rendering_shader_container_metal.h +++ b/drivers/metal/rendering_shader_container_metal.h @@ -51,6 +51,9 @@ public: NEEDS_VIEW_MASK_BUFFER = 1 << 0, USES_ARGUMENT_BUFFERS = 1 << 1, NEEDS_DEBUG_LOGGING = 1 << 2, + + /// A special value indicating that the shader failed to compile, so the compiled library data is invalid and should be ignored. + INVALID_SHADER = 0xFFFFFFFF, }; /// The base profile that was used to generate this shader. @@ -107,8 +110,21 @@ public: flags &= ~NEEDS_DEBUG_LOGGING; } } + + bool is_invalid() const { + return flags == INVALID_SHADER; + } + + void mark_invalid() { + flags = INVALID_SHADER; + } }; + /// @brief Returns `true` if the shader failed to compile and the compiled library data is invalid. + bool is_invalid() const { + return mtl_reflection_data.is_invalid(); + } + struct StageData { uint32_t vertex_input_binding_mask = 0; uint32_t is_position_invariant = 0; ///< true if the position output is invariant @@ -148,6 +164,8 @@ public: return slot; case IndexType::ARG: return arg_buffer; + default: + CRASH_NOW_MSG("Unreachable"); } } }; @@ -176,7 +194,7 @@ private: Error compile_metal_source(const char *p_source, const StageData &p_stage_data, Vector &r_binary_data); - Error reflect_spirv(const ReflectShader &p_shader); + MetalDeviceProfile::MinimumRequirements inspect_spirv(const ReflectShader &p_shader); public: static constexpr uint32_t FORMAT_VERSION = 2;