Skip to content

fix(agc): accept a copy's sample selection on a draw that copies nothing - #5937

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
milescoviello:fix/db-render-control-copy-sample
Oct 16, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
milescoviello:fix/db-render-control-copy-sample

Conversation

@milescoviello

@milescoviello milescoviello commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What

Forspoken (PPSA03984) stops 43 s after start on main:

terminate called after throwing an instance of 'std::runtime_error'
  what():  DB_RENDER_CONTROL depth copy, resummarize or decompress is unsupported (register 0x0 = 0x000000c0)

0xc0 is DEPTH_COMPRESS_DISABLE (bit 6) and COPY_CENTROID (bit 7), with DEPTH_COPY and STENCIL_COPY (bits 2 and 3) clear (Mesa's GFX10.3 register definitions). COPY_CENTROID and COPY_SAMPLE (bits 8–11) select the sample a depth or stencil copy reads; AMD's PAL sets them only together with a copy (gfx9DepthStencilView.cpp), and #5499 uses them that way to pick the copied sample. On a draw with neither copy bit set, nothing is copied and they have no effect. DepthMaintenanceRejection and decodeDepth both rejected them as if they were a copy.

Both checks now accept bits 7–11. A draw with DEPTH_COPY or STENCIL_COPY is still rejected, whatever sample it selects.

depthMaintenanceTests gains 0x80, 0xc0 (Forspoken's value), 0x100, 0xf00 and 0x2fe3 as ordinary controls, and keeps a copy with a sample selection (0x84, 0x108, 0xf88) as maintenance. The clear test decodes a combined depth and stencil clear with 0xc0 and with 0xf00 added and requires the same clear, and still rejects a clear with a copy and a sample selection. On main's State.cpp the test stops at "ordinary depth controls were mistaken for maintenance"; with only DepthMaintenanceRejection changed it stops in decodeDepth on 0xe3.

Tested

Ubuntu 24.04, GCC 13.3 (CI's compiler), Release with CI's configure flags, on main e34c479, RTX 4070 Ti with NVIDIA 595.99.02:

Not checked on Windows or on a console.

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
  • Unblocks: none
  • AI-assisted: yes

🤖 Generated with Claude Code

@Dextroid17

Copy link
Copy Markdown
Contributor

I ran this PR's test on Linux.

Setup: this PR's diff applied to main at 8cd937e (the PR's base is 421 commits behind it), default CMake build (CMAKE_BUILD_TYPE empty, unoptimized), clean environment. ctest -R '^agc_driver_graphics$', 3 runs each. The test did not open a GPU device, so this is a CPU-only run.

  • with this PR: 3 of 3 pass.
  • with Graphics/src/State.cpp put back to main's version (the new test kept): 3 of 3 fail at "AGC graphics: ordinary depth controls were mistaken for maintenance".

So the new test catches the old behaviour here. I did not check Windows, macOS or a real GPU.

Note: #5855 also edits Graphics/src/State.cpp and the same DepthStencilTests area of tests/Graphics.cpp, so whichever of the two merges second will likely need a rebase.

AI-assisted: yes

@milescoviello
milescoviello force-pushed the fix/db-render-control-copy-sample branch from 802ff5d to 182b4c6 Compare October 15, 2026 02:13
@milescoviello

Copy link
Copy Markdown
Contributor Author

Thanks for running it. I've rebased the branch onto main 760ffa6; there were no conflicts, agc_driver_graphics passes on an RTX 4070 Ti, and tools/check_conventions.py passes. Noted about #5855 — whichever lands second gets a rebase.

AI-assisted: yes

@milescoviello
milescoviello force-pushed the fix/db-render-control-copy-sample branch 2 times, most recently from 6e0f3cf to 573bf5a Compare October 15, 2026 09:02
@Dextroid17

Copy link
Copy Markdown
Contributor

Thanks. I reran the same Linux check on the current head 573bf5a, after the latest push. Its change has the same git patch-id --stable as the head I tested before (802ff5d), so only the line numbers moved.

Setup: head 573bf5a merged into main at a08de68 (no conflicts; main has 14 commits that the head does not), default CMake build (CMAKE_BUILD_TYPE empty, unoptimized), clean environment (env -i, only HOME, PATH and VK_ICD_FILENAMES set). ctest -R '^agc_driver_graphics$', 3 runs each. CPU-only as before (the test uses a mock Vulkan device).

  • with this PR: 3 of 3 pass.
  • with Graphics/src/State.cpp put back to main's version (the PR's test changes kept): 3 of 3 fail at "AGC graphics: ordinary depth controls were mistaken for maintenance".

So the result from my first run holds on this head. I did not check Windows, macOS or a real GPU.

AI-assisted: yes

@github-actions

github-actions Bot commented Oct 15, 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
PR 6146 core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp L258-261, L918
PR 6762 core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp L258-259

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 1395 core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 4040 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 4650 core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 5441 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 5499 core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 5500 core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 5925 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6188 core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6222 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6235 core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6312 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6320 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6481 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6523 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp

@milescoviello
milescoviello force-pushed the fix/db-render-control-copy-sample branch from ac8914f to bb0dbe4 Compare October 15, 2026 12:55
@milescoviello
milescoviello force-pushed the fix/db-render-control-copy-sample branch from bb0dbe4 to 5ce989d Compare October 15, 2026 13:15
DB_RENDER_CONTROL.COPY_CENTROID and COPY_SAMPLE select the sample that
DEPTH_COPY or STENCIL_COPY copies. Forspoken (PPSA03984) draws with
DB_RENDER_CONTROL 0xc0, DEPTH_COMPRESS_DISABLE and COPY_CENTROID with
neither copy bit set, and stopped at 43 s because both
DepthMaintenanceRejection and decodeDepth took the selection for a copy.

Accept the selection fields when no copy is enabled; draws that copy are
still rejected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@milescoviello
milescoviello force-pushed the fix/db-render-control-copy-sample branch from 5ce989d to 4cdd2f4 Compare October 15, 2026 16:55
@adriadam10 adriadam10 self-assigned this Oct 16, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the DB_RENDER_CONTROL masks against the GFX10.3 register layout (0x101c in decodeDepth, ~0x2fe3 in DepthMaintenanceRejection): copy and resummarize/decompress still throw, a sample selection without a copy is ordinary. agc_driver_graphics passes, and fails on main's State.cpp at "ordinary depth controls were mistaken for maintenance". Merging once the full Build is green.

@milescoviello

Copy link
Copy Markdown
Contributor Author

Thanks for checking the masks. One thing that will hold up the Build: main at 65d56a2 doesn't compile on Linux with libstdc++. #5875's journal guards MeshConfiguration at 36 bytes, and #5884 made it 40, so PipelineJournal.cpp stops on its static_asserts. #6784 fixes that (MarcZX100 approved it). Until it lands, the Linux job here fails for that reason, not this change.

AI-assisted: yes

@milescoviello

Copy link
Copy Markdown
Contributor Author

An update for the Build: #6784 is in, so main compiles on Linux again. One test still fails on main itself: agc_driver_graphics, since #5804 (#6792, a zero-filled vertex code span). #6789 fixes it and is approved, so a Build of this PR before #6789 lands will show that one failure regardless of this change. With #6789 applied, agc_driver_graphics passes with this PR, as you saw.

AI-assisted: yes

@adriadam10

Copy link
Copy Markdown
Collaborator

Rechecked on current main 93e54ce: head merges cleanly, agc_driver_graphics passes and still fails on main's State.cpp; merging once the full Build is green.

@adriadam10
adriadam10 merged commit 6fd4de2 into boykopovar:main Oct 16, 2026
9 checks passed
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.

3 participants