Map previousWorld0-3 so instanced motion vectors are correct

[bkaradzic-microsoft]

Fixes the nightly Playground validation failures on the three motion blur tests.

Problem

A mesh rendered with object based motion blur (or prepass velocity) declares previousWorld0-3 alongside world0-3, so an instanced or thin-instanced mesh has 8 per-instance vec4 attributes, 9 with instanceColor.

previousWorld0-3 had no built-in mapping in the shader compiler, and VertexArray::RecordVertexBuffer rejected anything past the 5th instanced buffer (Number of vertex buffer instances greater than 4 is not supported). Those buffers were never recorded, the shader read a zero previous world matrix, and produced a huge bogus velocity that smeared the whole object.

This is what the nightly has been failing on since MRT support (#1754) enabled object based motion blur on Native.

Why slots are now assigned per shader

The i_data slots cannot come from a fixed per-name table. bgfx requires the used slots to form a contiguous run starting at i_data0:

• the D3D11 input layout declares TEXCOORD31 down to TEXCOORD(31 - N + 1) at 16-byte-dense offsets;
• the GL path compacts the i_data locations it finds.

Since the declared set varies (world0-3 alone, plus instanceColor, plus previousWorld0-3), a fixed table leaves a hole in the run and every attribute past the hole reads zero.

So slots are assigned per shader from the set that shader actually declares, in reverse name order: alphabetically first gets the highest slot, last gets i_data0. That is dense by construction, stable between the base program compile and any instanced variant (identical declared set), and matches BuildInstanceDataBuffer, which packs the recorded buffers by descending attribute location.

It also reproduces the previous fixed D3D table exactly for every set that existed before previousWorld0-3 (world0-3 → i_data3..0, +instanceColor → i_data4, splatIndex0-3 → i_data3..0), so nothing else changes. OpenGL and Metal already assigned densely; they now share the same map instead of each counting attributes themselves.

The instanced buffer limit is raised to bgfx's real BGFX_CONFIG_MAX_INSTANCE_DATA_COUNT (16 — its doc comment still claims 5), and the off-by-one is fixed: the check ran before the insert, so it rejected the 5th buffer rather than the 6th.

Validation

Playground validation, Win32 D3D11, Babylon.js 9.21.2:

Test	before	after
Thin instances + dynamic buffer resize	3.346%	2.478%
Thin instances + render self motion blur	2.844%	2.355%
Instances + render self motion blur	2.531%	2.061%

All now under the 2.5% allowance, and the instance-limit errors go from 9 to 0. The numbers are bit-identical on hardware D3D11 and on forced WARP, so they are not adapter-flaky.

A full 720 test A/B sweep shows no other test changing by more than 0.001%, and an identical set of pre-existing hangs on both builds.

Known remaining difference

A smaller residual diff remains on these three tests, from a separate bug: Babylon.js's bindAttachments is a no-op on Native, so the scene clear is applied to every MRT attachment and wipes the velocity attachment's alpha-0 background. The motion blur shader multiplies velocity by that alpha, so the background gets a fixed non-zero velocity and moving objects get a faint halo. Fixing it needs a matching Babylon.js change plus per-attachment clear masking here, so it is not addressed in this PR.

The worst test passes at 2.478% against a 2.5% allowance, so there is little headroom until that second bug is fixed.

[Copilot]

Pull request overview

This PR fixes incorrect instanced motion vectors by adding built-in shader compiler handling for previousWorld0-3 and ensuring per-instance i_data slots are assigned densely per shader (as required by bgfx). It also lifts the NativeEngine instanced-buffer recording limit to match bgfx’s actual instance-data slot capacity.

Changes:

• Add previousWorld0-3 as built-in instanced attributes and assign built-in instance i_data slots per shader from the declared set (dense, stable mapping across variants).
• Add/align compile-time guards for instance-data slot/location invariants and maximum slot counts.
• Increase the NativeEngine instanced vertex-buffer instance limit to MAX_INSTANCE_DATA_SLOT_COUNT and fix the prior off-by-one behavior.

Reviewed changes

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

Show a summary per file

File	Description
Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.cpp	Adds previousWorld0-3 built-in instance detection and introduces per-shader dense built-in i_data slot assignment used across GL/Metal/D3D traversers.
Plugins/ShaderCompiler/Source/ShaderCompilerCommon.cpp	Updates static asserts/comments for the built-in instance-data location boundary and ensures built-in slot count doesn’t exceed max slot count.
Plugins/NativeEngine/Source/VertexArray.cpp	Raises instanced buffer instance limit to the bgfx max slot count and updates the thrown error message accordingly.
Plugins/NativeEngine/Source/NativeEngine.cpp	Updates documentation/comments for built-in instanced attribute rerouting behavior and the new built-in set.
Core/Graphics/InternalInclude/Babylon/Graphics/BgfxShaderInfo.h	Introduces MAX_INSTANCE_DATA_SLOT_COUNT, increases BUILTIN_INSTANCE_DATA_SLOT_COUNT to cover previousWorld0-3, and adjusts built-in location boundaries.

---

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

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.