From 44fd609c324aaff1dae8b02ee9ca55255b2a56aa Mon Sep 17 00:00:00 2001 From: Dustinspace217 Date: Fri, 16 Oct 2026 04:13:58 -0700 Subject: [PATCH 1/3] Vulkan: bind all four model-matrix attribute locations for decals The Vulkan shader input mask had one bit per listed attribute, so the pipeline dropped the MATRIX4 attributes at locations 9-11 and the decal vertex shader read three of its four instance-matrix columns from unbound inputs. No decal was ever drawn in Vulkan. The mask now covers four locations for VATTRIB_MODEL_MATRIX. It is built by shader_vertex_input_mask() in shader_types so both Vulkan call sites share it and a unit test covers it without a Vulkan build. Fixes #7866 --- code/graphics/shader_types.cpp | 12 ++++++++++++ code/graphics/shader_types.h | 8 ++++++++ code/graphics/vulkan/VulkanShader.cpp | 10 ++-------- test/src/graphics/test_shader_types.cpp | 25 +++++++++++++++++++++++++ test/src/source_groups.cmake | 1 + 5 files changed, 48 insertions(+), 8 deletions(-) create mode 100644 test/src/graphics/test_shader_types.cpp diff --git a/code/graphics/shader_types.cpp b/code/graphics/shader_types.cpp index 80a733b7c2a..f983803a28e 100644 --- a/code/graphics/shader_types.cpp +++ b/code/graphics/shader_types.cpp @@ -162,6 +162,18 @@ static ShaderVariantInfo SHADER_VARIANTS[] = { {SDR_TYPE_SHADOW_MAP_GEN, true, SDR_FLAG_SHADOW_FALLBACK, "GEOMETRY_FALLBACK", {}, "Use a geometry shader to select the cascade layer instead of writing gl_Layer from the vertex shader"} }; +uint32_t shader_vertex_input_mask(const ShaderTypeInfo& info) +{ + uint32_t mask = 0; + for (auto attr : info.attributes) { + // A mat4 input spans four consecutive locations; a single bit here would + // make the Vulkan pipeline drop columns 1-3 as unused. + const uint32_t locations = (attr == VATTRIB_MODEL_MATRIX) ? 4u : 1u; + mask |= ((1u << locations) - 1u) << attr; + } + return mask; +} + const ShaderTypeInfo* shader_get_type_info(shader_type type) { for (auto & i : SHADER_TYPES) { diff --git a/code/graphics/shader_types.h b/code/graphics/shader_types.h index 09860a78d50..d8846487347 100644 --- a/code/graphics/shader_types.h +++ b/code/graphics/shader_types.h @@ -55,6 +55,14 @@ const ShaderTypeInfo* shader_get_type_info(shader_type type); // Lookup helper: find variant info by (type, flag). Returns nullptr if not found. const ShaderVariantInfo* shader_get_variant_info(shader_type type, int flag); +/** + * @brief Bitmask of the vertex attribute locations a shader type reads. + * + * One bit per location; VATTRIB_MODEL_MATRIX contributes bits 8-11 because a + * mat4 input occupies four consecutive locations. + */ +uint32_t shader_vertex_input_mask(const ShaderTypeInfo& info); + // Iterate all variants matching type whose flag bit is set in flags. void shader_for_each_active_variant(shader_type type, unsigned int flags, const std::function& fn); diff --git a/code/graphics/vulkan/VulkanShader.cpp b/code/graphics/vulkan/VulkanShader.cpp index da9a29bc5dc..1986e90a30d 100644 --- a/code/graphics/vulkan/VulkanShader.cpp +++ b/code/graphics/vulkan/VulkanShader.cpp @@ -127,10 +127,7 @@ void VulkanShaderManager::recompileAllShaders(const std::functionattributes) { - shader.vertexInputMask |= (1u << attr); - } + shader.vertexInputMask = shader_vertex_input_mask(*typeInfo); SCP_string fragFile = typeInfo->frag; auto fragSpirv = m_compiler->compile(fragFile, vk::ShaderStageFlagBits::eFragment, type, flags, requiresRaytracing); @@ -216,10 +213,7 @@ int VulkanShaderManager::loadShader(shader_type type, unsigned int flags) vertFile.c_str(), e.what())); } } - shader.vertexInputMask = 0; - for (auto attr : typeInfo->attributes) { - shader.vertexInputMask |= (1u << attr); - } + shader.vertexInputMask = shader_vertex_input_mask(*typeInfo); // Compile fragment shader SCP_string fragFile = typeInfo->frag; diff --git a/test/src/graphics/test_shader_types.cpp b/test/src/graphics/test_shader_types.cpp new file mode 100644 index 00000000000..2eb988378fd --- /dev/null +++ b/test/src/graphics/test_shader_types.cpp @@ -0,0 +1,25 @@ +#include +#include + +// decal-v.sdr reads its per-instance matrix from locations 8-11; a mask that only +// sets bit 8 makes the Vulkan pipeline drop the other three columns, so every +// decal is transformed by garbage and none reaches the screen (#7866). +TEST(ShaderTypes, ModelMatrixInputMaskCoversFourLocations) +{ + const ShaderTypeInfo* decal = shader_get_type_info(SDR_TYPE_DECAL); + ASSERT_NE(decal, nullptr); + + const uint32_t mask = shader_vertex_input_mask(*decal); + for (uint32_t loc = VATTRIB_MODEL_MATRIX; loc < VATTRIB_MODEL_MATRIX + 4; ++loc) { + EXPECT_TRUE(mask & (1u << loc)) << "location " << loc << " missing from mask " << mask; + } + EXPECT_TRUE(mask & (1u << VATTRIB_POSITION)); +} + +TEST(ShaderTypes, ScalarAttributesSetOneBitEach) +{ + const ShaderTypeInfo* nanovg = shader_get_type_info(SDR_TYPE_NANOVG); + ASSERT_NE(nanovg, nullptr); + + EXPECT_EQ(shader_vertex_input_mask(*nanovg), (1u << VATTRIB_POSITION) | (1u << VATTRIB_TEXCOORD)); +} diff --git a/test/src/source_groups.cmake b/test/src/source_groups.cmake index 1c268dda86e..1de2e1c7463 100644 --- a/test/src/source_groups.cmake +++ b/test/src/source_groups.cmake @@ -25,6 +25,7 @@ add_file_folder("Globalincs" add_file_folder("Graphics" graphics/test_font.cpp + graphics/test_shader_types.cpp ) if (FSO_BUILD_WITH_VULKAN) From 63a4b6c914f4339f10e0141becdb25450e7ddf51 Mon Sep 17 00:00:00 2001 From: Dustinspace217 Date: Fri, 16 Oct 2026 04:30:22 -0700 Subject: [PATCH 2/3] Vulkan: build the decal pipeline with the material's shader flags The decal pipeline config set the shader type but not the flags, so the decal shader variant was always compiled without USE_NORMAL_MAP and fell back to screen-space derivative normals for angle rejection. Far more of the decal volume passed the angle test than in OpenGL, which lit the decals brighter and wider. The pipeline now takes the flags the material reports, as the other draw paths already do. --- code/graphics/vulkan/VulkanDeferred.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/code/graphics/vulkan/VulkanDeferred.cpp b/code/graphics/vulkan/VulkanDeferred.cpp index d4a48744347..f2dd15a1a46 100644 --- a/code/graphics/vulkan/VulkanDeferred.cpp +++ b/code/graphics/vulkan/VulkanDeferred.cpp @@ -959,6 +959,7 @@ void vulkan_render_decals(decal_material* material_info, // Build pipeline config for decal rendering PipelineConfig config; config.shaderType = SDR_TYPE_DECAL; + config.shaderFlags = material_info->get_shader_flags(); config.primitiveType = prim_type; config.depthMode = material_info->get_depth_mode(); config.depthWriteEnabled = false; From d0c4936fc0de11cc1188eef8ac1e99a5f6ac64cc Mon Sep 17 00:00:00 2001 From: Dustinspace217 Date: Fri, 16 Oct 2026 04:48:44 -0700 Subject: [PATCH 3/3] Review fixes: exact input-mask test, decal flag polarity test, mask width check Pin the full mask per shader type instead of the presence of the matrix bits, so an expansion of any other attribute fails; pin that a decal without a normal map is the one that reads the scene normal buffer; add a compile-time bound on the mask width; trim two comments. --- code/graphics/shader_types.cpp | 5 +++-- test/src/graphics/test_material.cpp | 16 ++++++++++++++ test/src/graphics/test_shader_types.cpp | 28 ++++++++++++++++++------- test/src/source_groups.cmake | 1 + 4 files changed, 40 insertions(+), 10 deletions(-) create mode 100644 test/src/graphics/test_material.cpp diff --git a/code/graphics/shader_types.cpp b/code/graphics/shader_types.cpp index f983803a28e..f8ab24eccf4 100644 --- a/code/graphics/shader_types.cpp +++ b/code/graphics/shader_types.cpp @@ -162,12 +162,13 @@ static ShaderVariantInfo SHADER_VARIANTS[] = { {SDR_TYPE_SHADOW_MAP_GEN, true, SDR_FLAG_SHADOW_FALLBACK, "GEOMETRY_FALLBACK", {}, "Use a geometry shader to select the cascade layer instead of writing gl_Layer from the vertex shader"} }; +static_assert(VATTRIB_MODEL_MATRIX + 4 <= 32, "vertex input mask is 32 bits wide"); + uint32_t shader_vertex_input_mask(const ShaderTypeInfo& info) { uint32_t mask = 0; for (auto attr : info.attributes) { - // A mat4 input spans four consecutive locations; a single bit here would - // make the Vulkan pipeline drop columns 1-3 as unused. + // The Vulkan pipeline drops every location the mask omits, so a mat4 needs all four of its bits. const uint32_t locations = (attr == VATTRIB_MODEL_MATRIX) ? 4u : 1u; mask |= ((1u << locations) - 1u) << attr; } diff --git a/test/src/graphics/test_material.cpp b/test/src/graphics/test_material.cpp new file mode 100644 index 00000000000..eec20b2cc5c --- /dev/null +++ b/test/src/graphics/test_material.cpp @@ -0,0 +1,16 @@ +#include +#include + +// USE_NORMAL_MAP means "read the scene normal buffer": set when the decal brings no normal map of its own. +TEST(DecalMaterial, SceneNormalFlagSetOnlyWithoutDecalNormalMap) +{ + decal_material without_normal; + without_normal.set_texture_map(TM_BASE_TYPE, 0); + without_normal.set_texture_map(TM_NORMAL_TYPE, -1); + EXPECT_EQ(without_normal.get_shader_flags(), static_cast(SDR_FLAG_DECAL_USE_NORMAL_MAP)); + + decal_material with_normal; + with_normal.set_texture_map(TM_BASE_TYPE, 0); + with_normal.set_texture_map(TM_NORMAL_TYPE, 1); + EXPECT_EQ(with_normal.get_shader_flags(), 0u); +} diff --git a/test/src/graphics/test_shader_types.cpp b/test/src/graphics/test_shader_types.cpp index 2eb988378fd..f2efc1a2096 100644 --- a/test/src/graphics/test_shader_types.cpp +++ b/test/src/graphics/test_shader_types.cpp @@ -1,9 +1,7 @@ #include #include -// decal-v.sdr reads its per-instance matrix from locations 8-11; a mask that only -// sets bit 8 makes the Vulkan pipeline drop the other three columns, so every -// decal is transformed by garbage and none reaches the screen (#7866). +// A mat4 input spans locations 8-11; the Vulkan pipeline drops any location the mask omits. TEST(ShaderTypes, ModelMatrixInputMaskCoversFourLocations) { const ShaderTypeInfo* decal = shader_get_type_info(SDR_TYPE_DECAL); @@ -16,10 +14,24 @@ TEST(ShaderTypes, ModelMatrixInputMaskCoversFourLocations) EXPECT_TRUE(mask & (1u << VATTRIB_POSITION)); } -TEST(ShaderTypes, ScalarAttributesSetOneBitEach) +TEST(ShaderTypes, VertexInputMaskIsExactPerType) { - const ShaderTypeInfo* nanovg = shader_get_type_info(SDR_TYPE_NANOVG); - ASSERT_NE(nanovg, nullptr); - - EXPECT_EQ(shader_vertex_input_mask(*nanovg), (1u << VATTRIB_POSITION) | (1u << VATTRIB_TEXCOORD)); + struct Case { + shader_type type; + uint32_t expected; + }; + const Case cases[] = { + {SDR_TYPE_DECAL, (1u << VATTRIB_POSITION) | (0xFu << VATTRIB_MODEL_MATRIX)}, + {SDR_TYPE_NANOVG, (1u << VATTRIB_POSITION) | (1u << VATTRIB_TEXCOORD)}, + {SDR_TYPE_MODEL, + (1u << VATTRIB_POSITION) | (1u << VATTRIB_TEXCOORD) | (1u << VATTRIB_NORMAL) | (1u << VATTRIB_TANGENT) | + (1u << VATTRIB_MODELID)}, + {SDR_TYPE_EFFECT_PARTICLE, + (1u << VATTRIB_POSITION) | (1u << VATTRIB_TEXCOORD) | (1u << VATTRIB_RADIUS) | (1u << VATTRIB_COLOR)}, + }; + for (const auto& c : cases) { + const ShaderTypeInfo* info = shader_get_type_info(c.type); + ASSERT_NE(info, nullptr) << "type " << static_cast(c.type); + EXPECT_EQ(shader_vertex_input_mask(*info), c.expected) << "type " << static_cast(c.type); + } } diff --git a/test/src/source_groups.cmake b/test/src/source_groups.cmake index 1de2e1c7463..103f0399349 100644 --- a/test/src/source_groups.cmake +++ b/test/src/source_groups.cmake @@ -25,6 +25,7 @@ add_file_folder("Globalincs" add_file_folder("Graphics" graphics/test_font.cpp + graphics/test_material.cpp graphics/test_shader_types.cpp )