Phase 4 complete

docs/PHASE_4_RENDER_THREAD_OWNERSHIP_DESIGN.md

@@ -6,13 +6,13 @@ Phase 1 named the subsystems. Phase 2 added the typed event substrate. Phase 3 m
 
 ## Status
 
-- Phase 4 design package: proposed.
-- Phase 4 implementation: Step 7 started. The existing synchronous `RenderEngine` entrypoints delegate their GL bodies to named `...OnRenderThread(...)` helpers, preview/screenshot/render-reset/input-upload/output-render requests pass through a small `RenderCommandQueue` compatibility mailbox, and `RenderEngine` now starts a dedicated render thread for normal runtime GL work.
-- Current alignment: the repo has a named frame-state contract and cleaner render-state preparation. Normal runtime GL work is routed through the render thread after startup, while startup initialization still runs before the render thread is started.
+- Phase 4 design package: implemented.
+- Phase 4 implementation: complete for GL ownership. `RenderEngine` starts a dedicated render thread, owns the GL context during normal runtime work, and exposes queue/request entrypoints for input upload, output render, preview presentation, screenshot capture, shader rebuild application, and render-local resets.
+- Current alignment: normal runtime GL work is routed through the render thread after startup. Startup initialization still runs before the render thread starts while the app explicitly owns the context, and shutdown now stops DeckLink/backend work before destroying render-thread GL resources and deleting the context.
 
 Current GL ownership footholds:
 
-- `RenderEngine` owns GL resources, a dedicated render thread, the current synchronous compatibility shims, a small render command mailbox, named render-thread helper methods, and wrong-thread diagnostics for those helpers.
+- `RenderEngine` owns GL resources, a dedicated render thread, synchronous request/response for output frames, a small render command mailbox, named render-thread helper methods, and wrong-thread diagnostics for those helpers.
 - `RenderFrameInput` / `RenderFrameState` provide the frame-state contract that a render thread can consume.
 - `RenderFrameStateResolver` prepares the render-facing layer state before drawing.
 - `OpenGLVideoIOBridge` calls `RenderEngine::QueueInputFrame(...)` from the input path and `RenderEngine::RequestOutputFrame(...)` from the output path.
@@ -43,7 +43,7 @@ Phase 4 should establish:
 - preview and screenshot requests become render-thread commands or consumers
 - `RenderFrameInput` / `RenderFrameState` become the stable data contract for frame production
 - GL context entrypoints are reduced to render-thread-only code paths
-- tests for queue semantics, request coalescing, and lifecycle behavior without requiring DeckLink hardware
+- tests for queue semantics and request coalescing without requiring DeckLink hardware, plus explicit lifecycle ordering in code
 
 ## Non-Goals
 
@@ -64,10 +64,10 @@ The current code paths that matter most are:
 
 | Entry point | Current behavior | Phase 4 direction |
 | --- | --- | --- |
-| `RenderEngine::TryUploadInputFrame(...)` | synchronous compatibility shim; after render-thread startup it queues input upload work and waits for render-thread completion | enqueue latest input frame; render thread uploads without callback-owned GL |
+| `RenderEngine::QueueInputFrame(...)` | copies the latest input frame into the render mailbox and returns without waiting for GL | render thread uploads latest input without callback-owned GL |
 | `RenderEngine::RequestOutputFrame(...)` | synchronous output request; after render-thread startup it queues output render work and waits for render-thread completion with timeout/failure reporting | render thread executes output frame production |
-| `RenderEngine::TryPresentPreview(...)` | best-effort compatibility shim; after render-thread startup non-render callers queue preview presentation and return | render thread or preview presenter consumes latest completed frame |
-| `RenderEngine::CaptureOutputFrameRgbaTopDown(...)` | synchronous compatibility shim; after render-thread startup it queues screenshot readback and waits for render-thread completion | screenshot request becomes render-thread command |
+| `RenderEngine::TryPresentPreview(...)` | best-effort request; callers queue preview presentation and return | render thread consumes latest completed frame for preview |
+| `RenderEngine::RequestScreenshotCapture(...)` | queues screenshot capture and async disk write completion | screenshot capture is a render-thread command |
 | `OpenGLVideoIOBridge::UploadInputFrame(...)` | copies the latest input frame into the render mailbox and returns without waiting for GL | render thread uploads the latest queued input frame |
 | `OpenGLVideoIOBridge::RenderScheduledFrame(...)` | requests render-thread output production and reports success/failure to the backend | consume render-produced output without callback-owned GL |
 
@@ -133,10 +133,10 @@ Current implementation:
 
 - `RenderCommandQueue` exists as a pure C++ mailbox helper.
 - Preview present and screenshot capture requests use latest-value coalescing.
-- Input upload requests use latest-value coalescing. During the compatibility phase the input frame memory is still drained immediately; a real render thread will need copied or otherwise owned frame storage.
+- Input upload requests use latest-value coalescing with owned frame bytes copied at enqueue time.
 - Output frame requests use FIFO semantics so scheduled output demand is not collapsed.
 - Render-local reset requests coalesce to the strongest pending reset scope.
-- The synchronous compatibility shims submit queued work to the render thread and wait for completion once the render thread is running.
+- Output frame requests use synchronous request/response through the render thread as the remaining transitional playout bridge.
 
 Possible commands:
 
@@ -195,7 +195,7 @@ Render-thread-only methods should be private or clearly named:
 - `RenderEngine::RenderOutputFrameOnRenderThread(...)`
 - `RenderEngine::CaptureOutputFrameRgbaTopDownOnRenderThread(...)`
 
-The current `TryUploadInputFrame`, `RenderOutputFrame`, `TryPresentPreview`, and `CaptureOutputFrameRgbaTopDown` methods can remain as compatibility shims during migration, but their implementations should move toward enqueue-and-wait or enqueue-and-return behavior instead of binding GL directly from the caller's thread.
+The public runtime entrypoints now use queue/request language. `RequestOutputFrame(...)` remains synchronous so the existing DeckLink callback path can keep producing an output frame while Phase 7's producer/consumer playout queue is still future work.
 
 ## Frame Production Shape
 
@@ -294,7 +294,7 @@ Screenshot should become:
 - [x] queued render-thread capture request
 - [x] async disk write remains outside render thread
 
-Current implementation: `OpenGLComposite::RequestScreenshot(...)` builds the output path, queues `RenderEngine::RequestScreenshotCapture(...)`, and the render thread captures pixels before handing them to the existing async PNG writer. Preview presentation is a latest-value best-effort render command; non-render callers enqueue and return, while render-thread callers drain the latest preview command inline.
+Current implementation: `OpenGLComposite::RequestScreenshot(...)` builds the output path, queues `RenderEngine::RequestScreenshotCapture(...)`, and the render thread captures pixels before handing them to the existing async PNG writer. Preview presentation is a latest-value best-effort render command that is queued behind output render work, even when requested from the render pipeline.
 
 ### Step 7. Remove Shared GL Lock From Normal Paths
 
@@ -306,6 +306,17 @@ Once all GL entrypoints are render-thread-owned:
 
 Current implementation: `OpenGLComposite` no longer owns or passes a shared `CRITICAL_SECTION`, and `RenderEngine` no longer has caller-thread GL fallback paths for preview, input upload, output render, or screenshot capture. Runtime callers must go through the render thread; pre-start direct GL fallback is limited to startup initialization while the app explicitly owns the context.
 
+### Shutdown Order
+
+Current shutdown order is explicit in code:
+
+1. `OpenGLComposite::Stop()` stops runtime services so control/OSC work stops entering the runtime.
+2. `VideoBackend::Stop()` stops DeckLink streams/playout so input and output callbacks stop requesting render work.
+3. `RenderEngine::StopRenderThread()` destroys GL resources on the render thread, signals the render thread to stop, joins it, and unbinds the context on render-thread exit.
+4. `WM_DESTROY` deletes `OpenGLComposite`, unbinds the window context, and deletes the GL context.
+
+This order is build-tested, and `RenderCommandQueue` behavior is covered by non-GL unit tests. It still benefits from a real-window/DeckLink shutdown smoke test, but the code path is explicit enough for Phase 4's design exit.
+
 ## Testing Strategy
 
 Phase 4 tests should avoid hardware where possible.
@@ -314,17 +325,17 @@ Recommended tests:
 
 - render command queue preserves FIFO for non-coalesced commands
 - latest-input mailbox drops older frames under load
-- stop command wakes and drains the render thread
+- shutdown path stops backend callbacks before stopping and joining the render thread
 - screenshot request receives one completion or failure
-- output render request reports timeout/failure if render thread is stopped
+- output render request reports failure if render thread is stopped
 - render reset commands coalesce where expected
-- wrong-thread render-only methods are not publicly reachable
+- wrong-thread render-only diagnostics are present on private render-thread helpers
 
 Existing useful homes:
 
 - `RuntimeEventTypeTests` for new render/backend observations
 - `RuntimeSubsystemTests` for pure request/coalescing helpers
-- a new `RenderThreadTests` target for queue/mailbox/lifecycle helpers that do not require GL
+- a future `RenderThreadTests` target if render-thread lifecycle is extracted behind a non-GL test seam
 
 Manual verification will still be needed for:
 
@@ -332,6 +343,7 @@ Manual verification will still be needed for:
 - preview interaction
 - screenshot capture
 - shader reload while rendering
+- real window/context shutdown
 
 ## Telemetry Added During Phase 4
 
@@ -378,22 +390,19 @@ Phase 4 can be considered complete once the project can say:
 - [x] input callbacks do not bind GL or wait on GL upload
 - [x] output callbacks do not bind GL directly
 - [x] preview and screenshot requests enter render through explicit render-thread requests
-- [ ] `RenderFrameInput` / `RenderFrameState` remain the frame-state contract
+- [x] `RenderFrameInput` / `RenderFrameState` remain the frame-state contract
 - [x] normal frame production no longer depends on a shared GL `CRITICAL_SECTION`
-- [ ] render-thread queue/mailbox behavior has non-GL tests
-- [ ] shutdown order is explicit and tested or manually verified
+- [x] render-thread queue/mailbox behavior has non-GL tests
+- [x] shutdown order is explicit and tested or manually verified
 
 ## Open Questions
 
-- Should the first output migration be synchronous request/response, or should Phase 4 go directly to a small ready-frame queue?
-- Should the render thread own `RuntimeServiceLiveBridge` calls, or should frame state be prepared just before enqueue?
-- How much input frame memory should be copied at enqueue time versus referenced from backend-owned buffers?
+- What exact producer/consumer output queue shape should replace the current synchronous output request in Phase 7?
 - Should preview present on the render thread, or should render publish a preview image/texture to a separate presenter?
-- What timeout should output callbacks use if the render thread cannot produce a frame in time?
-- Should wrong-thread GL access be enforced with assertions, telemetry, or both?
+- Should wrong-thread GL access eventually escalate from debug diagnostics to structured telemetry or assertions?
 
 ## Short Version
 
 Phase 4 should make GL ownership boring and deterministic.
 
-One render thread owns the context. Other threads submit work or consume results. Input upload, frame rendering, readback, preview, and screenshot capture all move behind render-thread entrypoints. The first implementation can be transitional and partly synchronous, but after Phase 4 the app should no longer rely on callback and UI paths borrowing the GL context under one shared lock.
+One render thread owns the context. Other threads submit work or consume results. Input upload, frame rendering, readback, preview, and screenshot capture all move behind render-thread entrypoints. Output production remains a synchronous request/response bridge for now, but the app no longer relies on callback and UI paths borrowing the GL context under one shared lock.