From e656093d8538f584967d3070020759b6e24d4151 Mon Sep 17 00:00:00 2001 From: squidbus <175574877+squidbus@users.noreply.github.com> Date: Fri, 10 Jan 2025 12:35:03 -0800 Subject: [PATCH] shader_recompiler: Fix some image view type issues. (#2118) --- .../backend/spirv/emit_spirv_image.cpp | 5 ++--- .../backend/spirv/spirv_emit_context.cpp | 4 ++-- .../backend/spirv/spirv_emit_context.h | 2 +- .../ir/passes/resource_tracking_pass.cpp | 20 ++++++++++--------- src/shader_recompiler/specialization.h | 2 +- src/video_core/amdgpu/resource.h | 17 +++++++++++----- src/video_core/texture_cache/image_view.cpp | 2 +- 7 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/shader_recompiler/backend/spirv/emit_spirv_image.cpp b/src/shader_recompiler/backend/spirv/emit_spirv_image.cpp index 182437b6..b5ae507a 100644 --- a/src/shader_recompiler/backend/spirv/emit_spirv_image.cpp +++ b/src/shader_recompiler/backend/spirv/emit_spirv_image.cpp @@ -174,13 +174,12 @@ Id EmitImageQueryDimensions(EmitContext& ctx, IR::Inst* inst, u32 handle, Id lod const auto sharp = ctx.info.images[handle & 0xFFFF].GetSharp(ctx.info); const Id zero = ctx.u32_zero_value; const auto mips{[&] { return has_mips ? ctx.OpImageQueryLevels(ctx.U32[1], image) : zero; }}; - const bool uses_lod{texture.bound_type != AmdGpu::ImageType::Color2DMsaa && - !texture.is_storage}; + const bool uses_lod{texture.view_type != AmdGpu::ImageType::Color2DMsaa && !texture.is_storage}; const auto query{[&](Id type) { return uses_lod ? ctx.OpImageQuerySizeLod(type, image, lod) : ctx.OpImageQuerySize(type, image); }}; - switch (texture.bound_type) { + switch (texture.view_type) { case AmdGpu::ImageType::Color1D: return ctx.OpCompositeConstruct(ctx.U32[4], query(ctx.U32[1]), zero, zero, mips()); case AmdGpu::ImageType::Color1DArray: diff --git a/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp b/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp index 6151c5c6..7e86dfb4 100644 --- a/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp +++ b/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp @@ -773,7 +773,7 @@ spv::ImageFormat GetFormat(const AmdGpu::Image& image) { Id ImageType(EmitContext& ctx, const ImageResource& desc, Id sampled_type) { const auto image = desc.GetSharp(ctx.info); const auto format = desc.is_atomic ? GetFormat(image) : spv::ImageFormat::Unknown; - const auto type = image.GetBoundType(desc.is_array); + const auto type = image.GetViewType(desc.is_array); const u32 sampled = desc.is_written ? 2 : 1; switch (type) { case AmdGpu::ImageType::Color1D: @@ -814,7 +814,7 @@ void EmitContext::DefineImagesAndSamplers() { .sampled_type = is_storage ? sampled_type : TypeSampledImage(image_type), .pointer_type = pointer_type, .image_type = image_type, - .bound_type = sharp.GetBoundType(image_desc.is_array), + .view_type = sharp.GetViewType(image_desc.is_array), .is_integer = is_integer, .is_storage = is_storage, }); diff --git a/src/shader_recompiler/backend/spirv/spirv_emit_context.h b/src/shader_recompiler/backend/spirv/spirv_emit_context.h index 80d0d4d9..f552055c 100644 --- a/src/shader_recompiler/backend/spirv/spirv_emit_context.h +++ b/src/shader_recompiler/backend/spirv/spirv_emit_context.h @@ -222,7 +222,7 @@ public: Id sampled_type; Id pointer_type; Id image_type; - AmdGpu::ImageType bound_type; + AmdGpu::ImageType view_type; bool is_integer = false; bool is_storage = false; }; diff --git a/src/shader_recompiler/ir/passes/resource_tracking_pass.cpp b/src/shader_recompiler/ir/passes/resource_tracking_pass.cpp index c00b8e6b..10d685ed 100644 --- a/src/shader_recompiler/ir/passes/resource_tracking_pass.cpp +++ b/src/shader_recompiler/ir/passes/resource_tracking_pass.cpp @@ -558,13 +558,14 @@ void PatchTextureBufferArgs(IR::Block& block, IR::Inst& inst, Info& info) { } void PatchImageSampleArgs(IR::Block& block, IR::Inst& inst, Info& info, - const AmdGpu::Image& image) { + const ImageResource& image_res, const AmdGpu::Image& image) { const auto handle = inst.Arg(0); const auto sampler_res = info.samplers[(handle.U32() >> 16) & 0xFFFF]; auto sampler = sampler_res.GetSharp(info); IR::IREmitter ir{block, IR::Block::InstructionList::s_iterator_to(inst)}; const auto inst_info = inst.Flags(); + const auto view_type = image.GetViewType(image_res.is_array); IR::Inst* body1 = inst.Arg(1).InstRecursive(); IR::Inst* body2 = inst.Arg(2).InstRecursive(); @@ -611,7 +612,7 @@ void PatchImageSampleArgs(IR::Block& block, IR::Inst& inst, Info& info, return ir.BitFieldExtract(IR::U32{arg}, ir.Imm32(off), ir.Imm32(6), true); }; - switch (image.GetType()) { + switch (view_type) { case AmdGpu::ImageType::Color1D: case AmdGpu::ImageType::Color1DArray: return read(0); @@ -631,7 +632,7 @@ void PatchImageSampleArgs(IR::Block& block, IR::Inst& inst, Info& info, if (!inst_info.has_derivatives) { return {}; } - switch (image.GetType()) { + switch (view_type) { case AmdGpu::ImageType::Color1D: case AmdGpu::ImageType::Color1DArray: // du/dx, du/dy @@ -675,7 +676,7 @@ void PatchImageSampleArgs(IR::Block& block, IR::Inst& inst, Info& info, // Now we can load body components as noted in Table 8.9 Image Opcodes with Sampler const IR::Value coords = [&] -> IR::Value { - switch (image.GetType()) { + switch (view_type) { case AmdGpu::ImageType::Color1D: // x addr_reg = addr_reg + 1; return get_coord(addr_reg - 1, 0); @@ -745,17 +746,18 @@ void PatchImageArgs(IR::Block& block, IR::Inst& inst, Info& info) { // Sample instructions must be handled separately using address register data. if (inst.GetOpcode() == IR::Opcode::ImageSampleRaw) { - PatchImageSampleArgs(block, inst, info, image); + PatchImageSampleArgs(block, inst, info, image_res, image); return; } IR::IREmitter ir{block, IR::Block::InstructionList::s_iterator_to(inst)}; const auto inst_info = inst.Flags(); + const auto view_type = image.GetViewType(image_res.is_array); // Now that we know the image type, adjust texture coordinate vector. IR::Inst* body = inst.Arg(1).InstRecursive(); const auto [coords, arg] = [&] -> std::pair { - switch (image.GetType()) { + switch (view_type) { case AmdGpu::ImageType::Color1D: // x, [lod] return {body->Arg(0), body->Arg(1)}; case AmdGpu::ImageType::Color1DArray: // x, slice, [lod] @@ -772,12 +774,12 @@ void PatchImageArgs(IR::Block& block, IR::Inst& inst, Info& info) { case AmdGpu::ImageType::Color3D: // x, y, z, [lod] return {ir.CompositeConstruct(body->Arg(0), body->Arg(1), body->Arg(2)), body->Arg(3)}; default: - UNREACHABLE_MSG("Unknown image type {}", image.GetType()); + UNREACHABLE_MSG("Unknown image type {}", view_type); } }(); - const auto has_ms = image.GetType() == AmdGpu::ImageType::Color2DMsaa || - image.GetType() == AmdGpu::ImageType::Color2DMsaaArray; + const auto has_ms = view_type == AmdGpu::ImageType::Color2DMsaa || + view_type == AmdGpu::ImageType::Color2DMsaaArray; ASSERT(!inst_info.has_lod || !has_ms); const auto lod = inst_info.has_lod ? IR::U32{arg} : IR::U32{}; const auto ms = has_ms ? IR::U32{arg} : IR::U32{}; diff --git a/src/shader_recompiler/specialization.h b/src/shader_recompiler/specialization.h index c03621c5..18b1df1f 100644 --- a/src/shader_recompiler/specialization.h +++ b/src/shader_recompiler/specialization.h @@ -113,7 +113,7 @@ struct StageSpecialization { }); ForEachSharp(binding, images, info->images, [](auto& spec, const auto& desc, AmdGpu::Image sharp) { - spec.type = sharp.GetBoundType(desc.is_array); + spec.type = sharp.GetViewType(desc.is_array); spec.is_integer = AmdGpu::IsInteger(sharp.GetNumberFmt()); spec.is_storage = desc.is_written; if (spec.is_storage) { diff --git a/src/video_core/amdgpu/resource.h b/src/video_core/amdgpu/resource.h index 467cecf1..60fb42ca 100644 --- a/src/video_core/amdgpu/resource.h +++ b/src/video_core/amdgpu/resource.h @@ -254,9 +254,12 @@ struct Image { return 1; } + bool IsCube() const noexcept { + return static_cast(type) == ImageType::Cube; + } + ImageType GetType() const noexcept { - const auto img_type = static_cast(type); - return img_type == ImageType::Cube ? ImageType::Color2DArray : img_type; + return IsCube() ? ImageType::Color2DArray : static_cast(type); } DataFormat GetDataFmt() const noexcept { @@ -288,8 +291,12 @@ struct Image { GetDataFmt() <= DataFormat::FormatFmask64_8; } - [[nodiscard]] ImageType GetBoundType(const bool is_array) const noexcept { + [[nodiscard]] ImageType GetViewType(const bool is_array) const noexcept { const auto base_type = GetType(); + if (IsCube()) { + // Cube needs to remain array type regardless of instruction array specifier. + return base_type; + } if (base_type == ImageType::Color1DArray && !is_array) { return ImageType::Color1D; } @@ -303,7 +310,7 @@ struct Image { } [[nodiscard]] u32 NumViewLevels(const bool is_array) const noexcept { - switch (GetBoundType(is_array)) { + switch (GetViewType(is_array)) { case ImageType::Color2DMsaa: case ImageType::Color2DMsaaArray: return 1; @@ -313,7 +320,7 @@ struct Image { } [[nodiscard]] u32 NumViewLayers(const bool is_array) const noexcept { - switch (GetBoundType(is_array)) { + switch (GetViewType(is_array)) { case ImageType::Color1D: case ImageType::Color2D: case ImageType::Color2DMsaa: diff --git a/src/video_core/texture_cache/image_view.cpp b/src/video_core/texture_cache/image_view.cpp index 56923816..d90b78c6 100644 --- a/src/video_core/texture_cache/image_view.cpp +++ b/src/video_core/texture_cache/image_view.cpp @@ -45,7 +45,7 @@ ImageViewInfo::ImageViewInfo(const AmdGpu::Image& image, const Shader::ImageReso range.base.layer = image.base_array; range.extent.levels = image.NumViewLevels(desc.is_array); range.extent.layers = image.NumViewLayers(desc.is_array); - type = ConvertImageViewType(image.GetBoundType(desc.is_array)); + type = ConvertImageViewType(image.GetViewType(desc.is_array)); if (!is_storage) { mapping = Vulkan::LiverpoolToVK::ComponentMapping(image.DstSelect());