Skip to content

Commit 0863371

Browse files
committed
cocoa_gl_ctx: drop destroy-time RELEASE(glk_view) - desyncs the static
from its real strong owner and freezes the iOS gl2 screen on content load Regression from bd45086 ("Normalise MRR safety in cocoa_gl_ctx.m"), which added an unconditional RELEASE(glk_view) to the iOS branch of cocoa_gl_gfx_ctx_destroy. Under ARC that expands to `glk_view = nil` and drops the static's strong reference; the GLKView itself stays alive because RetroArch_iOS._renderView retains it. What breaks: on a video reinit (the destroy/init pair fired by content load) cocoa_gl_gfx_ctx_init calls [apple_platform setViewType:APPLE_VIEW_TYPE_OPENGL_ES]; setViewType: short-circuits on `if (vt == _vt) return;` because _vt is already OPENGL_ES from the first init, so the OPENGL_ES arm at line 611 of ui_cocoatouch.m - the only call site that re-invokes glkitview_init() - is skipped. The static glk_view therefore stays nil for the rest of the session. Every subsequent [glk_view display] (swap_buffers) [glk_view bindDrawable] (glkitview_bind_fbo) glk_view.context = g_ctx (set_video_mode) is a silent msg-to-nil, and get_video_size reads zero from glk_view.bounds / contentScaleFactor. Net effect: screen freezes the moment a core is loaded. Mac is unaffected - cocoa_gl_gfx_ctx_swap_buffers' OSX path uses [g_ctx flushBuffer] / [g_hw_ctx flushBuffer] and never touches a GLKView. Fix is to leave glk_view alone in destroy. The GLKView's real owner is RetroArch_iOS._renderView (set up via setViewType:OPENGL_ES, which RARCH_RETAINs the singleton returned by glkitview_init); on iOS the RetroArch_iOS instance is the UIApplication delegate and lives until process exit, so the MRR leak the original RELEASE aimed to plug only matters at app teardown - which on iOS effectively never runs. The rest of bd45086 stays: - RELEASE(g_ctx) / RELEASE(g_hw_ctx) at destroy is a behavioural no-op on iOS/ARC (matches the prior `g_ctx = nil; g_hw_ctx = nil`) and a real fix under MRR; g_ctx / g_hw_ctx have no peer strong owner so releasing the static is correct. - The defensive RELEASE(g_ctx) / RELEASE(g_hw_ctx) in set_video_mode fires on already-nil pointers in the normal flow. - The RELEASE(glk_view) in glkitview_init (line 151) is also kept - the immediately-following `glk_view = [GLKView new]` would do the same store-strong release-then-retain anyway under ARC, so it's net-neutral there. A more principled fix later would be to have glkitview_init adopt _renderView when called with glk_view == nil and _renderView is already a GLKView, which would let destroy keep releasing the static safely; not doing that here, as the regression fix wants to be minimal.
1 parent 0e2ea06 commit 0863371

1 file changed

Lines changed: 15 additions & 5 deletions

File tree

gfx/drivers_context/cocoa_gl_ctx.m

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,21 @@ static void cocoa_gl_gfx_ctx_destroy(void *data)
195195
* was dropped with a raw 'g_ctx = nil'. */
196196
RELEASE(g_ctx);
197197
RELEASE(g_hw_ctx);
198-
#if defined(HAVE_COCOATOUCH)
199-
/* glk_view was created with +new (+1) in glkitview_init. Release
200-
* it on destroy so it does not leak across init/destroy cycles. */
201-
RELEASE(glk_view);
202-
#endif
198+
/* Deliberately NOT releasing glk_view here. Its real strong owner
199+
* is RetroArch_iOS._renderView, which retains the singleton via
200+
* setViewType:APPLE_VIEW_TYPE_OPENGL_ES on first init and holds it
201+
* for the rest of the process. On a video reinit (e.g. content
202+
* load) the ctx destroy/init pair runs, but setViewType: returns
203+
* early on (vt == _vt) and never re-invokes glkitview_init - so
204+
* nilling the static here would leave it nil for the remainder of
205+
* the session. The consequence is that swap_buffers'
206+
* if (glk_view) [glk_view display];
207+
* becomes a no-op, get_video_size reads zero from glk_view.bounds,
208+
* set_video_mode's `glk_view.context = g_ctx` silently misses, and
209+
* the screen freezes the moment a core is loaded. The MRR leak
210+
* this previously aimed to plug only matters at app teardown,
211+
* which on iOS effectively never runs (RetroArch_iOS is the
212+
* UIApplication delegate). */
203213

204214
free(cocoa_ctx);
205215
}

0 commit comments

Comments
 (0)