Remove DeviceUpdate usage and mark the shim deprecated#1653
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the deprecated Babylon::Graphics::DeviceUpdate compatibility shim and updates samples/apps/tests to rely solely on Device::StartRenderingCurrentFrame() / Device::FinishRenderingCurrentFrame() for frame synchronization.
Changes:
- Deleted
DeviceUpdateandDevice::GetUpdate()from the public graphics device header. - Removed
DeviceUpdateusage across unit tests and sample applications. - Updated ExternalTexture documentation examples to no longer reference
DeviceUpdate.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Plugins/ExternalTexture/Readme.md | Removes DeviceUpdate calls from the D3D12 example snippet. |
| Core/Graphics/Include/Shared/Babylon/Graphics/Device.h | Removes deprecated DeviceUpdate type and GetUpdate() API. |
| Apps/UnitTests/Source/Tests.UniformPadding.cpp | Drops DeviceUpdate usage around frame start/finish. |
| Apps/UnitTests/Source/Tests.ShaderCache.cpp | Drops DeviceUpdate usage in frame-pumping loop. |
| Apps/UnitTests/Source/Tests.ExternalTexture.D3D11.cpp | Drops DeviceUpdate usage (minor formatting/spelling nits remain). |
| Apps/UnitTests/Source/Tests.ExternalTexture.cpp | Drops DeviceUpdate usage around frame boundaries. |
| Apps/UnitTests/Source/Tests.Device.D3D11.cpp | Drops DeviceUpdate usage inside back-buffer iteration. |
| Apps/StyleTransferApp/Win32/App.cpp | Removes g_update lifecycle and frame sync calls. |
| Apps/PrecompiledShaderTest/Source/App.cpp | Removes deviceUpdate from render/unblock sequencing. |
| Apps/Playground/X11/App.cpp | Removes DeviceUpdate() calls from the main loop tick. |
| Apps/Playground/Win32/App.cpp | Removes DeviceUpdate() calls from loop and suspend/resume paths. |
| Apps/Playground/visionOS/LibNativeBridge.mm | Removes DeviceUpdate() calls from resize/render callbacks. |
| Apps/Playground/UWP/App.cpp | Removes DeviceUpdate() calls from run loop and suspend/resume. |
| Apps/Playground/Shared/AppContext.h | Removes DeviceUpdate() accessor and stored optional. |
| Apps/Playground/Shared/AppContext.cpp | Removes m_deviceUpdate initialization/teardown and usage. |
| Apps/Playground/macOS/ViewController.mm | Removes DeviceUpdate() calls from Metal view callbacks. |
| Apps/Playground/iOS/LibNativeBridge.mm | Removes DeviceUpdate() calls from resize/render entrypoints. |
| Apps/Playground/Android/BabylonNative/src/main/cpp/BabylonNativeJNI.cpp | Removes DeviceUpdate() calls from JNI render tick. |
| Apps/HeadlessScreenshotApp/Win32/App.cpp | Removes deviceUpdate usage from headless render flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bkaradzic-microsoft
force-pushed
the
remove-device-update
branch
2 times, most recently
from
98212e9 to
758fcdc
Compare
bkaradzic-microsoft
force-pushed
the
rework-thread-model
branch
from
9f25ed5 to
0b8bfad
Compare
bkaradzic-microsoft
force-pushed
the
rework-thread-model
branch
from
0b8bfad to
e97bfdc
Compare
bghgary
added a commit
that referenced
this pull request
Jun 9, 2026
[Updated by Copilot on behalf of @bghgary] ## Context Replaces the old per-thread encoder model (`GetEncoderForThread`/`EndEncoders` + `UpdateToken` wrapping `SafeTimespanGuarantor`) with a **single frame encoder** owned by `DeviceImpl`, synchronized by a `FrameCompletionScope` RAII gate. `SafeTimespanGuarantor`, `UpdateToken`, and the `Update` classes are removed. `DeviceUpdate` remains as a no-op shim — removing it is an API break and a separate PR (#1653). ## Model `StartRenderingCurrentFrame` acquires the encoder (`bgfx::begin`) and opens the gate; `FinishRenderingCurrentFrame` waits for all `FrameCompletionScope`s to release, closes the gate, ends the encoder, and submits the frame. All consumers (NativeEngine, Canvas, NativeXr) read the shared encoder via `GetActiveEncoder()`. `SubmitCommands` and `ReadTexture` each hold a stack-scoped `FrameCompletionScope`, so the encoder can't be ended mid-work; when invoked outside a frame (e.g. an XHR callback) the scope blocks until the next frame opens. ``` Main Thread JS Thread bgfx render thread StartRenderingCurrentFrame() m_frameEncoder = bgfx::begin(true) m_frameBlocked = false; CV notify tick frameStartDispatcher -----> RAF callbacks submitCommands() AcquireFrameCompletionScope (immediate; frame open) GetEncoder() -> m_frameEncoder; draw... scope released; pendingScopes-- -> CV notify FinishRenderingCurrentFrame() wait scopes == 0; m_frameBlocked = true tick beforeRenderDispatcher bgfx::end(m_frameEncoder) Frame() -> bgfx::frame() ------------------------------> GPU submit & render tick afterRenderDispatcher ``` ## Synchronization rules 1. Single encoder, acquired before the gate opens (mutex gives memory ordering) and ended only after all scopes release. 2. `SubmitCommands`/`ReadTexture` always hold a scope — the encoder can't be ended mid-command, regardless of microtask draining or reentrancy. 3. Apps must bracket any main-thread wait for JS work with `Start`/`Finish`, or `SubmitCommands` blocks forever waiting for the gate (HeadlessScreenshotApp, StyleTransferApp, UnitTests do this). 4. `Canvas::Flush` acquires a scope when called outside a frame, then discards residual encoder state before nanovg rendering (the shared encoder carries NativeEngine state). ## Notes - `Tests.ExternalTexture.Msaa.cpp` opens the next frame before waiting on `startup()` — a workaround for the legacy async `AddToContextAsync` pattern, obsoleted once #1646's sync `CreateForJavaScript` lands. - `Tests.ExternalTexture.RestoreAfterDeviceLoss` (added on master after this branch) uses the same async pattern; the same frame-pump workaround is applied here. Obsoleted once #1646's sync `CreateForJavaScript` lands. - bgfx: added `m_encoderBeginLock` in `bgfx::frame()` to prevent a deadlock with `bgfx::destroy()`. - Follow-up: removing the `DeviceUpdate` no-op shim is an API-breaking change handled in #1653. --------- Co-authored-by: Branimir Karadzic <branimirkaradzic@gmail.com> Co-authored-by: Gary Hsu <bghgary@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkaradzic-microsoft
force-pushed
the
remove-device-update
branch
from
758fcdc to
6ca1cca
Compare
bkaradzic-microsoft
force-pushed
the
remove-device-update
branch
2 times, most recently
from
6ddbf11 to
fa082bd
Compare
bkaradzic-microsoft
changed the title
DeviceUpdate became a no-op compatibility shim when the threading model was reworked (#1652) -- frame synchronization is now handled by FrameCompletionScope inside StartRenderingCurrentFrame/ FinishRenderingCurrentFrame. This removes all of its now-redundant callers across the apps, platform shells (Win32/UWP/X11/Android/iOS/macOS/visionOS), and unit tests, and marks DeviceUpdate::Start/Finish/RequestFinish and Device::GetUpdate [[deprecated]] so any remaining external callers get a warning. The shim itself is kept for source compatibility and will be removed in a future PR. The type is left un-deprecated so GetUpdate can keep returning it without a self-warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkaradzic-microsoft
force-pushed
the
remove-device-update
branch
from
fa082bd to
3123f8a
Compare
ryantrem
approved these changes
Jun 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DeviceUpdatebecame a no-op compatibility shim when the threading model was reworked (#1652) — frame synchronization is now handled byFrameCompletionScopeinsideStartRenderingCurrentFrame/FinishRenderingCurrentFrame.This PR:
DeviceUpdate::Start/Finish/RequestFinishandDevice::GetUpdate[[deprecated]]so any remaining external callers get a warning. The type itself is left un-deprecated soGetUpdatecan keep returning it without a self-warning (no compiler-specific pragmas needed). The shim implementation is kept for source compatibility and will be removed in a future PR.Rebuilt fresh on top of current
master(the original branch predated the squash-merge of #1652). Verified locally on Win32: Playground and UnitTests build clean with no deprecation-warning leakage, and all UnitTests pass.