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/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(); 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/Apps/UnitTests/Source/Tests.JavaScript.cpp b/Apps/UnitTests/Source/Tests.JavaScript.cpp index cbd943b72..283106483 100644 --- a/Apps/UnitTests/Source/Tests.JavaScript.cpp +++ b/Apps/UnitTests/Source/Tests.JavaScript.cpp @@ -90,6 +90,24 @@ 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(); + while (exitCodeFuture.wait_for(std::chrono::milliseconds(16)) != std::future_status::ready) + { + device.StartRenderingCurrentFrame(); + 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(); } 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..a6d3635a7 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); @@ -288,42 +275,66 @@ namespace Babylon::Graphics } m_rendering = true; + m_firstFrameStarted = true; // 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) { + 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 + // (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 +379,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 +443,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 +500,6 @@ namespace Babylon::Graphics { arcana::trace_region frameRegion{"DeviceImpl::Frame"}; - // Automatically end bgfx encoders. - EndEncoders(); - // Update bgfx state if necessary. UpdateBgfxState(); @@ -474,34 +519,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..963811643 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,12 +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{}; + bool m_firstFrameStarted{}; + + // 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}; @@ -151,19 +162,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 cd822de9b..e7689a573 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 @@ -318,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)) @@ -387,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)) @@ -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); } @@ -1646,31 +1648,33 @@ 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); - // 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. 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()}; + + 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; *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. if (targetTextureInfo.format != sourceTextureInfo.format) { std::vector convertedTextureBuffer(targetTextureInfo.storageSize); @@ -1680,47 +1684,32 @@ 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); } - 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 + 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, 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); } - if (result.has_error()) { - deferred.Reject(Napi::Error::New(env, result.error()).Value()); + deferred.Reject(Napi::Error::New(Env(), result.error()).Value()); } }); } @@ -1798,63 +1787,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); } @@ -1862,41 +1843,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}; @@ -1910,6 +1885,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 = @@ -1931,7 +1908,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) @@ -2103,27 +2080,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) @@ -2135,6 +2108,13 @@ 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. + Graphics::FrameCompletionScope scope{m_deviceContext.AcquireFrameCompletionScope()}; + try { NativeDataStream::Reader reader = m_commandStream->GetReader(); @@ -2151,7 +2131,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; @@ -2159,6 +2138,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 @@ -2209,7 +2200,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); @@ -2225,33 +2216,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; } @@ -2264,8 +2242,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"}; @@ -2274,6 +2267,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()) { @@ -2282,4 +2281,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..870ba9614 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,12 +142,9 @@ namespace Babylon JsRuntime& m_runtime; Graphics::DeviceContext& m_deviceContext; - Graphics::Update m_update; JsRuntimeScheduler m_runtimeScheduler; - std::optional m_updateToken{}; - 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 cd28e629d..85541e1e3 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(); @@ -300,7 +300,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..64a7fd1e0 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,36 @@ namespace Babylon::Polyfills::Internal void Context::Flush(const Napi::CallbackInfo&) { + // 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; + } + + // 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(); - auto updateToken{m_update.GetUpdateToken()}; - bgfx::Encoder* encoder = updateToken.GetEncoder(); - frameBuffer.Bind(*encoder); + 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 +648,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 74a984438..c98443850 100644 --- a/Polyfills/Canvas/Source/nanovg/nanovg_babylon.cpp +++ b/Polyfills/Canvas/Source/nanovg/nanovg_babylon.cpp @@ -799,7 +799,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); } @@ -861,7 +861,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); } @@ -906,7 +906,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); } @@ -946,7 +946,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); }