From 3f0c09f6ddde9ba0f995eb3d7e088bd650112891 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Foucault?= Date: Sun, 1 May 2022 22:26:54 +0200 Subject: [PATCH] GPUShaderCreateInfo: Add validation for overlapping vertex attribute Those are usually not supported but some driver silently fix them and most just silently fail (rendering error). --- .../overlay/shaders/infos/armature_info.hh | 4 +- .../overlay/shaders/infos/extra_info.hh | 4 +- .../gpu/intern/gpu_shader_create_info.cc | 46 +++++++++++++++++-- .../gpu/intern/gpu_shader_create_info.hh | 3 +- ...nstance_varying_color_varying_size_info.hh | 6 +-- 5 files changed, 51 insertions(+), 12 deletions(-) diff --git a/source/blender/draw/engines/overlay/shaders/infos/armature_info.hh b/source/blender/draw/engines/overlay/shaders/infos/armature_info.hh index f417fbe1fd4..d89272a50cf 100644 --- a/source/blender/draw/engines/overlay/shaders/infos/armature_info.hh +++ b/source/blender/draw/engines/overlay/shaders/infos/armature_info.hh @@ -43,8 +43,8 @@ GPU_SHADER_CREATE_INFO(overlay_armature_sphere_solid) .do_static_compilation(true) .vertex_in(0, Type::VEC2, "pos") /* Per instance. */ - .vertex_in(1, Type::MAT4, "inst_obmat") - .vertex_in(2, Type::VEC4, "color") + .vertex_in(1, Type::VEC4, "color") + .vertex_in(2, Type::MAT4, "inst_obmat") // .depth_layout(DepthLayout::GREATER) /* TODO */ .vertex_out(overlay_armature_sphere_solid_iface) .vertex_source("armature_sphere_solid_vert.glsl") diff --git a/source/blender/draw/engines/overlay/shaders/infos/extra_info.hh b/source/blender/draw/engines/overlay/shaders/infos/extra_info.hh index 1ab9ab3b23f..a765d881682 100644 --- a/source/blender/draw/engines/overlay/shaders/infos/extra_info.hh +++ b/source/blender/draw/engines/overlay/shaders/infos/extra_info.hh @@ -16,8 +16,8 @@ GPU_SHADER_CREATE_INFO(overlay_extra) .vertex_in(0, Type::VEC3, "pos") .vertex_in(1, Type::INT, "vclass") /* Instance attributes. */ - .vertex_in(2, Type::MAT4, "inst_obmat") - .vertex_in(3, Type::VEC4, "color") + .vertex_in(2, Type::VEC4, "color") + .vertex_in(3, Type::MAT4, "inst_obmat") .vertex_out(overlay_extra_iface) .fragment_out(0, Type::VEC4, "fragColor") .fragment_out(1, Type::VEC4, "lineOutput") diff --git a/source/blender/gpu/intern/gpu_shader_create_info.cc b/source/blender/gpu/intern/gpu_shader_create_info.cc index 6e82201b424..f5b90989481 100644 --- a/source/blender/gpu/intern/gpu_shader_create_info.cc +++ b/source/blender/gpu/intern/gpu_shader_create_info.cc @@ -41,6 +41,8 @@ void ShaderCreateInfo::finalize() Set deps_merged; + validate_vertex_attributes(); + for (auto &info_name : additional_infos_) { const ShaderCreateInfo &info = *reinterpret_cast( gpu_shader_create_info_get(info_name.c_str())); @@ -55,6 +57,8 @@ void ShaderCreateInfo::finalize() vertex_out_interfaces_.extend(info.vertex_out_interfaces_); geometry_out_interfaces_.extend(info.geometry_out_interfaces_); + validate_vertex_attributes(&info); + push_constants_.extend(info.push_constants_); defines_.extend(info.defines_); @@ -69,7 +73,7 @@ void ShaderCreateInfo::finalize() depth_write_ = info.depth_write_; } - validate(info); + validate_merge(info); auto assert_no_overlap = [&](const bool test, const StringRefNull error) { if (!test) { @@ -164,7 +168,7 @@ std::string ShaderCreateInfo::check_error() const return error; } -void ShaderCreateInfo::validate(const ShaderCreateInfo &other_info) +void ShaderCreateInfo::validate_merge(const ShaderCreateInfo &other_info) { if (!auto_resource_location_) { /* Check same bind-points usage in OGL. */ @@ -220,8 +224,42 @@ void ShaderCreateInfo::validate(const ShaderCreateInfo &other_info) } } } - { - /* TODO(@fclem): Push constant validation. */ +} + +void ShaderCreateInfo::validate_vertex_attributes(const ShaderCreateInfo *other_info) +{ + uint32_t attr_bits = 0; + for (auto &attr : vertex_inputs_) { + if (attr.index >= 16 || attr.index < 0) { + std::cout << name_ << ": \"" << attr.name + << "\" : Type::MAT3 unsupported as vertex attribute." << std::endl; + BLI_assert(0); + } + if (attr.index >= 16 || attr.index < 0) { + std::cout << name_ << ": Invalid index for attribute \"" << attr.name << "\"" << std::endl; + BLI_assert(0); + } + uint32_t attr_new = 0; + if (attr.type == Type::MAT4) { + for (int i = 0; i < 4; i++) { + attr_new |= 1 << (attr.index + i); + } + } + else { + attr_new |= 1 << attr.index; + } + + if ((attr_bits & attr_new) != 0) { + std::cout << name_ << ": Attribute \"" << attr.name + << "\" overlap one or more index from another attribute." + " Note that mat4 takes up 4 indices."; + if (other_info) { + std::cout << " While merging " << other_info->name_ << std::endl; + } + std::cout << std::endl; + BLI_assert(0); + } + attr_bits |= attr_new; } } diff --git a/source/blender/gpu/intern/gpu_shader_create_info.hh b/source/blender/gpu/intern/gpu_shader_create_info.hh index 7c827e237eb..4927ef75a75 100644 --- a/source/blender/gpu/intern/gpu_shader_create_info.hh +++ b/source/blender/gpu/intern/gpu_shader_create_info.hh @@ -801,7 +801,8 @@ struct ShaderCreateInfo { std::string check_error() const; /** Error detection that some backend compilers do not complain about. */ - void validate(const ShaderCreateInfo &other_info); + void validate_merge(const ShaderCreateInfo &other_info); + void validate_vertex_attributes(const ShaderCreateInfo *other_info = nullptr); /** \} */ diff --git a/source/blender/gpu/shaders/infos/gpu_shader_instance_varying_color_varying_size_info.hh b/source/blender/gpu/shaders/infos/gpu_shader_instance_varying_color_varying_size_info.hh index 4885efc66c2..fb7bbcbc3e5 100644 --- a/source/blender/gpu/shaders/infos/gpu_shader_instance_varying_color_varying_size_info.hh +++ b/source/blender/gpu/shaders/infos/gpu_shader_instance_varying_color_varying_size_info.hh @@ -10,9 +10,9 @@ GPU_SHADER_CREATE_INFO(gpu_shader_instance_varying_color_varying_size) .vertex_in(0, Type::VEC3, "pos") - .vertex_in(1, Type::MAT4, "InstanceModelMatrix") - .vertex_in(2, Type::VEC4, "color") - .vertex_in(3, Type::FLOAT, "size") + .vertex_in(1, Type::VEC4, "color") + .vertex_in(2, Type::FLOAT, "size") + .vertex_in(3, Type::MAT4, "InstanceModelMatrix") .vertex_out(flat_color_iface) .fragment_out(0, Type::VEC4, "fragColor") .push_constant(Type::MAT4, "ViewProjectionMatrix")