From 722a0e36be3486d2084bae557bc6722d7b895b3d Mon Sep 17 00:00:00 2001 From: TheTurtle <47210458+raphaelthegreat@users.noreply.github.com> Date: Fri, 13 Dec 2024 21:49:37 +0200 Subject: [PATCH] graphics: Improve handling of color buffer and storage image swizzles (#1763) * liverpool_to_vk: Remove wrong component swap formats * shader_recompiler: Handle storage and buffer format swizzles * shader_recompiler: Skip unsupported depth export * image_view: Remove image format swizzle * Platform support is not always guaranteed --- .../frontend/translate/export.cpp | 5 +++ .../ir/passes/resource_tracking_pass.cpp | 42 +++++++++++++++++++ src/shader_recompiler/specialization.h | 11 ++++- src/video_core/amdgpu/resource.h | 9 ++++ .../renderer_vulkan/liverpool_to_vk.cpp | 9 ---- src/video_core/texture_cache/image_view.cpp | 39 ----------------- 6 files changed, 66 insertions(+), 49 deletions(-) diff --git a/src/shader_recompiler/frontend/translate/export.cpp b/src/shader_recompiler/frontend/translate/export.cpp index f82f8fc1..f4914577 100644 --- a/src/shader_recompiler/frontend/translate/export.cpp +++ b/src/shader_recompiler/frontend/translate/export.cpp @@ -13,6 +13,11 @@ void Translator::EmitExport(const GcnInst& inst) { const auto& exp = inst.control.exp; const IR::Attribute attrib{exp.target}; + if (attrib == IR::Attribute::Depth && exp.en != 1) { + LOG_WARNING(Render_Vulkan, "Unsupported depth export"); + return; + } + const std::array vsrc = { IR::VectorReg(inst.src[0].code), IR::VectorReg(inst.src[1].code), diff --git a/src/shader_recompiler/ir/passes/resource_tracking_pass.cpp b/src/shader_recompiler/ir/passes/resource_tracking_pass.cpp index 99585104..398579ad 100644 --- a/src/shader_recompiler/ir/passes/resource_tracking_pass.cpp +++ b/src/shader_recompiler/ir/passes/resource_tracking_pass.cpp @@ -137,6 +137,35 @@ bool IsImageInstruction(const IR::Inst& inst) { } } +IR::Value SwizzleVector(IR::IREmitter& ir, auto sharp, IR::Value texel) { + boost::container::static_vector comps; + for (u32 i = 0; i < 4; i++) { + switch (sharp.GetSwizzle(i)) { + case AmdGpu::CompSwizzle::Zero: + comps.emplace_back(ir.Imm32(0.f)); + break; + case AmdGpu::CompSwizzle::One: + comps.emplace_back(ir.Imm32(1.f)); + break; + case AmdGpu::CompSwizzle::Red: + comps.emplace_back(ir.CompositeExtract(texel, 0)); + break; + case AmdGpu::CompSwizzle::Green: + comps.emplace_back(ir.CompositeExtract(texel, 1)); + break; + case AmdGpu::CompSwizzle::Blue: + comps.emplace_back(ir.CompositeExtract(texel, 2)); + break; + case AmdGpu::CompSwizzle::Alpha: + comps.emplace_back(ir.CompositeExtract(texel, 3)); + break; + default: + UNREACHABLE(); + } + } + return ir.CompositeConstruct(comps[0], comps[1], comps[2], comps[3]); +}; + class Descriptors { public: explicit Descriptors(Info& info_) @@ -388,6 +417,15 @@ void PatchTextureBufferInstruction(IR::Block& block, IR::Inst& inst, Info& info, IR::IREmitter ir{block, IR::Block::InstructionList::s_iterator_to(inst)}; inst.SetArg(0, ir.Imm32(binding)); ASSERT(!buffer.swizzle_enable && !buffer.add_tid_enable); + + // Apply dst_sel swizzle on formatted buffer instructions + if (inst.GetOpcode() == IR::Opcode::StoreBufferFormatF32) { + inst.SetArg(2, SwizzleVector(ir, buffer, inst.Arg(2))); + } else { + const auto inst_info = inst.Flags(); + const auto texel = ir.LoadBufferFormat(inst.Arg(0), inst.Arg(1), inst_info); + inst.ReplaceUsesWith(SwizzleVector(ir, buffer, texel)); + } } IR::Value PatchCubeCoord(IR::IREmitter& ir, const IR::Value& s, const IR::Value& t, @@ -732,6 +770,10 @@ void PatchImageInstruction(IR::Block& block, IR::Inst& inst, Info& info, Descrip }(); inst.SetArg(1, coords); + if (inst.GetOpcode() == IR::Opcode::ImageWrite) { + inst.SetArg(2, SwizzleVector(ir, image, inst.Arg(2))); + } + if (inst_info.has_lod) { ASSERT(inst.GetOpcode() == IR::Opcode::ImageFetch); ASSERT(image.GetType() != AmdGpu::ImageType::Color2DMsaa && diff --git a/src/shader_recompiler/specialization.h b/src/shader_recompiler/specialization.h index bc8627c1..9b5dd8fa 100644 --- a/src/shader_recompiler/specialization.h +++ b/src/shader_recompiler/specialization.h @@ -31,6 +31,7 @@ struct BufferSpecialization { struct TextureBufferSpecialization { bool is_integer = false; + u32 dst_select = 0; auto operator<=>(const TextureBufferSpecialization&) const = default; }; @@ -38,8 +39,12 @@ struct TextureBufferSpecialization { struct ImageSpecialization { AmdGpu::ImageType type = AmdGpu::ImageType::Color2D; bool is_integer = false; + u32 dst_select = 0; - auto operator<=>(const ImageSpecialization&) const = default; + bool operator==(const ImageSpecialization& other) const { + return type == other.type && is_integer == other.is_integer && + (dst_select != 0 ? dst_select == other.dst_select : true); + } }; struct FMaskSpecialization { @@ -103,11 +108,15 @@ struct StageSpecialization { ForEachSharp(binding, tex_buffers, info->texture_buffers, [](auto& spec, const auto& desc, AmdGpu::Buffer sharp) { spec.is_integer = AmdGpu::IsInteger(sharp.GetNumberFmt()); + spec.dst_select = sharp.DstSelect(); }); ForEachSharp(binding, images, info->images, [](auto& spec, const auto& desc, AmdGpu::Image sharp) { spec.type = sharp.GetBoundType(); spec.is_integer = AmdGpu::IsInteger(sharp.GetNumberFmt()); + if (desc.is_storage) { + spec.dst_select = sharp.DstSelect(); + } }); ForEachSharp(binding, fmasks, info->fmasks, [](auto& spec, const auto& desc, AmdGpu::Image sharp) { diff --git a/src/video_core/amdgpu/resource.h b/src/video_core/amdgpu/resource.h index ba87425f..5d741755 100644 --- a/src/video_core/amdgpu/resource.h +++ b/src/video_core/amdgpu/resource.h @@ -52,6 +52,10 @@ struct Buffer { return std::memcmp(this, &other, sizeof(Buffer)) == 0; } + u32 DstSelect() const { + return dst_sel_x | (dst_sel_y << 3) | (dst_sel_z << 6) | (dst_sel_w << 9); + } + CompSwizzle GetSwizzle(u32 comp) const noexcept { const std::array select{dst_sel_x, dst_sel_y, dst_sel_z, dst_sel_w}; return static_cast(select[comp]); @@ -204,6 +208,11 @@ struct Image { return dst_sel_x | (dst_sel_y << 3) | (dst_sel_z << 6) | (dst_sel_w << 9); } + CompSwizzle GetSwizzle(u32 comp) const noexcept { + const std::array select{dst_sel_x, dst_sel_y, dst_sel_z, dst_sel_w}; + return static_cast(select[comp]); + } + static char SelectComp(u32 sel) { switch (sel) { case 0: diff --git a/src/video_core/renderer_vulkan/liverpool_to_vk.cpp b/src/video_core/renderer_vulkan/liverpool_to_vk.cpp index fa8d28ba..ec0bb3bb 100644 --- a/src/video_core/renderer_vulkan/liverpool_to_vk.cpp +++ b/src/video_core/renderer_vulkan/liverpool_to_vk.cpp @@ -699,15 +699,6 @@ vk::Format AdjustColorBufferFormat(vk::Format base_format, default: break; } - } else if (comp_swap_reverse) { - switch (base_format) { - case vk::Format::eR8G8B8A8Unorm: - return vk::Format::eA8B8G8R8UnormPack32; - case vk::Format::eR8G8B8A8Srgb: - return vk::Format::eA8B8G8R8SrgbPack32; - default: - break; - } } return base_format; } diff --git a/src/video_core/texture_cache/image_view.cpp b/src/video_core/texture_cache/image_view.cpp index 12ad201d..cc467e9a 100644 --- a/src/video_core/texture_cache/image_view.cpp +++ b/src/video_core/texture_cache/image_view.cpp @@ -50,34 +50,6 @@ vk::ComponentSwizzle ConvertComponentSwizzle(u32 dst_sel) { } } -bool IsIdentityMapping(u32 dst_sel, u32 num_components) { - return (num_components == 1 && dst_sel == 0b001'000'000'100) || - (num_components == 2 && dst_sel == 0b001'000'101'100) || - (num_components == 3 && dst_sel == 0b001'110'101'100) || - (num_components == 4 && dst_sel == 0b111'110'101'100); -} - -vk::Format TrySwizzleFormat(vk::Format format, u32 dst_sel) { - // BGRA - if (dst_sel == 0b111100101110) { - switch (format) { - case vk::Format::eR8G8B8A8Unorm: - return vk::Format::eB8G8R8A8Unorm; - case vk::Format::eR8G8B8A8Snorm: - return vk::Format::eB8G8R8A8Snorm; - case vk::Format::eR8G8B8A8Uint: - return vk::Format::eB8G8R8A8Uint; - case vk::Format::eR8G8B8A8Sint: - return vk::Format::eB8G8R8A8Sint; - case vk::Format::eR8G8B8A8Srgb: - return vk::Format::eB8G8R8A8Srgb; - default: - break; - } - } - return format; -} - ImageViewInfo::ImageViewInfo(const AmdGpu::Image& image, const Shader::ImageResource& desc) noexcept : is_storage{desc.is_storage} { const auto dfmt = image.GetDataFmt(); @@ -120,17 +92,6 @@ ImageViewInfo::ImageViewInfo(const AmdGpu::Image& image, const Shader::ImageReso mapping.b = ConvertComponentSwizzle(image.dst_sel_z); mapping.a = ConvertComponentSwizzle(image.dst_sel_w); } - // Check for unfortunate case of storage images being swizzled - const u32 num_comps = AmdGpu::NumComponents(image.GetDataFmt()); - const u32 dst_sel = image.DstSelect(); - if (is_storage && !IsIdentityMapping(dst_sel, num_comps)) { - if (auto new_format = TrySwizzleFormat(format, dst_sel); new_format != format) { - format = new_format; - return; - } - LOG_ERROR(Render_Vulkan, "Storage image (num_comps = {}) requires swizzling {}", num_comps, - image.DstSelectName()); - } } ImageViewInfo::ImageViewInfo(const AmdGpu::Liverpool::ColorBuffer& col_buffer) noexcept {