From 2d9673bc957f97162c2350ebe18e208e6d705ca1 Mon Sep 17 00:00:00 2001 From: CamilleLaVey Date: Thu, 5 Mar 2026 21:45:38 -0400 Subject: [PATCH] [vulkan] Replaced old logic for DescriptorType for a numeric handling per type to avoid mismatches during format binding --- .../backend/spirv/emit_spirv_image.cpp | 70 +++++++++++++------ .../backend/spirv/spirv_emit_context.cpp | 35 +++++++--- .../backend/spirv/spirv_emit_context.h | 8 ++- src/shader_recompiler/ir_opt/texture_pass.cpp | 25 +++++-- src/shader_recompiler/shader_info.h | 12 +++- .../renderer_vulkan/vk_graphics_pipeline.cpp | 3 +- 6 files changed, 112 insertions(+), 41 deletions(-) diff --git a/src/shader_recompiler/backend/spirv/emit_spirv_image.cpp b/src/shader_recompiler/backend/spirv/emit_spirv_image.cpp index 4bff810547..2fd0f3bd1a 100644 --- a/src/shader_recompiler/backend/spirv/emit_spirv_image.cpp +++ b/src/shader_recompiler/backend/spirv/emit_spirv_image.cpp @@ -14,6 +14,25 @@ namespace Shader::Backend::SPIRV { namespace { +Id GetResultType(EmitContext& ctx, NumericType numeric_type) { + switch (numeric_type) { + case NumericType::Float: + return ctx.F32[4]; + case NumericType::SignedInt: + return ctx.S32[4]; + case NumericType::UnsignedInt: + return ctx.U32[4]; + } + throw LogicError("Invalid numeric type {}", static_cast(numeric_type)); +} + +NumericType GetTextureNumericType(EmitContext& ctx, const IR::TextureInstInfo& info) { + if (info.type == TextureType::Buffer) { + return ctx.texture_buffers.at(info.descriptor_index).numeric_type; + } + return ctx.textures.at(info.descriptor_index).numeric_type; +} + class ImageOperands { public: [[maybe_unused]] static constexpr bool ImageSampleOffsetAllowed = false; @@ -201,10 +220,10 @@ Id TextureImage(EmitContext& ctx, IR::TextureInstInfo info, const IR::Value& ind const TextureBufferDefinition& def{ctx.texture_buffers.at(info.descriptor_index)}; if (def.count > 1) { const Id idx{index.IsImmediate() ? ctx.Const(index.U32()) : ctx.Def(index)}; - const Id ptr{ctx.OpAccessChain(ctx.image_buffer_type, def.id, idx)}; - return ctx.OpLoad(ctx.image_buffer_type, ptr); + const Id ptr{ctx.OpAccessChain(def.pointer_type, def.id, idx)}; + return ctx.OpLoad(def.image_type, ptr); } - return ctx.OpLoad(ctx.image_buffer_type, def.id); + return ctx.OpLoad(def.image_type, def.id); } else { const TextureDefinition& def{ctx.textures.at(info.descriptor_index)}; if (def.count > 1) { @@ -216,23 +235,24 @@ Id TextureImage(EmitContext& ctx, IR::TextureInstInfo info, const IR::Value& ind } } -std::pair Image(EmitContext& ctx, const IR::Value& index, IR::TextureInstInfo info) { +std::pair Image(EmitContext& ctx, const IR::Value& index, + IR::TextureInstInfo info) { if (info.type == TextureType::Buffer) { const ImageBufferDefinition def{ctx.image_buffers.at(info.descriptor_index)}; if (def.count > 1) { const Id idx{index.IsImmediate() ? ctx.Const(index.U32()) : ctx.Def(index)}; const Id ptr{ctx.OpAccessChain(def.pointer_type, def.id, idx)}; - return {ctx.OpLoad(def.image_type, ptr), def.is_integer}; + return {ctx.OpLoad(def.image_type, ptr), def.numeric_type}; } - return {ctx.OpLoad(def.image_type, def.id), def.is_integer}; + return {ctx.OpLoad(def.image_type, def.id), def.numeric_type}; } else { const ImageDefinition def{ctx.images.at(info.descriptor_index)}; if (def.count > 1) { const Id idx{index.IsImmediate() ? ctx.Const(index.U32()) : ctx.Def(index)}; const Id ptr{ctx.OpAccessChain(def.pointer_type, def.id, idx)}; - return {ctx.OpLoad(def.image_type, ptr), def.is_integer}; + return {ctx.OpLoad(def.image_type, ptr), def.numeric_type}; } - return {ctx.OpLoad(def.image_type, def.id), def.is_integer}; + return {ctx.OpLoad(def.image_type, def.id), def.numeric_type}; } } @@ -461,8 +481,9 @@ Id EmitImageSampleImplicitLod(EmitContext& ctx, IR::Inst* inst, const IR::Value& if (ctx.stage == Stage::Fragment) { const ImageOperands operands(ctx, info.has_bias != 0, false, info.has_lod_clamp != 0, bias_lc, offset); + const Id result_type{GetResultType(ctx, GetTextureNumericType(ctx, info))}; return Emit(&EmitContext::OpImageSparseSampleImplicitLod, - &EmitContext::OpImageSampleImplicitLod, ctx, inst, ctx.F32[4], + &EmitContext::OpImageSampleImplicitLod, ctx, inst, result_type, Texture(ctx, info, index), coords, operands.MaskOptional(), operands.Span()); } else { // We can't use implicit lods on non-fragment stages on SPIR-V. Maxwell hardware behaves as @@ -470,8 +491,9 @@ Id EmitImageSampleImplicitLod(EmitContext& ctx, IR::Inst* inst, const IR::Value& // derivatives const Id lod{ctx.Const(0.0f)}; const ImageOperands operands(ctx, false, true, info.has_lod_clamp != 0, lod, offset); + const Id result_type{GetResultType(ctx, GetTextureNumericType(ctx, info))}; return Emit(&EmitContext::OpImageSparseSampleExplicitLod, - &EmitContext::OpImageSampleExplicitLod, ctx, inst, ctx.F32[4], + &EmitContext::OpImageSampleExplicitLod, ctx, inst, result_type, Texture(ctx, info, index), coords, operands.Mask(), operands.Span()); } } @@ -480,12 +502,14 @@ Id EmitImageSampleExplicitLod(EmitContext& ctx, IR::Inst* inst, const IR::Value& Id lod, const IR::Value& offset) { const auto info{inst->Flags()}; const ImageOperands operands(ctx, false, true, false, lod, offset); + const NumericType numeric_type{GetTextureNumericType(ctx, info)}; + const Id result_type{GetResultType(ctx, numeric_type)}; Id result = Emit(&EmitContext::OpImageSparseSampleExplicitLod, - &EmitContext::OpImageSampleExplicitLod, ctx, inst, ctx.F32[4], + &EmitContext::OpImageSampleExplicitLod, ctx, inst, result_type, Texture(ctx, info, index), coords, operands.Mask(), operands.Span()); #ifdef ANDROID - if (Settings::values.fix_bloom_effects.GetValue()) { + if (numeric_type == NumericType::Float && Settings::values.fix_bloom_effects.GetValue()) { result = ctx.OpVectorTimesScalar(ctx.F32[4], result, ctx.Const(0.98f)); } #endif @@ -529,8 +553,9 @@ Id EmitImageGather(EmitContext& ctx, IR::Inst* inst, const IR::Value& index, Id if (ctx.profile.need_gather_subpixel_offset) { coords = ImageGatherSubpixelOffset(ctx, info, TextureImage(ctx, info, index), coords); } + const Id result_type{GetResultType(ctx, GetTextureNumericType(ctx, info))}; return Emit(&EmitContext::OpImageSparseGather, &EmitContext::OpImageGather, ctx, inst, - ctx.F32[4], Texture(ctx, info, index), coords, ctx.Const(info.gather_component), + result_type, Texture(ctx, info, index), coords, ctx.Const(info.gather_component), operands.MaskOptional(), operands.Span()); } @@ -558,8 +583,10 @@ Id EmitImageFetch(EmitContext& ctx, IR::Inst* inst, const IR::Value& index, Id c lod = Id{}; } const ImageOperands operands(lod, ms); - return Emit(&EmitContext::OpImageSparseFetch, &EmitContext::OpImageFetch, ctx, inst, ctx.F32[4], - TextureImage(ctx, info, index), coords, operands.MaskOptional(), operands.Span()); + const Id result_type{GetResultType(ctx, GetTextureNumericType(ctx, info))}; + return Emit(&EmitContext::OpImageSparseFetch, &EmitContext::OpImageFetch, ctx, inst, + result_type, TextureImage(ctx, info, index), coords, operands.MaskOptional(), + operands.Span()); } Id EmitImageQueryDimensions(EmitContext& ctx, IR::Inst* inst, const IR::Value& index, Id lod, @@ -609,8 +636,9 @@ Id EmitImageGradient(EmitContext& ctx, IR::Inst* inst, const IR::Value& index, I ctx.Def(offset), {}, lod_clamp) : ImageOperands(ctx, info.has_lod_clamp != 0, derivatives, info.num_derivatives, offset, lod_clamp); + const Id result_type{GetResultType(ctx, GetTextureNumericType(ctx, info))}; return Emit(&EmitContext::OpImageSparseSampleExplicitLod, - &EmitContext::OpImageSampleExplicitLod, ctx, inst, ctx.F32[4], + &EmitContext::OpImageSampleExplicitLod, ctx, inst, result_type, Texture(ctx, info, index), coords, operands.Mask(), operands.Span()); } @@ -620,11 +648,11 @@ Id EmitImageRead(EmitContext& ctx, IR::Inst* inst, const IR::Value& index, Id co LOG_WARNING(Shader_SPIRV, "Typeless image read not supported by host"); return ctx.ConstantNull(ctx.U32[4]); } - const auto [image, is_integer] = Image(ctx, index, info); - const Id result_type{is_integer ? ctx.U32[4] : ctx.F32[4]}; + const auto [image, numeric_type] = Image(ctx, index, info); + const Id result_type{GetResultType(ctx, numeric_type)}; Id color{Emit(&EmitContext::OpImageSparseRead, &EmitContext::OpImageRead, ctx, inst, result_type, image, coords, std::nullopt, std::span{})}; - if (!is_integer) { + if (numeric_type == NumericType::Float) { color = ctx.OpBitcast(ctx.U32[4], color); } return color; @@ -632,8 +660,8 @@ Id EmitImageRead(EmitContext& ctx, IR::Inst* inst, const IR::Value& index, Id co void EmitImageWrite(EmitContext& ctx, IR::Inst* inst, const IR::Value& index, Id coords, Id color) { const auto info{inst->Flags()}; - const auto [image, is_integer] = Image(ctx, index, info); - if (!is_integer) { + const auto [image, numeric_type] = Image(ctx, index, info); + if (numeric_type == NumericType::Float) { color = ctx.OpBitcast(ctx.F32[4], color); } ctx.OpImageWrite(image, coords, color); diff --git a/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp b/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp index b9a24496c9..68b9cad859 100644 --- a/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp +++ b/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp @@ -28,9 +28,21 @@ enum class Operation { FPMax, }; +Id GetNumericTypeId(EmitContext& ctx, NumericType numeric_type) { + switch (numeric_type) { + case NumericType::Float: + return ctx.F32[1]; + case NumericType::SignedInt: + return ctx.S32[1]; + case NumericType::UnsignedInt: + return ctx.U32[1]; + } + throw InvalidArgument("Invalid numeric type {}", static_cast(numeric_type)); +} + Id ImageType(EmitContext& ctx, const TextureDescriptor& desc) { const spv::ImageFormat format{spv::ImageFormat::Unknown}; - const Id type{ctx.F32[1]}; + const Id type{GetNumericTypeId(ctx, desc.numeric_type)}; const bool depth{desc.is_depth}; const bool ms{desc.is_multisample}; switch (desc.type) { @@ -1304,22 +1316,26 @@ void EmitContext::DefineTextureBuffers(const Info& info, u32& binding) { if (info.texture_buffer_descriptors.empty()) { return; } - const spv::ImageFormat format{spv::ImageFormat::Unknown}; - image_buffer_type = TypeImage(F32[1], spv::Dim::Buffer, 0U, false, false, 1, format); - - const Id type{TypePointer(spv::StorageClass::UniformConstant, image_buffer_type)}; texture_buffers.reserve(info.texture_buffer_descriptors.size()); for (const TextureBufferDescriptor& desc : info.texture_buffer_descriptors) { if (desc.count != 1) { throw NotImplementedException("Array of texture buffers"); } + const spv::ImageFormat format{spv::ImageFormat::Unknown}; + const Id image_type{ + TypeImage(GetNumericTypeId(*this, desc.numeric_type), spv::Dim::Buffer, 0U, false, + false, 1, format)}; + const Id type{TypePointer(spv::StorageClass::UniformConstant, image_type)}; const Id id{AddGlobalVariable(type, spv::StorageClass::UniformConstant)}; Decorate(id, spv::Decoration::Binding, binding); Decorate(id, spv::Decoration::DescriptorSet, 0U); Name(id, NameOf(stage, desc, "texbuf")); texture_buffers.push_back({ .id = id, + .image_type = image_type, + .pointer_type = type, .count = desc.count, + .numeric_type = desc.numeric_type, }); if (profile.supported_spirv >= 0x00010400) { interfaces.push_back(id); @@ -1332,7 +1348,7 @@ void EmitContext::DefineImageBuffers(const Info& info, u32& binding) { image_buffers.reserve(info.image_buffer_descriptors.size()); for (const ImageBufferDescriptor& desc : info.image_buffer_descriptors) { const spv::ImageFormat format{GetImageFormat(desc.format)}; - const Id sampled_type{desc.is_integer ? U32[1] : F32[1]}; + const Id sampled_type{GetNumericTypeId(*this, desc.numeric_type)}; const Id image_type{ TypeImage(sampled_type, spv::Dim::Buffer, false, false, false, 2, format)}; const Id pointer_type{TypePointer(spv::StorageClass::UniformConstant, image_type)}; @@ -1345,7 +1361,7 @@ void EmitContext::DefineImageBuffers(const Info& info, u32& binding) { .image_type = image_type, .pointer_type = pointer_type, .count = desc.count, - .is_integer = desc.is_integer, + .numeric_type = desc.numeric_type, }); if (profile.supported_spirv >= 0x00010400) { interfaces.push_back(id); @@ -1372,6 +1388,7 @@ void EmitContext::DefineTextures(const Info& info, u32& binding, u32& scaling_in .image_type = image_type, .count = desc.count, .is_multisample = desc.is_multisample, + .numeric_type = desc.numeric_type, }); if (profile.supported_spirv >= 0x00010400) { interfaces.push_back(id); @@ -1387,7 +1404,7 @@ void EmitContext::DefineTextures(const Info& info, u32& binding, u32& scaling_in void EmitContext::DefineImages(const Info& info, u32& binding, u32& scaling_index) { images.reserve(info.image_descriptors.size()); for (const ImageDescriptor& desc : info.image_descriptors) { - const Id sampled_type{desc.is_integer ? U32[1] : F32[1]}; + const Id sampled_type{GetNumericTypeId(*this, desc.numeric_type)}; const Id image_type{ImageType(*this, desc, sampled_type)}; const Id pointer_type{TypePointer(spv::StorageClass::UniformConstant, image_type)}; const Id id{AddGlobalVariable(pointer_type, spv::StorageClass::UniformConstant)}; @@ -1399,7 +1416,7 @@ void EmitContext::DefineImages(const Info& info, u32& binding, u32& scaling_inde .image_type = image_type, .pointer_type = pointer_type, .count = desc.count, - .is_integer = desc.is_integer, + .numeric_type = desc.numeric_type, }); if (profile.supported_spirv >= 0x00010400) { interfaces.push_back(id); diff --git a/src/shader_recompiler/backend/spirv/spirv_emit_context.h b/src/shader_recompiler/backend/spirv/spirv_emit_context.h index de56809a98..396022eddf 100644 --- a/src/shader_recompiler/backend/spirv/spirv_emit_context.h +++ b/src/shader_recompiler/backend/spirv/spirv_emit_context.h @@ -41,11 +41,15 @@ struct TextureDefinition { Id image_type; u32 count; bool is_multisample; + NumericType numeric_type; }; struct TextureBufferDefinition { Id id; + Id image_type; + Id pointer_type; u32 count; + NumericType numeric_type; }; struct ImageBufferDefinition { @@ -53,7 +57,7 @@ struct ImageBufferDefinition { Id image_type; Id pointer_type; u32 count; - bool is_integer; + NumericType numeric_type; }; struct ImageDefinition { @@ -61,7 +65,7 @@ struct ImageDefinition { Id image_type; Id pointer_type; u32 count; - bool is_integer; + NumericType numeric_type; }; struct UniformDefinitions { diff --git a/src/shader_recompiler/ir_opt/texture_pass.cpp b/src/shader_recompiler/ir_opt/texture_pass.cpp index 20b8591072..5fc8f0f4a9 100644 --- a/src/shader_recompiler/ir_opt/texture_pass.cpp +++ b/src/shader_recompiler/ir_opt/texture_pass.cpp @@ -19,6 +19,7 @@ #include "shader_recompiler/host_translate_info.h" #include "shader_recompiler/ir_opt/passes.h" #include "shader_recompiler/shader_info.h" +#include "video_core/surface.h" namespace Shader::Optimization { namespace { @@ -33,6 +34,16 @@ using TextureInstVector = boost::container::small_vector; constexpr u32 DESCRIPTOR_SIZE = 8; constexpr u32 DESCRIPTOR_SIZE_SHIFT = static_cast(std::countr_zero(DESCRIPTOR_SIZE)); +NumericType GetNumericType(TexturePixelFormat format) { + const auto pixel_format = static_cast(format); + if (!VideoCore::Surface::IsPixelFormatInteger(pixel_format)) { + return NumericType::Float; + } + return VideoCore::Surface::IsPixelFormatSignedInteger(pixel_format) + ? NumericType::SignedInt + : NumericType::UnsignedInt; +} + IR::Opcode IndexedInstruction(const IR::Inst& inst) { switch (inst.GetOpcode()) { case IR::Opcode::BindlessImageSampleImplicitLod: @@ -430,7 +441,8 @@ public: u32 Add(const TextureBufferDescriptor& desc) { return Add(texture_buffer_descriptors, desc, [&desc](const auto& existing) { - return desc.cbuf_index == existing.cbuf_index && + return desc.numeric_type == existing.numeric_type && + desc.cbuf_index == existing.cbuf_index && desc.cbuf_offset == existing.cbuf_offset && desc.shift_left == existing.shift_left && desc.secondary_cbuf_index == existing.secondary_cbuf_index && @@ -449,13 +461,13 @@ public: })}; image_buffer_descriptors[index].is_written |= desc.is_written; image_buffer_descriptors[index].is_read |= desc.is_read; - image_buffer_descriptors[index].is_integer |= desc.is_integer; return index; } u32 Add(const TextureDescriptor& desc) { const u32 index{Add(texture_descriptors, desc, [&desc](const auto& existing) { return desc.type == existing.type && desc.is_depth == existing.is_depth && + desc.numeric_type == existing.numeric_type && desc.has_secondary == existing.has_secondary && desc.cbuf_index == existing.cbuf_index && desc.cbuf_offset == existing.cbuf_offset && @@ -479,7 +491,6 @@ public: })}; image_descriptors[index].is_written |= desc.is_written; image_descriptors[index].is_read |= desc.is_read; - image_descriptors[index].is_integer |= desc.is_integer; return index; } @@ -651,13 +662,13 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo } const bool is_written{inst->GetOpcode() != IR::Opcode::ImageRead}; const bool is_read{inst->GetOpcode() != IR::Opcode::ImageWrite}; - const bool is_integer{IsTexturePixelFormatIntegerCached(env, cbuf)}; + const NumericType numeric_type{GetNumericType(ReadTexturePixelFormatCached(env, cbuf))}; if (flags.type == TextureType::Buffer) { index = descriptors.Add(ImageBufferDescriptor{ .format = flags.image_format, .is_written = is_written, .is_read = is_read, - .is_integer = is_integer, + .numeric_type = numeric_type, .cbuf_index = cbuf.index, .cbuf_offset = cbuf.offset, .count = cbuf.count, @@ -669,7 +680,7 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo .format = flags.image_format, .is_written = is_written, .is_read = is_read, - .is_integer = is_integer, + .numeric_type = numeric_type, .cbuf_index = cbuf.index, .cbuf_offset = cbuf.offset, .count = cbuf.count, @@ -681,6 +692,7 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo default: if (flags.type == TextureType::Buffer) { index = descriptors.Add(TextureBufferDescriptor{ + .numeric_type = GetNumericType(ReadTexturePixelFormatCached(env, cbuf)), .has_secondary = cbuf.has_secondary, .cbuf_index = cbuf.index, .cbuf_offset = cbuf.offset, @@ -696,6 +708,7 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo .type = flags.type, .is_depth = flags.is_depth != 0, .is_multisample = is_multisample, + .numeric_type = GetNumericType(ReadTexturePixelFormatCached(env, cbuf)), .has_secondary = cbuf.has_secondary, .cbuf_index = cbuf.index, .cbuf_offset = cbuf.offset, diff --git a/src/shader_recompiler/shader_info.h b/src/shader_recompiler/shader_info.h index dfacc06802..87dd14fa46 100644 --- a/src/shader_recompiler/shader_info.h +++ b/src/shader_recompiler/shader_info.h @@ -38,6 +38,12 @@ enum class TextureType : u32 { }; constexpr u32 NUM_TEXTURE_TYPES = 9; +enum class NumericType : u8 { + Float, + SignedInt, + UnsignedInt, +}; + enum class TexturePixelFormat { A8B8G8R8_UNORM, A8B8G8R8_SNORM, @@ -177,6 +183,7 @@ struct StorageBufferDescriptor { }; struct TextureBufferDescriptor { + NumericType numeric_type; bool has_secondary; u32 cbuf_index; u32 cbuf_offset; @@ -195,7 +202,7 @@ struct ImageBufferDescriptor { ImageFormat format; bool is_written; bool is_read; - bool is_integer; + NumericType numeric_type; u32 cbuf_index; u32 cbuf_offset; u32 count; @@ -209,6 +216,7 @@ struct TextureDescriptor { TextureType type; bool is_depth; bool is_multisample; + NumericType numeric_type; bool has_secondary; u32 cbuf_index; u32 cbuf_offset; @@ -228,7 +236,7 @@ struct ImageDescriptor { ImageFormat format; bool is_written; bool is_read; - bool is_integer; + NumericType numeric_type; u32 cbuf_index; u32 cbuf_offset; u32 count; diff --git a/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp b/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp index 7c9a3b0b61..0ccad3f24d 100644 --- a/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp +++ b/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp @@ -867,7 +867,8 @@ void GraphicsPipeline::MakePipeline(VkRenderPass render_pass) { VK_COLOR_COMPONENT_A_BIT, }; const auto& blend{key.state.attachments[index]}; - const bool supports_blending = !VideoCore::Surface::IsPixelFormatInteger(key.color_formats[index]); + const PixelFormat color_format{DecodeFormat(key.state.color_formats[index])}; + const bool supports_blending = !VideoCore::Surface::IsPixelFormatInteger(color_format); const std::array mask{blend.Mask()}; VkColorComponentFlags write_mask{}; for (size_t i = 0; i < mask_table.size(); ++i) {