Skip to content

fix(agc): preserve clip control during preparation - #2290

Closed
Sanjaysrinivas wants to merge 6 commits into
boykopovar:mainfrom
Sanjaysrinivas:fix/clip-distance-preparation
Closed

Sanjaysrinivas wants to merge 6 commits into
boykopovar:mainfrom
Sanjaysrinivas:fix/clip-distance-preparation

Conversation

@Sanjaysrinivas

@Sanjaysrinivas Sanjaysrinivas commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

Depends on: #423. This follow-up carries PA_CL_VS_OUT_CNTL through registered shader preparation and static graphics ABI resolution. #423 passes the control at draw time; preparing an otherwise complete registered vertex shader without it produces a different artifact key, so the draw throws prepared shader artifact is missing for the requested static ABI.

Use complete shader headers in the driver regression and resolve the combined graphics ABI for each clip/cull control change. Both draw-key modes verify the rendered pixels. The fixture shuts the driver down explicitly.

Main already uses request version 13 for float modes. Reserve version 14 for the added vertex control instead of changing version 13's layout. The regression replays a version-13 vertex request with non-default float-mode fields and verifies that those fields survive while clip control defaults to zero.

Based on main d70b89989473ba1f6ae13e44e079e67f1f8a44b0. The branch includes #423's five prerequisite commits, retaining Michon's authorship; merge conflicts were reconciled with main's multisampled-image capabilities, null-descriptor feature and test registration. The additional follow-up is commit c16aaec2e. This draft should wait for #423, then be rebased to remove the prerequisite commits.

The existing position-export mapping and instruction semantics are reused. TechnicalDebt records the mapping's lack of PS5 hardware verification and the unsupported mesh/tessellation clip/cull paths.

Overlap audit, 9 October 2026, at main f8c2f072: #2169 adds MSAA sample-location state alongside #423's clip-control state and tests. It does not carry PA_CL_VS_OUT_CNTL through registered preparation. Reconciliation must keep both sets of state fields, decoding and tests. #2327 removes the redundant decode/CFG/structurize pre-check beside this follow-up's vertex-control assignment in PrepareRegistered; keep the clip-control assignment while removing that redundant pass, with preparation still validating unsupported code. Virtual merges confirm conflicts in the state/test files and ShaderRegistry.cpp; they do not establish combined behavior. Rebase and retest after either lands. The implementation dependency remains Depends on: #423.

Tested

Windows, MinGW-w64 GCC 15.2.0, Release, NVIDIA GeForce RTX 4070 Laptop GPU, Vulkan required:

  • Built the standalone latest-main branch and passed all 10 focused checks: shader memory, graphics validation, direct clip/cull rendering, registered driver rendering with the draw key enabled and disabled, and all five prepared-shader modes. No checks were skipped.
  • Removed only the three new preparation assignments: both registered driver draw modes failed with the missing-static-ABI error in 1.07 seconds. Restored the assignments and rebuilt: both passed their full pixel checks in 1.17 seconds.
  • The direct GPU fixture renders clip/cull patterns in wave32 and wave64, including sparse enables, clipping half-planes, and primitive culling. The request regression preserves version-13 float modes and existing legacy pixel requests.
  • A separate local Silent Hill 2 startup stack using this port reached the production logos at approximately 30 FPS, then stopped with a missing null-pixel artifact at push offset 68. Gameplay remains unverified. That runtime contains other startup fixes and is separate from this PR's latest-main build.
  • Committed-head conventions and whitespace checks passed. Linux, macOS and PS5 hardware were not tested locally. Full upstream Windows/Linux builds require a maintainer's manual workflow run.

AI-assisted: yes.

Checklist

  • Based on current main; no other open PR implements this preparation follow-up
  • One topic per PR; follow-ups go in a new PR
  • No comments added to code
  • Unimplemented paths throw; no silent stubs added
  • No notes, investigation files or images added to the repository
  • No new third-party code
  • Generic behaviour, not specific to one title
  • Depends on: feat(agc): vertex clip and cull distances #423
  • AI-assisted: yes

MichonGoddijn231849 and others added 6 commits October 9, 2026 16:28
PA_CL_VS_OUT_CNTL clip and cull distance enables (up to eight distances) are accepted.
The vertex stage info carries the register, so the recompiled vertex stage writes
ClipDistance/CullDistance, its cache key and the serialized request include it, the
validator accepts those vertex outputs (f32[1..8]) and capabilities, and the device
enables shaderClipDistance and shaderCullDistance.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
boykopovar#752 took version 7 for the pixel stage's conservative Z export, so a version 7 request
written without this change has no PA_CL_VS_OUT_CNTL word: the field moves to version 8.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
main took version 8 for the pixel stage's target export mapping, which
is the version this PR used. A version 8 request written by main has no
PA_CL_VS_OUT_CNTL and would have been misread, so this PR writes
version 9 and reads the field only from version 9 on.
…river; capabilities only where enabled

agc_driver_vertex_clip_cull_distances draws with clip planes, a sparse CLIP_DIST_ENA (0x5)
and cull enables; agc_driver_vertex_clip_cull_draw (and _no_key) submits the same through
PM4. ShaderValidation accepts the ClipDistance/CullDistance capabilities only when the device
enabled the features, and names PA_CL_VS_OUT_CNTL when it refuses. A version-12 vertex
request reads paClVsOutCntl as 0.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Carry PA_CL_VS_OUT_CNTL through registration and static graphics ABI
preparation so draws reuse the matching vertex artifact.

Reserve request version 14 for clip control; main uses version 13 for
float modes. Exercise complete registered headers in both draw-key modes
and preserve legacy version-13 float modes.

Refs boykopovar#423
@Sanjaysrinivas
Sanjaysrinivas marked this pull request as ready for review October 9, 2026 14:59
@github-actions

github-actions Bot commented Oct 9, 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
#2169 core/libs/prx/libSceAgcDriver/Graphics/include/State.hpp L100
core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp L514
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp L1092, L2846
#2327 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp L367-370

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: this puts PA_CL_VS_OUT_CNTL into the prepared shader key, and #423 is being reworked to pass the clip/cull enables as runtime data, which removes the missing-artifact error this works around. If #423 still needs something here after that rework, please reopen.

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

Labels

agc libSceAgc* build docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants