Thread aware phase2 unification - #104
Draft
dantros wants to merge 61 commits into
Draft
Conversation
…presence The sim-UI context enables keyboard nav, which made ImGui set WantCaptureKeyboard true whenever a panel had nav focus — pinning imguiWantsKeyboard() at true and making it useless for gating game input. Set ConfigNavCaptureKeyboard = false so capture reflects an actively-edited widget or open modal; nav (arrows/Tab/Enter), InputText, and clipboard are unaffected.
Collapse the duplicated buildSnapshot/renderSnapshot (run/tick) and commitFrame/renderFrameThreaded (commit/renderFrame) orchestration into one produce(FrameMode) + consume(FrameMode), gated by Single/Threaded. Extract the shared main-context ImGui setup into beginMainImguiFrame(). Entry points become thin call sites; dead buildSnapshot/renderSnapshot removed. No public API or behavior change. Goldens 17/17 (release+debug), OpenGL/Vulkan/SDL3 build, threaded demos TSan/ASan clean.
Resolve frame_runner conflicts: thread main's new ImguiImageManager (imguiVisual feature, #102) through this branch's unified produce/consume/ renderSnapshotContents methods. imguiImages is passed as a pointer (matching the branch's Single-only dependency convention) — non-null on the single-threaded run/tick path, nullptr on the threaded commit/render path where imguiVisual is not yet wired (resolveAndGarbageCollect is guarded).
…Bellota/removeBellota that's safe in both single-threaded and live-threaded-session contexts
… M5 gamepad path.
…m the sim thread during a live threaded session.
# Conflicts: # source/frame_runner.cpp
…cursor is marshalled to the render thread and applied before its main-context NewFrame, so I-beam/resize cursors work over threaded ImGui (B5). Reuses the platform backend's existing cursor handling — no backend changes; render-backend-agnostic.
…amepad on the main context (so the platform backend populates the ImGuiKey_Gamepad* keys that are already harvested+replayed) and on the sim-UI context (+HasGamepad) to consume them. Threaded-only (set in beginThreadedSession); single-threaded unchanged. Digital D-pad/face-button nav; analog stick nav is a documented follow-up (bool-only marshal).
…d getClipboardText/setClipboardText to the WindowBackend concept (GLFW glfwGet/SetClipboardString, SDL3 SDL_Get/SetClipboardText, Headless no-op). The sim-UI clipboard callbacks (sim thread) read/write a mutex-guarded marshal; the render thread flushes sim->OS writes immediately and refreshes the OS->sim cache throttled (~0.25s) to avoid a per-frame X11 round-trip. Replaces the in-process-only clipboard.
RenderSnapshot gains a ScreenSize field, stamped in both produce() arms.
consume(Threaded) and renderSnapshotContents now derive the viewport, world
transform, and endFrame size from the snapshot rather than the live mScreenSize
atomic, so the render thread draws each snapshot's explorer pool in the exact
logical size it was committed at. This removes the 1-frame letterbox transient
that A4's threaded setScreenSize otherwise produced (render was using the live
size while drawing the previous snapshot's pool).
Priming / never-produced triple-buffer slots are default-constructed with
screenSize {0,0}; reading that directly divides by a zero aspect ratio in
computeLetterboxViewport / computeWorldTransformMat (caught as a UBSan error in
verification). Both read sites fall back to the live atomic when the snapshot
size is degenerate. Single mode always stamps a valid size, so the fallback is
a no-op there.
Verified: builds across glfw/sdl3 x opengl/vulkan + headless; SwiftShader
goldens 21/21; nonvisual tests pass; TSan clean on GLFW and SDL3 (no
snapshot-path race; only environmental driver/libc noise); ASan/UBSan 0 errors.
B6 enabled ImGui gamepad nav on the threaded path but only marshalled digital key state (bool keyDown via IsKeyDown/AddKeyEvent), so the analog gamepad keys (L2/R2 triggers, L/R stick directions) crossed the render->sim boundary as mere on/off — losing ImGui's smooth, continuous gamepad nav. ThreadedImguiInput gains a float keyAnalog[] side-channel (same indexing as keyDown). harvestImguiInput now captures io.KeysData[index].AnalogValue (the public per-key state array) alongside the down bit, and the sim-side replay routes the analog gamepad keys through AddKeyAnalogEvent instead of AddKeyEvent (selected by a small isAnalogNavKey helper covering GamepadL2/R2 and the GamepadL/RStick* range). Digital keys still replay via AddKeyEvent. Render-backend-independent (the platform backend already populates the analog values on the main context). Verified: builds across glfw/sdl3 x opengl/vulkan + headless; SwiftShader goldens 21/21; TSan clean on GLFW and SDL3 (0 races in our code); ASan/UBSan 0 errors.
…es (C8) On the threaded path the stats overlay was drawn on the render-thread main context, but when an app commits its own ImGui the sim-UI clone is what gets presented — so stats only showed for apps with no uiCallback (e.g. the gamepad demo). Move the stats window into the sim-UI frame (produce, after the user uiCallback, before Render) so it lands in the cloned draw data and shows uniformly, with or without app ImGui. The overlay now reports both rates: the render cadence is marshalled to the sim via mRenderFps/mRenderMs atomics (written each consume from the existing mThreadedPerfMonitor), and the sim commit rate comes from a new sim-side PerformanceMonitor fed by an accumulated commit clock (the sim thread has no window clock). The render-thread main-context stats block is removed. hello_threaded_imgui now sets canvas.stats() = true to demonstrate the case (a uiCallback app) that previously hid the overlay. Verified: builds across glfw/sdl3 x opengl/vulkan + headless; SwiftShader goldens 21/21; TSan clean on GLFW and SDL3 (0 races in our code — the sim PerformanceMonitor is sim-exclusive, the render cadence crosses via atomics); ASan/UBSan 0 errors.
dantros
force-pushed
the
thread_aware_phase2_unification
branch
from
June 24, 2026 05:39
cdf83da to
ab23b6f
Compare
Window/monitor ops require the main thread in every mode (GLFW/SDL), so the render-thread guard no longer needs the mThreadedRunning gate. Anchor mRenderThreadId at construction (the Canvas ctor runs on the main thread, where the window is created) and assert unconditionally, so a window op invoked off the main thread is caught single-threaded and outside a session too — not only during a live threaded session. The sim-thread guard stays session-scoped: a distinct sim thread only exists while a threaded session is live, and the captured sim id would be stale during post-session teardown (where main-thread cleanup legitimately touches the scene), so gating it on mThreadedRunning is the correct, race-free predicate. beginThreadedSession drops its now-redundant render-id capture (still resets the sim id per session). Verified (debug, guards active): single-thread window op on the main thread passes; the same op from the sim thread trips with the message; all threaded + single-thread demos run clean. Release compiles the guards out; goldens 21/21.
Feeds the sim controller AND runs the ImGui uiCallback in one commit (produce already supports both). Backs the run(...) convenience.
…enience FrameRunner::runThreaded owns the sim thread + both loops (commit on the sim thread, renderFrame on the main thread, join on close); internal PerformanceMonitor (steady_clock) + limitFrameRate. Makes the two-thread split a one-liner.
…erController) ImGui moved from update to a ui callback; controller split into simController (W/S/A/D/SPACE/Q game input) and renderController (F fullscreen, ESC close). Now a genuine two-thread app via the run(...) convenience.
…nification # Conflicts: # source/canvas.cpp # source/frame_runner.cpp # source/frame_runner.h
Wave A: hello_animation, hello_text, hello_render_to_texture, hello_nested_render_targets. renderTo passes ride the frame snapshot, so they work threaded; empty ui callback (no ImGui/controllers).
Wave B: hello_tint, hello_mesh, hello_imgui_overlay, hello_custom_font. Scene logic stays in update (sim); ImGui moves to the ui callback. Flags shared between the two are plain (both run on the sim thread).
…oller demos Wave C: hello_layers, hello_direct_texture, hello_animation_state_machine, test_create_destroy, hello_tilemap, test_gamepad. Game input + gamepad -> simController; ESC/window ops -> renderController; display ImGui -> ui. hello_screenshot deferred: threaded screenshot needs cross-thread request/result marshaling that isn't built yet.
…, ui, sim, render) Wave D: hello_dense_land, hello_sparse_land. Camera pan + streaming/edit storm -> update; interactive ImGui (incl. addChunk/removeChunk/rebuild/ setScreenSize asset ops) -> ui; WASD -> simController, ESC -> renderController. TSan clean on hello_dense_land (sim-thread sharing race-free).
…recate run(update, Controller&) run() and run(update) now forward to the sim/render split (empty ui + controllers); the single-controller run(update, Controller&) is marked [[deprecated]] but kept as the single-thread survivor. The six demos whose threaded support isn't built yet (registered ImGui images, diegetic renderImguiTo, scheduled screenshot) are parked on it with TODOs; each emits the intended deprecation warning. Goldens 25/25.
Thread ImguiImageManager through runThreaded/commitFrame/renderFrameThreaded, call beginFrame + appendInternalPasses in the Threaded produce arm, guard the registry under the asset mutex, and defer GPU handle/RTT frees to the render thread. Migrate hello_imgui_visual / hello_imgui_image_registry / hello_markdown to run(update, ui).
…1 verification Registered-image threading verified end-to-end: - Builds --parallel across glfw-opengl, glfw-vulkan, sdl3-vulkan, sdl3-opengl (library + the 3 migrated demos) and the headless-vulkan test lib. - xvfb-run smoke of hello_imgui_visual / hello_imgui_image_registry / hello_markdown: render live, clean close (exit 124 timeout, no asserts). - TSan (glfw-opengl) on hello_imgui_image_registry + hello_imgui_visual under xvfb: 0 ThreadSanitizer warnings — the per-frame updateImguiImage -> render resolveImages sharing and the teardown retire-drain are race-free. - SwiftShader goldens unchanged: 25/25 (50 assertions). Flip the registered-image row to Supported in THREADED_DIEGETIC_IMGUI.md and update the THREADED_MODE.md per-demo caveats; diegetic renderImguiTo stays the only deferred ImGui feature.
Generalize the main-context ImGui clone to N secondary RTT contexts. Split ImguiRttManager::flushPending into produceClones (sim: run each renderImguiTo callback on its secondary context, deep-clone its draw data into the snapshot's new rttUi list) and replayClones (render: lazy backend init + RenderDrawData of the clones, no user code). Wire the RTT manager through the threaded produce arm (mThreadedImguiRtt); the render consume now holds the ImGui mutex outer of the asset mutex so the RTT-clone atlas access matches the sim's imgui>asset order and can't deadlock. Migrate hello_imgui_rtt / hello_dpi_scaling to run(update, ui).
… verification Diegetic renderImguiTo threading verified end-to-end: - Builds --parallel across glfw-opengl, glfw-vulkan, sdl3-vulkan, sdl3-opengl (all examples). Only remaining deprecation warning is hello_screenshot (its screenshot request/result handoff is a separate, still-deferred gap). - xvfb-run smoke of hello_imgui_rtt / hello_dpi_scaling threaded on glfw-opengl, glfw-vulkan, sdl3-vulkan: render, clean close; raw hello_threaded_imgui (nullptr manager path) unaffected. No Vulkan validation output. - Single-thread parity: the refactored produce/replay path renders the diegetic demos unchanged, and a headless content check confirms the RTT panel draws actual ImGui pixels (not a blank target). - TSan on hello_imgui_rtt + hello_dpi_scaling threaded: 0 warnings — the per-RTT secondary-context sharing and shared-atlas access under the imgui>asset lock order are race-free. - SwiftShader goldens unchanged: 25/25 (50 assertions), incl. the imguiImage single-mode tests that exercise the refactored retire-queue path. As-built the two ImGui features in THREADED_DIEGETIC_IMGUI.md / THREADED_MODE.md and note threaded support in CLAUDE.md; the deferred bucket is now only hello_screenshot.
… renderImguiTo The Phase 2 render consume held the ImGui mutex outer of the asset mutex around renderSnapshotContents unconditionally, which serialized the render draw against the sim UI-build for EVERY threaded frame — regressing sim/render overlap even for apps that never use diegetic RTT ImGui. Only replayClones (empty-guarded) touches the shared atlas, and registered-image resolve is asset-mutex-covered, so gate the coarse lock on snapshot.rttUi being non-empty: no-clone frames keep the pre-diegetic asset-only renderSnapshotContents + short imgui main draw (full overlap); only renderImguiTo frames pay the coarse lock. Neither path takes asset>imgui, so the choice stays deadlock-safe. Verified: builds glfw-opengl/glfw-vulkan; xvfb smoke of diegetic + non-diegetic threaded demos (opengl + vulkan, no VUID); TSan 0 on both lock-discipline paths (hello_imgui_rtt non-empty, hello_threaded_imgui + hello_threaded_dense_land empty); SwiftShader goldens 25/25.
The diegetic gate (f8ab7ca) still dragged the whole render — GPU uploads, sprite RTT passes, resolveImages, main sprite draw — under the ImGui mutex on frames that use renderImguiTo, so none of it overlapped the sim UI-build. Only the actual shared-atlas work needs that lock. Split renderSnapshotContents at the replayClones seam into renderSnapshotPreMain (uploads + sprite RTT passes + resolveImages) and renderSnapshotMain (main sprite draw), both atlas-free, and drive the phases from consume(Threaded) with per-phase locks: preMain -> asset only (overlaps sim) replayClones -> imgui>asset (only when rttUi non-empty) main sprite draw -> asset only (overlaps sim) main endFrame -> imgui drainPendingFrees moves under the existing render-side imgui block (imgui>asset): its render-target branch tears down secondary ImGui contexts (releaseContext -> DestroyContext), which races the sim's produceClones on ImGui globals. Releasing the asset mutex between phases is safe — GPU frees are deferred and each phase re-looks-up by id. No path takes asset>imgui, so still deadlock-free. Single mode is a thin orchestrator over the same phases (byte-identical order, no locks). Verified: 4 backends build; xvfb smoke of diegetic + non-diegetic + explorer demos (opengl/vulkan, no VUID); TSan 0 on hello_imgui_rtt/_dpi_scaling (diegetic phase churn + releaseContext-under-imgui) and hello_threaded_imgui/_dense_land (asset-only); headless content check still draws the RTT panel; goldens 25/25.
… and remove it Both ImGui features (registered images, diegetic renderImguiTo) are shipped, so the scoping doc is obsolete. Absorb its as-built essentials — the produceClones/replayClones split, the finer-grained imgui/asset lock phases, and the no-input v1 limitation — into THREADED_MODE.md's Done section, refresh the stale demo-migration status (all demos ported except hello_headless/hello_screenshot), and repoint the two CLAUDE.md links.
hello_screenshot was lumped under 'demos that deliberately stay off the threaded path' next to hello_headless, implying it was intentional like the manual-tick reference. It isn't — it's blocked by an unclosed gap. Split it out into a new Pending/deferred work section with the fix approach (marshal the screenshot request + result across the boundary via the existing harvest->POD->feed / atomic channels, then migrate the demo and drop the last deprecated caller). hello_headless stays as the only by-design single-thread demo.
…oundary requestScreenshot() previously armed the render-owned backend directly and finishScreenshot() wrote mScreenshotResult on the render thread while retrieveScreenshot() read it on the sim thread, all unsynchronized. Route the request sim->render via a new std::atomic<bool> mScreenshotRequested (the render thread observes it in consume(Threaded) and does armScreenshot there, sized to the snapshot), and guard mScreenshotResult with mScreenshotResultMutex for the render-write / sim-read handoff. mScreenshotArmed is now render-thread-local on the threaded path. Single-threaded keeps the eager arm (byte-identical).
…hader headless (drop xvfb/TSan)
…nification # Conflicts: # source/frame_runner.cpp
… by TSan) close() from a run(update, ...) callback runs on the sim thread and calls HeadlessBackend::requestClose(), which wrote the plain bool mRunning while the render thread read it via isRunning() in consume() (mThreadedRunning refresh). TSan (under SwiftShader headless) flagged this as a data race. Make mRunning a std::atomic<bool> (acquire/release), matching mScreenSize / mThreadedRunning.
Adds tools/sanitizers/: a small self-closing threaded workload (threaded_smoke.cpp) exercising sim-thread scene mutation, asset create/destroy, threaded ImGui, and the scheduled-screenshot channel; a TSan suppressions file scoped to third-party (SwiftShader ICD / validation layer / ImGui-Vulkan init-teardown) noise only; and run_sanitizers.sh which builds + runs TSan and ASan+UBSan against the SwiftShader Vulkan ICD (Mesa is buggy under sanitizers). Gated by NOTHOFAGUS_BUILD_SANITIZER_FIXTURE (default OFF). Both sanitizers report 0 issues on the current threaded path.
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
1 participant
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.
No description provided.