Skip to content

Commit 68e3506

Browse files
bghgaryCopilot
andcommitted
Apps: fix dispatch ordering merge artifact between BabylonJS#1652 and BabylonJS#1646
In BabylonJS#1674's combination of rework-thread-model (BabylonJS#1652) and the ExternalTexture sync API (BabylonJS#1646), three apps swapped the original async AddToContextAsync flow for synchronous CreateForJavaScript but kept the two-frame skeleton from the async version. The result is that the loader.Dispatch which runs the JS startup() callback now gets queued in frame 1 even though its observable wait (startup.get_future().wait()) is in frame 2 — the dispatch can land in either frame depending on JS-thread scheduling, breaking the "each Start/Finish pair wraps a phase of work" pattern. On rework-thread-model the JS thread will block on the closed gate between frames 1 and 2 anyway, so it functionally works, but the ordering is wrong. Move the dispatch into the second frame so the texture creation, startup() call, and the wait that observes them all run inside the same frame: frame 1: load scripts (no dispatch) frame 2: dispatch[startup] -> wait frame 3+: per-asset / render-loop Three apps had this issue; UnitTests' ExternalTexture tests already do it correctly. On bare rework-thread-model these apps still use AddToContextAsync and don't have the bug. Files: - Apps/HeadlessScreenshotApp/Win32/App.cpp - Apps/PrecompiledShaderTest/Source/App.cpp - Apps/StyleTransferApp/Win32/App.cpp Local build verification (Win32 RelWithDebInfo): - HeadlessScreenshotApp: builds cleanly - StyleTransferApp: builds cleanly - PrecompiledShaderTest: not configured in this Build/Win32; change is structurally identical to HeadlessScreenshotApp which compiled. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 55f1461 commit 68e3506

3 files changed

Lines changed: 30 additions & 24 deletions

File tree

Apps/HeadlessScreenshotApp/Win32/App.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,15 @@ int main()
127127
// Create a render target texture for the output.
128128
winrt::com_ptr<ID3D11Texture2D> outputTexture = CreateD3DRenderTargetTexture(d3dDevice.get());
129129

130+
// Close the script-load frame.
131+
deviceUpdate.Finish();
132+
device.FinishRenderingCurrentFrame();
133+
134+
// Open a new frame for `startup` so the JS-side resource creation and
135+
// startup() call run in the same frame as the wait that observes them.
136+
device.StartRenderingCurrentFrame();
137+
deviceUpdate.Start();
138+
130139
std::promise<void> startup{};
131140

132141
// Create an external texture for the render target texture and pass it to
@@ -142,17 +151,10 @@ int main()
142151
startup.set_value();
143152
});
144153

145-
deviceUpdate.Finish();
146-
device.FinishRenderingCurrentFrame();
147-
148-
// Reopen the gate so JS can continue running (startup may issue bgfx commands).
149-
device.StartRenderingCurrentFrame();
150-
deviceUpdate.Start();
151-
152154
// Wait for `startup` to finish.
153155
startup.get_future().wait();
154156

155-
// Close the frame opened above.
157+
// Close the startup frame.
156158
deviceUpdate.Finish();
157159
device.FinishRenderingCurrentFrame();
158160

Apps/PrecompiledShaderTest/Source/App.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,15 @@ int RunApp(
135135
Babylon::ScriptLoader loader{runtime};
136136
loader.LoadScript("app:///index.js");
137137

138+
// Close the script-load frame.
139+
deviceUpdate.Finish();
140+
device.FinishRenderingCurrentFrame();
141+
142+
// Open a new frame for `startup` so the JS-side resource creation and
143+
// startup() call run in the same frame as the wait that observes them.
144+
device.StartRenderingCurrentFrame();
145+
deviceUpdate.Start();
146+
138147
std::promise<void> startup{};
139148

140149
// Create an external texture for the render target texture and pass it to
@@ -150,17 +159,10 @@ int RunApp(
150159
startup.set_value();
151160
});
152161

153-
deviceUpdate.Finish();
154-
device.FinishRenderingCurrentFrame();
155-
156-
// Reopen the gate so JS can continue running (startup may issue bgfx commands).
157-
device.StartRenderingCurrentFrame();
158-
deviceUpdate.Start();
159-
160162
// Wait for `startup` to finish.
161163
startup.get_future().wait();
162164

163-
// Close the frame opened above.
165+
// Close the startup frame.
164166
deviceUpdate.Finish();
165167
device.FinishRenderingCurrentFrame();
166168

Apps/StyleTransferApp/Win32/App.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,15 @@ int APIENTRY wWinMain(_In_ HINSTANCE hInstance,
334334
loader.LoadScript("app:///Scripts/babylonjs.loaders.js");
335335
loader.LoadScript("app:///Scripts/index.js");
336336

337+
// Close the script-load frame.
338+
g_update->Finish();
339+
g_device->FinishRenderingCurrentFrame();
340+
341+
// Open a new frame for `startup` so the JS-side resource creation and
342+
// startup() call run in the same frame as the wait that observes them.
343+
g_device->StartRenderingCurrentFrame();
344+
g_update->Start();
345+
337346
std::promise<void> startup{};
338347

339348
// Create an external texture for the render target texture and pass it to
@@ -349,17 +358,10 @@ int APIENTRY wWinMain(_In_ HINSTANCE hInstance,
349358
startup.set_value();
350359
});
351360

352-
g_update->Finish();
353-
g_device->FinishRenderingCurrentFrame();
354-
355-
// Reopen the gate so JS can continue running (startup may issue bgfx commands).
356-
g_device->StartRenderingCurrentFrame();
357-
g_update->Start();
358-
359361
// Wait for `startup` to finish.
360362
startup.get_future().wait();
361363

362-
// Close the frame opened above.
364+
// Close the startup frame.
363365
g_update->Finish();
364366
g_device->FinishRenderingCurrentFrame();
365367

0 commit comments

Comments
 (0)