fix(agc): sample resident depth surfaces when a stage-A texture record hits

[orbz22]

What

ShaderResources::fastTexture (the stage-B check for a stage-A image record) reused the Textures() cache entry for the descriptor without asking whether a depth surface is resident at the address. cachedTexture returns DepthSurfaceTexture before it looks at that cache; the fast path skipped that step.

So once a guest-memory entry exists for a depth target's descriptor (made while no surface was resident there, or while it was retired), every later sampled read of the target takes the fast path and gets the guest-memory texture. The memory under a depth target is never written (the depth lives only in the Vulkan depth image), and UnchangedSince keeps proving that entry current, so the reads stay on it for good.

fastTexture now returns nullptr while DepthSurfaceAt(address) finds a resident depth or stencil plane there. The lookup then falls back to cachedTexture, which asks DepthSurfaceTexture first.

Found in Astro's Playroom (PPSA01325):

• The deferred-lighting tile classifier (compute 0x5002a8500, 240x135 groups over the 3840x2160 D32S8 target 0x517370000) reduces each 16x16 tile's depth in LDS. It skips a tile when the max depth is <= 0 or the min depth is >= 1.
• It sampled the target through the fast path and read zeros, so it skipped every tile:
  • the GDS tile counters stayed 0 after it ran (read after a full drain);
  • the per-tile class buffer was all zero;
  • the four indirect lighting dispatches ran 1x1x1.
• Every deferred-lit object rendered black: the controllers and the cube in the game-start scene, and Astro on the main menu.
• With APS5_NO_STAGE_A_IMAGES=1, or with this change, the classifier counts about 2900 tiles on the title screen and those objects are lit.
• DepthSurfaceTexture was never reached for those dispatches before the change. After it, the lookup picks the same image the G-buffer pass draws into.
• The depth downsample 0x50028e000, which feeds the game's AO and particle soft fade, read the same zeros.

Related open PRs, no overlap in the changed lines: #1430 changes fastRevalidate, the reuse proof of a whole set; this is the per-image record path. #425 and #454 change DepthSurfaceTexture and depth surface retirement.

Tested

Windows 11, WinLibs GCC 15.2, RTX 3060 Laptop:

• agc_driver_tests, agc_driver_graphics_tests and agc_driver_recorder_tests built on this branch; all pass.
• tools/check_conventions.py --base upstream/main passes.
• Astro's Playroom, on a local branch that has this change plus unrelated experiments: title screen, main menu (Astro lit) and the game-start space scene (controllers and rocks lit). Before the change, the same build rendered those objects black.

Checklist

• Based on current main; no other open PR implements the same functions
• One topic per PR; follow-ups go in a new PR
• No comments in code except technical debt
• Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
• No notes, investigation .md files or images added to the repository (attach them to this PR)
• New third-party code is a submodule built from source
• Generic behaviour, not specific to one title
• Depends on: none
• AI-assisted: yes (Claude Code)

🤖 Generated with Claude Code

https://claude.ai/code/session_01UTjpbMX7BUFMZZaxJE98zV

[MarcZX10]

Reviewed the full diff and the surrounding lookup paths. The regular cachedTexture path asks DepthSurfaceTexture before the guest-memory cache; this guard makes a stage-A cache hit take that same full lookup whenever the address belongs to a resident depth/stencil surface. DepthSurfaceAt is synchronized and checks both depth and stencil addresses. Current-main ancestry is exact. Upstream Build passed on Linux and Windows, along with Conventions; the PR also reports targeted AGC suites and a before/after Astro reproduction. No targeted automated regression test was added, so future coverage of this fast-path fallback would be useful, but the reported GPU reproduction and full CI provide good evidence for this focused change.

[Dextroid17]

Verified on Linux, which checks the claim in code and re-runs the suites on a second device.

The claim, in code — confirmed. cachedTexture asks DepthSurfaceTexture before it looks at the cache (ShaderResources.cpp:288), the diff adds exactly the one line to the fast path (if (DepthSurfaceAt(address)) return nullptr;), and DepthSurfaceAt is the same depth/stencil predicate already used for the storage-image refusal at line 567. So the fast path was the only route to a guest-memory texture that skipped the resident-surface question, as the body says.

