Skip to content

fix(agc): bound tessellation patch records by the draw's patch count - #4114

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
Celegans12:fix/agc-tessellation-num-patches
Oct 13, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
Celegans12:fix/agc-tessellation-num-patches

Conversation

@Celegans12

@Celegans12 Celegans12 commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What

Tessellation lowering proves that every ring and LDS address is affine in the control point and the relative patch, with the relative patch bounded by a fixed 256. A hull that lays its patch records out downwards from a base, such as 0x8000 - 208 * (rel + 1), then fails as "not affine", because record 255 would be negative even though the draw never has that many patches.

  • DecodeShaderStages reads VGT_LS_HS_CONFIG.NUM_PATCHES (bits 7:0, the patches per hull threadgroup) into the tessellation configuration as patchesPerGroup, and refuses 0.
  • Hull and domain lowering bound the relative patch by it; local programs keep their own bound.
  • It joins the draw, validation and pipeline keys, the hull and domain cache keys (not the local one), and the recompile request (version 22; older requests read as 256, the old bound).

SharpEmu decodes the field the same way (PatchesPerGroup = configuration & 0xFF, 0 rejected). PPSA26344's first terrain hull (VGT_LS_HS_CONFIG 0x1043f, 63 patches) stores its patch constants at exactly that descending stride and stopped there after #4082.

#4451 also adds a request version; whichever lands second renumbers.

Tested

Based on main c63dd44. Windows 11 (MinGW-w64 GCC 15.2.0) and Linux: configure -DBUILD_TESTING=ON, full build, --target libs, ctest. Measured on #4222's head, which contains this PR:

  • Windows NVIDIA: 1049/1051 before this rebase (log_macros fails on every branch and on main; patched_libraries cannot load main's libkernel_sys.prx); merged with main 14036cb, agc_driver_tessellation, agc_driver_graphics, agc_shader_memory, agc_driver_bda_device and agc_tessellation_factor_ring pass (5/5)
  • Linux (upstream's job in WSL, GCC 13.3, llvmpipe), merged with main 14036cb: 1047/1049 pass; agc_driver_depth_compare_copy and guest_math also fail on main

agc_driver_tessellation gains a hull storing patch constants at 0x8000 - 208 * (rel + 1): refused at 256 and 158 patches, rendered correctly at 157 (the boundary). agc_driver_graphics decodes 63 and 85 patches and refuses 0; agc_shader_memory covers version 22 and the hull/domain-only key. Each fails without it.

Not tested: console behaviour.

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 under Silent stubs in their library's technical debt file
  • 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
  • Unblocks: PPSA26344 Ghost of Yōtei (PPSA26344) #675
  • AI-assisted: yes

@tibzejoker

Copy link
Copy Markdown
Contributor

GTA V (PPSA04264) data point for this PR and #4082. Windows 11, RTX 4090, MinGW GCC 15.2.

GTA's hull stores its patch constants downward from 0x8000 (the 0x8000 - 208 * (rel + 1) shape you describe), with VGT_LS_HS_CONFIG 0xc338 and a maximum tessellation level of 15. On main 325e0a8 plus the other PRs GTA needs, this PR and #4082 replace the local patches I had for the same two things, and the hull program now compiles.

The next stop is the domain program, which reads those constants:

tessellation lowering: domain shader reads of patch constants or other patches are unsupported at pc 0x9c

I opened #4175 for that, stacked on this PR (its last commit only). With it, GTA's Story mode runs past that draw.

AI-assisted (Claude Code).

@blawesom

Copy link
Copy Markdown
Contributor

Rebase handoff — this branch was ~362 commits behind main (labeled needs-rebase for staleness; the rebase itself applied cleanly, no conflicts). Rebased onto current main, both commits' authorship unchanged.

Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, BUILD_TESTING=ON, AMD RX 9070 XT / RADV):

  • builds; tools/check_conventions.py --base origin/main passes
  • full suite: 949/949 passed — including the tessellation tests

To pick it up:

git fetch https://github.com/blawesom/AnyPS5 rebase/4114
git checkout <your branch>
git merge --ff-only FETCH_HEAD   # or reset to it

Head is d0ea5f4d1. Since #4163, #4172 and #4201 stack on this branch, I can rebase the chain on top of this if useful — say the word.

@blawesom

Copy link
Copy Markdown
Contributor

Independent verification of your rebase — the new head builds and passes the full local suite on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, BUILD_TESTING=ON, AMD RX 9070 XT / RADV): 905/905, conventions green. From my side this is merge-ready.

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 12, 2026
@Celegans12
Celegans12 force-pushed the fix/agc-tessellation-num-patches branch from d3f88f9 to 6b15b72 Compare October 13, 2026 01:44
@Celegans12

Copy link
Copy Markdown
Contributor Author

Thanks for the rebase handoffs across the chain, blawesom. I used your rebase/* heads to check my own rebase. They also give the stack a RADV/AMD data point that I can't get here. Main has moved a lot since then, so I rebased all six again onto main 510a218 and force-pushed them. Two things on main changed the result. First, docs/dev/TechnicalDebt.md was split into docs/dev/debt/, so the chain's entries now live in debt/shader.md. Second, three new request versions landed: main writes 19 now.

That second point is easy to miss. Your heads kept the chain's request versions at 17/18/19, but main had already written 17 with a different layout, and git can't flag that because the lines look the same. The chain now uses 20 (#4082), 21 (this PR) and 22 (#4201).

About listing every link under "Depends on" on #4222: each PR names only its direct parent, and the stack order follows from that. tools/pr_overlap.py also treats every number on that line as a dependency. So I've left it as one number per PR.

Details

@fdfawef324

Copy link
Copy Markdown
Contributor

Evidence from another title, and a bound that may help this PR.

GTA V (PPSA04264, #1203) hits exactly this refusal: a hull shader with 3 control points stores its patch constants downward from the top of a 32 KiB off-chip area, at 0x8000 minus 0x150 × (relative patch + 1). That form is affine; it is refused only by the range check, because PatchIndexCount assumes relative patch ids up to 255 and 0x7eb0 − 0x150 × 255 is negative.

The hardware bound is smaller than 256 regardless of VGT_LS_HS_CONFIG.NUM_PATCHES: Mesa computes the patches per threadgroup as 256 / max(input, output control points), because the in and out vertices per threadgroup "are at most 256, which is the hw limit" (ac_shader_util.c L948-L954 at 5c142e46). Using min(NUM_PATCHES, 256 / max(in, out)) as the bound would also cover requests saved before the new field, which otherwise read as 256 patches and still fail on this shader.

I have a small branch that applies only that hardware bound, without touching the request format or the keys, with a hull test using 336-byte records from 0x8000 and an IR test that checks the lowered offsets for several patches: fdfawef324:fix/recompiler-hull-patch-range. I am not opening it as a separate PR since this one covers the same refusal; feel free to take anything from it, or tell me if you would rather I open it so the two bounds can be combined.

@fdfawef324

Copy link
Copy Markdown
Contributor

GTA V (PPSA04264) with this PR and #4163. Windows Server 2025, RTX 3060, MinGW-w64 GCC 15.2.0.

Base. An integration branch on main 7e181fc that already has #4082 rebased (7e3eefa) and other open PRs. I cherry-picked 6b15b72 (this PR) and then f4aa18d (#4163). The branch also carries the hull bound of 256 / max(input, output control points) from my earlier comment. In the merge it applies only when patchesPerGroup is unknown (256, i.e. requests older than version 21), so in game the bound is VGT_LS_HS_CONFIG.NUM_PATCHES.

Conflicts. Against current main (fc1f063) this head conflicts in one file: core/libs/prx/libSceAgcDriver/tests/execution/Tessellation.cpp, in the list in main(), where #4727 added HullHalfSlotIndex and RequirePatches(..., HullDynamicIndex, ...). #4082 hits the same place. Everything else merges cleanly. main still writes request version 19, so 20 and 21 are free.

Tests. agc_driver_tessellation, agc_driver_graphics, agc_shader_memory and agc_tessellation_factor_ring pass. check_conventions.py passes on the two commits.

GTA V. The tessellated draw has VGT_LS_HS_CONFIG 0xc338: 3 input and 3 output control points, NUM_PATCHES 56. Hull 0x25cb4ea00 writes 336-byte patch-constant records at 0x8000 - 0x150 * (rel + 1), so record 255 would be negative.

  • Offline, I replayed the title's hull and domain requests, rewritten to version 21. With 56 patches per group, both compile. With 256, the hull is refused at pc 0xc7c, where GTA stopped in game before.
  • In game, the run gets past that draw. On this branch without the two PRs, GTA aborted 43-46 s in after 57-205 frames: first on the hull at pc 0xc7c, then, with only the hull bound, on the domain at pc 0x9c. With both PRs there is no tessellation refusal in three runs (231 s, 150 s and 375 s). The loading screen reaches 351-445 frames at 23-29 FPS.

The next stop is outside this PR.

  • Presents stop. After those frames the game thread spends about 5.5 minutes in sceAgcCreateInterpolantMapping, where the driver prepares each program synchronously in ResolveShaderAbi: about 4,100 from the disk cache and 3,300 new compiles.
  • Abort. At 375 s it aborts on pixel shader 0x25cb2dc00: GetImageResource dword 0 is not a valid runtime value. That is the same refusal that six programs already log at registration without aborting.

@Celegans12
Celegans12 force-pushed the fix/agc-tessellation-num-patches branch from 6b15b72 to c3f418f Compare October 13, 2026 09:42
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked: the request version is written as 21 and read up to 21 (older requests read 256), the new field is in the draw, validation and pipeline keys and in the hull/domain cache keys only. agc_driver_graphics, agc_shader_memory and agc_tessellation_factor_ring pass, and agc_driver_tessellation passes on CI's lavapipe. The new tests do not build without the change. GTA V runs by @fdfawef324 and @tibzejoker confirm the hull and domain now compile. Merging once the full Build is green.

TessellationLowering proved ring and LDS addresses affine over a
relative patch index of up to 256. A hull that lays its patch-constant
records out downwards, at 0x8000 - 208 * (rel + 1) like the quad
particle hull in Ghost of Yotei, then reached a negative address for
the last slot and aborted as "not affine". The relative patch index of
hull and domain programs stays below VGT_LS_HS_CONFIG.NUM_PATCHES
(bits 7:0, patches per threadgroup), which is 63 for that draw.

DecodeShaderStages now carries NUM_PATCHES as patchesPerGroup in the
tessellation configuration (draw keys, hull and domain cache keys,
recompile request version 22) and refuses zero, and the lowering bounds
the relative patch index by it. Configurations without it keep 256.
@Celegans12
Celegans12 force-pushed the fix/agc-tessellation-num-patches branch from c3f418f to 25a8372 Compare October 13, 2026 12:17
@Celegans12

Copy link
Copy Markdown
Contributor Author

@adriadam10 I rebased the tessellation chain onto current main, and one thing in this PR had to change: #3763 moved main's recompile requests to version 21 without adding a field, and this PR also used 21 for the new patchesPerGroup word. Git merged the two identical lines silently, so a request written by main at version 21 would have been read here with a patchesPerGroup it never had.

This PR now writes version 22 and reads patchesPerGroup only from 22 on; the agc_shader_memory expectations moved with it. Nothing else changed, the rest of the diff is line-for-line the one you looked at. #4201 moves from 22 to 23 the same way, and #4163, #4172 and #4222 are plain rebases.

Details

@github-actions

github-actions Bot commented Oct 13, 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
#4650 core/shader/recompiler/ControlFlow/src/RequestSerializer.cpp L751, L783

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the NUM_PATCHES decode (bits 7:0 of VGT_LS_HS_CONFIG, 0 refused), the bound in hull/domain lowering (Local keeps its own), the draw/validation/pipeline keys and the hull/domain-only cache key, and the version 22 request (older requests read 256). agc_driver_graphics, agc_shader_memory and agc_tessellation_factor_ring pass; agc_driver_tessellation passes on CI's lavapipe, with the descending-records hull refused at 158 and 256 patches and rendered at 157. The new tests do not build without the change. Confirmed on GTA V by @tibzejoker and @fdfawef324. Merging once the full Build is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants