Repository navigation
docs(agc): correct five stale statements in the HTILE and multisampled target entries - #9077
Conversation
|
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.
|
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main:
- HTILE address match:
HtileFillCovers(DepthSurface.cpp:2119-2122) accepts any range containing the HTILE start and reachingHtileMetadataBytespast it, andtakeClearedHtile(:1375-1384) uses it, so "covers that surface's whole HTILE, which need not start where the clear does" is right. - HTILE other writers:
DepthSurfaceViewcallsstoreBackClearedHtile(:1720-1721), which reads HTILE written since the last read (htileReads, :1503-1535) and takes the fast clear. The pointer to the next entry is accurate. - CMASK fill codes:
MultisampledCmaskKeys(MultisampleTarget.cpp:739-744) returns a key only for 0 and for tiles 0xc to 0xf with equal nibbles, andNoteFill(:747-764) needs the 32-bit pattern to repeat that byte, so 0xcccccccc, 0xdddddddd, 0xeeeeeeee and 0xffffffff are exactly right. - Tile modes:
createFilled(:658-663) maps Standard4KB, Standard64KB, Z64KBX, D64KBX and R64KBX;guard(:578-580) uses fragment-aware blocks for the 64 KiB modes except Standard64KB (MultisampledBlockLog2, :32-38). Both rewordings match. check_conventions.py --base main --head pr-9077: 0 errors; the two debt-length warnings are on entries already over the limit, and the HTILE one gets shorter.- Optional nit: the sentence before the first change still says the clear applies to "the surface with that
DB_HTILE_DATA_BASEaddress", which now reads slightly against "need not start where the clear does".
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
b53bfe6 to
4157678
Compare
4157678 to
a3f1e60
Compare
a3f1e60 to
8e95e2d
Compare
8e95e2d to
8743fc7
Compare
…d target entries Co-Authored-By: Claude <noreply@anthropic.com>
8743fc7 to
5536623
Compare
|
Rebased onto current |
What
Five sentences in
docs/dev/debt/libSceAgcDriver.mddescribe code that has since changed. Each is replaced with whatmaindoes now; nothing else in the entries changes.HtileFillCovers(DepthSurface.cpp:2417-2420onmain), which accepts any cleared range that contains the surface's HTILE start and reachesHtileMetadataBytespast it;takeClearedHtile(:1565-1572) uses it. Now: "A clear hits a surface only when its range covers that surface's whole HTILE, which need not start where the clear does."MultisampledCmaskKeys(MultisampleTarget.cpp:901-907) returns the expanded key for the codes 0xcc, 0xdd, 0xee and 0xff, andNoteFill(:909-926) takes a whole-CMASK fill whose pattern repeats one of them. Now: "one with 0xcccccccc, 0xdddddddd, 0xeeeeeeee or 0xffffffff marks it expanded".createFilled(MultisampleTarget.cpp:819-826) also mapskZ64KBXandkD64KBX; only the default case refuses. Now: "tile modes other than Standard4KB, Standard64KB, Z64KBX, D64KBX and RenderTarget".guard(MultisampleTarget.cpp:736-746) takes the fragment-aware size for every 64 KiB mode thatMultisampledBlockLog2(:35-41) knows except Standard64KB, so Z64KBX and D64KBX as well as RenderTarget. Now: "uses fragment-aware 64 KiB blocks for the RenderTarget, Z64KBX and D64KBX tile modes."The
Storage image write-backentry also links a file that no longer exists (Draw/DrawPipeline.hpp; the file isDrawPipeline.cpp). It is left out here because open #7912 rewrites that line.Overlap: #7803 and #7948, which add entries right next to the edited lines, have both merged; this branch is rebased on top of them, and neither changed these sentences.
Tested
Docs only; nothing was built or run. Each statement was checked by reading the code cited above on
maind34da5d694, and the commits were found withgit log -Son the changed lines. Rebased ontomain990f45e17d(#7803 added the 'Sample 0 of a multisampled color target' entry right after the 'Multisampled texture no draw rendered to' entry; it stays asmainhas it) and rechecked there: the five statements still hold, atDepthSurface.cpp:2415-2418(HtileFillCovers) and:1563-1570(takeClearedHtile),MultisampleTarget.cpp:901-907(MultisampledCmaskKeys) and the fill check at:919, the tile-mode mapping at:819-826, andguardat:736-746withMultisampledBlockLog2at:35-41; the 'HTILE written by other kernels' entry is still the next entry. Of the 96 other open pull requests that edit this file (checked on990f45e17d), only #9700 (EQAA color targets) edits one of these entries: it changes other sentences of the Multisampled target guest access entry and keeps the one corrected here, so the two do not conflict in content, but they edit the same line and whichever merges second rebases it. Rebased ontomaine78acf4bdafor the conflict with #8283 (merged) indocs/dev/debt/libSceAgcDriver.md: main's text is kept and this PR's change is unchanged; the statements were rechecked on that commit.python tools/check_conventions.py --base origin/mainwith this title and body: 0 errors and twodebt-lengthwarnings, for entries that were already over 1000 characters: HTILE depth fast clears (2008 characters before, 1967 after) and Multisampled target guest access (1831 before, 1868 after). Multisampled CMASK state stays under 1000 (958 to 996).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)🤖 Generated with Claude Code