Skip to content

fix(agc): honor bounded direct index fetches - #4824

Closed
krishalivaibhav wants to merge 2 commits into
boykopovar:mainfrom
krishalivaibhav:codex/bounded-index-fetch
Closed

krishalivaibhav wants to merge 2 commits into
boykopovar:mainfrom
krishalivaibhav:codex/bounded-index-fetch

Conversation

@krishalivaibhav

@krishalivaibhav krishalivaibhav commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

Direct DRAW_INDEX_2 and DRAW_INDEX_OFFSET_2 packets can request more indices than their max_size readable prefix. Keep the requested draw count, read only min(count, max_size) guest indices, and supply zero indices for the remaining fetches. For OFFSET_2, max_size is already relative to the packet offset. This replaces the count-greater-than-max rejection and prevents reads beyond the guest range.

The bounded 16/32-bit vertex path uses an owned padded buffer, preserves primitive restart behavior, and does not reuse snapshots or in-place imports for the synthetic tail. Alias checks cover only the real guest prefix. Bounded mesh fetches and the existing unsupported 8-bit graphics path remain explicit errors; indirect packet semantics are unchanged.

The packet contract is consistent with PAL index packet construction and Mesa draw emission.

Tested

  • Apple M2 / MoltenVK local integrated runtime: agc_driver_pm4, agc_driver_pm4_failure, agc_driver_viewport_depth, agc_driver_graphics and agc_driver_draw_offset_indices passed.
  • New guarded-page GPU fixture, with Vulkan required: a saved pre-fix local driver fails on the protected index tail; the candidate passes. The fixture compares 16/32-bit rendering against explicitly zero-padded references for readable counts 0, 1, 3 and full count, preserves 42 requested invocations, and covers restart followed by zero fetches. The baseline/candidate share the fixture and relevant header contract. These are local integrated-runtime binaries, not a pristine-main full-suite A/B.
  • All five changed C++ translation units compile against main 43e3a70f8143f014f621c48abbdb137fffd7346e in an isolated checkout; git diff --check passes. Independent agent review found no blocking issue.

Focused clean-head Linux linking and execution now pass as recorded below; full suites, Windows execution, and a merge onto the latest main remain unverified. The local complete macOS build uses additional unpublished runtime changes. This PR does not claim stable game completion or a performance improvement.

Additional verified Linux evidence

Clean Linux/lavapipe execution on exact head 0976b1eb421061b75077b3e537ea7c8160330b45: built the actual graphics runtime and bounded-index/PM4 targets. agc_driver_bounded_index_fetch, agc_driver_pm4_failure, and agc_driver_pm4_priority_refused all passed three consecutive runs, with skipped tests rejected. Hosted logs. Lavapipe is a software Vulkan renderer; this is neither Windows GPU coverage nor a passing full game session. The production-only negative control remains the earlier local integrated-runtime result described above.

Checklist

  • Depends on: none
  • AI-assisted: yes
  • Added behavioral PM4 and guarded-page GPU regression coverage.
  • Changes isolated from local macOS startup/shutdown and game-specific diagnostic work.

@krishalivaibhav

Copy link
Copy Markdown
Contributor Author

The corrected head is 0976b1eb421061b75077b3e537ea7c8160330b45; conventions passed. The automatic Build run completed with all platform jobs skipped: its eligibility log enables full Linux/Windows builds only for a manual collaborator dispatch. Could a maintainer run Build for PR 4824 before review?

Local evidence is scoped in the description: the protected-page GPU fixture fails with the saved pre-fix driver and passes with the candidate on M2/MoltenVK, five focused local checks pass, and the changed translation units compile against the stated main base. Full clean-main linking and Linux/Windows execution remain unverified. Keeping this draft until that evidence is available.

AI-assisted: yes.

@github-actions

github-actions Bot commented Oct 13, 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
#4855 core/libs/prx/libSceAgcDriver/tests/Pm4.cpp L23
#5205 core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp L1015-1017

@mrfelek

