Skip to content

fix(agc): address texture views that start past the surface's last mip - #490

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
oneandonlydean:fix/agc-views-past-max-mip
Oct 5, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
oneandonlydean:fix/agc-views-past-max-mip

Conversation

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Title: fix(agc): address texture views that start past the surface's last mip

What

Replaces #483, as @ThisIsAkill asked (comment): instead of dropping the stores, a view whose BASE_LEVEL is past MAX_MIP addresses that level where the hardware puts it, when it lies inside the surface's allocation.

PPSA21564 (and PPSA21567, see #483) ends each 1920x1080 RGBA16F bloom downsample chain (MAX_MIP 5) with a compute pass that stores level 6 through a view of levels 6..6, every frame from the PS logo on. DecodeTextureResource throws for that view.

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, counted from the first tail level. A level past MAX_MIP is addressed the same way. ThisIsAkill's own addrlib check on #483 (Addr2ComputeSurfaceInfo, five GB_ADDR_CONFIG settings) and mine below agree: for this surface, 6 and 7 levels give the same layout and size (0x1640000), the tail starts at level 5 in both, and level 6 is the next slot of that tail block. So the title's stores land in memory the surface owns, and levels 0 to 5 do not move.

Two commits:

  1. Decode (GuestTextureResource.cpp, TextureTiling.cpp): when BASE_LEVEL > MAX_MIP, LevelsFitAllocation describes the chain extended to the view's LAST_LEVEL and accepts it only if the guest size, the layer size and every allocated level (offset, size, pitch, tail slot) are unchanged, and the extra levels end inside the layer. The view then decodes with mipCount = LAST_LEVEL + 1 over the same bytes. Anything else still throws ("starts past the surface's last mip level at levels that would move the surface's own"), for example linear chains or chains without a tail, where an extra level would change the size. Views that start at or below MAX_MIP are clamped to it as before.
  2. One storage image (ShaderResources.cpp): storage images are shared through a surface key that includes the level count, so the extended view would get a second image of the same memory, and the two would flush and re-upload each other every frame. A surface once viewed past its allocated levels now keeps one image holding the extended chain, and its other descriptors find that image; the allocated-level image is evicted with its results stored. GuestTextureResource gets allocatedMipCount (MAX_MIP + 1) for this.

No recompiler change is needed: the shader side only rejects such descriptors for MSAA types.

Base: current main. The inventory note that this waited on #394 was about textual overlap in the tiling files; the change needs nothing from #394. It merges cleanly with #394 and with my thick-mip-tail follow-up of #394 (feat/agc-3d-thick-mip-tails), in either order, and agc_driver_graphics and the new execution test pass on that merge. With #394, a 3D view past MAX_MIP goes through the same check (#394 throws for 3D chains that reach a tail until the follow-up lands).

Tested

  • Linux (CachyOS, GCC 16.2, RTX 4090): full build with BUILD_TESTING=ON; ctest passes (169/169).

  • Expected values from addrlib: Mesa's src/amd/addrlib at e5f06878, built standalone, created as Navi10 (family 0x8F, revision 1) with GB_ADDR_CONFIG 0x44 (16 pipes, like the PS5 equations in TextureSwizzleEquations.hpp). Addr2ComputeSurfaceInfo (ADDR_RSRC_TEX_2D) gave the sizes, level offsets and tail coordinates, and Addr2ComputeSurfaceAddrFromCoord the texel addresses. The harness (addrlib-2d-harness.cpp, about 120 lines) and its output (navi10-2d.txt) are linked here.

    Surface Levels addrlib size Notes
    1920x1080, 64 bpp, SW_64KB_R_X 6, 7, 9, 11 0x1640000 levels 0 to 4 at 0x650000, 0x1d0000, 0x90000, 0x30000, 0x10000 in all; tail block at 0, level 5 at (64, 0), level 6 at (0, 32), 7 at (32, 0), 8 at (0, 16)
    1920x1080, 64 bpp, SW_64KB_R_X 1, 2 0xff0000, 0x1470000 no tail: a second level grows the surface, refused
    16x16, 32 bpp, linear 2, 3 0x1800, 0x1c00 a third level grows the chain, refused
    256x256, 32 bpp, SW_64KB_R_X 3, 4 0x60000 level 2 at (64, 0), level 3 at (0, 64); texels (0..7, 0) of level 3 at 0x4800..0x480c and 0x4880..0x488c
  • agc_driver_graphics (GuestTextureResource cases): the 1920x1080 view of levels 6..6 over MAX_MIP 5 decodes to 7 levels; size, every level's offset and the tail slots of levels 5 to 8 match the table, and levels 0 to 5 are identical to the 6-level layout. A view to level 8 covers 9 levels. The linear and untailed views are refused, and the sizes behind the refusals match addrlib.

  • New execution test agc_driver_view_past_last_mip (runs on lavapipe in CI): a compute shader stores through a level-3 view of a 256x256 R32_UINT SW_64KB_R_X surface with MAX_MIP 2. After write-back, addrlib's eight texel addresses of level 3 hold the stored value and levels 0 and 1 are untouched. A second store through the level-2 view lands at addrlib's level-2 addresses and keeps the level-3 texels.

  • agc_driver_recorder_tests (not registered in ctest; run by hand): after a view past MAX_MIP, the allocated-level storage image leaves the cache, and both views get the same 4-level image, whose levels 2 and 3 are read from their addrlib tail slots.

  • Fails without the change: on main with only the new tests, all three stop at "guest texture descriptor starts past the surface's last mip level". With the first commit only, the recorder test fails with "the allocated levels' image is still the surface's after a view past its last mip".

  • PPSA21564: the same two changes have been in my integration branch since 2026-10-03; both bloom chains' level-6 passes run every frame. This branch itself was not run in the title.

  • Fork CI (Linux and Windows) passes.

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

PPSA21564 ends each of its two 1920x1080 RGBA16F bloom downsample
chains (MAX_MIP 5) with a compute pass that stores level 6 through a
view of levels 6..6, every frame from the PS logo on. The descriptor
decode threw for such a view.

GFX10 uses MAX_MIP only to place the levels: 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 counted from the first tail level. A level past MAX_MIP
is addressed the same way, so when the chain extended to the view's last
level keeps every allocated level, the tail and the surface size
unchanged, the view reaches that slot inside the surface's allocation.
addrlib (GFX10, Navi10 with 16 pipes) computes the same 0x1640000-byte
layout for 6, 7, 9 and 11 levels of this surface, with level 6 in the
tail at (0, 32). The view then decodes with the extended chain over the
same guest bytes. Views whose extra levels would move the allocated
ones, such as linear chains or chains without a tail, still throw.
The descriptors of one surface share its storage image through the
surface key, which includes the level count. A view past MAX_MIP
counts the extra levels, so its storage image was a second image of
the same memory, and the two flushed and re-uploaded each other every
frame (PPSA21564 writes level 6 of each 1920x1080 bloom chain after
building levels 1 to 5 through MAX_MIP 5 views).

A surface once viewed past its allocated levels now keeps a single
image holding the extended chain, and its other descriptors look that
image up; the allocated-level image it replaces is evicted with its
results stored. The extra levels lie in the allocation (the decode
proved that), so levels 0 to MAX_MIP keep their layout and the views
of them read the same texels.
@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

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.

Pull request Conflicting files (lines on main) Same exports Same new files
#293 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp L519
#425 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp L519-526

@github-actions github-actions Bot added the agc libSceAgc* label Oct 4, 2026
@oneandonlydean
oneandonlydean merged commit ef88b5b into boykopovar:main Oct 5, 2026
3 checks passed
oneandonlydean added a commit to oneandonlydean/AnyPS5 that referenced this pull request Oct 5, 2026
Takes upstream's merged versions of our boykopovar#586 boykopovar#611 boykopovar#612 boykopovar#505 boykopovar#490 boykopovar#619
boykopovar#503 boykopovar#627 boykopovar#628 boykopovar#618 and drops astrobot's local copies, plus gate-batch-3
(boykopovar#469 boykopovar#500 boykopovar#504 boykopovar#509 boykopovar#510 boykopovar#514 boykopovar#525 boykopovar#526 boykopovar#528), round o/p (boykopovar#617 boykopovar#620
boykopovar#621 boykopovar#622 boykopovar#623 boykopovar#624 boykopovar#501 boykopovar#508 boykopovar#629 boykopovar#630 boykopovar#631).

Conflicts:
- SpirvAnalysis.hpp/.cpp, SpirvBufferAccess.cpp: upstream's boykopovar#586/boykopovar#612.
  The 64-bit LDS atomics keep the full array (f27d6e5 no longer needed).
- SpirvMemoryInstructions.cpp: upstream's constant slot plus offset;
  astrobot's split-wave append/consume kept.
- GuestTextureResource.cpp, TextureTiling.cpp and their tests: upstream's
  boykopovar#490/boykopovar#503 (astrobot had no other change there).
- MaskedSelect.cpp test, ShaderMemory.cpp LDS test: upstream's.
- State.cpp: upstream's boykopovar#505 export count and boykopovar#469 null pixel program;
  astrobot's depth export rejection and depth target model kept.
- Pipeline.cpp: upstream's pipeline key; astrobot's depth attachment and
  blend check (a trailing unused attachment stays valid, as
  colorGapPipelineTests expects).
- Draw.cpp, Recorder.cpp, ShaderResources.cpp: astrobot's (renderPassKey
  with depth, moved sample functions, residency maintenance), which
  already hold the merged boykopovar#505/boykopovar#628/boykopovar#490 changes.
- ShaderInputState.cpp: both register sets.
- State.hpp: the duplicate exportIndex member from the auto-merge removed.
- CMakeLists.txt: astrobot's test list (a superset of upstream's).
- tests/Graphics.cpp: upstream's CompactedExportTests replaces astrobot's
  inline copy; tests/Recorder.cpp: upstream's viewPastLastMipTests added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant