From ca85709efdf6deda109f3d166aa4e50951fc0980 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Tue, 24 Mar 2026 21:54:57 -0700 Subject: [PATCH 01/24] Reworked threading model. --- Apps/HeadlessScreenshotApp/Win32/App.cpp | 8 + Apps/StyleTransferApp/Win32/App.cpp | 8 + Core/Graphics/CMakeLists.txt | 2 - .../Include/Shared/Babylon/Graphics/Device.h | 38 +--- .../Babylon/Graphics/DeviceContext.h | 89 ++++----- .../Babylon/Graphics/FrameBuffer.h | 10 +- .../Babylon/Graphics/SafeTimespanGuarantor.h | 56 ------ Core/Graphics/Source/Device.cpp | 13 -- Core/Graphics/Source/DeviceContext.cpp | 57 ++++-- Core/Graphics/Source/DeviceImpl.cpp | 133 +++++++------ Core/Graphics/Source/DeviceImpl.h | 68 +++++-- Core/Graphics/Source/FrameBuffer.cpp | 22 +-- .../Graphics/Source/SafeTimespanGuarantor.cpp | 83 -------- Plugins/NativeEngine/Source/NativeEngine.cpp | 181 ++++++++++-------- Plugins/NativeEngine/Source/NativeEngine.h | 13 +- Plugins/NativeEngine/Source/PerFrameValue.h | 4 +- Plugins/NativeXr/Source/NativeXrImpl.cpp | 6 +- Plugins/NativeXr/Source/NativeXrImpl.h | 2 - Polyfills/Canvas/Source/Context.cpp | 24 ++- Polyfills/Canvas/Source/Context.h | 1 - .../Canvas/Source/nanovg/nanovg_babylon.cpp | 8 +- 21 files changed, 383 insertions(+), 443 deletions(-) delete mode 100644 Core/Graphics/InternalInclude/Babylon/Graphics/SafeTimespanGuarantor.h delete mode 100644 Core/Graphics/Source/SafeTimespanGuarantor.cpp diff --git a/Apps/HeadlessScreenshotApp/Win32/App.cpp b/Apps/HeadlessScreenshotApp/Win32/App.cpp index c89d59b74..d192b8bc5 100644 --- a/Apps/HeadlessScreenshotApp/Win32/App.cpp +++ b/Apps/HeadlessScreenshotApp/Win32/App.cpp @@ -159,9 +159,17 @@ int main() deviceUpdate.Finish(); device.FinishRenderingCurrentFrame(); + // Reopen the gate so JS can continue running (startup may issue bgfx commands). + device.StartRenderingCurrentFrame(); + deviceUpdate.Start(); + // Wait for `startup` to finish. startup.get_future().wait(); + // Close the frame opened above. + deviceUpdate.Finish(); + device.FinishRenderingCurrentFrame(); + struct Asset { const char* Name; diff --git a/Apps/StyleTransferApp/Win32/App.cpp b/Apps/StyleTransferApp/Win32/App.cpp index 3688a9752..338b817c2 100644 --- a/Apps/StyleTransferApp/Win32/App.cpp +++ b/Apps/StyleTransferApp/Win32/App.cpp @@ -362,9 +362,17 @@ int APIENTRY wWinMain(_In_ HINSTANCE hInstance, g_update->Finish(); g_device->FinishRenderingCurrentFrame(); + // Reopen the gate so JS can continue running (startup may issue bgfx commands). + g_device->StartRenderingCurrentFrame(); + g_update->Start(); + // Wait for `startup` to finish. startup.get_future().wait(); + // Close the frame opened above. + g_update->Finish(); + g_device->FinishRenderingCurrentFrame(); + // --------------------------- Rendering loop ------------------------- HACCEL hAccelTable = LoadAccelerators(hInstance, MAKEINTRESOURCE(IDC_PLAYGROUNDWIN32)); diff --git a/Core/Graphics/CMakeLists.txt b/Core/Graphics/CMakeLists.txt index 8758dfa17..552b9cd0c 100644 --- a/Core/Graphics/CMakeLists.txt +++ b/Core/Graphics/CMakeLists.txt @@ -6,7 +6,6 @@ set(SOURCES "InternalInclude/Babylon/Graphics/continuation_scheduler.h" "InternalInclude/Babylon/Graphics/FrameBuffer.h" "InternalInclude/Babylon/Graphics/DeviceContext.h" - "InternalInclude/Babylon/Graphics/SafeTimespanGuarantor.h" "InternalInclude/Babylon/Graphics/Texture.h" "Source/BgfxCallback.cpp" "Source/FrameBuffer.cpp" @@ -16,7 +15,6 @@ set(SOURCES "Source/DeviceImpl.h" "Source/DeviceImpl_${BABYLON_NATIVE_PLATFORM}.${BABYLON_NATIVE_PLATFORM_IMPL_EXT}" "Source/DeviceImpl_${GRAPHICS_API}.${BABYLON_NATIVE_PLATFORM_IMPL_EXT}" - "Source/SafeTimespanGuarantor.cpp" "Source/Texture.cpp") add_library(Graphics ${SOURCES}) diff --git a/Core/Graphics/Include/Shared/Babylon/Graphics/Device.h b/Core/Graphics/Include/Shared/Babylon/Graphics/Device.h index 8a529cdee..f57a95b2d 100644 --- a/Core/Graphics/Include/Shared/Babylon/Graphics/Device.h +++ b/Core/Graphics/Include/Shared/Babylon/Graphics/Device.h @@ -58,45 +58,23 @@ namespace Babylon::Graphics DepthStencilFormat BackBufferDepthStencilFormat{DepthStencilFormat::Depth24Stencil8}; }; - class Device; + class DeviceImpl; + // Deprecated: DeviceUpdate is a no-op compatibility shim. Frame synchronization + // is now handled by FrameCompletionScope inside StartRenderingCurrentFrame/ + // FinishRenderingCurrentFrame. This class will be removed in a future PR. class DeviceUpdate { public: - void Start() - { - m_start(); - } + void Start() {} + void Finish() {} void RequestFinish(std::function onFinishCallback) { - m_requestFinish(std::move(onFinishCallback)); - } - - void Finish() - { - std::promise promise{}; - auto future = promise.get_future(); - RequestFinish([&promise] { promise.set_value(); }); - future.wait(); - } - - private: - friend class Device; - - template - DeviceUpdate(StartCallableT&& start, RequestEndCallableT&& requestEnd) - : m_start{std::forward(start)} - , m_requestFinish{std::forward(requestEnd)} - { + onFinishCallback(); } - - std::function m_start{}; - std::function)> m_requestFinish{}; }; - class DeviceImpl; - class Device { public: @@ -128,7 +106,7 @@ namespace Babylon::Graphics void EnableRendering(); void DisableRendering(); - DeviceUpdate GetUpdate(const char* updateName); + DeviceUpdate GetUpdate(const char* /*updateName*/) { return {}; } void StartRenderingCurrentFrame(); void FinishRenderingCurrentFrame(); diff --git a/Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h b/Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h index f0a9aeae2..5b9fcf186 100644 --- a/Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h +++ b/Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h @@ -3,7 +3,8 @@ #include "BgfxCallback.h" #include #include "continuation_scheduler.h" -#include "SafeTimespanGuarantor.h" + +#include #include @@ -15,7 +16,6 @@ namespace Babylon::Graphics { - class Update; class DeviceContext; class DeviceImpl; @@ -29,53 +29,38 @@ namespace Babylon::Graphics bgfx::TextureFormat::Enum Format{}; }; - class UpdateToken final + // FrameCompletionScope is an RAII guard that keeps a frame "open" on the JS thread. + // While any scope is alive, FinishRenderingCurrentFrame() on the main thread will + // block — it cannot call bgfx::frame() until all scopes are destroyed. + // + // This prevents a race where the main thread submits a bgfx frame while the JS + // thread is still recording encoder commands (which would cause bgfx deadlocks + // or lost draw calls). + // + // Three usage patterns: + // 1. RAF scheduling: scope acquired on main thread during StartRenderingCurrentFrame, + // transferred to JS thread, released after RAF callbacks complete + one extra + // dispatch cycle (to cover GC-triggered resource destruction). + // 2. NativeEngine::GetEncoder(): scope acquired lazily when JS code uses the encoder + // outside RAF (e.g., async texture loads, LOD switches). Released on next dispatch. + // 3. Canvas::Flush(): stack-scoped for the duration of nanovg rendering. + // + // Construction blocks if m_frameBlocked is true (frame submission in progress). + // Destruction decrements counter and wakes main thread via condition variable. + class FrameCompletionScope final { public: - UpdateToken(const UpdateToken& other) = delete; - UpdateToken& operator=(const UpdateToken& other) = delete; - - UpdateToken(UpdateToken&&) noexcept = default; - - // The move assignment of `SafeTimespanGuarantor::SafetyGuarantee` is marked as delete. - // See https://github.com/Microsoft/GSL/issues/705. - //UpdateToken& operator=(UpdateToken&& other) = delete; + FrameCompletionScope(const FrameCompletionScope&) = delete; + FrameCompletionScope& operator=(const FrameCompletionScope&) = delete; + FrameCompletionScope& operator=(FrameCompletionScope&&) = delete; - bgfx::Encoder* GetEncoder(); - - private: - friend class Update; - - UpdateToken(DeviceContext&, SafeTimespanGuarantor&); - - DeviceContext& m_context; - SafeTimespanGuarantor::SafetyGuarantee m_guarantee; - }; - - class Update - { - public: - continuation_scheduler<>& Scheduler() - { - return m_safeTimespanGuarantor.OpenScheduler(); - } - - UpdateToken GetUpdateToken() - { - return {m_context, m_safeTimespanGuarantor}; - } + FrameCompletionScope(FrameCompletionScope&&) noexcept; + ~FrameCompletionScope(); private: friend class DeviceContext; - - Update(SafeTimespanGuarantor& safeTimespanGuarantor, DeviceContext& context) - : m_safeTimespanGuarantor{safeTimespanGuarantor} - , m_context{context} - { - } - - SafeTimespanGuarantor& m_safeTimespanGuarantor; - DeviceContext& m_context; + FrameCompletionScope(DeviceImpl&); + DeviceImpl* m_impl; }; class DeviceContext @@ -93,7 +78,19 @@ namespace Babylon::Graphics continuation_scheduler<>& BeforeRenderScheduler(); continuation_scheduler<>& AfterRenderScheduler(); - Update GetUpdate(const char* updateName); + // Scheduler that fires when StartRenderingCurrentFrame ticks the frame start dispatcher. + // Use this to schedule work (e.g., requestAnimationFrame callbacks) that should run each frame. + continuation_scheduler<>& FrameStartScheduler(); + + // Acquire a scope that prevents FinishRenderingCurrentFrame from completing. + // The scope must be held while JS frame callbacks are running. + FrameCompletionScope AcquireFrameCompletionScope(); + + // Active encoder for the current frame. Managed by DeviceImpl in + // StartRenderingCurrentFrame/FinishRenderingCurrentFrame. + // Used by NativeEngine, Canvas, and NativeXr. + void SetActiveEncoder(bgfx::Encoder* encoder); + bgfx::Encoder* GetActiveEncoder(); void RequestScreenShot(std::function)> callback); void SetRenderResetCallback(std::function callback); @@ -113,7 +110,7 @@ namespace Babylon::Graphics using CaptureCallbackTicketT = arcana::ticketed_collection>::ticket; CaptureCallbackTicketT AddCaptureCallback(std::function callback); - bgfx::ViewId AcquireNewViewId(bgfx::Encoder&); + bgfx::ViewId AcquireNewViewId(); // TODO: find a different way to get the texture info for frame capture void AddTexture(bgfx::TextureHandle handle, uint16_t width, uint16_t height, bool hasMips, uint16_t numLayers, bgfx::TextureFormat::Enum format); @@ -122,8 +119,6 @@ namespace Babylon::Graphics static bx::AllocatorI& GetDefaultAllocator() { return m_allocator; } private: - friend UpdateToken; - DeviceImpl& m_graphicsImpl; std::unordered_map m_textureHandleToInfo{}; diff --git a/Core/Graphics/InternalInclude/Babylon/Graphics/FrameBuffer.h b/Core/Graphics/InternalInclude/Babylon/Graphics/FrameBuffer.h index d3e64d406..d970ecc97 100644 --- a/Core/Graphics/InternalInclude/Babylon/Graphics/FrameBuffer.h +++ b/Core/Graphics/InternalInclude/Babylon/Graphics/FrameBuffer.h @@ -33,12 +33,12 @@ namespace Babylon::Graphics uint16_t Height() const; bool DefaultBackBuffer() const; - void Bind(bgfx::Encoder& encoder); - void Unbind(bgfx::Encoder& encoder); + void Bind(); + void Unbind(); void Clear(bgfx::Encoder& encoder, uint16_t flags, uint32_t rgba, float depth, uint8_t stencil); - void SetViewPort(bgfx::Encoder& encoder, float x, float y, float width, float height); - void SetScissor(bgfx::Encoder& encoder, float x, float y, float width, float height); + void SetViewPort(float x, float y, float width, float height); + void SetScissor(float x, float y, float width, float height); void Submit(bgfx::Encoder& encoder, bgfx::ProgramHandle programHandle, uint8_t flags); void SetStencil(bgfx::Encoder& encoder, uint32_t stencilState); void Blit(bgfx::Encoder& encoder, bgfx::TextureHandle dst, uint16_t dstX, uint16_t dstY, bgfx::TextureHandle src, uint16_t srcX = 0, uint16_t srcY = 0, uint16_t width = UINT16_MAX, uint16_t height = UINT16_MAX); @@ -48,7 +48,7 @@ namespace Babylon::Graphics private: Rect GetBgfxScissor(float x, float y, float width, float height) const; - void SetBgfxViewPortAndScissor(bgfx::Encoder& encoder, const Rect& viewPort, const Rect& scissor); + void SetBgfxViewPortAndScissor(const Rect& viewPort, const Rect& scissor); DeviceContext& m_deviceContext; const uintptr_t m_deviceID{}; diff --git a/Core/Graphics/InternalInclude/Babylon/Graphics/SafeTimespanGuarantor.h b/Core/Graphics/InternalInclude/Babylon/Graphics/SafeTimespanGuarantor.h deleted file mode 100644 index 98c556b8c..000000000 --- a/Core/Graphics/InternalInclude/Babylon/Graphics/SafeTimespanGuarantor.h +++ /dev/null @@ -1,56 +0,0 @@ -#pragma once - -#include "continuation_scheduler.h" - -#include -#include - -#include - -#include -#include -#include - -namespace Babylon::Graphics -{ - class SafeTimespanGuarantor - { - public: - SafeTimespanGuarantor(std::optional&); - - continuation_scheduler<>& OpenScheduler() - { - return m_openDispatcher.scheduler(); - } - - continuation_scheduler<>& CloseScheduler() - { - return m_closeDispatcher.scheduler(); - } - - using SafetyGuarantee = gsl::final_action>; - SafetyGuarantee GetSafetyGuarantee(); - - void Open(); - void RequestClose(); - void Lock(); - void Unlock(); - - private: - enum class State - { - Open, - Closing, - Closed, - Locked - }; - - std::optional& m_cancellation; - State m_state{State::Locked}; - uint32_t m_count{}; - std::mutex m_mutex{}; - std::condition_variable m_condition_variable{}; - continuation_dispatcher<> m_openDispatcher{}; - continuation_dispatcher<> m_closeDispatcher{}; - }; -} diff --git a/Core/Graphics/Source/Device.cpp b/Core/Graphics/Source/Device.cpp index ec9365ea6..1dcdb7608 100644 --- a/Core/Graphics/Source/Device.cpp +++ b/Core/Graphics/Source/Device.cpp @@ -66,19 +66,6 @@ namespace Babylon::Graphics m_impl->DisableRendering(); } - DeviceUpdate Device::GetUpdate(const char* updateName) - { - auto& guarantor = m_impl->GetSafeTimespanGuarantor(updateName); - return { - [&guarantor] { - guarantor.Open(); - }, - [&guarantor](std::function callback) { - guarantor.CloseScheduler()(std::move(callback)); - guarantor.RequestClose(); - }}; - } - void Device::StartRenderingCurrentFrame() { m_impl->StartRenderingCurrentFrame(); diff --git a/Core/Graphics/Source/DeviceContext.cpp b/Core/Graphics/Source/DeviceContext.cpp index 8db350110..de1611933 100644 --- a/Core/Graphics/Source/DeviceContext.cpp +++ b/Core/Graphics/Source/DeviceContext.cpp @@ -4,20 +4,6 @@ #include -namespace Babylon::Graphics -{ - UpdateToken::UpdateToken(DeviceContext& context, SafeTimespanGuarantor& guarantor) - : m_context{context} - , m_guarantee{guarantor.GetSafetyGuarantee()} - { - } - - bgfx::Encoder* UpdateToken::GetEncoder() - { - return m_context.m_graphicsImpl.GetEncoderForThread(); - } -} - namespace Babylon::Graphics { DeviceContext& DeviceContext::GetFromJavaScript(Napi::Env env) @@ -51,9 +37,44 @@ namespace Babylon::Graphics return m_graphicsImpl.AfterRenderScheduler(); } - Update DeviceContext::GetUpdate(const char* updateName) + continuation_scheduler<>& DeviceContext::FrameStartScheduler() + { + return m_graphicsImpl.FrameStartScheduler(); + } + + FrameCompletionScope::FrameCompletionScope(DeviceImpl& impl) + : m_impl{&impl} + { + m_impl->IncrementPendingFrameScopes(); + } + + FrameCompletionScope::FrameCompletionScope(FrameCompletionScope&& other) noexcept + : m_impl{other.m_impl} + { + other.m_impl = nullptr; + } + + FrameCompletionScope::~FrameCompletionScope() + { + if (m_impl) + { + m_impl->DecrementPendingFrameScopes(); + } + } + + FrameCompletionScope DeviceContext::AcquireFrameCompletionScope() + { + return FrameCompletionScope{m_graphicsImpl}; + } + + void DeviceContext::SetActiveEncoder(bgfx::Encoder* encoder) + { + m_graphicsImpl.SetActiveEncoder(encoder); + } + + bgfx::Encoder* DeviceContext::GetActiveEncoder() { - return {m_graphicsImpl.GetSafeTimespanGuarantor(updateName), *this}; + return m_graphicsImpl.GetActiveEncoder(); } void DeviceContext::RequestScreenShot(std::function)> callback) @@ -101,9 +122,9 @@ namespace Babylon::Graphics return m_graphicsImpl.AddCaptureCallback(std::move(callback)); } - bgfx::ViewId DeviceContext::AcquireNewViewId(bgfx::Encoder& encoder) + bgfx::ViewId DeviceContext::AcquireNewViewId() { - return m_graphicsImpl.AcquireNewViewId(encoder); + return m_graphicsImpl.AcquireNewViewId(); } void DeviceContext::AddTexture(bgfx::TextureHandle handle, uint16_t width, uint16_t height, bool hasMips, uint16_t numLayers, bgfx::TextureFormat::Enum format) diff --git a/Core/Graphics/Source/DeviceImpl.cpp b/Core/Graphics/Source/DeviceImpl.cpp index 895177247..15ba9cedf 100644 --- a/Core/Graphics/Source/DeviceImpl.cpp +++ b/Core/Graphics/Source/DeviceImpl.cpp @@ -257,19 +257,6 @@ namespace Babylon::Graphics } } - SafeTimespanGuarantor& DeviceImpl::GetSafeTimespanGuarantor(const char* updateName) - { - std::scoped_lock lock{m_updateSafeTimespansMutex}; - std::string updateNameStr{updateName}; - auto found = m_updateSafeTimespans.find(updateNameStr); - if (found == m_updateSafeTimespans.end()) - { - m_updateSafeTimespans.emplace(std::piecewise_construct, std::forward_as_tuple(updateNameStr), std::forward_as_tuple(m_cancellationSource)); - found = m_updateSafeTimespans.find(updateNameStr); - } - return found->second; - } - void DeviceImpl::SetDiagnosticOutput(std::function diagnosticOutput) { ASSERT_THREAD_AFFINITY(m_renderThreadAffinity); @@ -292,38 +279,56 @@ namespace Babylon::Graphics // Ensure rendering is enabled. EnableRendering(); - // Unlock the update safe timespans. + // Acquire the frame encoder BEFORE opening the gate. This guarantees + // that when JS code unblocks from AcquireFrameCompletionScope, the + // encoder is already available in DeviceContext. + m_frameEncoder = bgfx::begin(true); + + // Open the gate: allow JS thread to acquire FrameCompletionScopes and use the encoder. { - std::scoped_lock lock{m_updateSafeTimespansMutex}; - for (auto& [key, value] : m_updateSafeTimespans) - { - value.Unlock(); - } + std::lock_guard lock{m_frameSyncMutex}; + m_frameBlocked = false; } + m_frameSyncCV.notify_all(); + + // Tick the frame start dispatcher. This fires requestAnimationFrame tasks that + // were scheduled by NativeEngine/NativeXr. Those tasks acquire FrameCompletionScopes + // (keeping the gate reference count > 0) and dispatch JS callbacks to the JS thread. + m_frameStartDispatcher.tick(*m_cancellationSource); } void DeviceImpl::FinishRenderingCurrentFrame() { - // Lock the update safe timespans. - { - std::scoped_lock lock{m_updateSafeTimespansMutex}; - for (auto& [key, value] : m_updateSafeTimespans) - { - value.Lock(); - } - } - arcana::trace_region finishRenderingRegion{"DeviceImpl::FinishRenderingCurrentFrame"}; ASSERT_THREAD_AFFINITY(m_renderThreadAffinity); if (!m_rendering) { - throw std::runtime_error{"Current frame cannot be finished prior to having been started."}; + // First call at startup — no frame in progress yet, nothing to finish. + return; + } + + // Close the gate: wait until JS thread has released all FrameCompletionScopes + // (meaning all encoder work for this frame is done), then block new acquisitions. + // After this point, no bgfx encoder calls can be in flight on the JS thread. + { + std::unique_lock lock{m_frameSyncMutex}; + m_frameSyncCV.wait(lock, [this] { return m_pendingFrameScopes == 0; }); + m_frameBlocked = true; } m_beforeRenderDispatcher.tick(*m_cancellationSource); + // End the frame encoder before calling bgfx::frame(). frame() waits for + // all encoders to be returned via encoderApiWait(), so the encoder must + // be ended first to avoid a deadlock on the same thread. + if (m_frameEncoder) + { + bgfx::end(m_frameEncoder); + m_frameEncoder = nullptr; + } + Frame(); m_afterRenderDispatcher.tick(*m_cancellationSource); @@ -368,6 +373,43 @@ namespace Babylon::Graphics return m_afterRenderDispatcher.scheduler(); } + continuation_scheduler<>& DeviceImpl::FrameStartScheduler() + { + return m_frameStartDispatcher.scheduler(); + } + + // Called by FrameCompletionScope constructor (on any thread). + // Blocks if the gate is closed (m_frameBlocked), meaning bgfx::frame() is running + // or no frame has started yet. Once unblocked, increments the scope counter. + void DeviceImpl::IncrementPendingFrameScopes() + { + std::unique_lock lock{m_frameSyncMutex}; + m_frameSyncCV.wait(lock, [this] { return !m_frameBlocked; }); + m_pendingFrameScopes++; + } + + // Called by FrameCompletionScope destructor (on any thread). + // Decrements the scope counter and wakes the main thread, which may be waiting + // in FinishRenderingCurrentFrame for all scopes to be released. + void DeviceImpl::DecrementPendingFrameScopes() + { + { + std::lock_guard lock{m_frameSyncMutex}; + m_pendingFrameScopes--; + } + m_frameSyncCV.notify_all(); + } + + void DeviceImpl::SetActiveEncoder(bgfx::Encoder* encoder) + { + m_frameEncoder = encoder; + } + + bgfx::Encoder* DeviceImpl::GetActiveEncoder() const + { + return m_frameEncoder; + } + void DeviceImpl::RequestScreenShot(std::function)> callback) { m_screenShotCallbacks.push(std::move(callback)); @@ -395,7 +437,7 @@ namespace Babylon::Graphics return m_captureCallbacks.insert(std::move(callback), m_captureCallbacksMutex); } - bgfx::ViewId DeviceImpl::AcquireNewViewId(bgfx::Encoder&) + bgfx::ViewId DeviceImpl::AcquireNewViewId() { bgfx::ViewId viewId = m_nextViewId.fetch_add(1); if (viewId >= bgfx::getCaps()->limits.maxViews) @@ -452,9 +494,6 @@ namespace Babylon::Graphics { arcana::trace_region frameRegion{"DeviceImpl::Frame"}; - // Automatically end bgfx encoders. - EndEncoders(); - // Update bgfx state if necessary. UpdateBgfxState(); @@ -474,34 +513,6 @@ namespace Babylon::Graphics m_nextViewId.store(0); } - bgfx::Encoder* DeviceImpl::GetEncoderForThread() - { - assert(!m_renderThreadAffinity.check()); - std::scoped_lock lock{m_threadIdToEncoderMutex}; - - const auto threadId{std::this_thread::get_id()}; - auto it{m_threadIdToEncoder.find(threadId)}; - if (it == m_threadIdToEncoder.end()) - { - bgfx::Encoder* encoder{bgfx::begin(true)}; - it = m_threadIdToEncoder.emplace(threadId, encoder).first; - } - - return it->second; - } - - void DeviceImpl::EndEncoders() - { - std::scoped_lock lock{m_threadIdToEncoderMutex}; - - for (auto [threadId, encoder] : m_threadIdToEncoder) - { - bgfx::end(encoder); - } - - m_threadIdToEncoder.clear(); - } - void DeviceImpl::CaptureCallback(const BgfxCallback::CaptureData& data) { std::scoped_lock callbackLock{m_captureCallbacksMutex}; diff --git a/Core/Graphics/Source/DeviceImpl.h b/Core/Graphics/Source/DeviceImpl.h index c42222275..66e9b9242 100644 --- a/Core/Graphics/Source/DeviceImpl.h +++ b/Core/Graphics/Source/DeviceImpl.h @@ -3,7 +3,6 @@ #include #include #include -#include #include #include @@ -16,8 +15,9 @@ #include #include +#include +#include #include -#include #include #include @@ -59,8 +59,6 @@ namespace Babylon::Graphics void EnableRendering(); void DisableRendering(); - SafeTimespanGuarantor& GetSafeTimespanGuarantor(const char* updateName); - void SetDiagnosticOutput(std::function diagnosticOutput); void StartRenderingCurrentFrame(); @@ -84,6 +82,7 @@ namespace Babylon::Graphics continuation_scheduler<>& BeforeRenderScheduler(); continuation_scheduler<>& AfterRenderScheduler(); + continuation_scheduler<>& FrameStartScheduler(); void RequestScreenShot(std::function)> callback); @@ -92,7 +91,15 @@ namespace Babylon::Graphics using CaptureCallbackTicketT = arcana::ticketed_collection>::ticket; CaptureCallbackTicketT AddCaptureCallback(std::function callback); - bgfx::ViewId AcquireNewViewId(bgfx::Encoder&); + bgfx::ViewId AcquireNewViewId(); + + // Frame completion scope support + void IncrementPendingFrameScopes(); + void DecrementPendingFrameScopes(); + + // Active encoder for the current frame. + void SetActiveEncoder(bgfx::Encoder* encoder); + bgfx::Encoder* GetActiveEncoder() const; /* ********** END DEVICE CONTEXT CONTRACT ********** */ @@ -103,7 +110,7 @@ namespace Babylon::Graphics } private: - friend class UpdateToken; + friend class FrameCompletionScope; static const bgfx::RendererType::Enum s_bgfxRenderType; static void ConfigureBgfxPlatformData(bgfx::PlatformData& pd, WindowT window); @@ -114,13 +121,16 @@ namespace Babylon::Graphics void UpdateBgfxResolution(); void RequestScreenShots(); void Frame(); - bgfx::Encoder* GetEncoderForThread(); - void EndEncoders(); void CaptureCallback(const BgfxCallback::CaptureData&); arcana::affinity m_renderThreadAffinity{}; bool m_rendering{}; + // The single bgfx encoder for the current frame. Acquired in + // StartRenderingCurrentFrame, ended in FinishRenderingCurrentFrame. + // Read by all consumers via DeviceContext::GetActiveEncoder() → DeviceImpl::GetActiveEncoder(). + bgfx::Encoder* m_frameEncoder{nullptr}; + std::atomic m_nextViewId{0}; std::optional m_cancellationSource{}; @@ -151,19 +161,49 @@ namespace Babylon::Graphics continuation_dispatcher<> m_beforeRenderDispatcher{}; continuation_dispatcher<> m_afterRenderDispatcher{}; + // Ticked by StartRenderingCurrentFrame(). NativeEngine and NativeXr schedule + // requestAnimationFrame tasks here so they fire once per frame at the right time. + continuation_dispatcher<> m_frameStartDispatcher{}; + + // --- Frame synchronization between main thread and JS thread --- + // + // bgfx itself can handle concurrent bgfx::begin() and bgfx::frame() safely + // (begin blocks on a mutex until frame releases it). However, BabylonNative needs + // to ensure LOGICAL frame correctness: all encoder commands intended for frame N + // must be submitted (bgfx::end) before bgfx::frame() for frame N runs, otherwise + // draw calls are lost or appear in the wrong frame (causing flickering/artifacts). + // + // Solution: a blocking gate with a reference counter. + // + // m_frameBlocked (bool): + // - true = JS cannot acquire new FrameCompletionScopes (blocks in IncrementPendingFrameScopes) + // - false = JS is free to acquire scopes and use encoders + // - Set to false in StartRenderingCurrentFrame (opens the gate) + // - Set to true in FinishRenderingCurrentFrame after all scopes are released (closes the gate) + // - Starts as true (no frame in progress at init) + // + // m_pendingFrameScopes (int): + // - Count of active FrameCompletionScope instances + // - FinishRenderingCurrentFrame waits for this to reach 0 + // - Incremented by IncrementPendingFrameScopes (called by FrameCompletionScope constructor) + // - Decremented by DecrementPendingFrameScopes (called by FrameCompletionScope destructor) + // + // m_frameSyncMutex + m_frameSyncCV: + // - Protects m_frameBlocked and m_pendingFrameScopes + // - CV is waited on by: main thread (for scopes==0) and JS thread (for !blocked) + // - CV is notified by: JS thread (scope released) and main thread (unblocked) + std::mutex m_frameSyncMutex{}; + std::condition_variable m_frameSyncCV{}; + int m_pendingFrameScopes{0}; + bool m_frameBlocked{true}; + std::mutex m_captureCallbacksMutex{}; arcana::ticketed_collection> m_captureCallbacks{}; arcana::blocking_concurrent_queue)>> m_screenShotCallbacks{}; - std::map m_threadIdToEncoder{}; - std::mutex m_threadIdToEncoderMutex{}; - std::queue>> m_readTextureRequests{}; - std::map m_updateSafeTimespans{}; - std::mutex m_updateSafeTimespansMutex{}; - DeviceContext m_context; uintptr_t m_bgfxId = 0; std::function m_renderResetCallback; diff --git a/Core/Graphics/Source/FrameBuffer.cpp b/Core/Graphics/Source/FrameBuffer.cpp index 35959f606..94f26345a 100644 --- a/Core/Graphics/Source/FrameBuffer.cpp +++ b/Core/Graphics/Source/FrameBuffer.cpp @@ -69,19 +69,19 @@ namespace Babylon::Graphics return m_defaultBackBuffer; } - void FrameBuffer::Bind(bgfx::Encoder&) + void FrameBuffer::Bind() { m_viewId.reset(); } - void FrameBuffer::Unbind(bgfx::Encoder&) + void FrameBuffer::Unbind() { } void FrameBuffer::Clear(bgfx::Encoder& encoder, uint16_t flags, uint32_t rgba, float depth, uint8_t stencil) { // BGFX requires us to create a new viewID, this will ensure that the view gets cleared. - m_viewId = m_deviceContext.AcquireNewViewId(encoder); + m_viewId = m_deviceContext.AcquireNewViewId(); bgfx::setViewMode(m_viewId.value(), bgfx::ViewMode::Sequential); bgfx::setViewClear(m_viewId.value(), flags, rgba, depth, stencil); @@ -124,28 +124,28 @@ namespace Babylon::Graphics encoder.touch(m_viewId.value()); } - void FrameBuffer::SetViewPort(bgfx::Encoder& encoder, float x, float y, float width, float height) + void FrameBuffer::SetViewPort(float x, float y, float width, float height) { m_desiredViewPort = {x, y, width, height}; - SetBgfxViewPortAndScissor(encoder, m_desiredViewPort, m_desiredScissor); + SetBgfxViewPortAndScissor(m_desiredViewPort, m_desiredScissor); } - void FrameBuffer::SetScissor(bgfx::Encoder& encoder, float x, float y, float width, float height) + void FrameBuffer::SetScissor(float x, float y, float width, float height) { m_desiredScissor = GetBgfxScissor(x, y, width, height); - SetBgfxViewPortAndScissor(encoder, m_desiredViewPort, m_desiredScissor); + SetBgfxViewPortAndScissor(m_desiredViewPort, m_desiredScissor); } void FrameBuffer::Submit(bgfx::Encoder& encoder, bgfx::ProgramHandle programHandle, uint8_t flags) { - SetBgfxViewPortAndScissor(encoder, m_desiredViewPort, m_desiredScissor); + SetBgfxViewPortAndScissor(m_desiredViewPort, m_desiredScissor); encoder.submit(m_viewId.value(), programHandle, 0, flags); } void FrameBuffer::Blit(bgfx::Encoder& encoder, bgfx::TextureHandle dst, uint16_t dstX, uint16_t dstY, bgfx::TextureHandle src, uint16_t srcX, uint16_t srcY, uint16_t width, uint16_t height) { // In order for Blit to work properly we need to force the creation of a new ViewID. - SetBgfxViewPortAndScissor(encoder, m_desiredViewPort, m_desiredScissor); + SetBgfxViewPortAndScissor(m_desiredViewPort, m_desiredScissor); encoder.blit(m_viewId.value(), dst, dstX, dstY, src, srcX, srcY, width, height); } @@ -195,14 +195,14 @@ namespace Babylon::Graphics return Rect{x, y, width, height}; } - void FrameBuffer::SetBgfxViewPortAndScissor(bgfx::Encoder& encoder, const Rect& viewPort, const Rect& scissor) + void FrameBuffer::SetBgfxViewPortAndScissor(const Rect& viewPort, const Rect& scissor) { if (m_viewId.has_value() && viewPort.Equals(m_bgfxViewPort) && scissor.Equals(m_bgfxScissor)) { return; } - m_viewId = m_deviceContext.AcquireNewViewId(encoder); + m_viewId = m_deviceContext.AcquireNewViewId(); bgfx::setViewMode(m_viewId.value(), bgfx::ViewMode::Sequential); bgfx::setViewClear(m_viewId.value(), BGFX_CLEAR_NONE, 0, 1.0f, 0); diff --git a/Core/Graphics/Source/SafeTimespanGuarantor.cpp b/Core/Graphics/Source/SafeTimespanGuarantor.cpp deleted file mode 100644 index 29c7e14fc..000000000 --- a/Core/Graphics/Source/SafeTimespanGuarantor.cpp +++ /dev/null @@ -1,83 +0,0 @@ -#include - -namespace Babylon::Graphics -{ - SafeTimespanGuarantor::SafeTimespanGuarantor(std::optional& cancellation) - : m_cancellation{cancellation} - { - } - - void SafeTimespanGuarantor::Open() - { - { - std::scoped_lock lock{m_mutex}; - if (m_state != State::Closed) - { - throw std::runtime_error{"Safe timespan cannot begin if guarantor state is not closed"}; - } - m_state = State::Open; - } - - m_condition_variable.notify_all(); - std::this_thread::yield(); - - m_openDispatcher.tick(*m_cancellation); - } - - void SafeTimespanGuarantor::RequestClose() - { - std::scoped_lock lock{m_mutex}; - if (m_state != State::Open) - { - throw std::runtime_error{"Safe timespan cannot end if guarantor state is not open"}; - } - if (m_count == 0) - { - m_state = State::Closed; - m_closeDispatcher.tick(*m_cancellation); - } - else - { - m_state = State::Closing; - } - } - - void SafeTimespanGuarantor::Lock() - { - std::scoped_lock lock{m_mutex}; - if (m_state != State::Closed) - { - throw std::runtime_error{"SafeTimespanGuarantor can only be locked from a closed state"}; - } - m_state = State::Locked; - } - - void SafeTimespanGuarantor::Unlock() - { - std::scoped_lock lock{m_mutex}; - if (m_state != State::Locked) - { - throw std::runtime_error{"SafeTimespanGuarantor can only be unlocked if it was locked"}; - } - m_state = State::Closed; - } - - SafeTimespanGuarantor::SafetyGuarantee SafeTimespanGuarantor::GetSafetyGuarantee() - { - std::unique_lock lock{m_mutex}; - if (m_state == State::Closed || m_state == State::Locked) - { - m_condition_variable.wait(lock, [this]() { return m_state != State::Closed && m_state != State::Locked; }); - } - m_count++; - - return gsl::finally(std::function{[this] { - std::scoped_lock lock{m_mutex}; - if (--m_count == 0 && m_state == State::Closing) - { - m_state = State::Closed; - m_closeDispatcher.tick(*m_cancellation); - } - }}); - } -} diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 38cfb856d..492edde88 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -21,7 +21,9 @@ #include #include +#include #include +#include #ifdef WEBP #include @@ -727,8 +729,12 @@ namespace Babylon InstanceMethod("submitCommands", &NativeEngine::SubmitCommands), InstanceMethod("populateFrameStats", &NativeEngine::PopulateFrameStats), + InstanceMethod("beginFrame", &NativeEngine::BeginFrame), + InstanceMethod("endFrame", &NativeEngine::EndFrame), InstanceMethod("setDeviceLostCallback", &NativeEngine::SetRenderResetCallback), + + }); JsRuntime::NativeObject::GetFromJavaScript(env).Set(JS_CONSTRUCTOR_NAME, func); @@ -744,7 +750,6 @@ namespace Babylon , m_cancellationSource{std::make_shared()} , m_runtime{runtime} , m_deviceContext{Graphics::DeviceContext::GetFromJavaScript(info.Env())} - , m_update{m_deviceContext.GetUpdate("update")} , m_runtimeScheduler{runtime} , m_defaultFrameBuffer{m_deviceContext, BGFX_INVALID_HANDLE, 0, 0, true, true, true} , m_boundFrameBuffer{&m_defaultFrameBuffer} @@ -1342,12 +1347,11 @@ namespace Babylon void NativeEngine::CopyTexture(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder = GetUpdateToken().GetEncoder(); - const auto textureSource = data.ReadPointer(); const auto textureDestination = data.ReadPointer(); - GetBoundFrameBuffer(*encoder).Blit(*encoder, textureDestination->Handle(), 0, 0, textureSource->Handle()); + bgfx::Encoder* encoder = GetEncoder(); + GetBoundFrameBuffer().Blit(*encoder, textureDestination->Handle(), 0, 0, textureSource->Handle()); } void NativeEngine::LoadRawTexture(const Napi::CallbackInfo& info) @@ -1569,26 +1573,24 @@ namespace Babylon void NativeEngine::SetTexture(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder = GetUpdateToken().GetEncoder(); - const UniformInfo* uniformInfo = data.ReadPointer(); const Graphics::Texture* texture = data.ReadPointer(); + bgfx::Encoder* encoder = GetEncoder(); encoder->setTexture(uniformInfo->Stage, uniformInfo->Handle, texture->Handle(), texture->SamplerFlags()); } void NativeEngine::UnsetTexture(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder = GetUpdateToken().GetEncoder(); - const UniformInfo* uniformInfo = data.ReadPointer(); + bgfx::Encoder* encoder = GetEncoder(); encoder->setTexture(uniformInfo->Stage, uniformInfo->Handle, BGFX_INVALID_HANDLE); } void NativeEngine::DiscardAllTextures(NativeDataStream::Reader&) { - bgfx::Encoder* encoder = GetUpdateToken().GetEncoder(); + bgfx::Encoder* encoder = GetEncoder(); encoder->discard(BGFX_DISCARD_BINDINGS); } @@ -1649,25 +1651,29 @@ namespace Babylon bgfx::TextureHandle sourceTextureHandle{texture->Handle()}; auto tempTexture = std::make_shared(false); - // If the image needs to be cropped (not starting at 0, or less than full width/height (accounting for requested mip level)), - // or if the texture was not created with the BGFX_TEXTURE_READ_BACK flag, then blit it to a temp texture. + // If the image needs to be cropped or the texture lacks the READ_BACK flag, blit to a temp texture. + // The blit is scheduled via BeforeRenderScheduler and runs on the main thread + // right before the encoder is ended — guaranteeing it executes before bgfx::frame(). if (x != 0 || y != 0 || width != (texture->Width() >> mipLevel) || height != (texture->Height() >> mipLevel) || (texture->Flags() & BGFX_TEXTURE_READ_BACK) == 0) { const bgfx::TextureHandle blitTextureHandle{bgfx::createTexture2D(width, height, /*hasMips*/ false, /*numLayers*/ 1, sourceTextureFormat, BGFX_TEXTURE_BLIT_DST | BGFX_TEXTURE_READ_BACK)}; - bgfx::Encoder* encoder{GetUpdateToken().GetEncoder()}; - encoder->blit(static_cast(bgfx::getCaps()->limits.maxViews - 1), blitTextureHandle, /*dstMip*/ 0, /*dstX*/ 0, /*dstY*/ 0, /*dstZ*/ 0, sourceTextureHandle, mipLevel, x, y, /*srcZ*/ 0, width, height, /*depth*/ 0); + + arcana::make_task(m_deviceContext.BeforeRenderScheduler(), *m_cancellationSource, + [this, src = sourceTextureHandle, dst = blitTextureHandle, mipLevel, x, y, width, height]() { + bgfx::Encoder* encoder = m_deviceContext.GetActiveEncoder(); + assert(encoder != nullptr); + encoder->blit(static_cast(bgfx::getCaps()->limits.maxViews - 1), dst, /*dstMip*/ 0, /*dstX*/ 0, /*dstY*/ 0, /*dstZ*/ 0, src, mipLevel, x, y, /*srcZ*/ 0, width, height, /*depth*/ 0); + }); sourceTextureHandle = blitTextureHandle; *tempTexture = true; - - // The requested mip level was blitted, so the source texture now has just one mip, so reset the mip level to 0. mipLevel = 0; } // Allocate a buffer to store the source pixel data. std::vector textureBuffer(sourceTextureInfo.storageSize); - // Read the source texture. + // Read the source texture (async — completes after bgfx::frame). m_deviceContext.ReadTextureAsync(sourceTextureHandle, textureBuffer, mipLevel) .then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo]() mutable { // If the source texture format does not match the target texture format, convert it. @@ -1681,10 +1687,8 @@ namespace Babylon textureBuffer = convertedTextureBuffer; } - // Ensure the final texture buffer has the expected size. assert(textureBuffer.size() == targetTextureInfo.storageSize); - // Flip the image vertically if needed. if (bgfx::getCaps()->originBottomLeft) { FlipImage(textureBuffer, targetTextureInfo.height); @@ -1693,15 +1697,11 @@ namespace Babylon return textureBuffer; }) .then(m_runtimeScheduler, *m_cancellationSource, [this, bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred, tempTexture, sourceTextureHandle](std::vector textureBuffer) mutable { - // Double check the destination buffer length. This is redundant with prior checks, but we'll be extra sure before the memcpy. assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); - // Copy the pixel data into the JS ArrayBuffer. uint8_t* buffer{static_cast(bufferRef.Value().Data())}; std::memcpy(buffer + bufferOffset, textureBuffer.data(), textureBuffer.size()); - // Dispose of the texture handle before resolving the promise. - // TODO: Handle properly handle stale handles after BGFX shutdown if (*tempTexture && !m_cancellationSource->cancelled()) { bgfx::destroy(sourceTextureHandle); @@ -1710,9 +1710,7 @@ namespace Babylon deferred.Resolve(bufferRef.Value()); }) - .then(m_runtimeScheduler, arcana::cancellation::none(), [this, env, deferred, tempTexture, sourceTextureHandle](const arcana::expected& result) { - // Dispose of the texture handle if not yet disposed. - // TODO: Handle properly handle stale handles after BGFX shutdown + .then(m_runtimeScheduler, arcana::cancellation::none(), [this, deferred, tempTexture, sourceTextureHandle](const arcana::expected& result) { if (*tempTexture && !m_cancellationSource->cancelled()) { bgfx::destroy(sourceTextureHandle); @@ -1720,7 +1718,7 @@ namespace Babylon if (result.has_error()) { - deferred.Reject(Napi::Error::New(env, result.error()).Value()); + deferred.Reject(Napi::Error::New(Env(), result.error()).Value()); } }); } @@ -1804,63 +1802,55 @@ namespace Babylon void NativeEngine::BindFrameBuffer(NativeDataStream::Reader& data) { - auto encoder = GetUpdateToken().GetEncoder(); - Graphics::FrameBuffer* frameBuffer = data.ReadPointer(); - m_boundFrameBuffer->Unbind(*encoder); + m_boundFrameBuffer->Unbind(); m_boundFrameBuffer = frameBuffer; - m_boundFrameBuffer->Bind(*encoder); - m_boundFrameBufferNeedsRebinding.Set(*encoder, false); + m_boundFrameBuffer->Bind(); + m_boundFrameBufferNeedsRebinding.Set(false); } void NativeEngine::UnbindFrameBuffer(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder = GetUpdateToken().GetEncoder(); - const Graphics::FrameBuffer* frameBuffer = data.ReadPointer(); assert(m_boundFrameBuffer == frameBuffer); UNUSED(frameBuffer); - m_boundFrameBuffer->Unbind(*encoder); + m_boundFrameBuffer->Unbind(); m_boundFrameBuffer = nullptr; - m_boundFrameBufferNeedsRebinding.Set(*encoder, false); + m_boundFrameBufferNeedsRebinding.Set(false); } // Note: For legacy reasons JS might call this function for instance drawing. // In that case the instanceCount will be calculated inside the SetVertexBuffers method. void NativeEngine::DrawIndexed(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder{GetUpdateToken().GetEncoder()}; - const uint32_t fillMode = data.ReadUint32(); const uint32_t indexStart = data.ReadUint32(); const uint32_t indexCount = data.ReadUint32(); + bgfx::Encoder* encoder = GetEncoder(); if (m_boundVertexArray != nullptr) { m_boundVertexArray->SetIndexBuffer(encoder, indexStart, indexCount); m_boundVertexArray->SetVertexBuffers(encoder, 0, std::numeric_limits::max()); } - DrawInternal(encoder, fillMode); } void NativeEngine::DrawIndexedInstanced(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder{GetUpdateToken().GetEncoder()}; - const uint32_t fillMode = data.ReadUint32(); const uint32_t indexStart = data.ReadUint32(); const uint32_t indexCount = data.ReadUint32(); const uint32_t instanceCount = data.ReadUint32(); + bgfx::Encoder* encoder = GetEncoder(); if (m_boundVertexArray != nullptr) { m_boundVertexArray->SetIndexBuffer(encoder, indexStart, indexCount); m_boundVertexArray->SetVertexBuffers(encoder, 0, std::numeric_limits::max(), instanceCount); } - DrawInternal(encoder, fillMode); } @@ -1868,41 +1858,35 @@ namespace Babylon // In that case the instanceCount will be calculated inside the SetVertexBuffers method. void NativeEngine::Draw(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder{GetUpdateToken().GetEncoder()}; - const uint32_t fillMode = data.ReadUint32(); const uint32_t verticesStart = data.ReadUint32(); const uint32_t verticesCount = data.ReadUint32(); + bgfx::Encoder* encoder = GetEncoder(); if (m_boundVertexArray != nullptr) { m_boundVertexArray->SetVertexBuffers(encoder, verticesStart, verticesCount); } - DrawInternal(encoder, fillMode); } void NativeEngine::DrawInstanced(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder{GetUpdateToken().GetEncoder()}; - const uint32_t fillMode = data.ReadUint32(); const uint32_t verticesStart = data.ReadUint32(); const uint32_t verticesCount = data.ReadUint32(); const uint32_t instanceCount = data.ReadUint32(); + bgfx::Encoder* encoder = GetEncoder(); if (m_boundVertexArray != nullptr) { m_boundVertexArray->SetVertexBuffers(encoder, verticesStart, verticesCount, instanceCount); } - DrawInternal(encoder, fillMode); } void NativeEngine::Clear(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder{GetUpdateToken().GetEncoder()}; - uint16_t flags{0}; uint32_t rgba{0x000000ff}; @@ -1916,6 +1900,8 @@ namespace Babylon const bool shouldClearStencil{static_cast(data.ReadUint32())}; const uint8_t stencil{static_cast(data.ReadUint32())}; + bgfx::Encoder* encoder = GetEncoder(); + if (shouldClearColor) { rgba = @@ -1937,7 +1923,7 @@ namespace Babylon flags |= BGFX_CLEAR_STENCIL; } - GetBoundFrameBuffer(*encoder).Clear(*encoder, flags, rgba, depth, stencil); + GetBoundFrameBuffer().Clear(*encoder, flags, rgba, depth, stencil); } Napi::Value NativeEngine::GetRenderWidth(const Napi::CallbackInfo& info) @@ -2109,27 +2095,23 @@ namespace Babylon void NativeEngine::SetViewPort(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder{GetUpdateToken().GetEncoder()}; - const float x{data.ReadFloat32()}; const float y{data.ReadFloat32()}; const float width{data.ReadFloat32()}; const float height{data.ReadFloat32()}; const float yOrigin = bgfx::getCaps()->originBottomLeft ? y : (1.f - y - height); - GetBoundFrameBuffer(*encoder).SetViewPort(*encoder, x, yOrigin, width, height); + GetBoundFrameBuffer().SetViewPort(x, yOrigin, width, height); } void NativeEngine::SetScissor(NativeDataStream::Reader& data) { - bgfx::Encoder* encoder{GetUpdateToken().GetEncoder()}; - const float x{data.ReadFloat32()}; const float y{data.ReadFloat32()}; const float width{data.ReadFloat32()}; const float height{data.ReadFloat32()}; - GetBoundFrameBuffer(*encoder).SetScissor(*encoder, x, y, width, height); + GetBoundFrameBuffer().SetScissor(x, y, width, height); } void NativeEngine::SetCommandDataStream(const Napi::CallbackInfo& info) @@ -2141,6 +2123,21 @@ namespace Babylon void NativeEngine::SubmitCommands(const Napi::CallbackInfo& info) { + // If called outside the frame cycle (e.g., scene.dispose() from a + // getFrameBufferData callback), acquire a FrameCompletionScope. This + // blocks until StartRenderingCurrentFrame provides the encoder, + // then keeps the frame open so the encoder stays valid. + // The scope is released via deferred dispatch on the next JS tick. + if (m_deviceContext.GetActiveEncoder() == nullptr) + { + // Release any stale scope from a previous frame before acquiring a new one. + m_outsideFrameScope.reset(); + m_outsideFrameScope.emplace(m_deviceContext.AcquireFrameCompletionScope()); + m_runtime.Dispatch([this](auto) { + m_outsideFrameScope.reset(); + }); + } + try { NativeDataStream::Reader reader = m_commandStream->GetReader(); @@ -2157,7 +2154,6 @@ namespace Babylon void NativeEngine::PopulateFrameStats(const Napi::CallbackInfo& info) { - const auto updateToken{m_update.GetUpdateToken()}; const auto stats{bgfx::getStats()}; const double toGpuNs = 1000000000.0 / double(stats->gpuTimerFreq); const double gpuTimeNs = (stats->gpuTimeEnd - stats->gpuTimeBegin) * toGpuNs; @@ -2165,6 +2161,18 @@ namespace Babylon jsStatsObject.Set("gpuTimeNs", gpuTimeNs); } + void NativeEngine::BeginFrame(const Napi::CallbackInfo&) + { + // Encoder is managed by StartRenderingCurrentFrame/FinishRenderingCurrentFrame. + // Nothing to do here. + } + + void NativeEngine::EndFrame(const Napi::CallbackInfo&) + { + // Encoder is managed by StartRenderingCurrentFrame/FinishRenderingCurrentFrame. + // Nothing to do here. + } + void NativeEngine::DrawInternal(bgfx::Encoder* encoder, uint32_t fillMode) { uint64_t fillModeState{0}; // indexed triangle list @@ -2215,7 +2223,7 @@ namespace Babylon encoder->setUniform({it.first}, value.Data.data(), value.ElementLength); } - auto& boundFrameBuffer = GetBoundFrameBuffer(*encoder); + auto& boundFrameBuffer = GetBoundFrameBuffer(); if (boundFrameBuffer.HasDepth()) { encoder->setState(m_engineState | fillModeState); @@ -2231,33 +2239,20 @@ namespace Babylon boundFrameBuffer.Submit(*encoder, m_currentProgram->Handle(), BGFX_DISCARD_ALL & ~BGFX_DISCARD_BINDINGS); } - Graphics::UpdateToken& NativeEngine::GetUpdateToken() - { - if (!m_updateToken) - { - m_updateToken.emplace(m_update.GetUpdateToken()); - m_runtime.Dispatch([this](auto) { - m_updateToken.reset(); - }); - } - - return m_updateToken.value(); - } - - Graphics::FrameBuffer& NativeEngine::GetBoundFrameBuffer(bgfx::Encoder& encoder) + Graphics::FrameBuffer& NativeEngine::GetBoundFrameBuffer() { if (m_boundFrameBuffer == nullptr) { m_boundFrameBuffer = &m_defaultFrameBuffer; - m_defaultFrameBuffer.Bind(encoder); + m_defaultFrameBuffer.Bind(); } - else if (m_boundFrameBufferNeedsRebinding.Get(encoder)) + else if (m_boundFrameBufferNeedsRebinding.Get()) { - m_boundFrameBuffer->Unbind(encoder); - m_boundFrameBuffer->Bind(encoder); + m_boundFrameBuffer->Unbind(); + m_boundFrameBuffer->Bind(); } - m_boundFrameBufferNeedsRebinding.Set(encoder, false); + m_boundFrameBufferNeedsRebinding.Set(false); return *m_boundFrameBuffer; } @@ -2270,8 +2265,23 @@ namespace Babylon m_requestAnimationFrameCallbacksScheduled = true; - arcana::make_task(m_update.Scheduler(), *m_cancellationSource, [this, cancellationSource{m_cancellationSource}]() { - return arcana::make_task(m_runtimeScheduler, *m_cancellationSource, [this, updateToken{m_update.GetUpdateToken()}, cancellationSource{m_cancellationSource}]() { + // Schedule a two-phase task: + // Phase 1 (FrameStartScheduler, runs on main thread during StartRenderingCurrentFrame): + // Acquires a FrameCompletionScope to keep the frame open, then dispatches to JS. + // Phase 2 (runtimeScheduler, runs on JS thread): + // Executes RAF callbacks (which call scene.render → beginFrame/endFrame). + // Defers scope release to the NEXT dispatch cycle so that any bgfx API calls + // triggered after callbacks return (e.g., GC-driven resource destruction) are + // still protected from concurrent bgfx::frame(). + arcana::make_task(m_deviceContext.FrameStartScheduler(), *m_cancellationSource, [this, cancellationSource{m_cancellationSource}]() { + return arcana::make_task( + m_runtimeScheduler + , *m_cancellationSource + , [this + , frameScope{std::make_shared(m_deviceContext.AcquireFrameCompletionScope())} + , cancellationSource{m_cancellationSource} + ]() + { m_requestAnimationFrameCallbacksScheduled = false; arcana::trace_region scheduleRegion{"NativeEngine::ScheduleRequestAnimationFrameCallbacks invoke JS callbacks"}; @@ -2280,6 +2290,12 @@ namespace Babylon { callback.Value().Call({}); } + + // Defer scope release to next dispatch cycle. The shared_ptr captured by + // the Dispatch lambda is the last reference — when that lambda runs and + // returns, the scope is destroyed, decrementing the counter and allowing + // FinishRenderingCurrentFrame to proceed. + m_runtime.Dispatch([prevent_frame = frameScope](auto) {}); }).then(arcana::inline_scheduler, *m_cancellationSource, [this, cancellationSource{m_cancellationSource}](const arcana::expected& result) { if (!cancellationSource->cancelled() && result.has_error()) { @@ -2288,4 +2304,11 @@ namespace Babylon }); }); } + + bgfx::Encoder* NativeEngine::GetEncoder() + { + bgfx::Encoder* encoder = m_deviceContext.GetActiveEncoder(); + assert(encoder != nullptr); + return encoder; + } } diff --git a/Plugins/NativeEngine/Source/NativeEngine.h b/Plugins/NativeEngine/Source/NativeEngine.h index 809ba3406..2d22ffc1d 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.h +++ b/Plugins/NativeEngine/Source/NativeEngine.h @@ -24,6 +24,7 @@ #include #include +#include namespace Babylon { @@ -126,10 +127,12 @@ namespace Babylon void SetCommandDataStream(const Napi::CallbackInfo& info); void SubmitCommands(const Napi::CallbackInfo& info); void PopulateFrameStats(const Napi::CallbackInfo& info); + void BeginFrame(const Napi::CallbackInfo&); + void EndFrame(const Napi::CallbackInfo&); void DrawInternal(bgfx::Encoder* encoder, uint32_t fillMode); - Graphics::UpdateToken& GetUpdateToken(); - Graphics::FrameBuffer& GetBoundFrameBuffer(bgfx::Encoder& encoder); + bgfx::Encoder* GetEncoder(); + Graphics::FrameBuffer& GetBoundFrameBuffer(); std::shared_ptr m_cancellationSource{}; @@ -139,11 +142,13 @@ namespace Babylon JsRuntime& m_runtime; Graphics::DeviceContext& m_deviceContext; - Graphics::Update m_update; JsRuntimeScheduler m_runtimeScheduler; - std::optional m_updateToken{}; + // When bgfx API calls happen outside the frame cycle (e.g., scene.dispose() + // from a callback), a FrameCompletionScope keeps the frame encoder alive. + // Released via deferred dispatch on the next JS tick. + std::optional m_outsideFrameScope; void ScheduleRequestAnimationFrameCallbacks(); bool m_requestAnimationFrameCallbacksScheduled{}; diff --git a/Plugins/NativeEngine/Source/PerFrameValue.h b/Plugins/NativeEngine/Source/PerFrameValue.h index f594fb955..0932381a9 100644 --- a/Plugins/NativeEngine/Source/PerFrameValue.h +++ b/Plugins/NativeEngine/Source/PerFrameValue.h @@ -21,12 +21,12 @@ namespace Babylon { } - T Get(bgfx::Encoder&) const + T Get() const { return m_value; } - void Set(bgfx::Encoder&, bool value) + void Set(bool value) { m_value = value; if (!m_isResetScheduled) diff --git a/Plugins/NativeXr/Source/NativeXrImpl.cpp b/Plugins/NativeXr/Source/NativeXrImpl.cpp index 72a1185e0..0855b87ca 100644 --- a/Plugins/NativeXr/Source/NativeXrImpl.cpp +++ b/Plugins/NativeXr/Source/NativeXrImpl.cpp @@ -163,10 +163,10 @@ namespace Babylon // reason requestAnimationFrame is being called twice when starting XR. m_sessionState->ScheduleFrameCallbacks.emplace_back(callback); - m_sessionState->FrameTask = arcana::make_task(m_sessionState->Update.Scheduler(), m_sessionState->CancellationSource, [this, thisRef{shared_from_this()}] { + m_sessionState->FrameTask = arcana::make_task(m_sessionState->GraphicsContext.FrameStartScheduler(), m_sessionState->CancellationSource, [this, thisRef{shared_from_this()}] { BeginFrame(); - return arcana::make_task(m_runtimeScheduler, m_sessionState->CancellationSource, [this, updateToken{m_sessionState->Update.GetUpdateToken()}, thisRef{shared_from_this()}]() { + return arcana::make_task(m_runtimeScheduler, m_sessionState->CancellationSource, [this, frameScope{std::make_shared(m_sessionState->GraphicsContext.AcquireFrameCompletionScope())}, thisRef{shared_from_this()}]() { m_sessionState->FrameScheduled = false; BeginUpdate(); @@ -307,7 +307,7 @@ namespace Babylon // WebXR, at least in its current implementation, specifies an implicit default clear to black. // https://immersive-web.github.io/webxr/#xrwebgllayer-interface - frameBuffer.Clear(*m_sessionState->Update.GetUpdateToken().GetEncoder(), BGFX_CLEAR_COLOR | BGFX_CLEAR_DEPTH | BGFX_CLEAR_STENCIL, 0, 1.0f, 0); + frameBuffer.Clear(*m_sessionState->GraphicsContext.GetActiveEncoder(), BGFX_CLEAR_COLOR | BGFX_CLEAR_DEPTH | BGFX_CLEAR_STENCIL, 0, 1.0f, 0); viewConfig.FrameBuffers[eyeIdx] = frameBufferPtr; diff --git a/Plugins/NativeXr/Source/NativeXrImpl.h b/Plugins/NativeXr/Source/NativeXrImpl.h index 4c431d16e..2ce058598 100644 --- a/Plugins/NativeXr/Source/NativeXrImpl.h +++ b/Plugins/NativeXr/Source/NativeXrImpl.h @@ -111,12 +111,10 @@ namespace Babylon { explicit SessionState(Graphics::DeviceContext& graphicsContext) : GraphicsContext{graphicsContext} - , Update{GraphicsContext.GetUpdate("update")} { } Graphics::DeviceContext& GraphicsContext; - Graphics::Update Update; Napi::FunctionReference CreateRenderTexture{}; Napi::FunctionReference DestroyRenderTexture{}; std::vector ActiveViewConfigurations{}; diff --git a/Polyfills/Canvas/Source/Context.cpp b/Polyfills/Canvas/Source/Context.cpp index 8cdde6a22..9c82570b2 100644 --- a/Polyfills/Canvas/Source/Context.cpp +++ b/Polyfills/Canvas/Source/Context.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #ifdef __GNUC__ @@ -105,7 +106,6 @@ namespace Babylon::Polyfills::Internal , m_canvas{NativeCanvas::Unwrap(info[0].As())} , m_nvg{std::make_shared(nvgCreate(1))} , m_graphicsContext{m_canvas->GetGraphicsContext()} - , m_update{m_graphicsContext.GetUpdate("update")} , m_cancellationSource{std::make_shared()} , m_runtimeScheduler{Babylon::JsRuntime::GetFromJavaScript(info.Env())} , Polyfills::Canvas::Impl::MonitoredResource{Polyfills::Canvas::Impl::GetFromJavaScript(info.Env())} @@ -608,18 +608,26 @@ namespace Babylon::Polyfills::Internal void Context::Flush(const Napi::CallbackInfo&) { + // If called outside the frame cycle (e.g., during initialization/font loading), + // acquire a FrameCompletionScope which blocks until StartRenderingCurrentFrame + // provides the encoder, and keeps the frame open while we use it. + std::optional scope; + if (m_graphicsContext.GetActiveEncoder() == nullptr) + { + scope.emplace(m_graphicsContext.AcquireFrameCompletionScope()); + } + bool needClear = m_canvas->UpdateRenderTarget(); Graphics::FrameBuffer& frameBuffer = m_canvas->GetFrameBuffer(); - auto updateToken{m_update.GetUpdateToken()}; - bgfx::Encoder* encoder = updateToken.GetEncoder(); - frameBuffer.Bind(*encoder); + bgfx::Encoder* encoder = m_graphicsContext.GetActiveEncoder(); + frameBuffer.Bind(); if (needClear) { frameBuffer.Clear(*encoder, BGFX_CLEAR_COLOR | BGFX_CLEAR_DEPTH | BGFX_CLEAR_STENCIL, 0, 1.f, 0); } - frameBuffer.SetViewPort(*encoder, 0.f, 0.f, 1.f, 1.f); + frameBuffer.SetViewPort(0.f, 0.f, 1.f, 1.f); const auto width = m_canvas->GetWidth(); const auto height = m_canvas->GetHeight(); @@ -630,21 +638,21 @@ namespace Babylon::Polyfills::Internal } std::function acquire = [this, encoder]() -> Babylon::Graphics::FrameBuffer* { Babylon::Graphics::FrameBuffer *frameBuffer = this->m_canvas->m_frameBufferPool.Acquire(); - frameBuffer->Bind(*encoder); + frameBuffer->Bind(); return frameBuffer; }; std::function release = [this, encoder](Babylon::Graphics::FrameBuffer* frameBuffer) -> void { // clear framebuffer when released frameBuffer->Clear(*encoder, BGFX_CLEAR_COLOR | BGFX_CLEAR_DEPTH | BGFX_CLEAR_STENCIL, 0, 1.f, 0); this->m_canvas->m_frameBufferPool.Release(frameBuffer); - frameBuffer->Unbind(*encoder); + frameBuffer->Unbind(); }; nvgBeginFrame(*m_nvg, float(width), float(height), 1.0f); nvgSetFrameBufferAndEncoder(*m_nvg, frameBuffer, encoder); nvgSetFrameBufferPool(*m_nvg, { acquire, release }); nvgEndFrame(*m_nvg); - frameBuffer.Unbind(*encoder); + frameBuffer.Unbind(); for (auto& buffer : m_canvas->m_frameBufferPool.GetPoolBuffers()) { diff --git a/Polyfills/Canvas/Source/Context.h b/Polyfills/Canvas/Source/Context.h index e4472964e..9a9074818 100644 --- a/Polyfills/Canvas/Source/Context.h +++ b/Polyfills/Canvas/Source/Context.h @@ -110,7 +110,6 @@ namespace Babylon::Polyfills::Internal int m_currentFontId{-1}; Graphics::DeviceContext& m_graphicsContext; - Graphics::Update m_update; bool m_isClipped{false}; diff --git a/Polyfills/Canvas/Source/nanovg/nanovg_babylon.cpp b/Polyfills/Canvas/Source/nanovg/nanovg_babylon.cpp index 9f668ba08..46f30481b 100644 --- a/Polyfills/Canvas/Source/nanovg/nanovg_babylon.cpp +++ b/Polyfills/Canvas/Source/nanovg/nanovg_babylon.cpp @@ -801,7 +801,7 @@ namespace outBuffer->Submit(*gl->encoder, prog, BGFX_DISCARD_ALL); }; Babylon::Graphics::FrameBuffer *finalFrameBuffer = gl->frameBuffer; - finalFrameBuffer->Bind(*gl->encoder); // Should this be bound elsewhere? + finalFrameBuffer->Bind(); // Should this be bound elsewhere? call->filterStack.Render(firstProg, setUniform, firstPass, filterPass, finalPass, finalFrameBuffer, gl->frameBufferPool.acquire, gl->frameBufferPool.release); } @@ -863,7 +863,7 @@ namespace outBuffer->Submit(*gl->encoder, prog, BGFX_DISCARD_ALL); }; Babylon::Graphics::FrameBuffer *finalFrameBuffer = gl->frameBuffer; - finalFrameBuffer->Bind(*gl->encoder); // Should this be bound elsewhere? + finalFrameBuffer->Bind(); // Should this be bound elsewhere? call->filterStack.Render(firstProg, setUniform, firstPass, filterPass, finalPass, finalFrameBuffer, gl->frameBufferPool.acquire, gl->frameBufferPool.release); } @@ -908,7 +908,7 @@ namespace outBuffer->Submit(*gl->encoder, prog, BGFX_DISCARD_ALL); }; Babylon::Graphics::FrameBuffer *finalFrameBuffer = gl->frameBuffer; - finalFrameBuffer->Bind(*gl->encoder); // Should this be bound elsewhere? + finalFrameBuffer->Bind(); // Should this be bound elsewhere? call->filterStack.Render(firstProg, setUniform, firstPass, filterPass, finalPass, finalFrameBuffer, gl->frameBufferPool.acquire, gl->frameBufferPool.release); } @@ -948,7 +948,7 @@ namespace outBuffer->Submit(*gl->encoder, prog, BGFX_DISCARD_ALL); }; Babylon::Graphics::FrameBuffer *finalFrameBuffer = gl->frameBuffer; - finalFrameBuffer->Bind(*gl->encoder); // Should this be bound elsewhere? + finalFrameBuffer->Bind(); // Should this be bound elsewhere? call->filterStack.Render(firstProg, setUniform, firstPass, filterPass, finalPass, finalFrameBuffer, gl->frameBufferPool.acquire, gl->frameBufferPool.release); } From 9daf7907a464f83cdb7ffe3cb7add74b16ff711a Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Mon, 6 Apr 2026 16:51:21 -0700 Subject: [PATCH 02/24] Added first frame started check. --- Core/Graphics/Source/DeviceImpl.cpp | 10 ++++++++-- Core/Graphics/Source/DeviceImpl.h | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Core/Graphics/Source/DeviceImpl.cpp b/Core/Graphics/Source/DeviceImpl.cpp index 15ba9cedf..a6d3635a7 100644 --- a/Core/Graphics/Source/DeviceImpl.cpp +++ b/Core/Graphics/Source/DeviceImpl.cpp @@ -275,6 +275,7 @@ namespace Babylon::Graphics } m_rendering = true; + m_firstFrameStarted = true; // Ensure rendering is enabled. EnableRendering(); @@ -305,8 +306,13 @@ namespace Babylon::Graphics if (!m_rendering) { - // First call at startup — no frame in progress yet, nothing to finish. - return; + if (!m_firstFrameStarted) + { + // First call at startup - no frame in progress yet, nothing to finish. + return; + } + + throw std::runtime_error{"Current frame cannot be finished prior to having been started."}; } // Close the gate: wait until JS thread has released all FrameCompletionScopes diff --git a/Core/Graphics/Source/DeviceImpl.h b/Core/Graphics/Source/DeviceImpl.h index 66e9b9242..963811643 100644 --- a/Core/Graphics/Source/DeviceImpl.h +++ b/Core/Graphics/Source/DeviceImpl.h @@ -125,6 +125,7 @@ namespace Babylon::Graphics arcana::affinity m_renderThreadAffinity{}; bool m_rendering{}; + bool m_firstFrameStarted{}; // The single bgfx encoder for the current frame. Acquired in // StartRenderingCurrentFrame, ended in FinishRenderingCurrentFrame. From 9fa86ff605f444e130ec5a5b80a2d0c431c0c038 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 08:39:17 -0700 Subject: [PATCH 03/24] Fixed ordering. --- Plugins/NativeEngine/Source/NativeEngine.cpp | 138 +++++++++++-------- 1 file changed, 81 insertions(+), 57 deletions(-) diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 492edde88..3b07ef5fc 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -1651,76 +1651,100 @@ namespace Babylon bgfx::TextureHandle sourceTextureHandle{texture->Handle()}; auto tempTexture = std::make_shared(false); + // Calculate storage size for the source pixel data. + std::vector textureBuffer(sourceTextureInfo.storageSize); + // If the image needs to be cropped or the texture lacks the READ_BACK flag, blit to a temp texture. - // The blit is scheduled via BeforeRenderScheduler and runs on the main thread - // right before the encoder is ended — guaranteeing it executes before bgfx::frame(). + // Both the blit and readTexture must be submitted in the correct order within + // the same frame, so when a blit is needed they are both scheduled together + // via BeforeRenderScheduler (runs on main thread before encoder is ended). if (x != 0 || y != 0 || width != (texture->Width() >> mipLevel) || height != (texture->Height() >> mipLevel) || (texture->Flags() & BGFX_TEXTURE_READ_BACK) == 0) { const bgfx::TextureHandle blitTextureHandle{bgfx::createTexture2D(width, height, /*hasMips*/ false, /*numLayers*/ 1, sourceTextureFormat, BGFX_TEXTURE_BLIT_DST | BGFX_TEXTURE_READ_BACK)}; arcana::make_task(m_deviceContext.BeforeRenderScheduler(), *m_cancellationSource, - [this, src = sourceTextureHandle, dst = blitTextureHandle, mipLevel, x, y, width, height]() { + [this, src = sourceTextureHandle, dst = blitTextureHandle, mipLevel, x, y, width, height, textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo, bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred, tempTexture]() mutable { bgfx::Encoder* encoder = m_deviceContext.GetActiveEncoder(); assert(encoder != nullptr); encoder->blit(static_cast(bgfx::getCaps()->limits.maxViews - 1), dst, /*dstMip*/ 0, /*dstX*/ 0, /*dstY*/ 0, /*dstZ*/ 0, src, mipLevel, x, y, /*srcZ*/ 0, width, height, /*depth*/ 0); - }); - sourceTextureHandle = blitTextureHandle; - *tempTexture = true; - mipLevel = 0; + // Submit readTexture immediately after blit so both land in the same frame. + m_deviceContext.ReadTextureAsync(dst, textureBuffer, 0) + .then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo]() mutable { + if (targetTextureInfo.format != sourceTextureInfo.format) + { + std::vector convertedTextureBuffer(targetTextureInfo.storageSize); + if (!bimg::imageConvert(&Graphics::DeviceContext::GetDefaultAllocator(), convertedTextureBuffer.data(), bimg::TextureFormat::Enum(targetTextureInfo.format), textureBuffer.data(), bimg::TextureFormat::Enum(sourceTextureInfo.format), sourceTextureInfo.width, sourceTextureInfo.height, /*depth*/ 1)) + { + throw std::runtime_error{"Texture conversion to RBGA8 failed."}; + } + textureBuffer = convertedTextureBuffer; + } + assert(textureBuffer.size() == targetTextureInfo.storageSize); + if (bgfx::getCaps()->originBottomLeft) + { + FlipImage(textureBuffer, targetTextureInfo.height); + } + return textureBuffer; + }) + .then(m_runtimeScheduler, *m_cancellationSource, [this, bufferRef{std::move(bufferRef)}, bufferOffset, deferred, tempTexture, dst](std::vector textureBuffer) mutable { + assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); + uint8_t* buffer{static_cast(bufferRef.Value().Data())}; + std::memcpy(buffer + bufferOffset, textureBuffer.data(), textureBuffer.size()); + if (*tempTexture && !m_cancellationSource->cancelled()) + { + bgfx::destroy(dst); + *tempTexture = false; + } + deferred.Resolve(bufferRef.Value()); + }) + .then(m_runtimeScheduler, arcana::cancellation::none(), [this, deferred, tempTexture, dst](const arcana::expected& result) { + if (*tempTexture && !m_cancellationSource->cancelled()) + { + bgfx::destroy(dst); + } + if (result.has_error()) + { + deferred.Reject(Napi::Error::New(Env(), result.error()).Value()); + } + }); + }); } - - // Allocate a buffer to store the source pixel data. - std::vector textureBuffer(sourceTextureInfo.storageSize); - - // Read the source texture (async — completes after bgfx::frame). - m_deviceContext.ReadTextureAsync(sourceTextureHandle, textureBuffer, mipLevel) - .then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo]() mutable { - // If the source texture format does not match the target texture format, convert it. - if (targetTextureInfo.format != sourceTextureInfo.format) - { - std::vector convertedTextureBuffer(targetTextureInfo.storageSize); - if (!bimg::imageConvert(&Graphics::DeviceContext::GetDefaultAllocator(), convertedTextureBuffer.data(), bimg::TextureFormat::Enum(targetTextureInfo.format), textureBuffer.data(), bimg::TextureFormat::Enum(sourceTextureInfo.format), sourceTextureInfo.width, sourceTextureInfo.height, /*depth*/ 1)) + else + { + // No blit needed — texture already has READ_BACK flag and correct region. + // ReadTextureAsync can be called directly from the JS thread. + m_deviceContext.ReadTextureAsync(sourceTextureHandle, textureBuffer, mipLevel) + .then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo]() mutable { + if (targetTextureInfo.format != sourceTextureInfo.format) { - throw std::runtime_error{"Texture conversion to RBGA8 failed."}; + std::vector convertedTextureBuffer(targetTextureInfo.storageSize); + if (!bimg::imageConvert(&Graphics::DeviceContext::GetDefaultAllocator(), convertedTextureBuffer.data(), bimg::TextureFormat::Enum(targetTextureInfo.format), textureBuffer.data(), bimg::TextureFormat::Enum(sourceTextureInfo.format), sourceTextureInfo.width, sourceTextureInfo.height, /*depth*/ 1)) + { + throw std::runtime_error{"Texture conversion to RBGA8 failed."}; + } + textureBuffer = convertedTextureBuffer; } - textureBuffer = convertedTextureBuffer; - } - - assert(textureBuffer.size() == targetTextureInfo.storageSize); - - if (bgfx::getCaps()->originBottomLeft) - { - FlipImage(textureBuffer, targetTextureInfo.height); - } - - return textureBuffer; - }) - .then(m_runtimeScheduler, *m_cancellationSource, [this, bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred, tempTexture, sourceTextureHandle](std::vector textureBuffer) mutable { - assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); - - uint8_t* buffer{static_cast(bufferRef.Value().Data())}; - std::memcpy(buffer + bufferOffset, textureBuffer.data(), textureBuffer.size()); - - if (*tempTexture && !m_cancellationSource->cancelled()) - { - bgfx::destroy(sourceTextureHandle); - *tempTexture = false; - } - - deferred.Resolve(bufferRef.Value()); - }) - .then(m_runtimeScheduler, arcana::cancellation::none(), [this, deferred, tempTexture, sourceTextureHandle](const arcana::expected& result) { - if (*tempTexture && !m_cancellationSource->cancelled()) - { - bgfx::destroy(sourceTextureHandle); - } - - if (result.has_error()) - { - deferred.Reject(Napi::Error::New(Env(), result.error()).Value()); - } - }); + assert(textureBuffer.size() == targetTextureInfo.storageSize); + if (bgfx::getCaps()->originBottomLeft) + { + FlipImage(textureBuffer, targetTextureInfo.height); + } + return textureBuffer; + }) + .then(m_runtimeScheduler, *m_cancellationSource, [this, bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred, tempTexture, sourceTextureHandle](std::vector textureBuffer) mutable { + assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); + uint8_t* buffer{static_cast(bufferRef.Value().Data())}; + std::memcpy(buffer + bufferOffset, textureBuffer.data(), textureBuffer.size()); + deferred.Resolve(bufferRef.Value()); + }) + .then(m_runtimeScheduler, arcana::cancellation::none(), [this, deferred](const arcana::expected& result) { + if (result.has_error()) + { + deferred.Reject(Napi::Error::New(Env(), result.error()).Value()); + } + }); + } } return deferred.Promise(); From 55712c83a445aa97303bb91d312f981f94236f59 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 09:09:10 -0700 Subject: [PATCH 04/24] Capture just POD values. --- Plugins/NativeEngine/Source/NativeEngine.cpp | 107 ++++++++++--------- 1 file changed, 58 insertions(+), 49 deletions(-) diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 3b07ef5fc..3e3e226e5 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -1651,9 +1651,6 @@ namespace Babylon bgfx::TextureHandle sourceTextureHandle{texture->Handle()}; auto tempTexture = std::make_shared(false); - // Calculate storage size for the source pixel data. - std::vector textureBuffer(sourceTextureInfo.storageSize); - // If the image needs to be cropped or the texture lacks the READ_BACK flag, blit to a temp texture. // Both the blit and readTexture must be submitted in the correct order within // the same frame, so when a blit is needed they are both scheduled together @@ -1662,58 +1659,70 @@ namespace Babylon { const bgfx::TextureHandle blitTextureHandle{bgfx::createTexture2D(width, height, /*hasMips*/ false, /*numLayers*/ 1, sourceTextureFormat, BGFX_TEXTURE_BLIT_DST | BGFX_TEXTURE_READ_BACK)}; - arcana::make_task(m_deviceContext.BeforeRenderScheduler(), *m_cancellationSource, - [this, src = sourceTextureHandle, dst = blitTextureHandle, mipLevel, x, y, width, height, textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo, bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred, tempTexture]() mutable { + sourceTextureHandle = blitTextureHandle; + *tempTexture = true; + mipLevel = 0; + + // Schedule blit + readTexture on the main thread. Only bgfx calls + // go here — no N-API objects (they aren't thread-safe). + auto readTextureTask = arcana::make_task(m_deviceContext.BeforeRenderScheduler(), *m_cancellationSource, + [this, src = texture->Handle(), dst = blitTextureHandle, origMipLevel = static_cast(info[1].As().Uint32Value()), x, y, width, height, storageSize = sourceTextureInfo.storageSize]() { bgfx::Encoder* encoder = m_deviceContext.GetActiveEncoder(); assert(encoder != nullptr); - encoder->blit(static_cast(bgfx::getCaps()->limits.maxViews - 1), dst, /*dstMip*/ 0, /*dstX*/ 0, /*dstY*/ 0, /*dstZ*/ 0, src, mipLevel, x, y, /*srcZ*/ 0, width, height, /*depth*/ 0); - - // Submit readTexture immediately after blit so both land in the same frame. - m_deviceContext.ReadTextureAsync(dst, textureBuffer, 0) - .then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo]() mutable { - if (targetTextureInfo.format != sourceTextureInfo.format) - { - std::vector convertedTextureBuffer(targetTextureInfo.storageSize); - if (!bimg::imageConvert(&Graphics::DeviceContext::GetDefaultAllocator(), convertedTextureBuffer.data(), bimg::TextureFormat::Enum(targetTextureInfo.format), textureBuffer.data(), bimg::TextureFormat::Enum(sourceTextureInfo.format), sourceTextureInfo.width, sourceTextureInfo.height, /*depth*/ 1)) - { - throw std::runtime_error{"Texture conversion to RBGA8 failed."}; - } - textureBuffer = convertedTextureBuffer; - } - assert(textureBuffer.size() == targetTextureInfo.storageSize); - if (bgfx::getCaps()->originBottomLeft) - { - FlipImage(textureBuffer, targetTextureInfo.height); - } - return textureBuffer; - }) - .then(m_runtimeScheduler, *m_cancellationSource, [this, bufferRef{std::move(bufferRef)}, bufferOffset, deferred, tempTexture, dst](std::vector textureBuffer) mutable { - assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); - uint8_t* buffer{static_cast(bufferRef.Value().Data())}; - std::memcpy(buffer + bufferOffset, textureBuffer.data(), textureBuffer.size()); - if (*tempTexture && !m_cancellationSource->cancelled()) - { - bgfx::destroy(dst); - *tempTexture = false; - } - deferred.Resolve(bufferRef.Value()); - }) - .then(m_runtimeScheduler, arcana::cancellation::none(), [this, deferred, tempTexture, dst](const arcana::expected& result) { - if (*tempTexture && !m_cancellationSource->cancelled()) - { - bgfx::destroy(dst); - } - if (result.has_error()) - { - deferred.Reject(Napi::Error::New(Env(), result.error()).Value()); - } + encoder->blit(static_cast(bgfx::getCaps()->limits.maxViews - 1), dst, /*dstMip*/ 0, /*dstX*/ 0, /*dstY*/ 0, /*dstZ*/ 0, src, origMipLevel, x, y, /*srcZ*/ 0, width, height, /*depth*/ 0); + + std::vector textureBuffer(storageSize); + return m_deviceContext.ReadTextureAsync(dst, textureBuffer, 0) + .then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}]() mutable { + return std::move(textureBuffer); }); }); + + // Continue on JS thread with N-API objects. + std::move(readTextureTask) + .then(arcana::inline_scheduler, *m_cancellationSource, [sourceTextureInfo, targetTextureInfo](std::vector textureBuffer) mutable { + if (targetTextureInfo.format != sourceTextureInfo.format) + { + std::vector convertedTextureBuffer(targetTextureInfo.storageSize); + if (!bimg::imageConvert(&Graphics::DeviceContext::GetDefaultAllocator(), convertedTextureBuffer.data(), bimg::TextureFormat::Enum(targetTextureInfo.format), textureBuffer.data(), bimg::TextureFormat::Enum(sourceTextureInfo.format), sourceTextureInfo.width, sourceTextureInfo.height, /*depth*/ 1)) + { + throw std::runtime_error{"Texture conversion to RBGA8 failed."}; + } + textureBuffer = convertedTextureBuffer; + } + assert(textureBuffer.size() == targetTextureInfo.storageSize); + if (bgfx::getCaps()->originBottomLeft) + { + FlipImage(textureBuffer, targetTextureInfo.height); + } + return textureBuffer; + }) + .then(m_runtimeScheduler, *m_cancellationSource, [this, bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred, tempTexture, blitTextureHandle](std::vector textureBuffer) mutable { + assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); + uint8_t* buf{static_cast(bufferRef.Value().Data())}; + std::memcpy(buf + bufferOffset, textureBuffer.data(), textureBuffer.size()); + if (*tempTexture && !m_cancellationSource->cancelled()) + { + bgfx::destroy(blitTextureHandle); + *tempTexture = false; + } + deferred.Resolve(bufferRef.Value()); + }) + .then(m_runtimeScheduler, arcana::cancellation::none(), [this, deferred, tempTexture, blitTextureHandle](const arcana::expected& result) { + if (*tempTexture && !m_cancellationSource->cancelled()) + { + bgfx::destroy(blitTextureHandle); + } + if (result.has_error()) + { + deferred.Reject(Napi::Error::New(Env(), result.error()).Value()); + } + }); } else { // No blit needed — texture already has READ_BACK flag and correct region. - // ReadTextureAsync can be called directly from the JS thread. + std::vector textureBuffer(sourceTextureInfo.storageSize); m_deviceContext.ReadTextureAsync(sourceTextureHandle, textureBuffer, mipLevel) .then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo]() mutable { if (targetTextureInfo.format != sourceTextureInfo.format) @@ -1734,8 +1743,8 @@ namespace Babylon }) .then(m_runtimeScheduler, *m_cancellationSource, [this, bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred, tempTexture, sourceTextureHandle](std::vector textureBuffer) mutable { assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); - uint8_t* buffer{static_cast(bufferRef.Value().Data())}; - std::memcpy(buffer + bufferOffset, textureBuffer.data(), textureBuffer.size()); + uint8_t* buf{static_cast(bufferRef.Value().Data())}; + std::memcpy(buf + bufferOffset, textureBuffer.data(), textureBuffer.size()); deferred.Resolve(bufferRef.Value()); }) .then(m_runtimeScheduler, arcana::cancellation::none(), [this, deferred](const arcana::expected& result) { From b80caa4c66644df8f0e302aa9bc5ced443f3ac0e Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 10:11:45 -0700 Subject: [PATCH 05/24] Removed unused captures. --- Plugins/NativeEngine/Source/NativeEngine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 3e3e226e5..7c3539a7a 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -1741,7 +1741,7 @@ namespace Babylon } return textureBuffer; }) - .then(m_runtimeScheduler, *m_cancellationSource, [this, bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred, tempTexture, sourceTextureHandle](std::vector textureBuffer) mutable { + .then(m_runtimeScheduler, *m_cancellationSource, [bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred](std::vector textureBuffer) mutable { assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); uint8_t* buf{static_cast(bufferRef.Value().Data())}; std::memcpy(buf + bufferOffset, textureBuffer.data(), textureBuffer.size()); From a2b1679cec8e905ae5ffc23ebd63f76e259714f3 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 11:27:11 -0700 Subject: [PATCH 06/24] Fix null encoder crash in Canvas::Flush by adding defensive check after FrameCompletionScope acquisition. --- Polyfills/Canvas/Source/Context.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Polyfills/Canvas/Source/Context.cpp b/Polyfills/Canvas/Source/Context.cpp index 9c82570b2..618a4a2e4 100644 --- a/Polyfills/Canvas/Source/Context.cpp +++ b/Polyfills/Canvas/Source/Context.cpp @@ -608,20 +608,25 @@ namespace Babylon::Polyfills::Internal void Context::Flush(const Napi::CallbackInfo&) { - // If called outside the frame cycle (e.g., during initialization/font loading), - // acquire a FrameCompletionScope which blocks until StartRenderingCurrentFrame - // provides the encoder, and keeps the frame open while we use it. + // If called outside the frame cycle (e.g., during initialization/font loading + // or async texture load callbacks), acquire a FrameCompletionScope which blocks + // until StartRenderingCurrentFrame provides the encoder. std::optional scope; if (m_graphicsContext.GetActiveEncoder() == nullptr) { scope.emplace(m_graphicsContext.AcquireFrameCompletionScope()); } + bgfx::Encoder* encoder = m_graphicsContext.GetActiveEncoder(); + if (encoder == nullptr) + { + return; + } + bool needClear = m_canvas->UpdateRenderTarget(); Graphics::FrameBuffer& frameBuffer = m_canvas->GetFrameBuffer(); - bgfx::Encoder* encoder = m_graphicsContext.GetActiveEncoder(); frameBuffer.Bind(); if (needClear) { From 05e682bc4a81f2d41800b185916ad069afc85cde Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Thu, 12 Mar 2026 08:52:33 -0700 Subject: [PATCH 07/24] Fix LoadCubeTexture*. --- CMakeLists.txt | 2 +- Core/Graphics/CMakeLists.txt | 8 ++++++++ Dependencies/CMakeLists.txt | 1 + 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6b102c3f5..d5dbb49d2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -207,7 +207,7 @@ elseif(WIN32) set(GRAPHICS_API OpenGL) set(BABYLON_NATIVE_OPENGLES_FROM_BROWSER ON) else() - if(NOT GRAPHICS_API STREQUAL Vulkan AND NOT GRAPHICS_API STREQUAL D3D11 AND NOT GRAPHICS_API STREQUAL D3D12) + if(NOT GRAPHICS_API STREQUAL Vulkan AND NOT GRAPHICS_API STREQUAL D3D11 AND NOT GRAPHICS_API STREQUAL D3D12 AND NOT GRAPHICS_API STREQUAL OpenGL) message(FATAL_ERROR "Unrecognized/Unsupported render API: ${GRAPHICS_API}") endif() endif() diff --git a/Core/Graphics/CMakeLists.txt b/Core/Graphics/CMakeLists.txt index 552b9cd0c..29aca2e68 100644 --- a/Core/Graphics/CMakeLists.txt +++ b/Core/Graphics/CMakeLists.txt @@ -26,6 +26,10 @@ target_include_directories(Graphics PRIVATE "Include/RendererType/${GRAPHICS_API}" PRIVATE "InternalInclude") +if(GRAPHICS_API STREQUAL "OpenGL") + target_include_directories(Graphics PRIVATE "${BGFX_KHRONOS_INCLUDE_DIR}") +endif() + target_compile_definitions(Graphics PRIVATE NOMINMAX) @@ -72,6 +76,10 @@ target_include_directories(GraphicsDevice INTERFACE "Include/Platform/${BABYLON_NATIVE_PLATFORM}" INTERFACE "Include/RendererType/${GRAPHICS_API}") +if(GRAPHICS_API STREQUAL "OpenGL") + target_include_directories(GraphicsDevice INTERFACE "${BGFX_KHRONOS_INCLUDE_DIR}") +endif() + target_link_libraries(GraphicsDevice INTERFACE Graphics INTERFACE JsRuntime) diff --git a/Dependencies/CMakeLists.txt b/Dependencies/CMakeLists.txt index a07c9943f..bfda0eecb 100644 --- a/Dependencies/CMakeLists.txt +++ b/Dependencies/CMakeLists.txt @@ -66,6 +66,7 @@ elseif(GRAPHICS_API STREQUAL "OpenGL") target_compile_definitions(bgfx PRIVATE BGFX_CONFIG_RENDERER_OPENGLES=30) target_compile_definitions(bgfx PRIVATE BGFX_GL_CONFIG_BLIT_EMULATION=1) target_compile_definitions(bgfx PRIVATE BGFX_GL_CONFIG_TEXTURE_READ_BACK_EMULATION=1) + set(BGFX_KHRONOS_INCLUDE_DIR "${bgfx.cmake_SOURCE_DIR}/bgfx/3rdparty/khronos" CACHE INTERNAL "") elseif(GRAPHICS_API STREQUAL "Vulkan") target_compile_definitions(bgfx PRIVATE BGFX_CONFIG_RENDERER_VULKAN=1) endif() From 212eb4a790090aab6b3d675557744ad17630f69b Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 14:50:19 -0700 Subject: [PATCH 08/24] Fix null encoder crash in SubmitCommands by using stack-scoped FrameCompletionScope instead of deferred member release. --- Plugins/NativeEngine/Source/NativeEngine.cpp | 9 ++------- Plugins/NativeEngine/Source/NativeEngine.h | 5 ----- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 7c3539a7a..e8779e746 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -2160,15 +2160,10 @@ namespace Babylon // getFrameBufferData callback), acquire a FrameCompletionScope. This // blocks until StartRenderingCurrentFrame provides the encoder, // then keeps the frame open so the encoder stays valid. - // The scope is released via deferred dispatch on the next JS tick. + std::optional scope; if (m_deviceContext.GetActiveEncoder() == nullptr) { - // Release any stale scope from a previous frame before acquiring a new one. - m_outsideFrameScope.reset(); - m_outsideFrameScope.emplace(m_deviceContext.AcquireFrameCompletionScope()); - m_runtime.Dispatch([this](auto) { - m_outsideFrameScope.reset(); - }); + scope.emplace(m_deviceContext.AcquireFrameCompletionScope()); } try diff --git a/Plugins/NativeEngine/Source/NativeEngine.h b/Plugins/NativeEngine/Source/NativeEngine.h index 2d22ffc1d..870ba9614 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.h +++ b/Plugins/NativeEngine/Source/NativeEngine.h @@ -145,11 +145,6 @@ namespace Babylon JsRuntimeScheduler m_runtimeScheduler; - // When bgfx API calls happen outside the frame cycle (e.g., scene.dispose() - // from a callback), a FrameCompletionScope keeps the frame encoder alive. - // Released via deferred dispatch on the next JS tick. - std::optional m_outsideFrameScope; - void ScheduleRequestAnimationFrameCallbacks(); bool m_requestAnimationFrameCallbacksScheduled{}; From ed656ebfac8c62633f5b31c97b59a16337e57419 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 14:56:35 -0700 Subject: [PATCH 09/24] Always acquire FrameCompletionScope in SubmitCommands to prevent encoder race. --- Plugins/NativeEngine/Source/NativeEngine.cpp | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index e8779e746..069efd314 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -2156,15 +2156,12 @@ namespace Babylon void NativeEngine::SubmitCommands(const Napi::CallbackInfo& info) { - // If called outside the frame cycle (e.g., scene.dispose() from a - // getFrameBufferData callback), acquire a FrameCompletionScope. This - // blocks until StartRenderingCurrentFrame provides the encoder, - // then keeps the frame open so the encoder stays valid. - std::optional scope; - if (m_deviceContext.GetActiveEncoder() == nullptr) - { - scope.emplace(m_deviceContext.AcquireFrameCompletionScope()); - } + // Acquire a FrameCompletionScope to ensure the encoder stays valid for + // the duration of command processing. When called within a RAF callback, + // the frame is already open and this returns immediately. When called + // outside (e.g., scene.dispose() from an XHR callback), this blocks + // until StartRenderingCurrentFrame provides the encoder. + Graphics::FrameCompletionScope scope{m_deviceContext.AcquireFrameCompletionScope()}; try { From 8cb30d5c1608b2e9e47909daecbadd1c086d87fb Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 15:45:22 -0700 Subject: [PATCH 10/24] Revert "Fix LoadCubeTexture*." This reverts commit 0d315bbf635264335a8a5670b45cc2f5ba2e71bf. --- CMakeLists.txt | 2 +- Core/Graphics/CMakeLists.txt | 8 -------- Dependencies/CMakeLists.txt | 1 - Plugins/NativeEngine/Source/NativeEngine.cpp | 4 ++-- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d5dbb49d2..6b102c3f5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -207,7 +207,7 @@ elseif(WIN32) set(GRAPHICS_API OpenGL) set(BABYLON_NATIVE_OPENGLES_FROM_BROWSER ON) else() - if(NOT GRAPHICS_API STREQUAL Vulkan AND NOT GRAPHICS_API STREQUAL D3D11 AND NOT GRAPHICS_API STREQUAL D3D12 AND NOT GRAPHICS_API STREQUAL OpenGL) + if(NOT GRAPHICS_API STREQUAL Vulkan AND NOT GRAPHICS_API STREQUAL D3D11 AND NOT GRAPHICS_API STREQUAL D3D12) message(FATAL_ERROR "Unrecognized/Unsupported render API: ${GRAPHICS_API}") endif() endif() diff --git a/Core/Graphics/CMakeLists.txt b/Core/Graphics/CMakeLists.txt index 29aca2e68..552b9cd0c 100644 --- a/Core/Graphics/CMakeLists.txt +++ b/Core/Graphics/CMakeLists.txt @@ -26,10 +26,6 @@ target_include_directories(Graphics PRIVATE "Include/RendererType/${GRAPHICS_API}" PRIVATE "InternalInclude") -if(GRAPHICS_API STREQUAL "OpenGL") - target_include_directories(Graphics PRIVATE "${BGFX_KHRONOS_INCLUDE_DIR}") -endif() - target_compile_definitions(Graphics PRIVATE NOMINMAX) @@ -76,10 +72,6 @@ target_include_directories(GraphicsDevice INTERFACE "Include/Platform/${BABYLON_NATIVE_PLATFORM}" INTERFACE "Include/RendererType/${GRAPHICS_API}") -if(GRAPHICS_API STREQUAL "OpenGL") - target_include_directories(GraphicsDevice INTERFACE "${BGFX_KHRONOS_INCLUDE_DIR}") -endif() - target_link_libraries(GraphicsDevice INTERFACE Graphics INTERFACE JsRuntime) diff --git a/Dependencies/CMakeLists.txt b/Dependencies/CMakeLists.txt index bfda0eecb..a07c9943f 100644 --- a/Dependencies/CMakeLists.txt +++ b/Dependencies/CMakeLists.txt @@ -66,7 +66,6 @@ elseif(GRAPHICS_API STREQUAL "OpenGL") target_compile_definitions(bgfx PRIVATE BGFX_CONFIG_RENDERER_OPENGLES=30) target_compile_definitions(bgfx PRIVATE BGFX_GL_CONFIG_BLIT_EMULATION=1) target_compile_definitions(bgfx PRIVATE BGFX_GL_CONFIG_TEXTURE_READ_BACK_EMULATION=1) - set(BGFX_KHRONOS_INCLUDE_DIR "${bgfx.cmake_SOURCE_DIR}/bgfx/3rdparty/khronos" CACHE INTERNAL "") elseif(GRAPHICS_API STREQUAL "Vulkan") target_compile_definitions(bgfx PRIVATE BGFX_CONFIG_RENDERER_VULKAN=1) endif() diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 069efd314..549e56cc3 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -320,7 +320,7 @@ namespace Babylon texture->Create2D(static_cast(image->m_width), static_cast(image->m_height), (image->m_numMips > 1), 1, Cast(image->m_format), flags); } - for (uint8_t mip = 0, numMips = image->m_numMips; mip < numMips; ++mip) + for (uint8_t mip = 0; mip < image->m_numMips; ++mip) { bimg::ImageMip imageMip{}; if (bimg::imageGetRawData(*image, 0, mip, image->m_data, image->m_size, imageMip)) @@ -389,7 +389,7 @@ namespace Babylon for (uint8_t side = 0; side < 6; ++side) { bimg::ImageContainer* image{images[side]}; - for (uint8_t mip = 0, numMips = image->m_numMips; mip < numMips; ++mip) + for (uint8_t mip = 0; mip < image->m_numMips; ++mip) { bimg::ImageMip imageMip{}; if (bimg::imageGetRawData(*image, 0, mip, image->m_data, image->m_size, imageMip)) From 3d491d80ffe491c259039e1a88743c376f4aa087 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 15:51:55 -0700 Subject: [PATCH 11/24] Pump frames in JavaScript unit test to prevent deadlock from always-acquired FrameCompletionScope in SubmitCommands. --- Apps/UnitTests/Source/Tests.JavaScript.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Apps/UnitTests/Source/Tests.JavaScript.cpp b/Apps/UnitTests/Source/Tests.JavaScript.cpp index cbd943b72..8b01ab63a 100644 --- a/Apps/UnitTests/Source/Tests.JavaScript.cpp +++ b/Apps/UnitTests/Source/Tests.JavaScript.cpp @@ -90,6 +90,17 @@ TEST(JavaScript, All) device.StartRenderingCurrentFrame(); device.FinishRenderingCurrentFrame(); - auto exitCode{exitCodePromise.get_future().get()}; + // Pump frames while JS tests run — tests use RAF internally and + // SubmitCommands requires an active frame. + auto exitCodeFuture = exitCodePromise.get_future(); + device.StartRenderingCurrentFrame(); + while (exitCodeFuture.wait_for(std::chrono::milliseconds(0)) != std::future_status::ready) + { + device.FinishRenderingCurrentFrame(); + device.StartRenderingCurrentFrame(); + } + device.FinishRenderingCurrentFrame(); + + auto exitCode = exitCodeFuture.get(); EXPECT_EQ(exitCode, 0); } From bf32f2dd6393d4bdceaf07ebd653ae6f8be91654 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 16:22:18 -0700 Subject: [PATCH 12/24] Fix unit test shutdown race by using 16ms frame interval instead of spin-loop. --- Apps/UnitTests/Source/Tests.JavaScript.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.JavaScript.cpp b/Apps/UnitTests/Source/Tests.JavaScript.cpp index 8b01ab63a..aaf36665d 100644 --- a/Apps/UnitTests/Source/Tests.JavaScript.cpp +++ b/Apps/UnitTests/Source/Tests.JavaScript.cpp @@ -93,13 +93,11 @@ TEST(JavaScript, All) // Pump frames while JS tests run — tests use RAF internally and // SubmitCommands requires an active frame. auto exitCodeFuture = exitCodePromise.get_future(); - device.StartRenderingCurrentFrame(); - while (exitCodeFuture.wait_for(std::chrono::milliseconds(0)) != std::future_status::ready) + while (exitCodeFuture.wait_for(std::chrono::milliseconds(16)) != std::future_status::ready) { - device.FinishRenderingCurrentFrame(); device.StartRenderingCurrentFrame(); + device.FinishRenderingCurrentFrame(); } - device.FinishRenderingCurrentFrame(); auto exitCode = exitCodeFuture.get(); EXPECT_EQ(exitCode, 0); From eb86445d37e94017ae2a21e8780f6aaf67d7ea57 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 17:08:52 -0700 Subject: [PATCH 13/24] Keep frame open during unit test shutdown to prevent JS thread deadlock on FrameCompletionScope. --- Apps/UnitTests/Source/Tests.JavaScript.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Apps/UnitTests/Source/Tests.JavaScript.cpp b/Apps/UnitTests/Source/Tests.JavaScript.cpp index aaf36665d..283106483 100644 --- a/Apps/UnitTests/Source/Tests.JavaScript.cpp +++ b/Apps/UnitTests/Source/Tests.JavaScript.cpp @@ -99,6 +99,15 @@ TEST(JavaScript, All) device.FinishRenderingCurrentFrame(); } + // Keep the frame open during shutdown so any pending JS work + // (e.g., SubmitCommands acquiring a FrameCompletionScope) can complete. + device.StartRenderingCurrentFrame(); + auto exitCode = exitCodeFuture.get(); EXPECT_EQ(exitCode, 0); + + // Runtime destructor joins the JS thread; must happen before Finish. + nativeCanvas.reset(); + + device.FinishRenderingCurrentFrame(); } From d5560b6f4059067b8a621760be69a070c7b7b450 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 20:24:26 -0700 Subject: [PATCH 14/24] Fix PrecompiledShaderTest deadlock by keeping frame open during startup wait. --- Apps/PrecompiledShaderTest/Source/App.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Apps/PrecompiledShaderTest/Source/App.cpp b/Apps/PrecompiledShaderTest/Source/App.cpp index 1ae0dee10..fdc9b8cfd 100644 --- a/Apps/PrecompiledShaderTest/Source/App.cpp +++ b/Apps/PrecompiledShaderTest/Source/App.cpp @@ -166,9 +166,17 @@ int RunApp( deviceUpdate.Finish(); device.FinishRenderingCurrentFrame(); + // Reopen the gate so JS can continue running (startup may issue bgfx commands). + device.StartRenderingCurrentFrame(); + deviceUpdate.Start(); + // Wait for `startup` to finish. startup.get_future().wait(); + // Close the frame opened above. + deviceUpdate.Finish(); + device.FinishRenderingCurrentFrame(); + // Start a new frame for rendering the scene. device.StartRenderingCurrentFrame(); deviceUpdate.Start(); From c73534ccfdd502edd5a4912aea067cf980f2de0e Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 21:03:26 -0700 Subject: [PATCH 15/24] =?UTF-8?q?Revert=20ReadTexture=20to=20inline=20blit?= =?UTF-8?q?=20with=20FrameCompletionScope=20=E2=80=94=20BeforeRenderSchedu?= =?UTF-8?q?ler=20fires=20one=20frame=20late=20when=20called=20outside=20RA?= =?UTF-8?q?F.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Plugins/NativeEngine/Source/NativeEngine.cpp | 140 +++++++------------ 1 file changed, 48 insertions(+), 92 deletions(-) diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 549e56cc3..880f6cc4e 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -1652,108 +1652,64 @@ namespace Babylon auto tempTexture = std::make_shared(false); // If the image needs to be cropped or the texture lacks the READ_BACK flag, blit to a temp texture. - // Both the blit and readTexture must be submitted in the correct order within - // the same frame, so when a blit is needed they are both scheduled together - // via BeforeRenderScheduler (runs on main thread before encoder is ended). if (x != 0 || y != 0 || width != (texture->Width() >> mipLevel) || height != (texture->Height() >> mipLevel) || (texture->Flags() & BGFX_TEXTURE_READ_BACK) == 0) { const bgfx::TextureHandle blitTextureHandle{bgfx::createTexture2D(width, height, /*hasMips*/ false, /*numLayers*/ 1, sourceTextureFormat, BGFX_TEXTURE_BLIT_DST | BGFX_TEXTURE_READ_BACK)}; + // Acquire a scope to ensure the encoder is available, then blit inline. + Graphics::FrameCompletionScope blitScope{m_deviceContext.AcquireFrameCompletionScope()}; + bgfx::Encoder* encoder = m_deviceContext.GetActiveEncoder(); + assert(encoder != nullptr); + encoder->blit(static_cast(bgfx::getCaps()->limits.maxViews - 1), blitTextureHandle, /*dstMip*/ 0, /*dstX*/ 0, /*dstY*/ 0, /*dstZ*/ 0, sourceTextureHandle, mipLevel, x, y, /*srcZ*/ 0, width, height, /*depth*/ 0); + sourceTextureHandle = blitTextureHandle; *tempTexture = true; mipLevel = 0; - - // Schedule blit + readTexture on the main thread. Only bgfx calls - // go here — no N-API objects (they aren't thread-safe). - auto readTextureTask = arcana::make_task(m_deviceContext.BeforeRenderScheduler(), *m_cancellationSource, - [this, src = texture->Handle(), dst = blitTextureHandle, origMipLevel = static_cast(info[1].As().Uint32Value()), x, y, width, height, storageSize = sourceTextureInfo.storageSize]() { - bgfx::Encoder* encoder = m_deviceContext.GetActiveEncoder(); - assert(encoder != nullptr); - encoder->blit(static_cast(bgfx::getCaps()->limits.maxViews - 1), dst, /*dstMip*/ 0, /*dstX*/ 0, /*dstY*/ 0, /*dstZ*/ 0, src, origMipLevel, x, y, /*srcZ*/ 0, width, height, /*depth*/ 0); - - std::vector textureBuffer(storageSize); - return m_deviceContext.ReadTextureAsync(dst, textureBuffer, 0) - .then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}]() mutable { - return std::move(textureBuffer); - }); - }); - - // Continue on JS thread with N-API objects. - std::move(readTextureTask) - .then(arcana::inline_scheduler, *m_cancellationSource, [sourceTextureInfo, targetTextureInfo](std::vector textureBuffer) mutable { - if (targetTextureInfo.format != sourceTextureInfo.format) - { - std::vector convertedTextureBuffer(targetTextureInfo.storageSize); - if (!bimg::imageConvert(&Graphics::DeviceContext::GetDefaultAllocator(), convertedTextureBuffer.data(), bimg::TextureFormat::Enum(targetTextureInfo.format), textureBuffer.data(), bimg::TextureFormat::Enum(sourceTextureInfo.format), sourceTextureInfo.width, sourceTextureInfo.height, /*depth*/ 1)) - { - throw std::runtime_error{"Texture conversion to RBGA8 failed."}; - } - textureBuffer = convertedTextureBuffer; - } - assert(textureBuffer.size() == targetTextureInfo.storageSize); - if (bgfx::getCaps()->originBottomLeft) - { - FlipImage(textureBuffer, targetTextureInfo.height); - } - return textureBuffer; - }) - .then(m_runtimeScheduler, *m_cancellationSource, [this, bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred, tempTexture, blitTextureHandle](std::vector textureBuffer) mutable { - assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); - uint8_t* buf{static_cast(bufferRef.Value().Data())}; - std::memcpy(buf + bufferOffset, textureBuffer.data(), textureBuffer.size()); - if (*tempTexture && !m_cancellationSource->cancelled()) - { - bgfx::destroy(blitTextureHandle); - *tempTexture = false; - } - deferred.Resolve(bufferRef.Value()); - }) - .then(m_runtimeScheduler, arcana::cancellation::none(), [this, deferred, tempTexture, blitTextureHandle](const arcana::expected& result) { - if (*tempTexture && !m_cancellationSource->cancelled()) - { - bgfx::destroy(blitTextureHandle); - } - if (result.has_error()) - { - deferred.Reject(Napi::Error::New(Env(), result.error()).Value()); - } - }); } - else - { - // No blit needed — texture already has READ_BACK flag and correct region. - std::vector textureBuffer(sourceTextureInfo.storageSize); - m_deviceContext.ReadTextureAsync(sourceTextureHandle, textureBuffer, mipLevel) - .then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo]() mutable { - if (targetTextureInfo.format != sourceTextureInfo.format) - { - std::vector convertedTextureBuffer(targetTextureInfo.storageSize); - if (!bimg::imageConvert(&Graphics::DeviceContext::GetDefaultAllocator(), convertedTextureBuffer.data(), bimg::TextureFormat::Enum(targetTextureInfo.format), textureBuffer.data(), bimg::TextureFormat::Enum(sourceTextureInfo.format), sourceTextureInfo.width, sourceTextureInfo.height, /*depth*/ 1)) - { - throw std::runtime_error{"Texture conversion to RBGA8 failed."}; - } - textureBuffer = convertedTextureBuffer; - } - assert(textureBuffer.size() == targetTextureInfo.storageSize); - if (bgfx::getCaps()->originBottomLeft) - { - FlipImage(textureBuffer, targetTextureInfo.height); - } - return textureBuffer; - }) - .then(m_runtimeScheduler, *m_cancellationSource, [bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred](std::vector textureBuffer) mutable { - assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); - uint8_t* buf{static_cast(bufferRef.Value().Data())}; - std::memcpy(buf + bufferOffset, textureBuffer.data(), textureBuffer.size()); - deferred.Resolve(bufferRef.Value()); - }) - .then(m_runtimeScheduler, arcana::cancellation::none(), [this, deferred](const arcana::expected& result) { - if (result.has_error()) + + // Allocate a buffer to store the source pixel data. + std::vector textureBuffer(sourceTextureInfo.storageSize); + + // Read the source texture (async — completes after bgfx::frame). + m_deviceContext.ReadTextureAsync(sourceTextureHandle, textureBuffer, mipLevel) + .then(arcana::inline_scheduler, *m_cancellationSource, [textureBuffer{std::move(textureBuffer)}, sourceTextureInfo, targetTextureInfo]() mutable { + if (targetTextureInfo.format != sourceTextureInfo.format) + { + std::vector convertedTextureBuffer(targetTextureInfo.storageSize); + if (!bimg::imageConvert(&Graphics::DeviceContext::GetDefaultAllocator(), convertedTextureBuffer.data(), bimg::TextureFormat::Enum(targetTextureInfo.format), textureBuffer.data(), bimg::TextureFormat::Enum(sourceTextureInfo.format), sourceTextureInfo.width, sourceTextureInfo.height, /*depth*/ 1)) { - deferred.Reject(Napi::Error::New(Env(), result.error()).Value()); + throw std::runtime_error{"Texture conversion to RBGA8 failed."}; } - }); - } + textureBuffer = convertedTextureBuffer; + } + assert(textureBuffer.size() == targetTextureInfo.storageSize); + if (bgfx::getCaps()->originBottomLeft) + { + FlipImage(textureBuffer, targetTextureInfo.height); + } + return textureBuffer; + }) + .then(m_runtimeScheduler, *m_cancellationSource, [this, bufferRef{Napi::Persistent(buffer)}, bufferOffset, deferred, tempTexture, sourceTextureHandle](std::vector textureBuffer) mutable { + assert(bufferRef.Value().ByteLength() - bufferOffset >= textureBuffer.size()); + uint8_t* buf{static_cast(bufferRef.Value().Data())}; + std::memcpy(buf + bufferOffset, textureBuffer.data(), textureBuffer.size()); + if (*tempTexture && !m_cancellationSource->cancelled()) + { + bgfx::destroy(sourceTextureHandle); + *tempTexture = false; + } + deferred.Resolve(bufferRef.Value()); + }) + .then(m_runtimeScheduler, arcana::cancellation::none(), [this, deferred, tempTexture, sourceTextureHandle](const arcana::expected& result) { + if (*tempTexture && !m_cancellationSource->cancelled()) + { + bgfx::destroy(sourceTextureHandle); + } + if (result.has_error()) + { + deferred.Reject(Napi::Error::New(Env(), result.error()).Value()); + } + }); } return deferred.Promise(); From 935bf4a70cf797bee06570df7b0aea234a28988d Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 21:22:54 -0700 Subject: [PATCH 16/24] Discard encoder state before Canvas Flush to prevent NativeEngine state leaking into nanovg rendering. --- Polyfills/Canvas/Source/Context.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Polyfills/Canvas/Source/Context.cpp b/Polyfills/Canvas/Source/Context.cpp index 618a4a2e4..64a7fd1e0 100644 --- a/Polyfills/Canvas/Source/Context.cpp +++ b/Polyfills/Canvas/Source/Context.cpp @@ -623,6 +623,11 @@ namespace Babylon::Polyfills::Internal return; } + // Discard any residual encoder state from NativeEngine rendering. + // In the old model Canvas had its own per-thread encoder with clean state; + // now it shares the frame encoder with NativeEngine. + encoder->discard(BGFX_DISCARD_ALL); + bool needClear = m_canvas->UpdateRenderTarget(); Graphics::FrameBuffer& frameBuffer = m_canvas->GetFrameBuffer(); From e97bfdc4ef5978f2f5b500e2080f31415d805540 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 8 Apr 2026 21:42:26 -0700 Subject: [PATCH 17/24] =?UTF-8?q?Add=20FrameCompletionScope=20to=20ReadTex?= =?UTF-8?q?ture=20=E2=80=94=20called=20during=20init=20and=20from=20getFra?= =?UTF-8?q?meBufferData=20outside=20RAF.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Plugins/NativeEngine/Source/NativeEngine.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 880f6cc4e..6dc32bf87 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -1648,6 +1648,11 @@ namespace Babylon } else { + // Acquire a FrameCompletionScope for the duration of the read operation. + // This ensures the encoder is available for the blit (if needed) and that + // bgfx::readTexture lands in the same frame as the blit. + Graphics::FrameCompletionScope scope{m_deviceContext.AcquireFrameCompletionScope()}; + bgfx::TextureHandle sourceTextureHandle{texture->Handle()}; auto tempTexture = std::make_shared(false); @@ -1656,10 +1661,7 @@ namespace Babylon { const bgfx::TextureHandle blitTextureHandle{bgfx::createTexture2D(width, height, /*hasMips*/ false, /*numLayers*/ 1, sourceTextureFormat, BGFX_TEXTURE_BLIT_DST | BGFX_TEXTURE_READ_BACK)}; - // Acquire a scope to ensure the encoder is available, then blit inline. - Graphics::FrameCompletionScope blitScope{m_deviceContext.AcquireFrameCompletionScope()}; - bgfx::Encoder* encoder = m_deviceContext.GetActiveEncoder(); - assert(encoder != nullptr); + bgfx::Encoder* encoder = GetEncoder(); encoder->blit(static_cast(bgfx::getCaps()->limits.maxViews - 1), blitTextureHandle, /*dstMip*/ 0, /*dstX*/ 0, /*dstY*/ 0, /*dstZ*/ 0, sourceTextureHandle, mipLevel, x, y, /*srcZ*/ 0, width, height, /*depth*/ 0); sourceTextureHandle = blitTextureHandle; From 282a9a65b310afe797e7faa9aff2b6e74d87b264 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Wed, 29 Apr 2026 15:59:57 -0700 Subject: [PATCH 18/24] Apps/UnitTests: hold the frame open across the JS test pump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unit-test pump has been calling Start; Finish back-to-back per loop iteration with the wait_for AFTER Finish, which collapses the FrameCompletionScope gate's open window to ~zero. With the new shared-gate model on rework-thread-model, JS-thread SubmitCommands (and similar) acquire a scope that blocks on the gate's CV; whenever they wake on notify_all from Start, they have to win a scheduler race against the pump thread re-entering Finish to land their scope before the gate closes. On contended Linux CI runners that race is lost frequently, accumulating 16ms+ per missed acquisition into 30+ minute test runtimes (vs ~5 minutes on master). Restructure the pump to follow the "Start ASAP / Finish; Start tick / final Finish" pattern: open the frame immediately after device creation so the JS thread can submit at any time, then in the pump loop just tick bgfx (Finish; Start) once per iteration, and rely on the existing trailing Finish (after nativeCanvas.reset()) to close the gate at shutdown. Drops the dead Start; Finish priming pair and the redundant post-loop Start. Validated locally (Win32 RelWithDebInfo): UnitTests --gtest_filter= JavaScript.* passes in 2.4s with all 24 sub-tests green. Validated on bghgary fork CI (4 parallel runs x 2 jobs, Linux JSC, prior version of this fix on bghgary/unittest-watchdog 140894be): all 8 jobs ran the Unit Tests step in 4 seconds consistently with total job duration 352-393s — back in master baseline range. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Apps/UnitTests/Source/Tests.JavaScript.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.JavaScript.cpp b/Apps/UnitTests/Source/Tests.JavaScript.cpp index 283106483..817d3f06d 100644 --- a/Apps/UnitTests/Source/Tests.JavaScript.cpp +++ b/Apps/UnitTests/Source/Tests.JavaScript.cpp @@ -40,6 +40,13 @@ TEST(JavaScript, All) Babylon::Graphics::Device device{g_deviceConfig}; + // Start rendering a frame to unblock the JavaScript from queuing graphics + // commands. The frame is held open through script load and the test pump + // (which only ticks bgfx via Finish; Start) so the JS thread can submit + // at any time without racing the gate. A final Finish closes it after + // runtime teardown. + device.StartRenderingCurrentFrame(); + std::optional nativeCanvas; Babylon::AppRuntime::Options options{}; @@ -87,22 +94,17 @@ TEST(JavaScript, All) loader.LoadScript("app:///Assets/babylonjs.materials.js"); loader.LoadScript("app:///Assets/tests.javaScript.all.js"); - device.StartRenderingCurrentFrame(); - device.FinishRenderingCurrentFrame(); - // Pump frames while JS tests run — tests use RAF internally and - // SubmitCommands requires an active frame. + // SubmitCommands requires an active frame. The frame was opened + // immediately after device creation; the loop just ticks bgfx + // (Finish; Start) once per iteration so commands can advance. auto exitCodeFuture = exitCodePromise.get_future(); while (exitCodeFuture.wait_for(std::chrono::milliseconds(16)) != std::future_status::ready) { - device.StartRenderingCurrentFrame(); device.FinishRenderingCurrentFrame(); + device.StartRenderingCurrentFrame(); } - // Keep the frame open during shutdown so any pending JS work - // (e.g., SubmitCommands acquiring a FrameCompletionScope) can complete. - device.StartRenderingCurrentFrame(); - auto exitCode = exitCodeFuture.get(); EXPECT_EQ(exitCode, 0); From 7f98c7c21f5824971b8e089996adb21a956d8ec3 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Mon, 4 May 2026 15:23:10 -0700 Subject: [PATCH 19/24] NativeEngine: hold FrameCompletionScope across the JS-thread frame The X11 Validation Tests flake on Linux JSC was caused by a gap in FrameCompletionScope coverage between two scope acquisitions in one logical JS-side update. SubmitCommands acquired a scope locally, processed its command stream, and released the scope when the function returned. If a single JS task made multiple submit calls (or did any non-bgfx work between them), the scope count dropped to 0 between calls. A concurrent FinishRenderingCurrentFrame on the render thread could then proceed through the !m_pendingFrameScopes wait, close the gate, and run bgfx::frame() with only a partial scene submitted -- producing the empty/clear-color screenshots observed in the flake's pixel diffs. Fix: capture the scope into an m_runtime.Dispatch lambda so it survives to the end of the current JS-thread task and is released when the runtime services its queue next. Continuation work in the same JS frame sees count > 0 and remains protected from concurrent Finish. Same shape as the existing RAF dispatcher's "prevent_frame" pattern, applied at the SubmitCommands funnel which is reached by every render path. Validated locally on Win32 RelWithDebInfo (UnitTests --gtest_filter= JavaScript.* passes in 3.3s, all 24 sub-tests green). [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Plugins/NativeEngine/Source/NativeEngine.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 190bc4947..ac38d58fc 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -2174,6 +2174,14 @@ namespace Babylon { throw Napi::Error::New(info.Env(), exception); } + + // Defer scope release to the next JS-thread dispatch cycle so the + // frame stays open across the rest of the current JS frame, not just + // across this command stream's processing. Any continuation work in + // the same JS frame (further submitCommands calls, immediate promise + // resolutions touching bgfx, etc.) sees count > 0 and remains + // protected from a concurrent FinishRenderingCurrentFrame. + m_runtime.Dispatch([scope = std::move(scope)](auto) {}); } void NativeEngine::PopulateFrameStats(const Napi::CallbackInfo& info) From bb16582ee06d5cd96f448e538d2664419cee0387 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Mon, 4 May 2026 15:38:38 -0700 Subject: [PATCH 20/24] NativeEngine: fix Linux gcc build of SubmitCommands scope capture, refresh docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The std::move(scope) capture in 7f98c7c2 produced a move-only lambda. Dispatchable at the AppRuntime layer is move-only and accepts it, but JsRuntime downstream type-erases the callable through std::function which requires copy-constructibility: std_function.h:439:18: error: static assertion failed due to requirement 'is_copy_constructible<...>::value': std::function target must be copy-constructible MSVC accepted this; gcc 14 (Ubuntu CI) rejects it. Wrap the scope in shared_ptr so the lambda is copyable, matching the existing RAF dispatcher pattern in ScheduleRequestAnimationFrameCallbacks. Also refresh the comments on FrameCompletionScope and AcquireFrameCompletionScope. The previous class-level comment listed three usage patterns, but one of them ("NativeEngine::GetEncoder() acquires lazily") didn't match the code — GetEncoder doesn't acquire a scope. Replace with the two patterns that are actually in use: 1. JS-frame scoped via m_runtime.Dispatch capture (RAF, SubmitCommands) 2. Block scoped on the stack (Canvas::Flush fallback, ReadTextureAsync) [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Babylon/Graphics/DeviceContext.h | 43 +++++++++++-------- Plugins/NativeEngine/Source/NativeEngine.cpp | 9 +++- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h b/Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h index d25e61f6c..2bd5b7882 100644 --- a/Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h +++ b/Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h @@ -29,24 +29,27 @@ namespace Babylon::Graphics bgfx::TextureFormat::Enum Format{}; }; - // FrameCompletionScope is an RAII guard that keeps a frame "open" on the JS thread. - // While any scope is alive, FinishRenderingCurrentFrame() on the main thread will - // block — it cannot call bgfx::frame() until all scopes are destroyed. + // FrameCompletionScope is an RAII guard that prevents the render thread from + // closing a bgfx frame while JS-thread work is still in flight. While any + // scope is alive, FinishRenderingCurrentFrame() blocks before bgfx::frame() + // — so all encoder commands recorded while a scope was held land in the + // same bgfx frame, never split across two. // - // This prevents a race where the main thread submits a bgfx frame while the JS - // thread is still recording encoder commands (which would cause bgfx deadlocks - // or lost draw calls). + // Acquisition blocks if the gate is closed (m_frameBlocked == true), so a + // JS thread that picks up work between frames waits for the next Start + // before proceeding. // - // Three usage patterns: - // 1. RAF scheduling: scope acquired on main thread during StartRenderingCurrentFrame, - // transferred to JS thread, released after RAF callbacks complete + one extra - // dispatch cycle (to cover GC-triggered resource destruction). - // 2. NativeEngine::GetEncoder(): scope acquired lazily when JS code uses the encoder - // outside RAF (e.g., async texture loads, LOD switches). Released on next dispatch. - // 3. Canvas::Flush(): stack-scoped for the duration of nanovg rendering. - // - // Construction blocks if m_frameBlocked is true (frame submission in progress). - // Destruction decrements counter and wakes main thread via condition variable. + // Two scoping patterns are used: + // 1. JS-frame scoped: capture the scope into an m_runtime.Dispatch lambda + // so it survives until the JS-thread queue services the next + // continuation. Use this when subsequent work in the same JS task may + // also touch bgfx (chained submitCommands, RAF callbacks, scene + // cleanup, etc.). NativeEngine::SubmitCommands and + // ScheduleRequestAnimationFrameCallbacks both do this. + // 2. Block scoped: hold the scope on the stack across a single self- + // contained bgfx phase. Used for one-shot operations like + // Canvas::Flush() called outside an active frame, and + // ReadTextureAsync. class FrameCompletionScope final { public: @@ -82,8 +85,12 @@ namespace Babylon::Graphics // Use this to schedule work (e.g., requestAnimationFrame callbacks) that should run each frame. continuation_scheduler<>& FrameStartScheduler(); - // Acquire a scope that prevents FinishRenderingCurrentFrame from completing. - // The scope must be held while JS frame callbacks are running. + // Acquire a scope that prevents FinishRenderingCurrentFrame from + // completing until the scope is destroyed. JS-thread callers that + // need coverage across the whole current JS task should capture the + // scope into an m_runtime.Dispatch lambda; callers needing only a + // single phase can hold it stack-scoped. See the class comment on + // FrameCompletionScope above for the two patterns. FrameCompletionScope AcquireFrameCompletionScope(); // Active encoder for the current frame. Managed by DeviceImpl in diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index ac38d58fc..ce25e18a7 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -2160,7 +2160,12 @@ namespace Babylon // the frame is already open and this returns immediately. When called // outside (e.g., scene.dispose() from an XHR callback), this blocks // until StartRenderingCurrentFrame provides the encoder. - Graphics::FrameCompletionScope scope{m_deviceContext.AcquireFrameCompletionScope()}; + // + // Wrapped in shared_ptr so the captured lambda below is copy- + // constructible (Dispatchable is move-only at the AppRuntime layer + // but gets type-erased via std::function downstream). + auto scope = std::make_shared( + m_deviceContext.AcquireFrameCompletionScope()); try { @@ -2181,7 +2186,7 @@ namespace Babylon // the same JS frame (further submitCommands calls, immediate promise // resolutions touching bgfx, etc.) sees count > 0 and remains // protected from a concurrent FinishRenderingCurrentFrame. - m_runtime.Dispatch([scope = std::move(scope)](auto) {}); + m_runtime.Dispatch([scope](auto) {}); } void NativeEngine::PopulateFrameStats(const Napi::CallbackInfo& info) From 606369aa674887212e6a6012fd3e61dca6bdc053 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Mon, 4 May 2026 15:43:07 -0700 Subject: [PATCH 21/24] NativeEngine: dispatch the SubmitCommands frame scope at the top MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Schedule the FrameCompletionScope-releasing lambda before the command stream is processed, not after. Equivalent in the success path, but makes the JS-frame coverage exception-safe — if the command processing throws, the lambda is already queued and the scope survives to the next dispatch cycle, instead of being destroyed at the throw and dropping the count to 0 on the render thread's view. Also drops the explicit local in favour of an inline make_shared in the lambda capture — the local was only there to be captured. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Plugins/NativeEngine/Source/NativeEngine.cpp | 35 ++++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index ce25e18a7..f479939ad 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -2155,17 +2155,24 @@ namespace Babylon void NativeEngine::SubmitCommands(const Napi::CallbackInfo& info) { - // Acquire a FrameCompletionScope to ensure the encoder stays valid for - // the duration of command processing. When called within a RAF callback, - // the frame is already open and this returns immediately. When called - // outside (e.g., scene.dispose() from an XHR callback), this blocks - // until StartRenderingCurrentFrame provides the encoder. + // Acquire a FrameCompletionScope and capture it into a Dispatch + // lambda so the frame stays open across the rest of the current JS + // task, not just this command-stream pass. Any continuation work in + // the same JS task (further submitCommands calls, immediate promise + // resolutions touching bgfx, scene.dispose() cleanup, etc.) sees + // count > 0 and remains protected from a concurrent + // FinishRenderingCurrentFrame. // - // Wrapped in shared_ptr so the captured lambda below is copy- - // constructible (Dispatchable is move-only at the AppRuntime layer - // but gets type-erased via std::function downstream). - auto scope = std::make_shared( - m_deviceContext.AcquireFrameCompletionScope()); + // Dispatching at the top (before the try block) also makes the + // coverage exception-safe — if the command stream throws, the lambda + // is already queued so the scope still survives to the next dispatch + // cycle. + // + // shared_ptr because Dispatchable is move-only at the AppRuntime + // layer but JsRuntime downstream type-erases via std::function which + // requires copy-constructibility. + m_runtime.Dispatch([scope = std::make_shared( + m_deviceContext.AcquireFrameCompletionScope())](auto) {}); try { @@ -2179,14 +2186,6 @@ namespace Babylon { throw Napi::Error::New(info.Env(), exception); } - - // Defer scope release to the next JS-thread dispatch cycle so the - // frame stays open across the rest of the current JS frame, not just - // across this command stream's processing. Any continuation work in - // the same JS frame (further submitCommands calls, immediate promise - // resolutions touching bgfx, etc.) sees count > 0 and remains - // protected from a concurrent FinishRenderingCurrentFrame. - m_runtime.Dispatch([scope](auto) {}); } void NativeEngine::PopulateFrameStats(const Napi::CallbackInfo& info) From cc58c2d0d7c993d09778787297b4181c79b451c3 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Fri, 29 May 2026 12:21:35 -0700 Subject: [PATCH 22/24] Tests.ExternalTexture.Msaa: open frame 2 before waiting on startup() The MSAA tests deadlocked after merging master into rework-thread-model. The render thread waits on startupDone while the JS thread runs the AddToContextAsync .then() callback. That callback calls startup() -> new NativeEngine() / new Scene() / new RenderTargetTexture(), which eventually hits SubmitCommands. SubmitCommands now synchronously acquires a FrameCompletionScope, which blocks while m_frameBlocked is true. The gate only opens on the next StartRenderingCurrentFrame -- but the render thread is stuck on startupDone. Deadlock. Fix: open frame 2 before waiting on startupDone, and reuse the same frame for renderFrame(). Any JS work that touches the engine has to run while a frame is in progress under the new model. This workaround is temporary. Once #1646 lands, the test should migrate from AddToContextAsync to the new synchronous CreateForJavaScript, at which point startup() runs in the same JS task as the texture creation and the cross-frame dance disappears entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Apps/UnitTests/Source/Tests.ExternalTexture.Msaa.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.ExternalTexture.Msaa.cpp b/Apps/UnitTests/Source/Tests.ExternalTexture.Msaa.cpp index 419dfc5a1..d43bf2bc4 100644 --- a/Apps/UnitTests/Source/Tests.ExternalTexture.Msaa.cpp +++ b/Apps/UnitTests/Source/Tests.ExternalTexture.Msaa.cpp @@ -89,12 +89,17 @@ namespace addToContextCalled.get_future().wait(); update.Finish(); device.FinishRenderingCurrentFrame(); - startupDone.get_future().get(); - // New frame: drive a single renderFrame() on the JS side. + // Open the next frame BEFORE waiting for startup() to complete. The AddToContextAsync .then() + // callback runs on the JS thread and calls startup(), which constructs NativeEngine + Scene + RTT, + // each step submitting bgfx commands. In the threading model from #1652, SubmitCommands + // synchronously acquires a FrameCompletionScope and blocks until a frame is in progress, so a + // frame must be open while startup() runs. The same frame is reused for renderFrame(). device.StartRenderingCurrentFrame(); update.Start(); + startupDone.get_future().get(); + std::promise renderDone; loader.Dispatch([&renderDone](Napi::Env env) { auto jsPromise = env.Global().Get("renderFrame").As().Call({}).As(); From 0fa19c2b41ebdc2bbc6b1d30fa3d7207f585cadb Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Mon, 8 Jun 2026 16:55:53 -0700 Subject: [PATCH 23/24] Skip RestoreAfterDeviceLoss under the reworked threading model The test drives device loss/restore via the async AddToContextAsync + manual frame-pump pattern, which deadlocks under the single-frame-encoder model in this PR. It is re-enabled in #1646 after migration to the synchronous CreateForJavaScript API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../UnitTests/Source/Tests.ExternalTexture.DeviceLoss.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Apps/UnitTests/Source/Tests.ExternalTexture.DeviceLoss.cpp b/Apps/UnitTests/Source/Tests.ExternalTexture.DeviceLoss.cpp index 501f17847..c698cf155 100644 --- a/Apps/UnitTests/Source/Tests.ExternalTexture.DeviceLoss.cpp +++ b/Apps/UnitTests/Source/Tests.ExternalTexture.DeviceLoss.cpp @@ -74,6 +74,14 @@ TEST(ExternalTexture, RestoreAfterDeviceLoss) #if defined(SKIP_EXTERNAL_TEXTURE_TESTS) || defined(SKIP_RENDER_TESTS) GTEST_SKIP(); #else + // This test drives device loss/restore through the async AddToContextAsync + manual + // frame-pump pattern, which deadlocks under the reworked single-frame-encoder threading + // model in this PR. PR #1646 migrates ExternalTexture tests to the synchronous + // CreateForJavaScript API; once #1646 lands on top of this PR, this test should be ported + // to that API and re-enabled (see Tests.ExternalTexture.Msaa.cpp for the migration shape). + GTEST_SKIP() << "Re-enabled in #1646 after migration to synchronous CreateForJavaScript " + "(async device-loss frame-pump deadlocks under the reworked threading model)."; + Babylon::Graphics::DeviceT deviceA = Helpers::CreateDevice(); ASSERT_NE(deviceA, nullptr); Babylon::Graphics::DeviceT deviceB = nullptr; From ed7d10f9a5b2efcf3590516286f7a1cf2cfd381e Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Mon, 8 Jun 2026 17:16:07 -0700 Subject: [PATCH 24/24] Adapt RestoreAfterDeviceLoss to the reworked threading model Replaces the earlier skip. Under the single-frame-encoder model the async AddToContextAsync .then() runs startup()/restoreTexture() on the JS thread, whose SubmitCommands blocks until a frame is in progress. Open the next frame before each blocking wait on the async result (and reuse it for the following renderFrame), mirroring the workaround already applied to Tests.ExternalTexture.Msaa.cpp. Obsoleted once #1646 migrates these tests to the synchronous CreateForJavaScript API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Tests.ExternalTexture.DeviceLoss.cpp | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.ExternalTexture.DeviceLoss.cpp b/Apps/UnitTests/Source/Tests.ExternalTexture.DeviceLoss.cpp index c698cf155..de5d34cf3 100644 --- a/Apps/UnitTests/Source/Tests.ExternalTexture.DeviceLoss.cpp +++ b/Apps/UnitTests/Source/Tests.ExternalTexture.DeviceLoss.cpp @@ -74,14 +74,6 @@ TEST(ExternalTexture, RestoreAfterDeviceLoss) #if defined(SKIP_EXTERNAL_TEXTURE_TESTS) || defined(SKIP_RENDER_TESTS) GTEST_SKIP(); #else - // This test drives device loss/restore through the async AddToContextAsync + manual - // frame-pump pattern, which deadlocks under the reworked single-frame-encoder threading - // model in this PR. PR #1646 migrates ExternalTexture tests to the synchronous - // CreateForJavaScript API; once #1646 lands on top of this PR, this test should be ported - // to that API and re-enabled (see Tests.ExternalTexture.Msaa.cpp for the migration shape). - GTEST_SKIP() << "Re-enabled in #1646 after migration to synchronous CreateForJavaScript " - "(async device-loss frame-pump deadlocks under the reworked threading model)."; - Babylon::Graphics::DeviceT deviceA = Helpers::CreateDevice(); ASSERT_NE(deviceA, nullptr); Babylon::Graphics::DeviceT deviceB = nullptr; @@ -149,12 +141,16 @@ TEST(ExternalTexture, RestoreAfterDeviceLoss) addToContextCalled.get_future().wait(); update.Finish(); device.FinishRenderingCurrentFrame(); - startupDone.get_future().get(); - // --- Phase 1: render red into texture 1, readback --- + // Open the next frame BEFORE waiting on startup(): under the reworked single-frame-encoder + // model the AddToContextAsync .then() runs startup() on the JS thread, whose SubmitCommands + // blocks until a frame is in progress. The same frame is reused for Phase 1's renderFrame. device.StartRenderingCurrentFrame(); update.Start(); + startupDone.get_future().get(); + + // --- Phase 1: render red into texture 1, readback (reuses the open frame) --- std::promise render1Done; loader.Dispatch([&render1Done](Napi::Env env) { auto jsPromise = env.Global().Get("renderFrame").As().Call({}).As(); @@ -229,12 +225,15 @@ TEST(ExternalTexture, RestoreAfterDeviceLoss) addToContext2Called.get_future().wait(); update.Finish(); device.FinishRenderingCurrentFrame(); - restoreDone.get_future().get(); - // Render blue into restored RTT. + // Open the next frame before waiting on restoreTexture(): SubmitCommands needs an open frame + // under the reworked model. The same frame is reused for the blue render below. device.StartRenderingCurrentFrame(); update.Start(); + restoreDone.get_future().get(); + + // Render blue into restored RTT (reuses the open frame). std::promise render2Done; loader.Dispatch([&render2Done](Napi::Env env) { auto jsPromise = env.Global().Get("renderFrame").As().Call({}).As();