Skip to content

fix(agc): copy an indirect draw's vertex buffer only up to its last element - #5975

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
milescoviello:fix/indirect-vertex-extent
Oct 16, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
milescoviello:fix/indirect-vertex-extent

Conversation

@milescoviello

@milescoviello milescoviello commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What

Forspoken (PPSA03984) stops on an indirect draw:

terminate called after throwing an instance of 'std::runtime_error'
  what():  AGC driver: guest memory is not readable at 0x103a03c000 (range 0x103703c000+0x3000050)

The draw is instanced, with six per-instance attributes (fetch index 1) in one buffer of 96-byte records, at offsets 0, 0x10, 0x20, 0x30, 0x40 and 0x50. Each descriptor has 0x80000 records, so records * stride is the 48 MiB buffer, and the 16 KiB after it are not readable (a ---p region in /proc/self/maps). Because an indirect draw's counts are unknown, prepareDrawInputs copies each descriptor's VertexBufferExtent, which was records * stride from the descriptor's own address. For the attribute at 0x50 that ends 0x50 bytes past the buffer. The last record's element ends at (records - 1) * stride plus the element's bytes from the descriptor's address, inside the buffer, and that is as far as the fetch can read.

VertexBufferExtent now ends there. That is what VertexBufferReadSize gives for a direct draw that reads the last record. A zero stride, a null descriptor and a descriptor without records are handled as before. The comment above VertexBufferOutOfBoundsSelect, which described the old extent, is removed.

agc_vertex_input now expects 32 * (records - 1) + 16 where it expected 32 * records, checks that the extent equals VertexBufferReadSize at the last record, and checks that an attribute at offset 16 of a 32-byte record ends with the descriptor's records. With main's VertexInput.hpp it stops at "cached layout froze the indirect descriptor extent".

Tested

Ubuntu 24.04, GCC 13.3 (CI's compiler), Release with CI's configure flags, on main db06cb4, then rebased onto main 8b8bfe0, 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

@CnRJay CnRJay 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.

The extent now matches what the fetch can actually reach: (records - 1) * stride plus the element's bytes, which is also what VertexBufferReadSize gives for the last record. The added check that the extent equals the read of the last record pins the two together. The Forspoken case (attribute at 0x50 of a 96-byte record, buffer followed by a ---p region) is exactly the one the old records * stride got wrong. It also shortens every indirect draw's copy by stride - elementBytes for each attribute past offset 0, though that's small.

…lement

For an indirect draw, whose counts only the GPU knows, the driver copies
the whole range a vertex buffer descriptor covers, records * stride from
the descriptor's address. The last record's element ends sooner when the
attribute sits at an offset in its record, and the fetch can read at most
(records - 1) * stride plus the element.

Forspoken (PPSA03984) draws instances from a 48 MiB buffer of 96-byte
records with six attributes at offsets 0 to 0x50 in each record. The copy
for the attribute at 0x50 ran 0x50 bytes past the buffer into the
unmapped page after it, and the draw stopped with "guest memory is not
readable".

VertexBufferExtent now ends at the last record's element, as
VertexBufferReadSize does for the last record of a direct draw.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@milescoviello

Copy link
Copy Markdown
Contributor Author

@CnRJay thanks for reviewing this, and for checking the extent against VertexBufferReadSize for the last record.

AI-assisted: yes

@adriadam10 adriadam10 self-assigned this Oct 16, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the new extent (records - 1) * stride + element bytes against VertexBufferReadSize for the last record (same value; VertexBufferExtent is only used for indirect draws, Draw.cpp:1163). Built and ran the libSceAgcDriver tests on RADV: agc_vertex_input passes and fails with VertexInput.hpp reverted. Merging once the full Build is green.

@milescoviello

Copy link
Copy Markdown
Contributor Author

A re-check on current main 1b1f848, which now has #5937. Forspoken (PPSA03984), Linux, RTX 4070 Ti, 15 GiB cap, pad input, warm cache. #6814 is merged on top in both builds, because without it the title crashes at start-up on #6805's race.

So with #5937 merged, this is now the title's first stop on main after the start-up race.

AI-assisted: yes

@adriadam10
adriadam10 merged commit 57f2a59 into boykopovar:main Oct 16, 2026
11 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