Repository navigation
fix(agc): drop image stores to a view that starts past the surface's last mip level - #483
ThisIsAkill wants to merge 1 commit into
Conversation
…last mip level A T# whose BASE_LEVEL is past its own MAX_MIP names a level the resource does not have. PPSA21567 builds per-level views 0..6 of a 6-level surface (every view carries MAX_MIP = 5) and runs one pass of its per-level compute chain into the level-6 view; decoding it threw. Such a view now decodes as empty, clamped to the surface's last level. The recompiler specializes the image as empty: image stores to it are compiled out, and loads, samples and atomics fail compilation. The driver binds the clamped level as a placeholder that is never written back, and logs once per surface and level that the stores are dropped. Sampled bindings of an empty view still throw. The hardware behaviour is not confirmed by AMD documentation; KytyPS5 instead adds the missing level to its host image. Recorded in the technical debt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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.
|
|
Thanks for writing up both options so clearly. I hit the same chain in PPSA21564 (two 1920x1080 RGBA16F bloom chains, MAX_MIP 5, level 6 stored every frame) and went the other way on my integration branch, because I think the hardware does write that level. On GFX10, MAX_MIP only decides where the levels are placed: a level's address comes from the levels below it, and a level in the mip tail is a fixed slot of the tail block. For this surface addrlib gives the same layout for 6 and 7 levels, with level 6 in tail slot 1, so the store lands inside the surface's own allocation and levels 0 to 5 do not move. Dropping the stores then loses texels the console writes. My version addresses such a view only when extending the chain to the view's last level keeps the allocated levels and the surface size unchanged, keeps one storage image for the surface, and still throws otherwise (linear chains, chains without a tail). It is waiting on #394 because it touches the same tiling files. I can open it so the maintainer can compare the two; what do you think? |
|
You're right, and thanks for explaining it. I checked with Mesa's addrlib (GFX10 path,
So the title's level-6 stores land in memory the surface already owns, and dropping them loses texels the console writes. Please open your version. I'll close #483 in favour of it once it's up. |
|
Closing in favour of #490, which writes the level-6 view into the mip tail the way the hardware does (verified with addrlib above). Thanks @oneandonlydean for the explanation and for opening it. |
2 participants
What
A T# whose
BASE_LEVELis past its ownMAX_MIPnames a mip level the resource does not have.DecodeTextureResourcethrewguest texture descriptor starts past the surface's last mip level.PPSA21567 (Astro Bot) hits this in a per-level compute chain. For a 1920x1080 surface with 6 levels it creates views of levels 0..6, and every one of them carries
MAX_MIP = 5. It then runs one pass of the chain (same shader, same 60x34 grid as the valid levels) that stores into the level-6 view.There are two ways to handle it; maintainers may prefer the other one:
max(MAX_MIP + 1, BASE_LEVEL + view levels)host levels, so the level-6 view gets a seventh, host-only level. The title runs there.I chose to drop because growing invents a level that the title's own descriptors say does not exist. Writing that level back could address memory the title never allocated for this surface, and sizing a cached surface from a view rather than from
MAX_MIPwould change it for every other view of that surface. This also extends a precedent already inDecodeTextureResource: aLAST_LEVELpastMAX_MIPis clamped, because "the hardware never addresses them".Changes:
emptyflag (non-null T#, not dynamic-mip,BASE_LEVEL > MAX_MIP), part of the variant and disk-cache keys (static_assertsizes updated). The backend compilesimage_storeto an empty image away. Loads, samples, size queries and atomics on it fail compilation. The T# level fields are read through small named helpers, also used by the two existing sites inResourceMaterializer.cpp.DecodeTextureResourcereturns such a view marked empty and clamped toMAX_MIP. The storage binding uses that existing level as a placeholder that is never written back. Sampled bindings of an empty view still throw (RequireLevelsPresent). Dynamic-mip storage writes to an empty view throw.[gpu] storage image view of surface 0x... starts at mip 6, past its last mip 5: the shader's stores to it are dropped.Hardware behaviour: I found no AMD documentation for stores through a view whose
BASE_LEVELis pastMAX_MIP(dropped or clamped). The RDNA2 ISA reference was not retrievable (its published URLs now redirect to the docs.amd.com portal), and searching turned up only emulator code. So this is a conservative choice, not a documented one. For comparison, stores past a view's extent are already left to the GPU here (image stores carry no bounds check and no robustness feature is enabled), and the title's valid passes at levels 3..5 rely on that.Tested
ctest151/151 on currentmain+ this branch.agc_driver_empty_mip_view(2-level 8x8 R32_UINT surface): a store through the last real level reaches memory (positive control), a store through a level-2 view leaves memory untouched and logs once, and a load through the level-2 view fails to compile. New decode cases inGuestTextureResource.mainwith other open PRs and local changes (not cleanmain): the level-6 views of two surfaces log their dropped stores once each, and the title runs on (about 4 more minutes of rendering) to an unrelated texture error.Overlap with #425 / #426: this PR is independent of both. The pr-overlap check flags
ShaderResources.cppbecause the edits touch neighbouring lines (textual overlap only, no shared behaviour). I'll rebase after either one merges.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt (the dropped store is logged, not silent).mdfiles or images added to the repository🤖 Generated with Claude Code