Skip to content

fix(agc): read shader user data without tail padding - #2063

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:codex/fix-agc-user-data-tail-padding
Oct 10, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:codex/fix-agc-user-data-tail-padding

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

Read only the serialized ShaderUserData fields through sharp_resource_count; the in-memory C++ struct has trailing padding that is not present in the guest shader header. Pin the serialized size to 54 bytes and cover a header that ends at that boundary.

Overlap audit

The overlap report with #2261 is limited to core/libs/prx/libSceAgcDriver/tests/Graphics.cpp. This PR tests the serialized ShaderUserData boundary; #2261 tests registered NGG workgroup routing. They do not share production files or behavior, and this PR does not depend on #2261.

Tested

Current PR head 62298425 is rebased linearly onto upstream main at cb9734cd8a9921067c84229a529518589010175a. The complete source diff remains limited to two files.

  • Linux x86-64, GCC 12.2, Release: built libSceAgcDriver and agc_driver_graphics_tests; agc_driver_graphics passed (1/1, 8.70 s) on this head. The regression pins the serialized boundary with static_assert(userDataBytes == 54).
  • python3 tools/check_conventions.py --base upstream/main and git diff --check upstream/main...HEAD pass.
  • Windows 11 Pro 25H2, RTX 5060 Ti (NVIDIA 617.42), MinGW-w64 GCC 15.2.0, Release: the focused test passed and full CTest passed (510/510) on pre-rebase commit 1b5a314b cherry-picked onto main d70b8998 (report).
  • Independent title validation from Recapi used a build based on main f1451396 plus dependencies from AGC driver: PPSA99005 (ps5-opengl, Mesa shaders) aborts in DecodeVertexStageInfo: header pointer outside the registered shader header #2026: with this fix, PPSA99005 got past DecodeVertexStageInfo and stopped at the next known check (sceAgcCreatePrimState: invalid context register offsets); agc_driver_graphics passed. This was not tested on current main.
  • Eligibility, Conventions, report, and all three relinker jobs pass on this head. The automated PR workflow skipped the full Linux and Windows Build jobs; an authorized collaborator must start Build for those under ci(build): require manual full builds and auto-test relinker for contr #2113. No console hardware was available.

Checklist

  • Based on current main; no other open PR implements this correction
  • One topic: bound reads to the serialized shader user-data fields
  • #include <cstddef> is included directly where offsetof is used
  • No notes, investigation files or images added to the repository
  • No new third-party code
  • Generic behavior, not title-specific
  • Depends on: none; the distinct overlap with fix(agc): route registered NGG workgroups through mesh #2261 is documented above
  • AI-assisted: yes; independently reviewed and tested

@Recapi

Recapi commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Checked with the title from #2026. On the same build (main f145139 plus the PRs listed there), with this PR in place of my local test change, PPSA99005 gets past DecodeVertexStageInfo and stops at the next known check, sceAgcCreatePrimState: invalid context register offsets. agc_driver_graphics passes with the PR, and fails with only the test applied (the driver file at main), so the new case catches the bug.

Two small points:

  • The test derives its expected size from the same offsetof(ShaderUserData, sharp_resource_count) + sizeof(...) expression the fix uses, so a mistake there would pass in both places. A static_assert(userDataBytes == 54) (or a literal 54 in the test) would tie it to what opengnm writes, RESOURCE_LAYOUT_SIZE 54u.
  • offsetof comes from <cstddef>, which neither ShaderInputState.cpp nor tests/Graphics.cpp includes; it builds now through another header.

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Addressed: both files now include <cstddef> directly, and the regression has static_assert(userDataBytes == 54) so the serialized size is independently pinned to the opengnm layout. Rebuilt agc_driver_graphics_tests; agc_driver_graphics passes (9.11 s), and conventions plus diff checks pass.

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased the fix and its regression test onto current upstream main 25f10087; pushed head c22c8e9c. The complete diff still covers only the serialized ShaderUserData bound and the literal 54-byte regression. Linux agc_driver_graphics passes (1/1, 8.99 s); conventions and diff checks pass. Fork Build 37903776529 is running on this head.

@MarcZX10
MarcZX10 force-pushed the codex/fix-agc-user-data-tail-padding branch from c22c8e9 to 9783357 Compare October 9, 2026 08:26
@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased the reviewed diff onto current upstream main 6cc0503d and pushed head 97833579. The updated Linux agc_driver_graphics CTest passes (1/1, 8.69 s); conventions and diff checks pass. Fork Build 37904987707 is running on the new head. The description records the previous full-build failure from the missing SPIR-V test include path, addressed by #2093.

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Latest fork Build completed with the same baseline failure on Linux and Windows: agc_shader_fusion_tests cannot include spirv/unified1/spirv.hpp from VulkanTestDevice.hpp. The relinker jobs passed. This matches #2093; I recorded the exact result in the PR description and kept this PR's scope separate.

