From 35100396e4b56d4c28c6b1eb3b00e73d64235799 Mon Sep 17 00:00:00 2001 From: clayjohn Date: Wed, 12 Feb 2025 16:12:46 -0800 Subject: [PATCH] Validate varying count when compiling shaders This avoids crashing on devices when a number of varyings greater than the device limit is used. For now this accurately prints an error when compiling the shader, but the error text only pops up in the editor if the number of user varyings is above the limit. --- .../d3d12/rendering_device_driver_d3d12.cpp | 2 ++ drivers/gles3/storage/config.cpp | 5 +++ drivers/gles3/storage/config.h | 1 + drivers/gles3/storage/utilities.cpp | 12 +++++++ drivers/gles3/storage/utilities.h | 2 ++ drivers/metal/metal_device_properties.h | 1 + drivers/metal/metal_device_properties.mm | 1 + .../metal/rendering_device_driver_metal.mm | 2 ++ .../vulkan/rendering_device_driver_vulkan.cpp | 4 +++ servers/rendering/dummy/storage/utilities.h | 2 ++ .../renderer_rd/storage_rd/utilities.cpp | 8 +++++ .../renderer_rd/storage_rd/utilities.h | 2 ++ servers/rendering/rendering_device_commons.h | 1 + servers/rendering/shader_compiler.cpp | 19 ++---------- servers/rendering/shader_language.cpp | 31 +++++++++++++------ servers/rendering/shader_language.h | 24 ++++++++++++++ servers/rendering/storage/utilities.h | 2 ++ 17 files changed, 92 insertions(+), 27 deletions(-) diff --git a/drivers/d3d12/rendering_device_driver_d3d12.cpp b/drivers/d3d12/rendering_device_driver_d3d12.cpp index 88d0ac28ebe..29b36f14055 100644 --- a/drivers/d3d12/rendering_device_driver_d3d12.cpp +++ b/drivers/d3d12/rendering_device_driver_d3d12.cpp @@ -6234,6 +6234,8 @@ uint64_t RenderingDeviceDriverD3D12::limit_get(Limit p_limit) { case LIMIT_VRS_MAX_FRAGMENT_WIDTH: case LIMIT_VRS_MAX_FRAGMENT_HEIGHT: return vrs_capabilities.ss_max_fragment_size; + case LIMIT_MAX_SHADER_VARYINGS: + return MIN(D3D12_VS_OUTPUT_REGISTER_COUNT, D3D12_PS_INPUT_REGISTER_COUNT); default: { #ifdef DEV_ENABLED WARN_PRINT("Returning maximum value for unknown limit " + itos(p_limit) + "."); diff --git a/drivers/gles3/storage/config.cpp b/drivers/gles3/storage/config.cpp index 8e1d08274f4..f561cdac073 100644 --- a/drivers/gles3/storage/config.cpp +++ b/drivers/gles3/storage/config.cpp @@ -110,6 +110,11 @@ Config::Config() { glGetIntegerv(GL_MAX_TEXTURE_SIZE, &max_texture_size); glGetIntegerv(GL_MAX_VIEWPORT_DIMS, max_viewport_size); glGetInteger64v(GL_MAX_UNIFORM_BLOCK_SIZE, &max_uniform_buffer_size); + GLint max_vertex_output; + glGetIntegerv(GL_MAX_VERTEX_OUTPUT_COMPONENTS, &max_vertex_output); + GLint max_fragment_input; + glGetIntegerv(GL_MAX_FRAGMENT_INPUT_COMPONENTS, &max_fragment_input); + max_shader_varyings = (uint32_t)MIN(max_vertex_output, max_fragment_input) / 4; // sanity clamp buffer size to 16K..1MB max_uniform_buffer_size = CLAMP(max_uniform_buffer_size, 16384, 1048576); diff --git a/drivers/gles3/storage/config.h b/drivers/gles3/storage/config.h index 36fae084b0e..9727346cf60 100644 --- a/drivers/gles3/storage/config.h +++ b/drivers/gles3/storage/config.h @@ -62,6 +62,7 @@ public: GLint max_texture_size = 0; GLint max_viewport_size[2] = { 0, 0 }; GLint64 max_uniform_buffer_size = 0; + uint32_t max_shader_varyings = 0; int64_t max_renderable_elements = 0; int64_t max_renderable_lights = 0; diff --git a/drivers/gles3/storage/utilities.cpp b/drivers/gles3/storage/utilities.cpp index baf06c02bb1..4fa6444471b 100644 --- a/drivers/gles3/storage/utilities.cpp +++ b/drivers/gles3/storage/utilities.cpp @@ -465,4 +465,16 @@ Size2i Utilities::get_maximum_viewport_size() const { return Size2i(config->max_viewport_size[0], config->max_viewport_size[1]); } +uint32_t Utilities::get_maximum_shader_varyings() const { + Config *config = Config::get_singleton(); + ERR_FAIL_NULL_V(config, 31); + return config->max_shader_varyings; +} + +uint64_t Utilities::get_maximum_uniform_buffer_size() const { + Config *config = Config::get_singleton(); + ERR_FAIL_NULL_V(config, 65536); + return uint64_t(config->max_uniform_buffer_size); +} + #endif // GLES3_ENABLED diff --git a/drivers/gles3/storage/utilities.h b/drivers/gles3/storage/utilities.h index 88ac8020185..6be733b2e7a 100644 --- a/drivers/gles3/storage/utilities.h +++ b/drivers/gles3/storage/utilities.h @@ -226,6 +226,8 @@ public: virtual String get_video_adapter_api_version() const override; virtual Size2i get_maximum_viewport_size() const override; + virtual uint32_t get_maximum_shader_varyings() const override; + virtual uint64_t get_maximum_uniform_buffer_size() const override; }; } // namespace GLES3 diff --git a/drivers/metal/metal_device_properties.h b/drivers/metal/metal_device_properties.h index 853ddcaebc5..2f87f10ea4b 100644 --- a/drivers/metal/metal_device_properties.h +++ b/drivers/metal/metal_device_properties.h @@ -124,6 +124,7 @@ struct MetalLimits { uint32_t maxVertexInputBindings; uint32_t maxVertexInputBindingStride; uint32_t maxDrawIndexedIndexValue; + uint32_t maxShaderVaryings; double temporalScalerInputContentMinScale; double temporalScalerInputContentMaxScale; diff --git a/drivers/metal/metal_device_properties.mm b/drivers/metal/metal_device_properties.mm index 21553f7173d..2c7ff576f4b 100644 --- a/drivers/metal/metal_device_properties.mm +++ b/drivers/metal/metal_device_properties.mm @@ -303,6 +303,7 @@ void MetalDeviceProperties::init_limits(id p_device) { limits.maxVertexInputAttributes = 31; limits.maxVertexInputBindings = 31; limits.maxVertexInputBindingStride = (2 * KIBI); + limits.maxShaderVaryings = 31; // Accurate on Apple4 and above. See: https://developer.apple.com/metal/Metal-Feature-Set-Tables.pdf #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST limits.minUniformBufferOffsetAlignment = 64; diff --git a/drivers/metal/rendering_device_driver_metal.mm b/drivers/metal/rendering_device_driver_metal.mm index d66064249ac..b6f606bfeb9 100644 --- a/drivers/metal/rendering_device_driver_metal.mm +++ b/drivers/metal/rendering_device_driver_metal.mm @@ -3999,6 +3999,8 @@ uint64_t RenderingDeviceDriverMetal::limit_get(Limit p_limit) { return (uint64_t)((1.0 / limits.temporalScalerInputContentMaxScale) * 1000'000); case LIMIT_METALFX_TEMPORAL_SCALER_MAX_SCALE: return (uint64_t)((1.0 / limits.temporalScalerInputContentMinScale) * 1000'000); + case LIMIT_MAX_SHADER_VARYINGS: + return limits.maxShaderVaryings; UNKNOWN(LIMIT_VRS_TEXEL_WIDTH); UNKNOWN(LIMIT_VRS_TEXEL_HEIGHT); UNKNOWN(LIMIT_VRS_MAX_FRAGMENT_WIDTH); diff --git a/drivers/vulkan/rendering_device_driver_vulkan.cpp b/drivers/vulkan/rendering_device_driver_vulkan.cpp index f46015f1fc3..026c3660f90 100644 --- a/drivers/vulkan/rendering_device_driver_vulkan.cpp +++ b/drivers/vulkan/rendering_device_driver_vulkan.cpp @@ -5886,6 +5886,10 @@ uint64_t RenderingDeviceDriverVulkan::limit_get(Limit p_limit) { return vrs_capabilities.max_fragment_size.x; case LIMIT_VRS_MAX_FRAGMENT_HEIGHT: return vrs_capabilities.max_fragment_size.y; + case LIMIT_MAX_SHADER_VARYINGS: + // The Vulkan spec states that built in varyings like gl_FragCoord should count against this, but in + // practice, that doesn't seem to be the case. The validation layers don't even complain. + return MIN(limits.maxVertexOutputComponents / 4, limits.maxFragmentInputComponents / 4); default: ERR_FAIL_V(0); } diff --git a/servers/rendering/dummy/storage/utilities.h b/servers/rendering/dummy/storage/utilities.h index aa6ff80c6d1..d9c4d34d35a 100644 --- a/servers/rendering/dummy/storage/utilities.h +++ b/servers/rendering/dummy/storage/utilities.h @@ -94,6 +94,8 @@ public: virtual String get_video_adapter_api_version() const override { return String(); } virtual Size2i get_maximum_viewport_size() const override { return Size2i(); } + virtual uint32_t get_maximum_shader_varyings() const override { return 31; } // Fair assumption for everything except old OpenGL-only phones. + virtual uint64_t get_maximum_uniform_buffer_size() const override { return 65536; } // Fair assumption for all devices. }; } // namespace RendererDummy diff --git a/servers/rendering/renderer_rd/storage_rd/utilities.cpp b/servers/rendering/renderer_rd/storage_rd/utilities.cpp index 8ff1d2bc468..068d82d47cd 100644 --- a/servers/rendering/renderer_rd/storage_rd/utilities.cpp +++ b/servers/rendering/renderer_rd/storage_rd/utilities.cpp @@ -329,3 +329,11 @@ Size2i Utilities::get_maximum_viewport_size() const { int max_y = device->limit_get(RenderingDevice::LIMIT_MAX_VIEWPORT_DIMENSIONS_Y); return Size2i(max_x, max_y); } + +uint32_t Utilities::get_maximum_shader_varyings() const { + return RenderingDevice::get_singleton()->limit_get(RenderingDevice::LIMIT_MAX_SHADER_VARYINGS); +} + +uint64_t Utilities::get_maximum_uniform_buffer_size() const { + return RenderingDevice::get_singleton()->limit_get(RenderingDevice::LIMIT_MAX_UNIFORM_BUFFER_SIZE); +} diff --git a/servers/rendering/renderer_rd/storage_rd/utilities.h b/servers/rendering/renderer_rd/storage_rd/utilities.h index 96508fd3ff5..fbcc86b0942 100644 --- a/servers/rendering/renderer_rd/storage_rd/utilities.h +++ b/servers/rendering/renderer_rd/storage_rd/utilities.h @@ -117,6 +117,8 @@ public: virtual String get_video_adapter_api_version() const override; virtual Size2i get_maximum_viewport_size() const override; + virtual uint32_t get_maximum_shader_varyings() const override; + virtual uint64_t get_maximum_uniform_buffer_size() const override; }; } // namespace RendererRD diff --git a/servers/rendering/rendering_device_commons.h b/servers/rendering/rendering_device_commons.h index 67946fc4875..aa8beed3578 100644 --- a/servers/rendering/rendering_device_commons.h +++ b/servers/rendering/rendering_device_commons.h @@ -875,6 +875,7 @@ public: LIMIT_VRS_MAX_FRAGMENT_HEIGHT, LIMIT_METALFX_TEMPORAL_SCALER_MIN_SCALE, LIMIT_METALFX_TEMPORAL_SCALER_MAX_SCALE, + LIMIT_MAX_SHADER_VARYINGS, }; enum Features { diff --git a/servers/rendering/shader_compiler.cpp b/servers/rendering/shader_compiler.cpp index acb5d9b65be..501e60c888c 100644 --- a/servers/rendering/shader_compiler.cpp +++ b/servers/rendering/shader_compiler.cpp @@ -686,30 +686,14 @@ String ShaderCompiler::_dump_node_code(const SL::Node *p_node, int p_level, Gene vcode += _prestr(varying.precision, ShaderLanguage::is_float_type(varying.type)); vcode += _typestr(varying.type); vcode += " " + _mkid(varying_name); - uint32_t inc = 1U; + uint32_t inc = varying.get_size(); if (varying.array_size > 0) { - inc = (uint32_t)varying.array_size; - vcode += "["; vcode += itos(varying.array_size); vcode += "]"; } - switch (varying.type) { - case SL::TYPE_MAT2: - inc *= 2U; - break; - case SL::TYPE_MAT3: - inc *= 3U; - break; - case SL::TYPE_MAT4: - inc *= 4U; - break; - default: - break; - } - vcode += ";\n"; // GLSL ES 3.0 does not allow layout qualifiers for varyings if (!RS::get_singleton()->is_low_end()) { @@ -1481,6 +1465,7 @@ Error ShaderCompiler::compile(RS::ShaderMode p_mode, const String &p_code, Ident info.render_modes = ShaderTypes::get_singleton()->get_modes(p_mode); info.shader_types = ShaderTypes::get_singleton()->get_types(); info.global_shader_uniform_type_func = _get_global_shader_uniform_type; + info.base_varying_index = actions.base_varying_index; Error err = parser.compile(p_code, info); diff --git a/servers/rendering/shader_language.cpp b/servers/rendering/shader_language.cpp index c99117c4612..e122c80b0fd 100644 --- a/servers/rendering/shader_language.cpp +++ b/servers/rendering/shader_language.cpp @@ -33,6 +33,7 @@ #include "core/os/os.h" #include "core/templates/local_vector.h" #include "servers/rendering/renderer_compositor.h" +#include "servers/rendering/rendering_server_globals.h" #include "servers/rendering_server.h" #include "shader_types.h" @@ -9111,17 +9112,12 @@ Error ShaderLanguage::_parse_shader(const HashMap &p_f int prop_index = 0; #ifdef DEBUG_ENABLED uint64_t uniform_buffer_size = 0; - uint64_t max_uniform_buffer_size = 0; + uint64_t max_uniform_buffer_size = 65536; int uniform_buffer_exceeded_line = -1; - - bool check_device_limit_warnings = false; - { - RenderingDevice *device = RenderingDevice::get_singleton(); - if (device != nullptr) { - check_device_limit_warnings = check_warnings && HAS_WARNING(ShaderWarning::DEVICE_LIMIT_EXCEEDED_FLAG); - - max_uniform_buffer_size = device->limit_get(RenderingDevice::LIMIT_MAX_UNIFORM_BUFFER_SIZE); - } + bool check_device_limit_warnings = check_warnings && HAS_WARNING(ShaderWarning::DEVICE_LIMIT_EXCEEDED_FLAG); + // Can be false for internal shaders created in the process of initializing the engine. + if (RSG::utilities) { + max_uniform_buffer_size = RSG::utilities->get_maximum_uniform_buffer_size(); } #endif // DEBUG_ENABLED ShaderNode::Uniform::Scope uniform_scope = ShaderNode::Uniform::SCOPE_LOCAL; @@ -10959,6 +10955,12 @@ Error ShaderLanguage::_parse_shader(const HashMap &p_f tk = _get_token(); } + uint32_t varying_index = base_varying_index; + uint32_t max_varyings = 31; + // Can be false for internal shaders created in the process of initializing the engine. + if (RSG::utilities) { + max_varyings = RSG::utilities->get_maximum_shader_varyings(); + } for (const KeyValue &kv : shader->varyings) { if (kv.value.stage != ShaderNode::Varying::STAGE_FRAGMENT && (kv.value.type > TYPE_BVEC4 && kv.value.type < TYPE_FLOAT) && kv.value.interpolation != INTERPOLATION_FLAT) { @@ -10966,6 +10968,14 @@ Error ShaderLanguage::_parse_shader(const HashMap &p_f _set_error(vformat(RTR("Varying with integer data type must be declared with `%s` interpolation qualifier."), "flat")); return ERR_PARSE_ERROR; } + + if (varying_index + kv.value.get_size() > max_varyings) { + _set_tkpos(kv.value.tkpos); + _set_error(vformat(RTR("Too many varyings used in shader (%d used, maximum supported is %d)."), varying_index + kv.value.get_size(), max_varyings)); + return ERR_PARSE_ERROR; + } + + varying_index += kv.value.get_size(); } #ifdef DEBUG_ENABLED @@ -11183,6 +11193,7 @@ Error ShaderLanguage::compile(const String &p_code, const ShaderCompileInfo &p_i global_shader_uniform_get_type_func = p_info.global_shader_uniform_type_func; varying_function_names = p_info.varying_function_names; + base_varying_index = p_info.base_varying_index; nodes = nullptr; diff --git a/servers/rendering/shader_language.h b/servers/rendering/shader_language.h index 1a119682f07..a774f8674a5 100644 --- a/servers/rendering/shader_language.h +++ b/servers/rendering/shader_language.h @@ -631,6 +631,28 @@ public: int array_size = 0; TkPos tkpos; + uint32_t get_size() const { + uint32_t size = 1; + if (array_size > 0) { + size = (uint32_t)array_size; + } + + switch (type) { + case TYPE_MAT2: + size *= 2; + break; + case TYPE_MAT3: + size *= 3; + break; + case TYPE_MAT4: + size *= 4; + break; + default: + break; + } + return size; + } + Varying() {} }; @@ -1022,6 +1044,7 @@ private: String current_uniform_subgroup_name; VaryingFunctionNames varying_function_names; + uint32_t base_varying_index = 0; TkPos _get_tkpos() { TkPos tkp; @@ -1217,6 +1240,7 @@ public: HashSet shader_types; GlobalShaderUniformGetTypeFunc global_shader_uniform_type_func = nullptr; bool is_include = false; + uint32_t base_varying_index = 0; }; Error compile(const String &p_code, const ShaderCompileInfo &p_info); diff --git a/servers/rendering/storage/utilities.h b/servers/rendering/storage/utilities.h index 387f71f78a0..b7506735437 100644 --- a/servers/rendering/storage/utilities.h +++ b/servers/rendering/storage/utilities.h @@ -184,6 +184,8 @@ public: virtual String get_video_adapter_api_version() const = 0; virtual Size2i get_maximum_viewport_size() const = 0; + virtual uint32_t get_maximum_shader_varyings() const = 0; + virtual uint64_t get_maximum_uniform_buffer_size() const = 0; }; #endif // RENDERER_UTILITIES_H