Skip to content

Commit

Permalink
Validate varying count when compiling shaders
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
clayjohn committed Feb 13, 2025
1 parent 296de7d commit 679d63b
Show file tree
Hide file tree
Showing 17 changed files with 94 additions and 29 deletions.
4 changes: 3 additions & 1 deletion drivers/d3d12/rendering_device_driver_d3d12.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3236,7 +3236,7 @@ Vector<uint8_t> RenderingDeviceDriverD3D12::shader_compile_binary_from_spirv(Vec
DEV_ASSERT(binding_info.res_class == (uint32_t)RES_CLASS_INVALID || binding_info.res_class == (uint32_t)res_class);
binding_info.res_class = res_class;
} else if (p_dxil_type == DXIL_RES_SAMPLER) {
binding_info.has_sampler = (uint32_t)true;
binding_info.has_sampler = (uint32_t) true;
} else {
CRASH_NOW();
}
Expand Down Expand Up @@ -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) + ".");
Expand Down
5 changes: 5 additions & 0 deletions drivers/gles3/storage/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions drivers/gles3/storage/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class Config {
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;
Expand Down
12 changes: 12 additions & 0 deletions drivers/gles3/storage/utilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions drivers/gles3/storage/utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ class Utilities : public RendererUtilities {
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
Expand Down
1 change: 1 addition & 0 deletions drivers/metal/metal_device_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ struct MetalLimits {
uint32_t maxVertexInputBindings;
uint32_t maxVertexInputBindingStride;
uint32_t maxDrawIndexedIndexValue;
uint32_t maxShaderVaryings;

double temporalScalerInputContentMinScale;
double temporalScalerInputContentMaxScale;
Expand Down
1 change: 1 addition & 0 deletions drivers/metal/metal_device_properties.mm
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@
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;
Expand Down
2 changes: 2 additions & 0 deletions drivers/metal/rendering_device_driver_metal.mm
Original file line number Diff line number Diff line change
Expand Up @@ -3999,6 +3999,8 @@ bool isArrayTexture(MTLTextureType p_type) {
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);
Expand Down
4 changes: 4 additions & 0 deletions drivers/vulkan/rendering_device_driver_vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 2 additions & 0 deletions servers/rendering/dummy/storage/utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ class Utilities : public RendererUtilities {
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
Expand Down
8 changes: 8 additions & 0 deletions servers/rendering/renderer_rd/storage_rd/utilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
2 changes: 2 additions & 0 deletions servers/rendering/renderer_rd/storage_rd/utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ class Utilities : public RendererUtilities {
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
Expand Down
3 changes: 2 additions & 1 deletion servers/rendering/rendering_device_commons.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
#include "core/object/object.h"
#include "core/variant/type_info.h"

#define STEPIFY(m_number, m_alignment) ((((m_number) + ((m_alignment) - 1)) / (m_alignment)) * (m_alignment))
#define STEPIFY(m_number, m_alignment) ((((m_number) + ((m_alignment)-1)) / (m_alignment)) * (m_alignment))

class RenderingDeviceCommons : public Object {
////////////////////////////////////////////
Expand Down Expand Up @@ -875,6 +875,7 @@ class RenderingDeviceCommons : public Object {
LIMIT_VRS_MAX_FRAGMENT_HEIGHT,
LIMIT_METALFX_TEMPORAL_SCALER_MIN_SCALE,
LIMIT_METALFX_TEMPORAL_SCALER_MAX_SCALE,
LIMIT_MAX_SHADER_VARYINGS,
};

enum Features {
Expand Down
19 changes: 2 additions & 17 deletions servers/rendering/shader_compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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);

Expand Down
31 changes: 21 additions & 10 deletions servers/rendering/shader_language.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -9111,17 +9112,12 @@ Error ShaderLanguage::_parse_shader(const HashMap<StringName, FunctionInfo> &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;
Expand Down Expand Up @@ -10959,13 +10955,27 @@ Error ShaderLanguage::_parse_shader(const HashMap<StringName, FunctionInfo> &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<StringName, ShaderNode::Varying> &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) {
_set_tkpos(kv.value.tkpos);
_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
Expand Down Expand Up @@ -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;

Expand Down
24 changes: 24 additions & 0 deletions servers/rendering/shader_language.h
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,28 @@ class ShaderLanguage {
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() {}
};

Expand Down Expand Up @@ -1022,6 +1044,7 @@ class ShaderLanguage {
String current_uniform_subgroup_name;

VaryingFunctionNames varying_function_names;
uint32_t base_varying_index = 0;

TkPos _get_tkpos() {
TkPos tkp;
Expand Down Expand Up @@ -1217,6 +1240,7 @@ class ShaderLanguage {
HashSet<String> 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);
Expand Down
2 changes: 2 additions & 0 deletions servers/rendering/storage/utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ class RendererUtilities {
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

0 comments on commit 679d63b

Please sign in to comment.