Skip to content

Bound OpenGL CPU readbacks to pending GPU writes - #326

Merged
mstan merged 4 commits into
RetroPortingToolKit:masterfrom
Alexbeav:pr/gl-cpu-readback-regions-20260906
Sep 7, 2026
Merged

Bound OpenGL CPU readbacks to pending GPU writes#326
mstan merged 4 commits into
RetroPortingToolKit:masterfrom
Alexbeav:pr/gl-cpu-readback-regions-20260906

Conversation

@Alexbeav

@Alexbeav Alexbeav commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

A tiny GPU write followed by a CPU read currently copies all 524,288 native words. Keep a separate conservative rectangle for GPU writes that have not reached the CPU array, then read that region with an explicit full-image row stride. Uploads, queued draws and packing still complete in order. Texture packing must not erase CPU readback debt. Raster values, clocks, precision and GPU ownership remain unchanged.

Fork review: Alexbeav#23. Cubic completed the exact-head check 101476046529 with zero new issues. All earlier findings are resolved in source or explicitly reconciled in the review replies; prior review jobs are terminal. The public branch is identical to this reviewed head.

Base 155e269ba5d50fe36523846f908cf124dc1561a0; tested/reviewed head e57d9b9f4fd220be6f65825282329215f8109a6f; four focused commits; 6 changed files:

  • docs/GPU_GL_READBACK_REGIONS.md
  • runtime/CMakeLists.txt
  • runtime/src/gpu_gl_renderer.c
  • runtime/tests/run_gl_readback_region.py
  • runtime/tests/test_gl_readback_region.c
  • runtime/tests/test_gl_readback_runner.py

The renderer change owns coherence; the C fixture verifies native pixels and transfer size; the Python wrapper runs the real hidden GL context; CMake registers the optional hardware test; documentation states the invariant and limitations. No title configuration, private route tooling or retail payload is included.

Validation on this review head, Windows x64 / WinLibs GCC 16.1 / RTX 4070 Ti, NVIDIA 610.88:

  • Runtime CTest: 70 enabled pass; 2 pre-existing disabled. This includes 71 real OpenGL checks at native and 4x. Every native word matches full hardware readback after each ordered synthetic workload. The earlier 67-check fixture against pristine upstream passed pixel comparisons and failed only the small-transfer assertion at both scales. The expanded fixture also detects stale CPU words after a depth24 exit: the pre-fix source failed 2 checks with 7,676 differing words. The new head passes, including an immediate read and a newer overlapping texture upload. A negative run that has this coherence error is rejected, not treated as an expected bandwidth failure. This is coherence equivalence using the same hardware rasterizer, not independent raster accuracy.
  • Configure runtime with -DBUILD_TESTING=ON -DPSX_GL_READBACK_SDL_ROOT=<SDL3-deps> on MinGW, then ctest --test-dir <runtime-build> -R gl_readback_region_test --output-on-failure. Six parser unit tests reject malformed summaries, wrong exits and unexpected negative failures. The dependency root contains sdl3-src/include and sdl3-build/libSDL3.a. PSX_GL_READBACK_COMPILER_BIN can select GCC for a Clang/MinGW project; configure checks both compiler executables before hardware registration. Missing/restored compiler cases were verified. Both configure and runner reject missing SDL headers/static library before building; a bad dependency root is not counted as a hardware pass. The wrapper also accepts --compiler-bin, --sdl-root, --output; it has no fixed CPU affinity or priority and retains a fresh receipt directory on repeated runs. Other platforms are not qualified by this hardware runner.
  • Recompiler sources are identical to the interlace review branch. Its broader suite passes 58/61 enabled tests, with 3 disabled. Three failures reproduce from pristine upstream source: dirty_text_continuation_guards stale source fragment, release_zip Windows LF expectation, aot_overlay_discovery hardcoded MSYS gcc subprocess. Python tests use PYTHONUTF8=1.
  • Exact standalone head: owned Timeshock USA SLUS-00639 current-base table checkpoint loads, accepts player actions, continues ball/flipper play, then exits normally at frame 1398. This does not establish frontend startup: the title requires the separate interlace correction in Preserve interlaced fields when drawing GPU primitives #327.
  • Separate combined integration 96928d118bdcce574947052b4053153717eb28ac contains the interlace correction plus this exact GL runtime change. Its fresh native OpenGL game-frontend route starts a player game, launches the ball, demonstrates flipper/ball play and closes normally at frame 6925. It is not this standalone review head. The final interlace contribution is Preserve interlaced fields when drawing GPU primitives #327 at a later reviewed head; this earlier combined route is not qualification of that final union. Current runs establish route correctness only under host GPU load, not a new speed result.

The game is a private local port. Owned generated game/SCPH5552 code is reused unchanged; no emitter-regeneration claim. Current public recomp-ui/master 773155ae lacks lobby/spectator fields used by upstream runtime, so these GPU retail tests use supported PSX_RECOMP_UI=OFF direct launch. No launcher compatibility, broad save/load, netplay, full depth24 playback, Vulkan, full-game or audio-wide acceptance is implied. Consumer runtime pin update is required; this correction does not require code regeneration. The two corrections can merge independently. Timeshock frontend startup requires the interlace correction.

Developed with AI assistance; validated as described (test evidence in PR body). AI writes the code and the PR, but I always test before I send something up. Happy to iterate on this process with your feedback.

mstan added a commit that referenced this pull request Sep 7, 2026
@mstan
mstan merged commit 0451621 into RetroPortingToolKit:master Sep 7, 2026
Alexbeav pushed a commit to Alexbeav/psxrecomp that referenced this pull request Sep 10, 2026
…e retail BIOS boot

Matched Ape Escape retail SCPH1001 LLE builds identify 0661ce7 as the visible boot regression: parent 155e269 reaches game code, while 0661ce7 stalls near BIOS frame 375. Revert its four dependent follow-ups too; retain PR RetroPortingToolKit#326 readback bounds and PR RetroPortingToolKit#328 Vulkan aspect changes.

The row-by-row raster dispatcher amplifies an existing malformed DMA list into massive GL work. Keep the broader CPU/DMA arbitration experiment separate for compatibility testing. Document attribution, validation, and the temporarily restored interlace limitation.

Validation: full Ape target reaches title/menu and frame 11669 on retail LLE; OpenBIOS control reaches frame 3701. Eight focused tests pass; retained GL readback fixture passes 71 checks at both 1x and 4x after linking local SDL with iconv. Beads: beads-iisy; DMA follow-up: beads-eio.3.122.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants