Preserve queued instance data during buffer updates

[bkaradzic-microsoft]

Root cause

updateDynamicVertexBuffer mutates native vertex-buffer data immediately, while draw commands can still be queued in the JavaScript command stream. Instance attributes are copied when those commands are decoded, so a later update can overtake an earlier draw and make it use the new values.

The command-stream pointer persists between frames. Empty submissions therefore must not acquire FrameCompletionScope, whose contract intentionally waits for the next active frame.

Fix

Submit pending commands before mutating a dynamic vertex buffer, preserving the data visible when each queued draw was issued. SubmitCommands first flushes the JavaScript staging buffer and returns when it is empty; it acquires frame lifetime protection only when commands require decoding.

Regression coverage

• Queues a thin-instance draw with matrix translation 0.25, then updates it to 0.75; captured red must remain 64.
  • Before fix: red was 191.
  • After fix: red is 64.
• Issues an empty-stream dynamic vertex-buffer update between frames.
  • Before follow-up: remained blocked until another frame started.
  • After follow-up: completes without waiting for a frame.
• Final focused run: 8/8 passed (NativeEngineInstanceData.*, ShaderCompilation.FragCoord*, and Device.FrameCompletionScopeProvidesEncoderOutsideFrame).
• Build: cmake --build build\win32 --target UnitTests --config Debug -- /m:2

This branch uses upstream dependency declarations unchanged and imports no shotgun dependency pins, visual-test enables, or tolerance changes.

[Copilot]

🟡 Changes recommended

Command submission may block, hide failures, and incur unnecessary synchronous work.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Preserves queued instance data during dynamic vertex-buffer updates and adds GPU regression coverage.

Changes:

• Flushes queued commands before buffer mutation.
• Adds thin-instance matrix regression coverage and test setup support.

File summaries

File	Description
Plugins/NativeEngine/Source/NativeEngine.cpp	Submits commands before dynamic buffer updates.
Apps/UnitTests/Source/Tests.ShaderCompilation.FragCoord.cpp	Adds regression test coverage.

Review details

Suppressed comments (2)

Plugins/NativeEngine/Source/NativeEngine.cpp:1270

• Because SubmitCommands is inside this try, any exception while decoding a queued command is caught by the update handler's catch (...) and reduced to Failed to update vertex buffer. The command stream can therefore be discarded while the JavaScript caller receives no submission failure; keep the flush outside this buffer-update error handler or handle its exception separately so command errors propagate.

            if (m_commandStream)
            {
                SubmitCommands(info);

Plugins/NativeEngine/Source/NativeEngine.cpp:1270

• This condition only checks whether a command stream is attached, so every dynamic vertex-buffer update—including updates with no queued commands—now runs SubmitCommands. That path still dispatches a frame-completion scope, invokes the stream's flush callback, and acquires the reader, which can make frequent per-frame/thin-instance updates pay an unnecessary synchronous submission cost. Please add a cheap pending-command check (or otherwise coalesce updates) and submit only when the stream contains work.

            if (m_commandStream)
            {
                SubmitCommands(info);

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

[bkaradzic-microsoft]

Follow-up on the two suppressed notes from the original Copilot review:

1. The exception concern was valid. A negative control throwing Napi::Error at the nested submission site logged the message but let the regression pass, proving the outer vertex-update catch swallowed it. dd2908ad moves SubmitCommands outside that handler: the same control now terminates through the JS error path (exit 3), while native buffer-update failures retain the existing logging convention.
2. Empty-stream scope acquisition was already addressed by 7a00f293. GetReader() must remain before the empty check because it invokes the JS stream's flush callback, transferring staged words into the native buffer; a native-only pending check before that flush could incorrectly skip real commands. Empty readers return before acquiring FrameCompletionScope.

Restored focused validation: 8/8 tests passed.