Reworked threading model.

[bkaradzic-microsoft]

What Changed

The old model used per-thread encoders (GetEncoderForThread / EndEncoders)
and UpdateToken (wrapping SafeTimespanGuarantor::SafetyGuarantee) for encoder
access. The new model replaces all of this with a single frame encoder owned
by DeviceImpl and FrameCompletionScope for synchronization.

SafeTimespanGuarantor, UpdateToken, and Update classes are fully removed.
DeviceUpdate remains as a no-op shim in Device.h — its removal is a
separate PR because it is an API breaking change.

Summary of Changes

Core/Graphics — Removed

• SafeTimespanGuarantor.h / SafeTimespanGuarantor.cpp — deleted
• UpdateToken, Update classes — removed from DeviceContext.h
• GetSafeTimespanGuarantor() — removed from DeviceImpl
• GetEncoderForThread(), EndEncoders() — removed from DeviceImpl
• m_threadIdToEncoder, m_updateSafeTimespans — removed from DeviceImpl
• Device::GetUpdate() — replaced with inline no-op in Device.h
• DeviceUpdate class — replaced with no-op shim (Start/Finish/RequestFinish do nothing)
• bgfx::Encoder& parameter — removed from AcquireNewViewId, FrameBuffer methods

Core/Graphics — Added

• m_frameEncoder — single bgfx::Encoder* in DeviceImpl, acquired in
  StartRenderingCurrentFrame, ended in FinishRenderingCurrentFrame
• FrameCompletionScope — RAII counter that prevents FinishRenderingCurrentFrame
  from proceeding while held
• m_frameBlocked + m_pendingFrameScopes + mutex/CV — blocking gate mechanism
• m_frameStartDispatcher — ticked in StartRenderingCurrentFrame for RAF scheduling
• m_firstFrameStarted — flag to distinguish first-call no-op from lifecycle errors
  in FinishRenderingCurrentFrame
• SetActiveEncoder() / GetActiveEncoder() — on DeviceImpl, forwarded by DeviceContext
• FrameStartScheduler(), AcquireFrameCompletionScope() — on DeviceContext

Core/Graphics — Modified

• StartRenderingCurrentFrame(): acquires encoder, opens gate, ticks frame start
  dispatcher
• FinishRenderingCurrentFrame(): waits for scopes==0, closes gate, ticks beforeRender
  dispatcher, ends encoder, calls Frame(). Returns early on first call before any
  Start has been called; throws on subsequent out-of-order calls.
• FrameBuffer methods: get encoder internally via GetActiveEncoder() instead of
  parameter

Plugins/NativeEngine

• Replaced all GetUpdateToken().GetEncoder() with GetEncoder() →
  GetActiveEncoder() + assert
• Added BeginFrame() / EndFrame() — no-ops (encoder lifecycle is in DeviceImpl)
• SubmitCommands() always acquires a stack-scoped FrameCompletionScope. When
  within a RAF callback the frame is already open and acquisition returns immediately.
  When outside (e.g., XHR callback), it blocks until StartRenderingCurrentFrame
  provides the encoder. The scope lives on the stack — no deferred release, no member
  variable, no risk of mid-execution release via microtask draining.
• ReadTexture(): acquires its own FrameCompletionScope to ensure the encoder
  is available. When a blit is needed, executes it inline using GetEncoder().
  Both blit and ReadTextureAsync land in the same frame. The scope keeps the
  frame open for the entire operation.