@MarcZX10
MarcZX10 force-pushed the codex/fix-agc-user-data-tail-padding branch from 9783357 to 4b3629b Compare October 9, 2026 09:24
@MarcZX10
MarcZX10 force-pushed the codex/fix-agc-user-data-tail-padding branch 7 times, most recently from a00d995 to 1b5a314 Compare October 9, 2026 11:36
@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased PR #2063 linearly onto current main (d70b8998). On head 1b5a314b, agc_driver_graphics_tests builds and agc_driver_graphics passes (1/1, 9.18 s); conventions and diff checks pass, and GitHub reports the PR mergeable. The previously reported PPSA99005 run remains identified as an older-main validation. Under #2113, could a collaborator with write access run Actions → Build for this PR so the full Linux, Windows and relinker checks are recorded on the current head?

@rmz-24

rmz-24 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Windows run: Windows 11 Pro 25H2 (10.0.26200.9457), RTX 5060 Ti (NVIDIA 617.42), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON, full build plus libs. 1b5a314 cherry-picked onto current main d70b899 (applies cleanly).

  • agc_driver_graphics passes.
  • Full ctest: 510/510.

AI-assisted: yes

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the Windows report: it covers the exact PR head 1b5a314b on current main, and the full Windows CTest passes (510/510), including agc_driver_graphics. That supplies the Windows evidence. The upstream PR still reports only Conventions; under CONTRIBUTING, could an authorized collaborator run the official Build for this head to record Linux and the relinker matrix?

@MarcZX10
MarcZX10 force-pushed the codex/fix-agc-user-data-tail-padding branch from 1b5a314 to 36762dc Compare October 9, 2026 19:27
@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased the existing PR linearly onto current main (ae3d5f95) at head 36762dcf. The production change and its 54-byte serialized-boundary regression remain intact. Linux agc_driver_graphics_tests builds and agc_driver_graphics passes (1/1); local and GitHub Conventions pass. I documented that the overlap with #2261 is only the shared test source file: its registered NGG-routing tests and this PR’s serialized-user-data test exercise different behavior, with no production-file overlap or dependency.\n\nCould a collaborator with write access run the required upstream Build and relinker matrix on this exact head under #2113?

@MarcZX10
MarcZX10 force-pushed the codex/fix-agc-user-data-tail-padding branch from 36762dc to 37e7963 Compare October 9, 2026 20:05
@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Updated head and validation

I rebased the existing branch linearly onto upstream main at 8b393b6e and pushed head 37e7963f. The complete diff remains limited to ShaderInputState.cpp and its 54-byte regression in Graphics.cpp. On the updated source, libSceAgcDriver and agc_driver_graphics_tests build, and agc_driver_graphics passes (1/1); local Conventions and git diff --check pass. The Windows full-suite report and title run remain labeled as pre-rebase evidence.

The overlap audit still concerns only the shared test source with #2261; that PR tests NGG routing, while this one tests the serialized user-data boundary. There is no shared production change or dependency. Could a collaborator with write access run Actions → Build for PR #2063 on this head? The required upstream Linux/Windows and relinker matrix has not run for it.

@MarcZX10
MarcZX10 force-pushed the codex/fix-agc-user-data-tail-padding branch from 37e7963 to 6229842 Compare October 9, 2026 22:56
@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebase and current-head validation

Rebased the existing branch onto upstream main at cb9734cd8a9921067c84229a529518589010175a; the new head is 62298425. The reviewed diff remains limited to the serialized ShaderUserData read and its 54-byte boundary regression.

On Linux x86-64 with GCC 12.2 in Release mode, libSceAgcDriver and agc_driver_graphics_tests build, and agc_driver_graphics passes (1/1, 8.70 s). The local Conventions check and git diff --check pass. The Windows full-suite report and PPSA99005 run in the description are from earlier bases and are not represented as current-head validation.

The official upstream Build and relinker matrix has not run on this head; under #2113 it needs a collaborator with write access to dispatch it.

oneandonlydean added a commit to oneandonlydean/AnyPS5 that referenced this pull request Oct 10, 2026
oneandonlydean added a commit to oneandonlydean/AnyPS5 that referenced this pull request Oct 10, 2026
@oneandonlydean
oneandonlydean merged commit 606d4c3 into boykopovar:main Oct 10, 2026
9 checks passed
SleglJan pushed a commit to SleglJan/AnyPS5 that referenced this pull request Oct 10, 2026
…data-tail-padding

fix(agc): read shader user data without tail padding
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.

4 participants