From edb13581172a0c7b1ce1a498d8d112ed73a7f477 Mon Sep 17 00:00:00 2001 From: Dean Galvin Date: Tue, 29 Sep 2026 13:17:24 +1300 Subject: [PATCH] fix(agc): bind a 2D storage access to a 2D array surface as its first layer A compute shader addressing an image with MIMG DIM 2D over a T# of type 2D array was refused ("guest storage texture dimension disagrees with the shader's declared image shape"). Such an access carries no slice coordinate, so the hardware reads and writes slice 0 of the resource view, which is the BASE_ARRAY layer of the surface. The storage element is now bound through a 2D view of that layer of the surface's shared storage image (StorageTexture::FirstLayerView, created on demand per mip and released with the image). Every other shape/dimension mismatch still throws, and both mismatch messages now name the shape and the dimension. The recorder test device sets formatProperties, which storage images need to pick their Vulkan format. --- .../Graphics/include/ShaderResources.hpp | 1 + .../Graphics/include/Texture.hpp | 4 ++- .../Graphics/src/ShaderResources.cpp | 8 +++-- .../libSceAgcDriver/Graphics/src/Texture.cpp | 17 ++++++++-- .../prx/libSceAgcDriver/tests/Recorder.cpp | 31 +++++++++++++++++++ 5 files changed, 54 insertions(+), 7 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp index eaf43678a..a66fce072 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp @@ -390,6 +390,7 @@ private: std::vector> textures; std::vector> storageTextures; std::vector storageMips; + std::vector storageFirstLayer; std::vector storageWritten; std::vector> samplers; bool reusable = false; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp index 6fc5ea467..0a88f70e4 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp @@ -109,6 +109,7 @@ public: // The image holds every mip of the surface; one storage view per written mip is made on demand, // so successive mip writes of a chain share one image and one write-back. VkImageView View(std::uint32_t mip); + VkImageView FirstLayerView(std::uint32_t mip); // Render targets live in the same images: draws attach mip 0 through a view of the color // buffer's format and mark the image dirty like a storage write. bool Attachable() const { return attachable; } @@ -363,7 +364,7 @@ private: void forgetBorrowed(std::uint32_t first, std::uint32_t count); bool overlaps(std::uint64_t address, std::size_t bytes) const; bool pendingUnitInside(std::uint64_t address, std::size_t bytes) const; - VkImageView createView(std::uint32_t mip) const; + VkImageView createView(std::uint32_t mip, bool firstLayer = false) const; void release() noexcept; Context context; @@ -403,6 +404,7 @@ private: VkImageView view = VK_NULL_HANDLE; std::uint32_t defaultMip = 0; std::map extraViews; + std::map firstLayerViews; bool attachable = false; std::map, VkImageView> attachmentViews; VkFormat storageFormat = VK_FORMAT_UNDEFINED; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp index 5d096a597..bd1e8a2cd 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp @@ -974,7 +974,7 @@ void ShaderResources::buildComplete() { break; case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: write.pImageInfo = images.data() + images.size(); - for (const auto index : binding.imageAllocations) images.push_back({VK_NULL_HANDLE, storageTextures[index]->View(storageMips[index]), VK_IMAGE_LAYOUT_GENERAL}); + for (const auto index : binding.imageAllocations) images.push_back({VK_NULL_HANDLE, storageFirstLayer[index] ? storageTextures[index]->FirstLayerView(storageMips[index]) : storageTextures[index]->View(storageMips[index]), VK_IMAGE_LAYOUT_GENERAL}); break; case VK_DESCRIPTOR_TYPE_SAMPLER: write.pImageInfo = images.data() + images.size(); @@ -2391,7 +2391,7 @@ void ShaderResources::resolveImageBinding(const ShaderRecompiler::DescriptorBind const auto words = std::span(binding.guestDescriptor).subspan(static_cast(element) * elementWords, elementWords); const auto* record = nextRecord(); const auto resource = record != nullptr && record->decoded ? record->resource : DecodeTextureResource(words); - Require(MatchesGuestDimension(*binding.imageShape, resource.dimension), "guest texture dimension disagrees with the shader's declared image shape"); + if (!MatchesGuestDimension(*binding.imageShape, resource.dimension)) throw std::runtime_error("AGC graphics: guest texture dimension disagrees with the shader's declared image shape (shape " + std::to_string(static_cast(*binding.imageShape)) + ", dimension " + std::to_string(static_cast(resource.dimension)) + ")"); const VkComponentMapping components{ComponentSwizzleFor(resource.dstSelX), ComponentSwizzleFor(resource.dstSelY), ComponentSwizzleFor(resource.dstSelZ), ComponentSwizzleFor(resource.dstSelW)}; const auto guestBytes = record != nullptr && record->decoded ? record->guestBytes : DescribeSurface(resource).guestBytes; std::shared_ptr texture; @@ -2418,7 +2418,8 @@ void ShaderResources::resolveImageBinding(const ShaderRecompiler::DescriptorBind else mipOffset = 0; const auto* record = nextRecord(); const auto resource = record != nullptr && record->decoded ? record->resource : DecodeTextureResource(words); - if (binding.imageShape.has_value()) Require(MatchesGuestDimension(*binding.imageShape, resource.dimension), "guest storage texture dimension disagrees with the shader's declared image shape"); + const bool firstLayer = binding.imageShape == ShaderRecompiler::DescriptorImageShape::Image2D && resource.dimension == TextureDimension::k2DArray; + if (binding.imageShape.has_value() && !firstLayer && !MatchesGuestDimension(*binding.imageShape, resource.dimension)) throw std::runtime_error("AGC graphics: guest storage texture dimension disagrees with the shader's declared image shape (shape " + std::to_string(static_cast(*binding.imageShape)) + ", dimension " + std::to_string(static_cast(resource.dimension)) + ")"); const auto mip = std::min(resource.baseLevel + mipOffset, resource.mipCount - 1u); Require(resource.minLod <= mip * 256u, "guest storage texture descriptor clamps its minimum LOD above the level it addresses, which is not implemented"); const auto guestBytes = record != nullptr && record->decoded ? record->guestBytes : DescribeSurface(resource).guestBytes; @@ -2426,6 +2427,7 @@ void ShaderResources::resolveImageBinding(const ShaderRecompiler::DescriptorBind if (sameAsPrevious && StorageDedupeEnabled()) storageTextures.push_back(storageTextures.back()); else storageTextures.push_back(cachedStorageTexture(context, words, resource, mip, guestBytes)); storageMips.push_back(mip); + storageFirstLayer.push_back(firstLayer); // Images the shader only reads have nothing to store back. storageWritten.push_back(element >= binding.imageWritten.size() || binding.imageWritten[element]); describedRanges.push_back({"storage", resource.baseAddress, guestBytes, resource.width, resource.height, resource.format, static_cast(resource.tileMode), resource.dccAddress}); diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp index 923f8ab0e..75768711d 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp @@ -879,13 +879,14 @@ bool AdjacentGenerationEnabled() { } -VkImageView StorageTexture::createView(std::uint32_t mip) const { +VkImageView StorageTexture::createView(std::uint32_t mip, bool firstLayer) const { Require(mip < descriptor.mipCount, "storage texture mip level is outside the texture"); - const auto viewLayerCount = geometry.imageLayers - descriptor.baseArray; + Require(!firstLayer || descriptor.dimension == TextureDimension::k2DArray, "a first-layer storage view needs a 2D array surface"); + const auto viewLayerCount = firstLayer ? 1u : geometry.imageLayers - descriptor.baseArray; VkImageViewCreateInfo viewInfo{VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO}; viewInfo.image = image; // Storage views address one mip; cube faces are written as array layers. - viewInfo.viewType = descriptor.dimension == TextureDimension::k1D ? VK_IMAGE_VIEW_TYPE_1D : descriptor.dimension == TextureDimension::k2D ? VK_IMAGE_VIEW_TYPE_2D : descriptor.dimension == TextureDimension::k3D ? VK_IMAGE_VIEW_TYPE_3D : VK_IMAGE_VIEW_TYPE_2D_ARRAY; + viewInfo.viewType = firstLayer ? VK_IMAGE_VIEW_TYPE_2D : descriptor.dimension == TextureDimension::k1D ? VK_IMAGE_VIEW_TYPE_1D : descriptor.dimension == TextureDimension::k2D ? VK_IMAGE_VIEW_TYPE_2D : descriptor.dimension == TextureDimension::k3D ? VK_IMAGE_VIEW_TYPE_3D : VK_IMAGE_VIEW_TYPE_2D_ARRAY; viewInfo.format = storageFormat; viewInfo.components = {VK_COMPONENT_SWIZZLE_IDENTITY, VK_COMPONENT_SWIZZLE_IDENTITY, VK_COMPONENT_SWIZZLE_IDENTITY, VK_COMPONENT_SWIZZLE_IDENTITY}; viewInfo.subresourceRange = {VK_IMAGE_ASPECT_COLOR_BIT, mip, 1u, descriptor.baseArray, viewLayerCount}; @@ -923,6 +924,14 @@ VkImageView StorageTexture::View(std::uint32_t mip) { return created; } +VkImageView StorageTexture::FirstLayerView(std::uint32_t mip) { + const auto found = firstLayerViews.find(mip); + if (found != firstLayerViews.end()) return found->second; + const auto created = createView(mip, true); + firstLayerViews.emplace(mip, created); + return created; +} + const char* LookupOutcomes::Name(Kind kind) { static constexpr const char* names[Count] = {"sampled fast hit", "sampled fast miss", "sampled hit view", "sampled hit cleared view", "sampled hit snapshot", "sampled made view", "sampled made snapshot", "storage hit", "storage made", "refresh unchanged", "refresh compared", "upload direct", "upload cpu", "upload clear", "dcc scan", "pending flush"}; return kind < Count ? names[kind] : "?"; @@ -3217,6 +3226,8 @@ StorageTexture::~StorageTexture() { void StorageTexture::release() noexcept { for (const auto& [mip, extra] : extraViews) context.Function("vkDestroyImageView")(context.device, extra, nullptr); extraViews.clear(); + for (const auto& [mip, extra] : firstLayerViews) context.Function("vkDestroyImageView")(context.device, extra, nullptr); + firstLayerViews.clear(); for (const auto& [format, attachment] : attachmentViews) context.Function("vkDestroyImageView")(context.device, attachment, nullptr); attachmentViews.clear(); if (view) context.Function("vkDestroyImageView")(context.device, view, nullptr); diff --git a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp index 6655fd732..722e87ff3 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp @@ -1721,6 +1721,36 @@ void minLodTests(const Device& device, Recorder& recorder) { expectRed(sample(0x100, 0.0f, 1), unorm(1), "minimum LOD clamp: MIN_LOD at the view's base level reads its base level"); } +void firstLayerViewTests(const Device& device, Recorder& recorder) { + const auto& context = device.GetContext(); + TextureDetiler detiler(context); + auto withDetiler = context; + withDetiler.detiler = &detiler; + GuestTextureResource resource{}; + resource.width = 64; + resource.height = 4; + resource.depthOrLastArray = 2; + resource.baseArray = 1; + resource.mipCount = 1; + resource.tileMode = TextureTileMode::kLinear; + resource.dimension = TextureDimension::k2DArray; + resource.format = 56; + resource.dstSelX = 4; + resource.dstSelY = 5; + resource.dstSelZ = 6; + resource.dstSelW = 7; + const auto geometry = DescribeSurface(resource); + std::vector memory(static_cast(geometry.guestBytes) + 256); + auto* surface = reinterpret_cast((reinterpret_cast(memory.data()) + 255) & ~std::uintptr_t{255}); + for (std::uint32_t layer = 0; layer < 3; ++layer) std::memset(surface + geometry.GuestLayerOffset(layer), 0x20 * (layer + 1), static_cast(geometry.layerBytes)); + resource.baseAddress = reinterpret_cast(surface); + auto image = std::make_shared(withDetiler, detiler, resource, 0); + recorder.Keep(image); + SampleProgram program(context, recorder); + expectRed(program.Red(image->FirstLayerView(0), VK_IMAGE_LAYOUT_GENERAL, 0.0f), 0x40 / 255.0f, "a first-layer view does not read the BASE_ARRAY layer"); + Require(image->FirstLayerView(0) == image->FirstLayerView(0), "first-layer views are not reused"); +} + int main() { try { Device device; @@ -1748,6 +1778,7 @@ int main() { dataWordPositionsTests(); dataRefreshTests(device, recorder); minLodTests(device, recorder); + firstLayerViewTests(device, recorder); std::cout << "Recorder read tracking and label tests passed\n"; return 0; } catch (const std::exception& error) {