Repository navigation
fix(gl): repaint thread vs AppKit cursor hide race (#1947) - #1973
Merged
Merged
Conversation
A handle app that binds an NSOpenGLView hands the runtime that view, and comp_gl_window_macos_setup_external adopted the view's openGLContext - the very CGL context the app renders with. The #868 repaint thread then replayed the weave on it while the app thread was drawing (RenderScene runs between xrBeginFrame and xrEndFrame, outside c->mutex and outside app_frame_in_progress), so one GLDContextRec was driven from two threads: SIGSEGV in GLRResourceList::addResource under the repaint's glBlitFramebuffer, or a Metal-validation abort in the app's draw. Hiding the OS cursor was only a timing trigger. With DXR_WEAVE_REPAINT_FORCE=1 the old code crashes 10/10 launches with the cursor extension off as well. No lock the runtime takes can cover the app's own GL calls, so the repaint thread is not started when the compositor's context is app-owned (one WARN says so). Private contexts (own window, plain NSView, offscreen texture) keep the repaint, now under CGLLockContext so it cannot interleave with AppKit's -[NSOpenGLContext update] on the main thread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Undo the #1946 opt-in (DISPLAYXR_CURSOR_DEPTH=1) now that the crash it worked around is fixed in the GL compositor: like the Metal and Vulkan cubes, the cursor is on by default and DISPLAYXR_CURSOR_DEPTH=0 opts out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1947.
Root cause
The crash is not an AppKit race. It is one CGL context in use on two threads at once.
cube_handle_gl_macosrenders with[g_glView openGLContext]and passesg_glViewthroughXR_DXR_cocoa_window_binding.NSOpenGLView,comp_gl_window_macos_setup_externaladopted the view's existingopenGLContext. That is the same context the app renders with, so the compositor had no private context.gl_crop_and_process_dp→glBlitFramebuffer). It is serialised against the app only byc->mutexandapp_frame_in_progress, and both coverxrEndFramealone. The app's own GL (RenderScene, betweenxrBeginFrameandxrEndFrame) runs outside both.GLDContextRecis driven from two threads. The crash reports show the faulting repaint thread inGLRResourceList::addResourcewhile, at the same moment, the main thread is inRenderScene→GLDContextRec::flushContext. When the main thread loses the race instead, it aborts with Metal validation inglDrawElements.Hiding the OS cursor was only a timing trigger: it shifts the app's frame cadence just enough for the repaint gate to open. With
DXR_WEAVE_REPAINT_FORCE=1(a repaint every refresh), the old code crashes 10/10 launches with the cursor extension OFF.The adapter in displayxr-common is not involved:
NSCursor hide/unhidealready runs on the main thread. So there is no displayxr-common PR.Fix
comp_gl_window_macos_context_is_app_owned()reports that case, and a one-time WARN says so:#868/#1947: weave repaint OFF — …. The app's frames weave and present exactly as before. The only loss is the between-frames replay for this one configuration (an app that binds anNSOpenGLView).NSViewand offscreen-texture paths. The macOS repaint arm now also takesCGLLockContext, the lock AppKit takes around-[NSOpenGLContext update]on the main thread.cube_handle_gl_macos: depth cursor default-on again. The feat(cursor): the runtime draws the depth cursor — Vulkan + GL (ADR-046 Amendment 2 §B) #1946 opt-in is undone;DISPLAYXR_CURSOR_DEPTH=0opts out, the same as the Metal and Vulkan cubes. The issue reference is removed from the app.Evidence
All runs are on this Mac (M-series), sim-display,
SIM_DISPLAY_OUTPUT=sbs, 10 s per launch. An early exit counts as a crash. Each crash left a.ipsreport.DISPLAYXR_CURSOR_DEPTH_UV=0.5,0.5)DXR_WEAVE_REPAINT_FORCE=1DISPLAYXR_CURSOR_DEPTH=0),DXR_WEAVE_REPAINT_FORCE=1DISPLAYXR_CURSOR_DEPTH_DRAW=app)DISPLAYXR_CURSOR_DEPTH_SPRITE=custom)cube_hosted_legacy_gl(private context, repaint still ON)count=273)Notes on the evidence:
gl_repaint_threadinGLRResourceListhash emplace, the same as the issue's stack 1.RenderScene.Not tested
NSOpenGLView+DXR_WEAVE_REPAINTloss in practice: a stalled GL handle app now holds its last presented frame instead of re-weaving. sim-display has no tracked phase, so this cannot be seen here.__APPLE__-only) and were not run.NSViewexternal path (private context attached to the app's view) under the new CGL lock. No test app takes it.🤖 Generated with Claude Code