• GetEncoder() includes assert(encoder != nullptr) — all call-site asserts removed
• Removed m_update, m_updateToken
• LoadTextureFromImage / LoadCubeTextureFromImages: cached m_numMips before
  loop to avoid reading from potentially freed image (from PR Fixed race condition when passing data to bgfx. Issue #1398 #1628)

Plugins/NativeXr

• Replaced Update.Scheduler() → FrameStartScheduler()
• Replaced Update.GetUpdateToken() → AcquireFrameCompletionScope()
• Replaced GetUpdateToken().GetEncoder() → GetActiveEncoder()
• Removed Update member from SessionState

Polyfills/Canvas

• Replaced m_update.GetUpdateToken().GetEncoder() → GetActiveEncoder()
• Flush(): acquires FrameCompletionScope when encoder is null (outside frame),
  plus defensive null check after acquisition — returns early if encoder still null
  (shutdown edge case). Discards encoder state before nanovg rendering to prevent
  NativeEngine state from leaking into Canvas rendering (old model had separate
  per-thread encoder with clean state).
• Removed m_update member

Apps

• HeadlessScreenshotApp / StyleTransferApp: added Start/Finish frame around
  startup wait to keep encoder available while JS runs
• UnitTests (Tests.JavaScript.cpp): pump frames in a loop with 16ms interval
  while waiting for JS test completion. Keep frame open during shutdown so JS
  thread can complete pending work.

bgfx Fix (applied to build deps)

• Added m_encoderBeginLock in bgfx::frame() to prevent deadlock with
  bgfx::destroy()

Architecture

Single Encoder Lifecycle

StartRenderingCurrentFrame()
  m_frameEncoder = bgfx::begin(true)
  m_frameBlocked = false; notify CV
  tick m_frameStartDispatcher          → RAF callbacks dispatched to JS

FinishRenderingCurrentFrame()
  wait for scopes == 0; m_frameBlocked = true
  tick m_beforeRenderDispatcher
  bgfx::end(m_frameEncoder)
  Frame() → bgfx::frame()
  tick m_afterRenderDispatcher

Encoder Access

NativeEngine::GetEncoder()   → DeviceImpl::GetActiveEncoder() → m_frameEncoder (+ assert)
Canvas::Flush()              → DeviceImpl::GetActiveEncoder() → m_frameEncoder
NativeXr                     → DeviceImpl::GetActiveEncoder() → m_frameEncoder

Per-Frame Sequence Diagram

Main Thread                  JS Thread                        bgfx render thread
    |                            |                                   |
    |  StartRenderingCurrentFrame()                                  |
    |-------+                    |                                   |
    |       | m_frameEncoder =   |                                   |
    |       | bgfx::begin(true)  |                                   |
    |       |                    |                                   |
    |       | m_frameBlocked =   |                                   |
    |       | false; CV notify   |                                   |
    |       |                    |                                   |
    |  tick frameStartDispatcher |                                   |
    |       |                    |                                   |
    |  [acquire scope]           |                                   |
    |  pendingScopes++           |                                   |
    |       |                    |                                   |
    |  dispatch to JS ---------> |                                   |
    |       |                    |                                   |
    | (return to app loop)       | RAF callbacks execute             |
    |       |                    |---+                               |
    |       |                    |   | submitCommands()              |
    |       |                    |   | AcquireFrameCompletionScope   |
    |       |                    |   | (returns immediately —        |
    |       |                    |   |  frame already open)          |
    |       |                    |   | GetEncoder() → m_frameEncoder |
    |       |                    |   | Draw, Clear, SetTexture...    |
    |       |                    |   | scope released on return      |
    |       |                    |<--+                               |
    |       |                    |                                   |
    |       |                    | Dispatch(RAF scope release)       |
    |       |                    |    (deferred to next tick)        |
    |       |                    |                                   |
    |       |                    | next dispatch cycle:              |
    |       |                    | pendingScopes--                   |
    |       |                    |----> CV notify                    |
    |       |                    |                                   |
    |  FinishRenderingCurrentFrame()                                 |
    |-------+                    |                                   |
    |       | wait scopes == 0   |                                   |
    |       |<-------------------|                                   |
    |       |                    |                                   |
    |       | m_frameBlocked =   |                                   |
    |       | true               |                                   |
    |       |                    |                                   |
    |  tick beforeRenderDispatcher                                   |
    |       |                    |                                   |
    |       |                    |                                   |
    |  bgfx::end(m_frameEncoder) |                                   |
    |       |                    |                                   |
    |  Frame()                   |                                   |
    |---+                        |                                   |
    |   | bgfx::frame() --------|---------------------------------->|
    |   |                        |                                   | GPU submit & render
    |   |<-----------------------|---------------------------------->|
    |<--+                        |                                   |
    |                            |                                   |
    |  tick afterRenderDispatcher|                                   |
    |                            |                                   |
    |  [next frame...]           |                                   |
    v                            v                                   v

ReadTexture

ReadTexture acquires its own FrameCompletionScope — it can be called during
RAF, from getFrameBufferData callbacks, or during initialization:

ReadTexture()
    FrameCompletionScope scope{AcquireFrameCompletionScope()};
    // blocks until frame open if called outside RAF

    needs blit? → GetEncoder() (assert non-null)
                   encoder->blit(src → dst)
                   sourceTexture = dst

    ReadTextureAsync(sourceTexture)
    // scope released on return → blit + read land in same frame

SubmitCommands Scope Acquisition

SubmitCommands always acquires a stack-scoped FrameCompletionScope:

SubmitCommands()
    FrameCompletionScope scope{AcquireFrameCompletionScope()};
    // If frame open: returns immediately (scope counter++)
    // If frame closed: blocks until StartRenderingCurrentFrame opens gate
    process commands...
    // scope destroyed on return → counter--

This eliminates:

• Race between initial null-check and command execution
• Mid-execution scope release via microtask draining (drainMicrotasks)
• The m_outsideFrameScope member variable and deferred release pattern

Canvas Flush

Canvas::Flush()
    if GetActiveEncoder() == nullptr:
        scope = AcquireFrameCompletionScope()   // blocks until frame starts
    encoder = GetActiveEncoder()
    if encoder == nullptr: return               // defensive: shutdown edge case
    encoder->discard(BGFX_DISCARD_ALL)          // clear NativeEngine state
    nanovg rendering...

Synchronization Rules

1. Single encoder, shared by all consumers — DeviceImpl acquires the encoder in
   StartRenderingCurrentFrame and all consumers read it via GetActiveEncoder().

2. Encoder acquired before gate opens — bgfx::begin(true) is called BEFORE
   m_frameBlocked is set to false. Mutex provides memory ordering guarantee.

3. Encoder ended after gate closes — bgfx::end() is called AFTER all
   FrameCompletionScopes are released and m_frameBlocked is set to true.

4. SubmitCommands always holds a scope — Prevents encoder from being ended
   mid-command-processing, regardless of microtask draining or reentrancy.

5. Apps must keep frame open when waiting for JS — Any main thread wait for JS
   work must be bracketed by Start/Finish. Otherwise SubmitCommands blocks
   forever waiting for the gate to open.

6. ReadTexture holds its own scope — Acquires FrameCompletionScope to ensure
   encoder is available for blit and ReadTextureAsync. Both land in the same frame.
   Works during RAF, from getFrameBufferData callbacks, and during initialization.

Files Changed (vs 413cb49)

File	Change
Core/Graphics/Source/DeviceImpl.h	+m_frameEncoder, +m_firstFrameStarted, +frame sync, +FrameStartScheduler
Core/Graphics/Source/DeviceImpl.cpp	+encoder in Start/Finish, +sync methods, −SafeTimespan, −GetEncoderForThread
Core/Graphics/InternalInclude/.../DeviceContext.h	+FrameCompletionScope, +new methods, −UpdateToken, −Update
Core/Graphics/Source/DeviceContext.cpp	+FrameCompletionScope impl, +forwarding, −UpdateToken impl
Core/Graphics/Include/.../Device.h	DeviceUpdate → no-op shim
Core/Graphics/Source/Device.cpp	−GetUpdate impl
Core/Graphics/InternalInclude/.../FrameBuffer.h	−encoder& params
Core/Graphics/Source/FrameBuffer.cpp	Gets encoder via GetActiveEncoder
Core/Graphics/InternalInclude/.../SafeTimespanGuarantor.h	Deleted
Core/Graphics/Source/SafeTimespanGuarantor.cpp	Deleted
Plugins/NativeEngine/Source/NativeEngine.h	+GetEncoder, +BeginFrame/EndFrame, −m_update/m_updateToken
Plugins/NativeEngine/Source/NativeEngine.cpp	Always-scoped SubmitCommands+ReadTexture, inline blit, +assert in GetEncoder
Plugins/NativeXr/Source/NativeXrImpl.h/cpp	Replace Update→FrameStartScheduler+scopes
Polyfills/Canvas/Source/Context.h/cpp	Replace UpdateToken→GetActiveEncoder+scope+null check+discard
Apps/HeadlessScreenshotApp/Win32/App.cpp	+Start/Finish around startup wait
Apps/StyleTransferApp/Win32/App.cpp	+Start/Finish around startup wait
Apps/UnitTests/Source/Tests.JavaScript.cpp	Frame pump loop + shutdown frame
bgfx/src/bgfx.cpp + bgfx_p.h	+m_encoderBeginLock

What Remains for Separate PR

Remove DeviceUpdate — DeviceUpdate is a no-op shim retained because its
removal is an API breaking change (apps call GetUpdate/Start/Finish).
All other changes in this branch are non-breaking. Removing DeviceUpdate will
be a separate PR that updates all app call sites.

[Copilot]

Pull request overview

This PR reworks the graphics threading/synchronization model by replacing per-thread encoders and the SafeTimespan/UpdateToken mechanism with a single per-frame bgfx encoder owned by DeviceImpl and a new FrameCompletionScope gate/counter for cross-thread frame completion synchronization.

Changes:

• Removed SafeTimespanGuarantor, UpdateToken/Update, and per-thread encoder management; introduced a single frame encoder lifecycle in DeviceImpl.
• Added FrameCompletionScope + frame-start/before-render/after-render scheduling to coordinate JS-thread work with bgfx::frame().
• Updated NativeEngine/NativeXR/Canvas/FrameBuffer APIs to use GetActiveEncoder() and internal encoder acquisition patterns instead of passing encoders through call chains.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
Core/Graphics/Source/DeviceImpl.h	Adds single-frame encoder pointer and frame gate (mutex/CV + pending scope count).
Core/Graphics/Source/DeviceImpl.cpp	Implements frame open/close sequencing, scope blocking, and frame-start dispatcher tick.
Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h	Replaces Update/UpdateToken with FrameCompletionScope, adds frame-start scheduler + encoder accessors.
Core/Graphics/Source/DeviceContext.cpp	Implements FrameCompletionScope RAII and forwards new scheduling/encoder methods.
Core/Graphics/Include/Shared/Babylon/Graphics/Device.h	Converts DeviceUpdate into a no-op compatibility shim and inlines GetUpdate.
Core/Graphics/Source/Device.cpp	Removes old GetUpdate implementation.
Core/Graphics/InternalInclude/Babylon/Graphics/FrameBuffer.h	Removes encoder parameters from bind/unbind/viewport/scissor APIs.
Core/Graphics/Source/FrameBuffer.cpp	Updates FrameBuffer internals to acquire view IDs without needing an encoder parameter.
Plugins/NativeEngine/Source/NativeEngine.h	Switches to active-encoder access + outside-frame scope holding; adds begin/end frame no-ops.
Plugins/NativeEngine/Source/NativeEngine.cpp	Updates command submission and RAF scheduling to use frame-start scheduling + scopes; updates ReadTexture blit scheduling.
Plugins/NativeEngine/Source/PerFrameValue.h	Adjusts API to remove encoder parameters (but currently has a type-signature issue).
Plugins/NativeXr/Source/NativeXrImpl.h	Removes stored Update member from XR session state.
Plugins/NativeXr/Source/NativeXrImpl.cpp	Migrates XR scheduling to frame-start + scopes; updates encoder usage.
Polyfills/Canvas/Source/Context.h	Removes stored Update member.
Polyfills/Canvas/Source/Context.cpp	Uses GetActiveEncoder() and acquires a FrameCompletionScope when flushing outside the frame cycle.
Polyfills/Canvas/Source/nanovg/nanovg_babylon.cpp	Updates FrameBuffer binding calls to new signature (no encoder param).
Apps/HeadlessScreenshotApp/Win32/App.cpp	Wraps startup wait with Start/Finish frame calls to keep gate open during startup.
Apps/StyleTransferApp/Win32/App.cpp	Same startup framing change as HeadlessScreenshotApp.
Core/Graphics/CMakeLists.txt	Removes SafeTimespanGuarantor sources from build.

Comments suppressed due to low confidence (1)

Plugins/NativeEngine/Source/PerFrameValue.h:33

• PerFrameValue is templated on T, but Set currently takes a bool parameter. This makes the template misleading and will break or silently convert if PerFrameValue is ever instantiated with a non-bool type. Change Set to accept T (e.g., const T& or T) to match the class template parameter.

        T Get() const
        {
            return m_value;
        }

        void Set(bool value)
        {
            m_value = value;
            if (!m_isResetScheduled)
            {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.