From a84afe931c2d5cd3a92ed7407f56f814dfdf4a7d Mon Sep 17 00:00:00 2001 From: XairRrR Date: Tue, 13 Oct 2026 14:03:42 +0200 Subject: [PATCH 1/2] fix(agc): store a written storage image over a depth plane into the surface DepthSurfaceStoreBack took only the storage image a depth surface had handed out for its plane. A storage image over the same plane that the surface did not hand out (one cached before the surface existed, kept by a dispatch whose resources revalidate from their records) was written and never reached the depth image: later depth tests and plane reads used the surface's old contents. A written storage image that covers a depth surface's depth or stencil plane exactly (the plane's address and extent, level 0, one 2D layer, the plane's 32-bit float, 16-bit unorm or 8-bit uint format) is now copied into that plane when it is stored back, as the surface's own view is. A view the surface's own view replaced still throws. Uncharted: Legacy of Thieves Collection (PPSA05684) writes a 640x360 downsampled depth with a compute shader and then tests its mist particles against it as a depth target; the depth target stayed at its clear and the mist drew over foreground geometry. --- .../Graphics/src/DepthSurface.cpp | 30 +++++++++++++++---- .../tests/execution/DepthStorage.cpp | 11 +++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp index ce18ed4384..2d6013b2b8 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp @@ -213,7 +213,7 @@ public: if (plane.storage != nullptr) plane.replaced.push_back(plane.storage); plane.storage = StorageTexture::Create(context, *context.detiler, resource, 0); } - copyPlane(plane, stencil, true); + copyPlane(plane, plane.storage, stencil, true); return plane.storage; } @@ -221,7 +221,7 @@ public: for (const bool stencil : {false, true}) { auto& plane = stencil ? stencilPlane : depthPlane; if (plane.storage.get() == &candidate) { - copyPlane(plane, stencil, false); + copyPlane(plane, plane.storage, stencil, false); written = ++depthWrites; return true; } @@ -230,6 +230,14 @@ public: std::snprintf(text, sizeof(text), "AGC graphics: storage image writes to the %s plane of depth surface 0x%llx through a view that another view of the plane (guest format %u, tile mode %d) replaced are not implemented", stencil ? "stencil" : "depth", static_cast(target.address), plane.storage->Descriptor().format, static_cast(plane.storage->Descriptor().tileMode)); throw std::runtime_error(text); } + for (const bool stencil : {false, true}) { + if (!CoversPlane(candidate, stencil)) continue; + auto& plane = stencil ? stencilPlane : depthPlane; + const auto source = std::const_pointer_cast(candidate.shared_from_this()); + copyPlane(plane, source, stencil, false); + written = ++depthWrites; + return true; + } return false; } @@ -250,20 +258,30 @@ private: PlaneStorage depthPlane; PlaneStorage stencilPlane; - void copyPlane(PlaneStorage& plane, bool stencil, bool toStorage) { + bool CoversPlane(const StorageTexture& storage, bool stencil) const { + const auto& resource = storage.Descriptor(); + const auto address = stencil ? target.stencilAddress : target.address; + if (address == 0 || resource.baseAddress != address || resource.width != target.extent.width || resource.height != target.extent.height || resource.baseLevel != 0 || resource.mipCount != 1) return false; + if (resource.dimension != TextureDimension::k2D && (resource.dimension != TextureDimension::k2DArray || resource.baseArray != 0 || resource.depthOrLastArray != 0)) return false; + if (target.samples != 1 || (!stencil && hostFormat != target.format)) return false; + const bool d16 = target.format == VK_FORMAT_D16_UNORM || target.format == VK_FORMAT_D16_UNORM_S8_UINT; + return ResolveTextureFormat(resource.format) == (stencil ? VK_FORMAT_R8_UINT : d16 ? VK_FORMAT_R16_UNORM : VK_FORMAT_R32_SFLOAT); + } + + void copyPlane(PlaneStorage& plane, const std::shared_ptr& storage, bool stencil, bool toStorage) { auto* recorder = Recorder::Active(); Require(recorder != nullptr, "depth surface storage copies need an active recorder"); const bool d16 = target.format == VK_FORMAT_D16_UNORM || target.format == VK_FORMAT_D16_UNORM_S8_UINT; const auto bytes = static_cast(target.extent.width) * target.extent.height * (stencil ? 1u : d16 ? 2u : 4u); if (plane.staging == nullptr) plane.staging = std::make_unique(context, bytes, VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT); const auto commands = recorder->Commands(); - recorder->Keep(plane.storage); + recorder->Keep(storage); const auto timing = recorder->BeginGpuTiming(Recorder::CommandClass::Copy); VkBufferImageCopy planeRegion{0, 0, 0, {stencil ? VK_IMAGE_ASPECT_STENCIL_BIT : VK_IMAGE_ASPECT_DEPTH_BIT, 0, 0, 1}, {0, 0, 0}, {target.extent.width, target.extent.height, 1}}; VkBufferImageCopy colorRegion = planeRegion; colorRegion.imageSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; - const auto sourceImage = toStorage ? image : plane.storage->Image(); - const auto destinationImage = toStorage ? plane.storage->Image() : image; + const auto sourceImage = toStorage ? image : storage->Image(); + const auto destinationImage = toStorage ? storage->Image() : image; RecordMemoryBarrier(context, commands, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_MEMORY_WRITE_BIT, VK_ACCESS_TRANSFER_READ_BIT); context.Function("vkCmdCopyImageToBuffer")(commands, sourceImage, VK_IMAGE_LAYOUT_GENERAL, plane.staging->Handle(), 1, toStorage ? &planeRegion : &colorRegion); RecordMemoryBarrier(context, commands, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_TRANSFER_READ_BIT | VK_ACCESS_TRANSFER_WRITE_BIT); diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp index 0739bd6b86..521dce3be5 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp @@ -246,6 +246,17 @@ void Run(Context context, std::uint8_t* block) { ExpectFailure([&] { static_cast(DepthSurfaceStorage(context, {}, layeredDepth, 0)); }, "with a layer pitch of", "a two-layer 2D array view of the depth plane with a layer pitch other than the depth slice size was accepted"); layeredDepth.baseArray = 1; ExpectFailure([&] { static_cast(DepthSurfaceStorage(context, {}, layeredDepth, 0)); }, "with a layer pitch of", "a 2D array view of layer 1 of the depth plane with a layer pitch other than the depth slice size was accepted"); + const auto own = StorageTexture::Create(context, *context.detiler, PlaneResource(address, Format32Float), 0); + const std::array eighth{std::bit_cast(0.125f), std::bit_cast(0.125f), std::bit_cast(0.125f), std::bit_cast(0.125f)}; + Require(own->FillClear(eighth, StorageTexture::WholeImage, refusal), refusal != nullptr ? refusal : "the storage image could not be filled"); + Require(DepthSurfaceStoreBack(context, *own), "a storage image over the depth plane that the depth surface did not hand out was not stored back"); + submit(recorder); + const auto reread = DepthSurfaceStorage(context, {}, PlaneResource(address, Format32Float), 0); + Require(reread != nullptr, "the depth plane has no storage image"); + submit(recorder); + readBack(*reread); + Require(CountFloats(block, 0.125f) == Side * Side, "a storage image written over the depth plane did not reach the depth surface"); + Require(!DepthSurfaceStoreBack(context, *StorageTexture::Create(context, *context.detiler, PlaneResource(address, Format32Float, Side / 2), 0)), "a storage image of another extent over the depth memory was stored into the depth surface"); } } From b3e5acc8cc48abf73943e95b0780c2d78312f95f Mon Sep 17 00:00:00 2001 From: XairRrR Date: Tue, 13 Oct 2026 18:50:44 +0200 Subject: [PATCH 2/2] fix(agc): store a covering storage image into the newest pairing of its plane DepthSurfaceStoreBack walked the surfaces oldest first and let the first one whose plane the storage image covers take the copy. A depth plane bound with several stencil planes has one surface per pairing, so the copy could land in an older pairing while DepthSurfaceStorage, DepthSurfaceTexture and DepthSurfaceAt read the newest one, and a store back through a replaced view was accepted by an older pairing instead of throwing. The surface's own view and the replaced views are now checked on every surface first; only then does the newest surface covering the plane take the copy, the order DepthSurfaceStorage resolves a plane in. Reported with the repro and the fix by nikowolf123; the repro is the new RunPairings test. AI-assisted: yes --- .../Graphics/src/DepthSurface.cpp | 7 ++++ .../tests/execution/DepthStorage.cpp | 37 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp index 2d6013b2b8..b9ce14abb5 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp @@ -230,6 +230,10 @@ public: std::snprintf(text, sizeof(text), "AGC graphics: storage image writes to the %s plane of depth surface 0x%llx through a view that another view of the plane (guest format %u, tile mode %d) replaced are not implemented", stencil ? "stencil" : "depth", static_cast(target.address), plane.storage->Descriptor().format, static_cast(plane.storage->Descriptor().tileMode)); throw std::runtime_error(text); } + return false; + } + + bool StoreBackCovering(const StorageTexture& candidate) { for (const bool stencil : {false, true}) { if (!CoversPlane(candidate, stencil)) continue; auto& plane = stencil ? stencilPlane : depthPlane; @@ -935,6 +939,9 @@ bool DepthSurfaceStoreBack(const Context& context, const StorageTexture& storage for (const auto& surface : surfaces()) { if (surface->context.device == context.device && surface->StoreBack(storage)) return true; } + for (auto surface = surfaces().rbegin(); surface != surfaces().rend(); ++surface) { + if ((*surface)->context.device == context.device && (*surface)->StoreBackCovering(storage)) return true; + } return false; } diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp index 521dce3be5..f83e70edb5 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp @@ -259,6 +259,41 @@ void Run(Context context, std::uint8_t* block) { Require(!DepthSurfaceStoreBack(context, *StorageTexture::Create(context, *context.detiler, PlaneResource(address, Format32Float, Side / 2), 0)), "a storage image of another extent over the depth memory was stored into the depth surface"); } +void RunPairings(Context context, std::uint8_t* block) { + const auto address = reinterpret_cast(block); + const auto submit = [&](Recorder& recorder) { + recorder.Submit(); + Check(context.Function("vkQueueWaitIdle")(context.queue), "vkQueueWaitIdle"); + }; + Recorder recorder(context); + recorder.Activate(); + const auto readBack = [&](StorageTexture& texture) { + texture.MarkDirty(); + texture.WriteBack(); + submit(recorder); + }; + const DepthTarget first{address, address + BlockBytes / 2, {Side, Side}, VK_FORMAT_D32_SFLOAT_S8_UINT, 0.25f, 7}; + const DepthTarget second{address, address + BlockBytes / 2 + 8192, {Side, Side}, VK_FORMAT_D32_SFLOAT_S8_UINT, 0.5f, 3}; + Require(DepthSurfaceView(context, first) != VK_NULL_HANDLE && DepthSurfaceView(context, second) != VK_NULL_HANDLE, "the two pairings of the depth plane were not created"); + const auto depth = DepthSurfaceStorage(context, {}, PlaneResource(address, Format32Float), 0); + Require(depth != nullptr, "the depth plane has no storage image"); + submit(recorder); + const std::array pattern{std::bit_cast(0.75f), std::bit_cast(0.75f), std::bit_cast(0.75f), std::bit_cast(0.75f)}; + const char* refusal = nullptr; + Require(depth->FillClear(pattern, StorageTexture::WholeImage, refusal), refusal != nullptr ? refusal : "the storage image could not be filled"); + Require(DepthSurfaceStoreBack(context, *depth), "the newest pairing's storage image was not stored back"); + submit(recorder); + const auto reread = DepthSurfaceStorage(context, {}, PlaneResource(address, Format32Float), 0); + Require(reread != nullptr, "the depth plane has no storage image"); + submit(recorder); + readBack(*reread); + Require(CountFloats(block, 0.75f) == Side * Side, "a storage image stored back went into an older pairing instead of the pairing that handed it out"); + auto layered = PlaneResource(address, Format32Float); + layered.dimension = TextureDimension::k2DArray; + Require(DepthSurfaceStorage(context, {}, layered, 0) != nullptr, "a one-layer 2D array view of the depth plane was not taken"); + ExpectFailure([&] { static_cast(DepthSurfaceStoreBack(context, *depth)); }, "through a view that another view of the plane", "a store back through a replaced view was taken by an older pairing that covers the plane"); +} + } int main() { @@ -280,6 +315,8 @@ int main() { std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); Run(context, block); ClearDepthSurfaces(context.device); + RunPairings(context, block); + ClearDepthSurfaces(context.device); } GuestAllocations::Mutation().Remove(block); std::cout << "depth surface storage access tests passed\n";