Skip to content

docs(agc): say that only a whole-plane storage write retires a depth surface - #8423

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
thuanquang:docs/debt-depth-surface-retire-whole-plane
Oct 24, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
thuanquang:docs/debt-depth-surface-retire-whole-plane

Conversation

@thuanquang

@thuanquang thuanquang commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What

The depth surface entry in docs/dev/debt/libSceAgcDriver.md says a depth surface is retired when "a written storage image it does not serve overlaps it". Since #7050 (a714d1b37), a written storage view retires only depth surfaces whose whole plane it covers: ShaderResources.cpp:1134 passes the written bytes and extents to RetireCoveredDepthSurfaces, and DepthSurface.cpp:2489 retires a surface only when CoveredBy holds (DepthSurface.cpp:552-573: the written bytes or a matching block extent cover the whole depth or stencil plane), so a partial overlap leaves it alive. The clause now reads "a color target overlaps it, an unserved storage view writes a whole plane, or it is sampled ...".

#7154, merged since, edited the next sentence of the same line; this branch is rebased onto it and keeps its text. #9026 (f57bbc9) has since changed the next entry, "Depth planes in guest memory"; this branch is rebased onto main 6ef5a1c and keeps that text too.

Tested

Docs only; nothing was built or run. Checked on main 6ef5a1c by reading ShaderResources.cpp:1134 and DepthSurface.cpp:552-573 and :2486-2491; the other retire path, RetireDepthSurfaces (DepthSurface.cpp:2479-2484), is called outside tests only for color targets (Graphics/src/Draw.cpp:2244). CoveredBy and both retire functions are unchanged since a07e985c8; only their line numbers moved.

  • python tools/check_conventions.py --base origin/main with this title and body passes.
  • Not tested: behaviour on any machine; this changes text only.

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
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes (Claude Code)

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 2434 docs/dev/debt/libSceAgcDriver.md
PR 4448 docs/dev/debt/libSceAgcDriver.md
PR 5058 docs/dev/debt/libSceAgcDriver.md
PR 5925 docs/dev/debt/libSceAgcDriver.md
PR 6186 docs/dev/debt/libSceAgcDriver.md
PR 6194 docs/dev/debt/libSceAgcDriver.md
PR 6223 docs/dev/debt/libSceAgcDriver.md
PR 6273 docs/dev/debt/libSceAgcDriver.md
PR 6280 docs/dev/debt/libSceAgcDriver.md
PR 6300 docs/dev/debt/libSceAgcDriver.md
PR 6412 docs/dev/debt/libSceAgcDriver.md
PR 6481 docs/dev/debt/libSceAgcDriver.md
PR 6548 docs/dev/debt/libSceAgcDriver.md
PR 6549 docs/dev/debt/libSceAgcDriver.md
PR 6808 docs/dev/debt/libSceAgcDriver.md
PR 6867 docs/dev/debt/libSceAgcDriver.md
PR 6977 docs/dev/debt/libSceAgcDriver.md
PR 7256 docs/dev/debt/libSceAgcDriver.md
PR 7446 docs/dev/debt/libSceAgcDriver.md
PR 7449 docs/dev/debt/libSceAgcDriver.md
PR 7604 docs/dev/debt/libSceAgcDriver.md
PR 7618 docs/dev/debt/libSceAgcDriver.md
PR 7642 docs/dev/debt/libSceAgcDriver.md
PR 7844 docs/dev/debt/libSceAgcDriver.md
PR 7910 docs/dev/debt/libSceAgcDriver.md
PR 8014 docs/dev/debt/libSceAgcDriver.md
PR 8223 docs/dev/debt/libSceAgcDriver.md
PR 8319 docs/dev/debt/libSceAgcDriver.md
PR 8365 docs/dev/debt/libSceAgcDriver.md
PR 8542 docs/dev/debt/libSceAgcDriver.md
PR 8592 docs/dev/debt/libSceAgcDriver.md
PR 8639 docs/dev/debt/libSceAgcDriver.md
PR 9348 docs/dev/debt/libSceAgcDriver.md
PR 9356 docs/dev/debt/libSceAgcDriver.md
PR 9387 docs/dev/debt/libSceAgcDriver.md
PR 9393 docs/dev/debt/libSceAgcDriver.md
PR 9431 docs/dev/debt/libSceAgcDriver.md
PR 9432 docs/dev/debt/libSceAgcDriver.md
PR 9508 docs/dev/debt/libSceAgcDriver.md
PR 9592 docs/dev/debt/libSceAgcDriver.md
PR 9672 docs/dev/debt/libSceAgcDriver.md
PR 9673 docs/dev/debt/libSceAgcDriver.md
PR 9674 docs/dev/debt/libSceAgcDriver.md
PR 9675 docs/dev/debt/libSceAgcDriver.md
PR 9676 docs/dev/debt/libSceAgcDriver.md
PR 9748 docs/dev/debt/libSceAgcDriver.md
PR 9842 docs/dev/debt/libSceAgcDriver.md
PR 9891 docs/dev/debt/libSceAgcDriver.md
PR 9892 docs/dev/debt/libSceAgcDriver.md
PR 9893 docs/dev/debt/libSceAgcDriver.md
PR 9895 docs/dev/debt/libSceAgcDriver.md
PR 9898 docs/dev/debt/libSceAgcDriver.md
PR 9904 docs/dev/debt/libSceAgcDriver.md
PR 9905 docs/dev/debt/libSceAgcDriver.md
PR 9906 docs/dev/debt/libSceAgcDriver.md
PR 10193 docs/dev/debt/libSceAgcDriver.md
PR 10420 docs/dev/debt/libSceAgcDriver.md

@github-actions github-actions Bot added docs needs rebase Conflicts with main, rebase needed labels Oct 20, 2026
@thuanquang
thuanquang force-pushed the docs/debt-depth-surface-retire-whole-plane branch from 96315d6 to 438cfcf Compare October 21, 2026 05:37
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 21, 2026

@harisibrar1 harisibrar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked against main:

  • ShaderResources.cpp:1127-1130 calls RetireCoveredDepthSurfaces only for a written storage view that DepthSurfaceStorage (:1116) did not serve, so "an unserved storage view" is right.
  • DepthSurface.cpp:2216-2220 retires a surface only when CoveredBy (DepthSurface.cpp:512-531) holds. It holds when the written bytes, or a matching block extent, cover the whole depth or stencil plane, so a partial overlap leaves the surface alive, as the new text says.
  • The color-target path is the other retire call: RetireDepthSurfaces at Draw.cpp:2215. "A color target overlaps it" is unchanged and accurate.
  • tools/check_conventions.py --base main --head pr-8423 passes.

Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 21, 2026
…surface

Co-Authored-By: Claude <noreply@anthropic.com>
@thuanquang
thuanquang force-pushed the docs/debt-depth-surface-retire-whole-plane branch from 438cfcf to 6e1f2f4 Compare October 22, 2026 04:06
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 22, 2026
@adriadam10 adriadam10 self-assigned this Oct 24, 2026
@adriadam10
adriadam10 merged commit 20af9f8 into boykopovar:main Oct 24, 2026
10 of 11 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the new clause against main: RetireCoveredDepthSurfaces is called only for a written storage view the depth path did not serve, and it retires a surface only when CoveredBy holds for the whole plane; color targets go through RetireDepthSurfaces. Text is accurate, conventions green; merged.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants