Repository navigation
feat(agc): depth planes sampled as 2D arrays, cubes and integer views - #425
oneandonlydean merged 3 commits into
Conversation
62171e4 to
d106b57
Compare
|
A case from PPSA21567 (Astro Bot) for this PR's depth storage-image path, in case it helps. Linux, on #426 with What the title does with one allocation (base
Every view has the same base, extent, slice 0 and mip 0. The colour view uses the render-target swizzle and DCC; the depth target and the R32F views use the depth swizzle. Order of uses, every frame once the scene starts:
In earlier frames (before the depth pass appears) the memory is only ever used through the R32F and B10G11R11 views. On this branch, step 3 throws What I could not observe: whether the B10G11R11 read in step 3 depends on the bytes the depth pass left. That depends on the view's DCC keys. The trace sees them as A suggestion, for you to judge:
That reproduces the hardware without guessing the title's intent, as long as the colour path itself is exact. The one remaining gap is the DCC keys being rewritten as uncompressed after write-backs. I'm trying this locally and can report how it behaves, if that's useful. |
|
@boykopovar this one needs your call on where depth bits live when something other than the depth path reads them. This PR copies the slices a host depth surface holds from its depth image and reads every other slice from guest memory. On main, depth surfaces never write back to guest memory ( ThisIsAkill described the other model in #425 (comment): write the depth image back to guest memory when a view with another layout touches it, mark the surface stale, and reseed it at the next depth bind. That changes how depth surfaces work for every title, so it's your decision. My read: the write-back model is the exact one, and this PR's reads are exact only while no host depth surface wrote those slices. It also has no unit test yet. |
|
@ThisIsAkill, thanks, that trace is very useful. You're both right about the gap. This PR copies the slices a host depth surface still holds and reads the rest from guest memory. Because depth surfaces never write back on My proposal, for @boykopovar to decide:
If you'd rather go straight to write-back, I'll fold it into this PR instead. |
|
Restacked on the updated #374 (per-aspect HTILE clears) and #294; both still ship inside this PR until they merge. The fast-clear merge keeps this PR's reset of the storage link ( |
d106b57 to
e608f4d
Compare
|
Thanks for the restack. Step 1 sounds right to me as long as one case keeps throwing. In ThisIsAkill's trace the B10G11R11 view comes right after about 50 depth draws into the same memory. Without the write-back, the ordinary colour path would read guest memory the depth image never wrote, so today's refusal would turn into a silent stale read. Could step 1 still refuse a view with another layout while a host depth surface at that address holds depth newer than guest memory, and route only views that alias no live depth surface? The write-back model itself is still @boykopovar's call. |
e608f4d to
cfb3c7e
Compare
cfb3c7e to
1f53a9e
Compare
|
Agreed, that's the right line: a view with another layout should only take the colour path when no live host depth surface at that address holds depth newer than guest memory, and keep throwing otherwise. One thing to coordinate: #454 (@SP4C3B4R-8) changes the same routing in Rebased onto current |
|
That split sounds right to me. #454's author already plans the same rule: retire the depth surface when a colour target, a storage write or a CPU write covers its range, and keep the throw while it's still the latest writer. So moving the other-layout routing there, together with that refusal, keeps one PR on that path. They mentioned being away until 7 October and planned to build on this branch. So this PR would stay with arrays, cubes, the R16_UINT view and storage access, and none of those reroute other-layout views. The write-back model is a separate question, and it's still with the maintainer. |
|
Thanks, then that's the split. This PR stays with arrays, cubes, the R16_UINT view and storage access. It never reroutes views with another layout: those still throw here, and the routing with the live-surface refusal goes into #454. I'll keep this branch stable for @SP4C3B4R-8 to build on. Restacked on today's rebase of #374 so the commits it carries match #374's head (now b0414a9), with no other changes. Builds; the driver tests pass apart from |
1f53a9e to
b0414a9
Compare
|
Ghost of Yōtei (PPSA26344) writes the stencil plane through this storage path, and on this branch that would stop throwing without being handled, so here's the case. Its deferred-decal compute shader (0x800027c200) does a per-texel read-modify-write of its 3840x2160 D32S8 target's stencil: it loads the value, ORs in 0x04 and stores it back through an 8_UINT T# at the DB stencil base (0x5004850000, same swizzle as the DB). On b0414a9, Things I noticed while reading, in case they help under whichever model gets picked:
A test that fails on both Details
|
|
@MichonGoddijn231849 agreed on the split, thanks for keeping the branch stable. #454 takes the routing of views with another layout in @Pacuka's case on #454 (a 2848x1600 R32 texture over a 1024x1024 D32 surface) should fall under the same rule; I'll ask for a run once the branch is up. Written with AI assistance (Claude Code). |
|
Repeat runs for the branch above (https://github.com/xXPBSXx/AnyPS5/tree/test/depth-harmony-425-on-main, AI-assisted (Claude Code). |
|
This needs a rebase on main and two fixes before it can go in:
Merged with main 27873b0 the rest holds: A patch that does the two fixes, if it helps: --- a/core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp
+++ b/core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp
@@ -441,6 +441,7 @@ std::shared_ptr<Texture> DepthSurfaceTexture(const Context& context, std::span<c
const bool cube = resource.dimension == TextureDimension::kCube;
const bool layered = cube || (resource.dimension == TextureDimension::k2DArray && resource.depthOrLastArray != 0);
if (stencil || (!layered && !integer)) return (*found)->Sampled(words, resource, components);
+ Require((*found)->hostFormat == base.format, "sampling several slices or an integer view of a depth plane that the device stores in another depth format is not implemented");
const auto copyFormat = integer ? VK_FORMAT_R16_UINT : d16 ? VK_FORMAT_R16_UNORM : VK_FORMAT_R32_SFLOAT;
const auto layers = cube ? 6u : layered ? resource.depthOrLastArray + 1u : 1u;
if ((format != copyFormat && !depthBits) || resource.width != base.extent.width || resource.height != base.extent.height || resource.baseLevel != 0 || resource.lastLevel != 0 || resource.baseArray != 0 || (cube && resource.depthOrLastArray != 0 && resource.depthOrLastArray != 5) || (resource.dimension != TextureDimension::k2D && resource.dimension != TextureDimension::k2DArray && !cube)) {
--- a/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
+++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
@@ -671,6 +671,11 @@ Texture::Texture(const Context& context, VkImage depthImage, VkFormat depthForma
viewInfo.subresourceRange = {aspect, 0, 1, 0, viewType == VK_IMAGE_VIEW_TYPE_2D ? 1u : VK_REMAINING_ARRAY_LAYERS};
Check(context.Function<PFN_vkCreateImageView>("vkCreateImageView")(context.device, &viewInfo, nullptr, &view), "vkCreateImageView depth plane");
viewRange = {viewInfo.viewType, viewInfo.subresourceRange.levelCount, viewInfo.subresourceRange.layerCount};
+ if (viewType != VK_IMAGE_VIEW_TYPE_2D_ARRAY) return;
+ viewInfo.viewType = VK_IMAGE_VIEW_TYPE_2D;
+ viewInfo.subresourceRange.layerCount = 1;
+ Check(context.Function<PFN_vkCreateImageView>("vkCreateImageView")(context.device, &viewInfo, nullptr, &firstLayerView), "vkCreateImageView depth plane first layer");
+ firstLayerRange = {viewInfo.viewType, viewInfo.subresourceRange.levelCount, viewInfo.subresourceRange.layerCount};
}
Texture::~Texture() {Data point from a title we test with: on |
A texture over a depth surface could only sample one 2D slice of the
plane's own format; 2D arrays, cubes and the R16_UINT view of a D16
plane were refused ("sampling the depth plane of depth surface ... is
not implemented"). A one-slice 2D array now views the depth image as an
array. Several slices, a cube, or an integer view of a D16 plane are
copied into a DepthPlaneCopy image at every use: each slice from the
depth surface bound at its address (base plus one 64 KiB-block depth
slice per layer), otherwise from guest memory, detiled. A depth-bits
R16_UINT view (a depth swizzle) keeps the depth copy, since the shader
converts the sampled depth to its bits itself (GTA V's compute
0x242d4a200 samples a D16 plane array that way, reported by
@tibzejoker). Such a texture is not kept in resource templates.
Storage-image access to depth surfaces is no longer part of this
change; boykopovar#4500 implements it.
TechnicalDebt (Depth plane copies) and ARCHITECTURE describe the copy and
what still throws.
image_load of row 0 through each view, against a live surface drawn at a known depth and guest-memory slices filled with known values: a 3-slice D32 2D array, a D32 cube (all six faces), a D16 plane as R16_UINT, and a D16 depth-bits R16_UINT array (both slices' raw values). A plane array from base slice 1 is still refused. Fails on main (the array is refused) and without the depth-bits format gate (GTA V's refusal message).
… of another host format A 2D-array T# over a depth surface bound to a shader image declared 2D had no first-layer view, so the descriptor got a null view and the shader read 0. The depth plane texture now creates the 2D view of its first slice when its view is a 2D array, for the depth image itself and for the plane copy. A 16-bit depth surface with stencil may be stored as D24_UNORM_S8_UINT or D32_SFLOAT_S8_UINT. The plane copy sizes and copies slices by the guest format, so several slices or an R16_UINT view of such a surface now throw.
e6d75b4 to
90d899d
Compare
The points of this review are met on the sampled half that remains (one per-use flag, the reuse check, pinned execution test, debt entry, ARCHITECTURE note); the storage half moved to #4500.
|
Pushed the rebase and the two fixes from the comment above so this can go in (head 90d899d). Your two commits are unchanged apart from the conflict in |
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
|
…ed path SILENT HILL: The Short Message (PPSA10112) stopped at "compute shader 0x2007a70000: AGC graphics: storage image access to depth/stencil surface 0x205da70000 is not implemented (storage=1024x1024 guest format=7 mip=0 dimension=2 tile=4)": a compute shader binds a depth surface as a 2D array storage image of more than one layer. Main's depth plane storage path (boykopovar#4500/boykopovar#2076), which replaced our boykopovar#425 port in the e965b46 merge, takes one layer only. DepthSurface::Storage now leaves a depth (not stencil) 2D array from layer 0 to the ordinary storage path, and cachedStorageTexture seeds that image from the depth slices again (SeedStorageFromDepth: every layer that is a depth surface of its own is copied in, the surface becomes its writer and takes its writes back when it is drawn to; the code the merge left without a caller). Stencil arrays and arrays from a later layer still refuse. Tests: agc_driver_depth_storage expects the two-layer depth view to be left to that path; agc_driver_depth_storage_dispatch now stores through a two-layer 2D array view and requires the depth test to see layer 0's stored row (was: expected the refusal). test_fast 1126/1127 (windows_import_modules = Defender WinError 225).
What
Depth surfaces could only be sampled as one 2D slice of their own format. 2D arrays, cubes and the R16_UINT integer view of a D16 plane were refused with
sampling the depth plane of depth surface ... is not implemented.They now work:
DepthPlaneCopy):DepthSliceBytesapart).0x242d4a200samples a D16 plane array that way (reported by @tibzejoker).Storage-image access to depth surfaces was part of this PR at first. #4500 implements it, with stencil planes and the reused-memory fall-through, so it is no longer in this PR.
Part of #1409 (the sampled views; #4500 covers its storage half).
Tested
Windows 11 Pro, AMD Ryzen 5 7600, NVIDIA GeForce RTX 5080 (driver 32.0.16.1074), MinGW-w64 GCC 15.2.0, on
main82b1b2e7a.New
agc_driver_depth_plane_samplingloads row 0 through each view and pins the values:It fails on
main(the array is refused) and without the depth-bits format gate (with GTA V's message).agc_driver_graphics,depth_compare_copy,depth_storage_reuse,depth_fast_clear(_submit),depth_bias_per_face,pending_exact_surface,prepared_graphicsandcheck_conventions.pypass.GTA V (PPSA04264): earlier heads of this PR (with the storage half) took the Prologue past its first depth-plane read. I have not rerun the title on this head.
Checklist
main; no other open PR implements the same functions (feat(libSceAgcDriver): storage image access to depth surfaces #4500 is the storage half)NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository🤖 Generated with Claude Code