Release native vertex-array layout references

[bkaradzic-microsoft]

Summary

Extracts the focused vertex-layout lifetime repair from the BabylonNative shotgun worktree, independently based on official master b8c93d24.

Each bgfx::createVertexLayout call acquires a reference, including when a matching layout already exists. VertexArray previously dropped its records without releasing these references, and duplicate attribute insertion could acquire another reference before throwing.

• Release owned layout references on disposal/destruction, once only.
• Reject duplicate attributes before allocating, balance insertion-failure cleanup, and report layout allocation failure.
• Reject recording into a disposed array.
• Track the device generation, as vertex/index buffers already do, so stale arrays cannot destroy reused handles after device loss.
• Add seven regressions covering lifecycle, duplicates, sharing, sequential allocation, device loss, and exhaustion/recovery.

Validation

Built this isolated branch with the official dependency pins, macOS/JavaScriptCore, RelWithDebInfo, and BABYLON_NATIVE_TESTS_USE_NOOP_METAL_DEVICE=ON.

UnitTests --gtest_filter="NativeEngineVertexArray.*": 7 passed. These exercise bgfx resource accounting with the Noop renderer; this is not a claim of a fresh GPU rendering sweep.

No bgfx/JsRuntimeHost changes, dependency updates, visual references, tolerances, or exclusions are included.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 2 · 1

Open (3)

• This test relies on directly mutating bgfx::VertexLayout internals (layout.m_stride) to force… · New
• try_emplace can fail without throwing (e.g., if the key already exists). In that case,… · New
• The new error message is accurate but not very actionable when diagnosing layout exhaustion or… · New

What changed in this PR

Fixes VertexArray lifetime management so bgfx::VertexLayoutHandle references are correctly released, including in failure paths and across device loss, and adds regression tests to prevent reintroducing leaks/exhaustion.

Changes:

• Make VertexArray track DeviceContext + device generation and release owned layout references on dispose/destruct (once).
• Prevent duplicate attribute recording before allocation, clean up on insertion failure, and throw when layout allocation fails.
• Add unit tests validating layout accounting across lifecycle, sharing, duplicates, exhaustion/recovery, and device loss.

File	Description
Plugins/​NativeEngine/​Source/​VertexArray.h	Makes VertexArray device-context-aware by storing DeviceContext + device id.
Plugins/​NativeEngine/​Source/​VertexArray.cpp	Implements layout-handle release on disposal, duplicate rejection, disposed-guard, and allocation failure handling.
Plugins/​NativeEngine/​Source/​NativeEngine.cpp	Updates VertexArray construction to pass m_deviceContext.
Apps/​UnitTests/​Source/​Tests.NativeEngine.VertexArray.cpp	Adds 7 regression tests for layout lifecycle/accounting and device loss.
Apps/​UnitTests/​CMakeLists.txt	Registers the new VertexArray test file in the unit test target.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 1 · 1

Open (3)

• VertexArray now captures m_deviceId and Dispose() conditionally destroys layouts only when… · New
• This block does the right cleanup, but the control flow is more complex than necessary given the… · New
• The static_cast<void>(attrib); is only needed because the structured binding names attrib but… · New

Resolved since last review (3)

• try_emplace can fail without throwing (e.g., if the key already exists). In that case,…
• This test relies on directly mutating bgfx::VertexLayout internals (layout.m_stride) to force…
• The new error message is accurate but not very actionable when diagnosing layout exhaustion or…

[bkaradzic-microsoft]

CI follow-up: all five failing Linux jobs in run 35795505850 failed compiling the new test at gtest-param-test.h:360 because X11 defines Bool before GoogleTest declares testing::Bool(). Commit 0dc7bf2 moves GoogleTest ahead of platform headers, matching the existing unit tests. I reproduced the exact macro/compiler error with the pinned GoogleTest headers and verified the reordered includes compile. The commit also addresses the stale-device review finding. All eight focused lifecycle regressions pass locally on macOS/JSC with the Noop renderer.

Update: the replacement CI run 35883901384 has completed successfully. All 35 PR checks now pass, including all five Linux jobs that previously failed.

[bghgary]

[Reviewed by Copilot on behalf of @bghgary]

LGTM