Skip to content

Commit 8a519b7

Browse files
committed
Merge remote-tracking branch 'origin/rework-thread-model' into partner-test-1652-1646
2 parents d63e9fb + e97bfdc commit 8a519b7

23 files changed

Lines changed: 416 additions & 455 deletions

File tree

Apps/HeadlessScreenshotApp/Win32/App.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,9 +159,17 @@ int main()
159159
deviceUpdate.Finish();
160160
device.FinishRenderingCurrentFrame();
161161

162+
// Reopen the gate so JS can continue running (startup may issue bgfx commands).
163+
device.StartRenderingCurrentFrame();
164+
deviceUpdate.Start();
165+
162166
// Wait for `startup` to finish.
163167
startup.get_future().wait();
164168

169+
// Close the frame opened above.
170+
deviceUpdate.Finish();
171+
device.FinishRenderingCurrentFrame();
172+
165173
struct Asset
166174
{
167175
const char* Name;

Apps/PrecompiledShaderTest/Source/App.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,17 @@ int RunApp(
166166
deviceUpdate.Finish();
167167
device.FinishRenderingCurrentFrame();
168168

169+
// Reopen the gate so JS can continue running (startup may issue bgfx commands).
170+
device.StartRenderingCurrentFrame();
171+
deviceUpdate.Start();
172+
169173
// Wait for `startup` to finish.
170174
startup.get_future().wait();
171175

176+
// Close the frame opened above.
177+
deviceUpdate.Finish();
178+
device.FinishRenderingCurrentFrame();
179+
172180
// Start a new frame for rendering the scene.
173181
device.StartRenderingCurrentFrame();
174182
deviceUpdate.Start();

Apps/StyleTransferApp/Win32/App.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,9 +362,17 @@ int APIENTRY wWinMain(_In_ HINSTANCE hInstance,
362362
g_update->Finish();
363363
g_device->FinishRenderingCurrentFrame();
364364

365+
// Reopen the gate so JS can continue running (startup may issue bgfx commands).
366+
g_device->StartRenderingCurrentFrame();
367+
g_update->Start();
368+
365369
// Wait for `startup` to finish.
366370
startup.get_future().wait();
367371

372+
// Close the frame opened above.
373+
g_update->Finish();
374+
g_device->FinishRenderingCurrentFrame();
375+
368376
// --------------------------- Rendering loop -------------------------
369377

370378
HACCEL hAccelTable = LoadAccelerators(hInstance, MAKEINTRESOURCE(IDC_PLAYGROUNDWIN32));

Apps/UnitTests/Source/Tests.JavaScript.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,24 @@ TEST(JavaScript, All)
9090
device.StartRenderingCurrentFrame();
9191
device.FinishRenderingCurrentFrame();
9292

93-
auto exitCode{exitCodePromise.get_future().get()};
93+
// Pump frames while JS tests run — tests use RAF internally and
94+
// SubmitCommands requires an active frame.
95+
auto exitCodeFuture = exitCodePromise.get_future();
96+
while (exitCodeFuture.wait_for(std::chrono::milliseconds(16)) != std::future_status::ready)
97+
{
98+
device.StartRenderingCurrentFrame();
99+
device.FinishRenderingCurrentFrame();
100+
}
101+
102+
// Keep the frame open during shutdown so any pending JS work
103+
// (e.g., SubmitCommands acquiring a FrameCompletionScope) can complete.
104+
device.StartRenderingCurrentFrame();
105+
106+
auto exitCode = exitCodeFuture.get();
94107
EXPECT_EQ(exitCode, 0);
108+
109+
// Runtime destructor joins the JS thread; must happen before Finish.
110+
nativeCanvas.reset();
111+
112+
device.FinishRenderingCurrentFrame();
95113
}

Core/Graphics/CMakeLists.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ set(SOURCES
66
"InternalInclude/Babylon/Graphics/continuation_scheduler.h"
77
"InternalInclude/Babylon/Graphics/FrameBuffer.h"
88
"InternalInclude/Babylon/Graphics/DeviceContext.h"
9-
"InternalInclude/Babylon/Graphics/SafeTimespanGuarantor.h"
109
"InternalInclude/Babylon/Graphics/Texture.h"
1110
"Source/BgfxCallback.cpp"
1211
"Source/FrameBuffer.cpp"
@@ -16,7 +15,6 @@ set(SOURCES
1615
"Source/DeviceImpl.h"
1716
"Source/DeviceImpl_${BABYLON_NATIVE_PLATFORM}.${BABYLON_NATIVE_PLATFORM_IMPL_EXT}"
1817
"Source/DeviceImpl_${GRAPHICS_API}.${BABYLON_NATIVE_PLATFORM_IMPL_EXT}"
19-
"Source/SafeTimespanGuarantor.cpp"
2018
"Source/Texture.cpp")
2119

2220
add_library(Graphics ${SOURCES})

Core/Graphics/Include/Shared/Babylon/Graphics/Device.h

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -58,45 +58,23 @@ namespace Babylon::Graphics
5858
DepthStencilFormat BackBufferDepthStencilFormat{DepthStencilFormat::Depth24Stencil8};
5959
};
6060

61-
class Device;
61+
class DeviceImpl;
6262

63+
// Deprecated: DeviceUpdate is a no-op compatibility shim. Frame synchronization
64+
// is now handled by FrameCompletionScope inside StartRenderingCurrentFrame/
65+
// FinishRenderingCurrentFrame. This class will be removed in a future PR.
6366
class DeviceUpdate
6467
{
6568
public:
66-
void Start()
67-
{
68-
m_start();
69-
}
69+
void Start() {}
70+
void Finish() {}
7071

7172
void RequestFinish(std::function<void()> onFinishCallback)
7273
{
73-
m_requestFinish(std::move(onFinishCallback));
74-
}
75-
76-
void Finish()
77-
{
78-
std::promise<void> promise{};
79-
auto future = promise.get_future();
80-
RequestFinish([&promise] { promise.set_value(); });
81-
future.wait();
82-
}
83-
84-
private:
85-
friend class Device;
86-
87-
template<typename StartCallableT, typename RequestEndCallableT>
88-
DeviceUpdate(StartCallableT&& start, RequestEndCallableT&& requestEnd)
89-
: m_start{std::forward<StartCallableT>(start)}
90-
, m_requestFinish{std::forward<RequestEndCallableT>(requestEnd)}
91-
{
74+
onFinishCallback();
9275
}
93-
94-
std::function<void()> m_start{};
95-
std::function<void(std::function<void()>)> m_requestFinish{};
9676
};
9777

98-
class DeviceImpl;
99-
10078
class Device
10179
{
10280
public:
@@ -128,7 +106,7 @@ namespace Babylon::Graphics
128106
void EnableRendering();
129107
void DisableRendering();
130108

131-
DeviceUpdate GetUpdate(const char* updateName);
109+
DeviceUpdate GetUpdate(const char* /*updateName*/) { return {}; }
132110

133111
void StartRenderingCurrentFrame();
134112
void FinishRenderingCurrentFrame();

Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h

Lines changed: 42 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
#include "BgfxCallback.h"
44
#include <bx/allocator.h>
55
#include "continuation_scheduler.h"
6-
#include "SafeTimespanGuarantor.h"
6+
7+
#include <arcana/threading/task.h>
78

89
#include <napi/env.h>
910

@@ -15,7 +16,6 @@
1516

1617
namespace Babylon::Graphics
1718
{
18-
class Update;
1919
class DeviceContext;
2020
class DeviceImpl;
2121

@@ -29,53 +29,38 @@ namespace Babylon::Graphics
2929
bgfx::TextureFormat::Enum Format{};
3030
};
3131

32-
class UpdateToken final
32+
// FrameCompletionScope is an RAII guard that keeps a frame "open" on the JS thread.
33+
// While any scope is alive, FinishRenderingCurrentFrame() on the main thread will
34+
// block — it cannot call bgfx::frame() until all scopes are destroyed.
35+
//
36+
// This prevents a race where the main thread submits a bgfx frame while the JS
37+
// thread is still recording encoder commands (which would cause bgfx deadlocks
38+
// or lost draw calls).
39+
//
40+
// Three usage patterns:
41+
// 1. RAF scheduling: scope acquired on main thread during StartRenderingCurrentFrame,
42+
// transferred to JS thread, released after RAF callbacks complete + one extra
43+
// dispatch cycle (to cover GC-triggered resource destruction).
44+
// 2. NativeEngine::GetEncoder(): scope acquired lazily when JS code uses the encoder
45+
// outside RAF (e.g., async texture loads, LOD switches). Released on next dispatch.
46+
// 3. Canvas::Flush(): stack-scoped for the duration of nanovg rendering.
47+
//
48+
// Construction blocks if m_frameBlocked is true (frame submission in progress).
49+
// Destruction decrements counter and wakes main thread via condition variable.
50+
class FrameCompletionScope final
3351
{
3452
public:
35-
UpdateToken(const UpdateToken& other) = delete;
36-
UpdateToken& operator=(const UpdateToken& other) = delete;
37-
38-
UpdateToken(UpdateToken&&) noexcept = default;
39-
40-
// The move assignment of `SafeTimespanGuarantor::SafetyGuarantee` is marked as delete.
41-
// See https://github.com/Microsoft/GSL/issues/705.
42-
//UpdateToken& operator=(UpdateToken&& other) = delete;
53+
FrameCompletionScope(const FrameCompletionScope&) = delete;
54+
FrameCompletionScope& operator=(const FrameCompletionScope&) = delete;
55+
FrameCompletionScope& operator=(FrameCompletionScope&&) = delete;
4356

44-
bgfx::Encoder* GetEncoder();
45-
46-
private:
47-
friend class Update;
48-
49-
UpdateToken(DeviceContext&, SafeTimespanGuarantor&);
50-
51-
DeviceContext& m_context;
52-
SafeTimespanGuarantor::SafetyGuarantee m_guarantee;
53-
};
54-
55-
class Update
56-
{
57-
public:
58-
continuation_scheduler<>& Scheduler()
59-
{
60-
return m_safeTimespanGuarantor.OpenScheduler();
61-
}
62-
63-
UpdateToken GetUpdateToken()
64-
{
65-
return {m_context, m_safeTimespanGuarantor};
66-
}
57+
FrameCompletionScope(FrameCompletionScope&&) noexcept;
58+
~FrameCompletionScope();
6759

6860
private:
6961
friend class DeviceContext;
70-
71-
Update(SafeTimespanGuarantor& safeTimespanGuarantor, DeviceContext& context)
72-
: m_safeTimespanGuarantor{safeTimespanGuarantor}
73-
, m_context{context}
74-
{
75-
}
76-
77-
SafeTimespanGuarantor& m_safeTimespanGuarantor;
78-
DeviceContext& m_context;
62+
FrameCompletionScope(DeviceImpl&);
63+
DeviceImpl* m_impl;
7964
};
8065

8166
class DeviceContext
@@ -93,7 +78,19 @@ namespace Babylon::Graphics
9378
continuation_scheduler<>& BeforeRenderScheduler();
9479
continuation_scheduler<>& AfterRenderScheduler();
9580

96-
Update GetUpdate(const char* updateName);
81+
// Scheduler that fires when StartRenderingCurrentFrame ticks the frame start dispatcher.
82+
// Use this to schedule work (e.g., requestAnimationFrame callbacks) that should run each frame.
83+
continuation_scheduler<>& FrameStartScheduler();
84+
85+
// Acquire a scope that prevents FinishRenderingCurrentFrame from completing.
86+
// The scope must be held while JS frame callbacks are running.
87+
FrameCompletionScope AcquireFrameCompletionScope();
88+
89+
// Active encoder for the current frame. Managed by DeviceImpl in
90+
// StartRenderingCurrentFrame/FinishRenderingCurrentFrame.
91+
// Used by NativeEngine, Canvas, and NativeXr.
92+
void SetActiveEncoder(bgfx::Encoder* encoder);
93+
bgfx::Encoder* GetActiveEncoder();
9794

9895
void RequestScreenShot(std::function<void(std::vector<uint8_t>)> callback);
9996
void SetRenderResetCallback(std::function<void()> callback);
@@ -113,7 +110,7 @@ namespace Babylon::Graphics
113110
using CaptureCallbackTicketT = arcana::ticketed_collection<std::function<void(const BgfxCallback::CaptureData&)>>::ticket;
114111
CaptureCallbackTicketT AddCaptureCallback(std::function<void(const BgfxCallback::CaptureData&)> callback);
115112

116-
bgfx::ViewId AcquireNewViewId(bgfx::Encoder&);
113+
bgfx::ViewId AcquireNewViewId();
117114

118115
// TODO: find a different way to get the texture info for frame capture
119116
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
122119
static bx::AllocatorI& GetDefaultAllocator() { return m_allocator; }
123120

124121
private:
125-
friend UpdateToken;
126-
127122
DeviceImpl& m_graphicsImpl;
128123

129124
std::unordered_map<uint16_t, TextureInfo> m_textureHandleToInfo{};

Core/Graphics/InternalInclude/Babylon/Graphics/FrameBuffer.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ namespace Babylon::Graphics
3333
uint16_t Height() const;
3434
bool DefaultBackBuffer() const;
3535

36-
void Bind(bgfx::Encoder& encoder);
37-
void Unbind(bgfx::Encoder& encoder);
36+
void Bind();
37+
void Unbind();
3838

3939
void Clear(bgfx::Encoder& encoder, uint16_t flags, uint32_t rgba, float depth, uint8_t stencil);
40-
void SetViewPort(bgfx::Encoder& encoder, float x, float y, float width, float height);
41-
void SetScissor(bgfx::Encoder& encoder, float x, float y, float width, float height);
40+
void SetViewPort(float x, float y, float width, float height);
41+
void SetScissor(float x, float y, float width, float height);
4242
void Submit(bgfx::Encoder& encoder, bgfx::ProgramHandle programHandle, uint8_t flags);
4343
void SetStencil(bgfx::Encoder& encoder, uint32_t stencilState);
4444
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
4848

4949
private:
5050
Rect GetBgfxScissor(float x, float y, float width, float height) const;
51-
void SetBgfxViewPortAndScissor(bgfx::Encoder& encoder, const Rect& viewPort, const Rect& scissor);
51+
void SetBgfxViewPortAndScissor(const Rect& viewPort, const Rect& scissor);
5252

5353
DeviceContext& m_deviceContext;
5454
const uintptr_t m_deviceID{};

Core/Graphics/InternalInclude/Babylon/Graphics/SafeTimespanGuarantor.h

Lines changed: 0 additions & 56 deletions
This file was deleted.

Core/Graphics/Source/Device.cpp

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -66,19 +66,6 @@ namespace Babylon::Graphics
6666
m_impl->DisableRendering();
6767
}
6868

69-
DeviceUpdate Device::GetUpdate(const char* updateName)
70-
{
71-
auto& guarantor = m_impl->GetSafeTimespanGuarantor(updateName);
72-
return {
73-
[&guarantor] {
74-
guarantor.Open();
75-
},
76-
[&guarantor](std::function<void()> callback) {
77-
guarantor.CloseScheduler()(std::move(callback));
78-
guarantor.RequestClose();
79-
}};
80-
}
81-
8269
void Device::StartRenderingCurrentFrame()
8370
{
8471
m_impl->StartRenderingCurrentFrame();

0 commit comments

Comments
 (0)