From 4c2e703f12debe655f9c2df223ac155caad200b4 Mon Sep 17 00:00:00 2001 From: Shokunin <30949000+tetrisgm@users.noreply.github.com> Date: Tue, 13 Oct 2026 01:22:15 -0700 Subject: [PATCH] Present ring reset after the render thread stops; fixture covers the worker Review (#654): forget the ring's names after gl_renderer_render_thread_stop (the worker presents and captures on its own thread, and stop drains it and hands the context back), before the context is deleted. The call is unconditional (an empty stub without debug tools). The fixture adds a render-thread pass: frames captured on the worker and drained, more left queued at shutdown, then checks the worker stopped first and every name and pending readback is gone. Fails if the reset runs before the stop. Co-Authored-By: Claude Opus 5.5 --- runtime/src/gpu_gl_renderer.c | 13 ++++-- runtime/tests/test_gl_present_ring_restart.c | 48 +++++++++++++++++--- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/runtime/src/gpu_gl_renderer.c b/runtime/src/gpu_gl_renderer.c index b35c7e77..c375e9ec 100644 --- a/runtime/src/gpu_gl_renderer.c +++ b/runtime/src/gpu_gl_renderer.c @@ -6351,14 +6351,12 @@ int gl_renderer_get_swap_interval(void) { static void pass_resources_release(void); -#ifndef PSX_NO_DEBUG_TOOLS static void present_image_ring_gl_forget(void); -#endif void gl_renderer_shutdown(void) { -#ifndef PSX_NO_DEBUG_TOOLS + gl_renderer_render_thread_stop(); /* drains; the context comes back to this thread */ + /* After the worker has stopped (it presents, and captures, on its own + * thread) and before the context dies: forget the ring's GL names. */ present_image_ring_gl_forget(); -#endif - gl_renderer_render_thread_stop(); /* the context comes back to this thread */ s_native_surface_enabled=s_native_surface_pending=0; gl_renderer_clear_hd_texture_cache(); pass_resources_release(); @@ -9078,7 +9076,9 @@ static int s_pir_disabled; /* The GL objects belong to the context: a renderer restart (lobby rematch, * in-game menu -> match) makes new ones, so forget the old names. Reading * back into a stale pack-buffer name reads into client memory instead. */ +static int s_pir_forget_rth_on = -1; /* tests: was a render thread still running? */ static void present_image_ring_gl_forget(void) { + s_pir_forget_rth_on = s_rth_on; s_pir_fbo = s_pir_tex = 0; s_pir_pbo[0] = s_pir_pbo[1] = 0; s_pir_pending[0] = s_pir_pending[1] = 0; @@ -9152,6 +9152,9 @@ static void present_image_ring_capture_gl(void) { s_pir_w = tw; } #endif +#ifdef PSX_NO_DEBUG_TOOLS +static void present_image_ring_gl_forget(void) {} /* no ring in this build */ +#endif /* Host overlays (OSD text, volume bar, rewind strip, savestate menu) are * rasterized by the emulation thread. A recorded present carries a copy taken diff --git a/runtime/tests/test_gl_present_ring_restart.c b/runtime/tests/test_gl_present_ring_restart.c index 46923d68..71b1eca3 100644 --- a/runtime/tests/test_gl_present_ring_restart.c +++ b/runtime/tests/test_gl_present_ring_restart.c @@ -58,6 +58,16 @@ static SDL_Window *open_ctx(void) { return win; } +static void check_forgotten(const char* when) { + char l[160]; + snprintf(l, sizeof l, "%s: ring names forgotten at shutdown", when); + check(s_pir_fbo == 0 && s_pir_tex == 0 && s_pir_pbo[0] == 0 && s_pir_pbo[1] == 0, l); + snprintf(l, sizeof l, "%s: no readback pending across contexts", when); + check(s_pir_pending[0] == 0 && s_pir_pending[1] == 0, l); + snprintf(l, sizeof l, "%s: forgotten after the render thread stopped", when); + check(s_pir_forget_rth_on == 0, l); +} + int main(void) { if (SDL_Init(SDL_INIT_VIDEO) != 0) { fprintf(stderr, "SKIP no video\n"); return 77; } SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 3); @@ -67,20 +77,46 @@ int main(void) { SDL_GL_SetAttribute(SDL_GL_CONTEXT_FLAGS, SDL_GL_CONTEXT_FORWARD_COMPATIBLE_FLAG); #endif for (int i = 0; i < SW * SH; i++) src[i] = 0xFF336699u; + + /* 1. Direct presents (no render thread), then shutdown. */ SDL_Window *win = open_ctx(); if (!win) { fprintf(stderr, "SKIP no GL context\n"); return 77; } for (int i = 0; i < 3; i++) gl_renderer_present(src, SW, SH, 0, 0, 0); check(s_pir_fbo != 0 && s_pir_pbo[0] != 0, "ring objects made in the first context"); gl_renderer_shutdown(); - /* The context and its objects are gone: no name may survive. A stale - * pack-buffer name made the next readback write through a null client - * pointer (glReadPixels crash after a lobby rematch). */ - check(s_pir_fbo == 0 && s_pir_tex == 0 && s_pir_pbo[0] == 0 && s_pir_pbo[1] == 0, - "ring names forgotten at shutdown"); - check(s_pir_pending[0] == 0 && s_pir_pending[1] == 0, "no readback pending across contexts"); + /* A stale pack-buffer name made the next readback write through a null + * client pointer (glReadPixels crash after a lobby rematch). */ + check_forgotten("direct"); SDL_DestroyWindow(win); + + /* 2. Render thread: presents (and ring captures) run on the worker. + * Queued frames are drained before the check, and shutdown must stop + * the worker before the names are forgotten. */ win = open_ctx(); if (!win) { fprintf(stderr, "SKIP no second GL context\n"); return 77; } + if (gl_renderer_render_thread_start(2) == 1) { + for (int i = 0; i < 6; i++) { + gl_renderer_present(src, SW, SH, 0, 0, 0); + gl_renderer_render_thread_frame_boundary(); + } + gl_renderer_render_thread_sync("ring test drain"); + check(s_pir_fbo != 0 && s_pir_pbo[0] != 0, "ring objects made by the render thread"); + for (int i = 0; i < 3; i++) { /* leave presents queued at shutdown */ + gl_renderer_present(src, SW, SH, 0, 0, 0); + gl_renderer_render_thread_frame_boundary(); + } + gl_renderer_shutdown(); + check(!gl_renderer_render_thread_active(), "render thread stopped by shutdown"); + check_forgotten("render thread"); + } else { + printf("render thread not eligible here: threaded case skipped\n"); + gl_renderer_shutdown(); + } + SDL_DestroyWindow(win); + + /* 3. A fresh context rebuilds live objects. */ + win = open_ctx(); + if (!win) { fprintf(stderr, "SKIP no third GL context\n"); return 77; } for (int i = 0; i < 3; i++) gl_renderer_present(src, SW, SH, 0, 0, 0); check(s_pir_fbo != 0 && s_pir_pbo[0] != 0, "ring rebuilt in the new context"); check(glIsTexture(s_pir_tex) == GL_TRUE, "ring texture is a live object of the new context");