mrfelek commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Heads-up from #5205 (skip an indexed draw with no indices or instances): it replaces the zero-count indexed draws are unsupported check in prepareDrawInputs, a few lines from your hunk in Draw.cpp. The two changes are compatible: this PR keeps that line as it is, and #5205 turns the zero-count case into a skipped draw. Whichever lands first, the other needs a small rebase; I can rebase #5205 if this one goes first.

Written with AI assistance (Claude Code).

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: no title is known to reach this. With the queue at 700+ open PRs, please hold off on new PRs that don't name a title needing the change until it's down. If a title stops on this on current main, reopen with the title ID and the log line.

@krishalivaibhav

krishalivaibhav commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Follow-up evidence, keeping this closed pending the requested current-main title reproduction.

A historical Balatro capture contains the exact bounded case: c0042700 00000001 c4ed7400 00000002 0000002a 00000000 (DRAW_INDEX_2, 42 requested indices, readable maximum 1). Its log terminates with index count exceeds maximum index size. This is distinct from the newer index-alignment failure.

I replayed that unchanged packet through Pm4::Validate compiled from the entire production Pm4.cpp on pinned main a94675fb17dd5b7775cd2960200a25bba41ab29f: it rejects with that exact message. The locally rebased candidate accepts it. All five changed translation units compile, conventions and diff checks pass; independent review verified that the rebase preserves main’s empty-draw skipping behavior.

This is validation-only evidence: the small caller links with dead stripping/dynamic lookup for unused execution dependencies. It does not verify a full driver link, GPU zero-fill rendering, or a game reaching this packet on a complete current-main runtime. Historical integrated-runtime A/B and guarded-page rendering evidence remain separately scoped. I am not presenting this as satisfying the full title-reproduction condition or as an FPS improvement.

Written with AI assistance.

Title and pristine-main attempt

The title is Balatro PPSA21402, version 01.015.000 (verified from the dump’s sce_sys/param.json). Historical terminal log: AGC driver: DRAW_INDEX_2 at DWORD 1105: index count exceeds maximum index size.

I attempted a pristine runtime build at the pinned main above, with Release/x86_64 and main’s exact dependency revisions. Configuration succeeds, but the runtime build stops in libc/src/General.cpp: CLOCK_REALTIME_COARSE is undeclared and stat has no st_mtim on macOS. No portability changes were applied to disguise this as a pristine-main run; that macOS attempt could not launch the game.

Could a contributor with a working current-main runtime and this Balatro version check whether it reaches the same bounded packet (42 requested, maximum 1), attach the title/error log, and compare rendering with this change? That current-main title run remains needed before reopening.

Linux baseline follow-up — 14 October 2026

I have now built the complete Release relinker and runtime libraries from the same pinned main (a94675fb17dd5b7775cd2960200a25bba41ab29f) with its exact dependency revisions: 1,381 build steps completed successfully. No AnyPS5 source changes were applied. This was an Ubuntu amd64 userspace under Rosetta in an ARM64 Linux VM on Apple M2, with Mesa llvmpipe software Vulkan and Xvfb; it is not native x86 hardware or MoltenVK coverage. BUILD_TESTING was OFF, so this is a build/startup result, not a full-suite result.

Balatro PPSA21402 01.015.000 was converted using that freshly built relinker and launched with those freshly processed runtime libraries. The clean ELF inputs were independently checked against the original dump's ELF headers, program headers and all loadable segment payloads. Direct Rosetta loading rejected the generated ELF layout before game execution; invoking the existing glibc loader explicitly allowed execution without modifying the generated executable or runtime libraries.

The effective startup probe ran for 150 seconds, executed game code, initialized Vulkan and compiled shaders, then ended at its intentional time limit (SIGTERM; no forced-kill fallback). It did not emit the index-count rejection above. The headless probe did not establish visible menu/gameplay or exercise a later gameplay scene, and software rendering limited progress. An independent reviewer checked the build/conversion logs, input provenance and executable/library hashes.

Therefore this does not yet satisfy the requested title reproduction, and I am keeping this PR closed. The packet-validator result remains separate from this actual title-startup result; neither establishes an FPS improvement.

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

Labels

agc libSceAgc* build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants