Repository navigation
docs(agc): name what shadPS4, fpPS4 and addrlib do for PS4 layered surfaces and display buffers - #13484
Conversation
…rfaces and display buffers Two sentences in the PS4 layered surfaces and display buffers entries. AMD's address library and shadPS4 XOR the bank rotation of an array layer into the bank, which this code does since the layer rotation change, and fpPS4 adds it. shadPS4 rounds the width of a tiled display buffer up to 128 itself, whereas DisplayBufferSize requires the pitch to be a multiple of 128 already. Nothing is claimed about the console. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — looks good
The documentation-only changes match the pinned comparison sources and the implementation at the reviewed head.
What I ran
Reviewed head cd67746df82be1d967dd3aed81f349b50b48cb1c against base 556547da71c5e25b742f17b1ed9b27253b6e9d4d; the read-only merge preview onto current main at that base was clean. Environment: Linux x86_64, Python 3.11.2, GCC 12.2.0.
git diff --check 556547da71c5e25b742f17b1ed9b27253b6e9d4d...cd67746df82be1d967dd3aed81f349b50b48cb1c
python3 tools/check_conventions.py --base 556547da71c5e25b742f17b1ed9b27253b6e9d4d --head cd67746df82be1d967dd3aed81f349b50b48cb1c
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 13484I also inspected the cited external source lines at their pinned commits and the corresponding AnyPS5 implementation at the reviewed head.
Results
| Check | Result |
|---|---|
| Diff whitespace check | Passed; no output |
| Conventions and PR template | Passed |
| Current-main/dependency merge previews | Clean for #13484 and the four dependencies parsed by the stack helper (#9672, #9674, #9891, #9895); those heads are not included in #13484. #9892 is named in the overlap note and is individually mergeable, but was not part of the parsed chain. |
| Source comparison | Confirmed: AMD addrlib and shadPS4 XOR the layer rotation into the bank; fpPS4 adds it. shadPS4 rounds tiled display width to 128 pixels and DisplayBufferSize requires a pitch divisible by 128. |
| Aggregate precheck | Incomplete: its test-quality helper was unavailable in the isolated checkout; this documentation-only diff has no tests to assess. |
Unedited output
All conventions checks passed.
git diff --check produced no output. The precheck output was:
/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13484-cd67746df82b/report.md
/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13484-cd67746df82b/report.json
Conclusion: checks failed
The failed helper emitted:
/usr/bin/python3: can't open file '/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13484-cd67746df82b/head-cd67746df82b-4094f416/tools/pr_test_quality.py': [Errno 2] No such file or directory
Findings
Blocking
None.
Non-blocking
- The PR description says its latest overlap scan ran at
2026-10-30 17:15 UTCagainst main9304896d89. At the live snapshot for this review, it was 16:42 UTC and main was556547da71c5e25b742f17b1ed9b27253b6e9d4d. Please replace the postdated scan and olderTestedbase reference with evidence from an actual current-main snapshot.
Sources
- AnyPS5 layer rotation implementation at this head: TextureTiling.cpp, TextureTiling.hpp.
- AnyPS5 display pitch validation at this head: DisplayBuffer.cpp.
- shadPS4
tiling.compand shadPS4image_info.cpp. - fpPS4
ps4_tiling.pasand AMD addrlibegbaddrlib.cpp.
Not verified
- No build or tests were run; the PR changes documentation only.
- Hosted conventions, intake, labels and eligibility checks were queued at the final check snapshot.
|
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.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Thanks for checking this. The times in the Overlap paragraph were my local time (UTC+1), not UTC, which is why the 17:15 measurement looked later than your 16:42 UTC snapshot; it was taken at 16:15 UTC. The body now gives every time in UTC (12:29, 13:25, 14:15 and 16:15 UTC). I measured again on 2026-10-30 at 16:56 UTC against main AI-assisted (Claude Code). |
|
Checked the added sentences against the code ( |
What
Two existing debt entries in
docs/dev/debt/libSceAgcDriver.mdget one sentence each about how other PS4 implementations differ from this code. The entries then read:The new sentence in the first entry only records the comparison: the rotation of the bank per array layer has been implemented here since #12998, and it records that AMD's address library and shadPS4 XOR the rotation into the bank, where fpPS4 adds it. The second says shadPS4 rounds the width of a tiled display buffer up to 128 itself while
DisplayBufferSizerequires the pitch to be a multiple of 128 already.Changed:
docs/dev/debt/libSceAgcDriver.md, two existing entries. No code change.Method: the three were read at the linked commits; only their behaviour is described, no code was taken over. shadPS4
tiling.compL322-L335 andimage_info.cppL54; fpPS4ps4_tiling.pasL2887-L2912; AMD addrlibegbaddrlib.cppL3002-L3049. In fpPS4,xorand+have the same precedence in Pascal, sobank xor bank_swizzle + slice_rotationis evaluated left to right. The statements about this code were read inGcnLayerBankXorandTextureDetiler::layerBankXor(rotation, PRT modes get none) and inDisplayBufferSize(DisplayBuffer.cpp).sceVideoOutRegisterBufferscurrently rejects a tiled PS4 buffer, so the display sentence is about that function, not about a title. None of this is measured on a console.Overlap
Open PRs that change
docs/dev/debt/libSceAgcDriver.md(file sets fromgit diff --name-onlyagainst each PR head's merge base, all 1422 open pull requests as of 2026-10-30 12:29 UTC, main42d009f5f3; the 49 pull requests opened up to 13:25 UTC (1465 open, mainda026c7065) change other files, and the pairs below were measured again at that time): 131 touch the file or are ours. Of the pull requests opened since 12:29 UTC, only #13373 (a new "Adaptive staging" entry) touches the file; it merges with and without this commit (checked at mained1e0ed08f). Measured again on 2026-10-30 at 16:15 UTC against main9304896d89(1548 open pull requests): of the 46 pull requests opened since 14:15 UTC, only #13471 touches the file, and it merges with and without this commit; the five pairs below give the same result as before. Measured once more on 2026-10-30 at 16:56 UTC against main556547da71(1564 open pull requests, the head of each fetched fresh): 134 touch the file or are ours; the only ones that conflict with this commit but not with main are the five below, and #13373, #13471, #13474 and our other PRs (#12797, #12765, #12742) merge with and without it; #13491 and #13492, opened after the fetch, do not touch the file. Five give a new conflict with this change, all on the same lines: #9674 and #9891 rewrite the thick and layered surfaces entry, #9672 and #9895 the render targets and display buffers entry, and #9892 adds an entry next to them. Whichever of these merges second needs a rebase of the added sentence; the texts do not contradict each other. The others give the same merge result with and without this commit. Our other PRs (#12797, #12765, #12742) have no conflict with it.Tested
Documentation only.
tools/check_conventions.py --base upstream/mainpasses; both entries stay under 1000 characters; based onmainda026c7065, and it merges withmain556547da71without a conflict (checked 2026-10-30 16:56 UTC). Not built, not run.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)AI-assisted (Claude Code).
🤖 Generated with Claude Code