Your suites, on lavapipe (the author's run was Windows/NVIDIA):

agc_driver_tests ............ exit 0
agc_driver_graphics_tests ... exit 0
agc_driver_recorder_tests ... exit 1

That third one is not this change: with the guard reverted to main, everything else identical, it fails the same way — AGC graphics: the partial publish did not copy exactly the partly covered unit, exit 1. Pre-existing and lavapipe-only (it is the same assertion I ran into verifying #1046), so it should not hold this up.

Not verified here: the effect. There is no Astro's Playroom dump on this machine, so the title evidence — the tile classifier counting ~2900 tiles, the lit objects, the GDS counters — stands on the author's setup. Worth saying plainly, because for a one-line change with no test that evidence is the whole proof. A test that asserts "the stage-A fast path refuses while a depth surface is resident at the address" would make it self-protecting.

Related-PR check: no changed-line overlap. #1430 touches DepthSurfaceTexture call sites but not fastTexture; #425 and #454 touch DepthSurfaceAt call sites (storage images, RetireDepthSurfaces) but not this guard. They do share the predicate, which is the same source of truth, so they should compose rather than conflict.

Written with AI assistance.

[fisherjoey]

Linux results, since this was tested on Windows only. Everything passes here.

Nobara 44 (Linux 7.2), GCC 16.2.1, Release, BUILD_TESTING=ON. This PR merged onto main f145139 cleanly.

	main f145139	main + this PR
RTX 5070 Ti, driver 595.91.07, ctest -R agc_driver -j1	207 / 207	207 / 207
RADV, Ryzen 5 7600X iGPU (gfx1036), Mesa 26.2.1, same	207 / 207	207 / 207
CPU tests (-E agc_driver)	286 / 287	286 / 287

The CPU failure is guest_dynamic_loader, which fails the same way on main.

[orbz22]

A side effect I found running Astro's Playroom further in, after opening this PR:

• Once the fast path stops hiding a resident depth surface, DepthSurfaceTexture also sees reads that don't match it.
• In PPSA01325, the lighting dispatch 0x5001c6a00 samples the shadow memory at 0x529430000 as a 1536x1536 2D array (guest format 7, R16 unorm). The surface resident there is still the earlier 2048x2048 D16 one.
• ValidateSampled throws sampling the depth plane of depth surface 0x529430000 (2048x2048, vk format 124) as a 1536x1536 texture ... is not implemented, and the title aborts.
• On main this read goes through the slow path whenever no stale cache entry exists, so the same throw is already reachable there. This change only removes the stale entry that was hiding it.
• fix(agc): treat other-shaped views of a depth surface's memory as reused memory #1495 makes DepthSurfaceTexture treat an other-shaped view as reused memory. With that rule (applied locally, plus preferring a resident surface of the sampled extent when several share the address), the scene runs on to the in-game DualSense platform with correct lighting.

I kept this PR to the fast-path check. The shape rule is #1495's topic.

[Dextroid17]

The reachability half checks out in the code, and it is worth putting on the record because it is what makes this a known gap rather than a regression from your PR.

The throw you quote is at DepthSurface.cpp:98, inside the depth-resolution path, and its only public caller is DepthSurfaceTexture (DepthSurface.cpp:166), called from ShaderResources.cpp:389 — the single file this PR changes. So the fast-path check and the throw sit in the same call chain, and on main a cold cache reaches the same function with the same mismatched shape, which is exactly the sequence you describe. I have not run PPSA01325 here, so I am confirming the path, not the run.

Two things I would keep as they are. Reporting that your change exposes an existing throw rather than causing one, with the main comparison, is the difference between a side effect and a regression — and a reviewer can act on it either way. And keeping the shape rule in #1495 rather than folding it in here means each PR can be judged on one change; a PR that fixed both would hide which edit did what.

Written with AI assistance.