Honor instance counts without per-instance vertex attributes by bkaradzic-microsoft · Pull Request #1888 · BabylonJS/BabylonNative

bkaradzic-microsoft · 2026-09-17T20:34:10Z

Summary

Call setInstanceCount when a draw requests instances but provides no per-instance vertex buffer, on backends supporting BGFX_CAPS_VERTEX_ID.
Preserve buffered instancing and ordinary draw behavior. Without the explicit count, bgfx draws only one instance even when the shader uses gl_InstanceID.
Add a GPU regression using the existing Mesh.forcedInstanceCount API: indexed and non-indexed geometry, ordinary draws, one instance, and three independently colored instances.
Require runtime BGFX_CAPS_VERTEX_ID, respect no-op Metal / disabled-render-test configurations, and wait for RTT-pass readiness without warmup renders. Vulkan remains enabled.
Correct Vulkan SPIR-V interface locations and vertex/instance built-ins exposed by the Linux regression.
Keep private bgfx capability probing out of the installed-SDK compile fixture; internal UnitTests retain the GPU coverage.

Independence

Based on master at bb01e283, with published Babylon.js 9.21.2 and stock bgfx. No protocol, dependency-pin, fixture, tolerance, compute, or storage-buffer changes.

This fixes the underlying draw behavior; it does not import or claim the separate JavaScript shader changes needed by some clustered-lighting scenes.

CI follow-up

d32b2f0e fixes the installation failures: shared JavaScript test code must also compile against the public SDK, which intentionally does not export private bgfx headers. Only the installed-consumer fixture omits the private capability probe. Actual Windows/D3D11 and Linux/OpenGL installed-SDK consumers both build without source-tree include paths or installed bgfx headers.

7f7b0083 fixes the reproduced Linux Vulkan failure. RTT readiness alone did not fix the count-zero control: the emitted SPIR-V lacked inter-stage Location decorations and used OpenGL-only VertexId/InstanceId built-ins. Preserve ordinary GLSL parsing, assign matching varying locations by name, reserve explicit locations and complete matrix/array ranges, assign fragment output locations, and emit Vulkan VertexIndex/InstanceIndex. The pinned GLSLANG_WEB build omits glslang's IO mapper, so this uses a Vulkan-local AST pass without changing dependencies or other backends.

Validation

The two initial direct compiler regressions fail before the compiler fix. Three final regressions cover reversed declaration order, explicit locations on either/both stages, matrix arrays, vertex-only outputs, fragment outputs, and both index built-ins.
Both emitted diagnostic shader modules pass spirv-val --target-env vulkan1.0.
Linux/JavaScriptCore/Vulkan/lavapipe: CI-equivalent native suite has 23 passes and 19 existing skips; JavaScript suite has 66 passes and 27 optional codec cases pending (plugins disabled).
Indexed/non-indexed 0/1/3-count GPU controls pass. Removing only setInstanceCount with the repaired compiler still fails count 3; restoring it passes.
Unchanged Vulkan CI visual controls pass: Capsule and Multi camera rendering.
Windows/D3D11/Chakra: all 10 selected JavaScript and shader-compilation cases pass. The GPU regression also previously passed Windows Vulkan.
No renderer exclusion, reference change, pixel-tolerance relaxation, or warmup render was added. The existing Vulkan CI exclusion for CompileComprehensiveGLSL remains unchanged (upstream Make CompileComprehensiveGLSL complete on Linux Vulkan #1877).

Interface-block review follow-up

9c024dfe keys linked interface blocks by block type rather than stage-local instance name. A regression with different instance names, a matrix member, and implicit / one-sided explicit block locations fails before and passes after in the existing BABYLON_NATIVE_DISABLE_WEBMIN=ON configuration. The default parser rejects shader IO blocks, so this new syntax-specific case explicitly skips there; all existing Vulkan shader and GPU controls remain enabled and pass in both configurations.

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

This PR fixes attribute-less instancing by explicitly passing the requested instance count to bgfx when no per-instance vertex buffer is present, and adds a native GPU regression test.

Changes:

Calls setInstanceCount for supported backends using BGFX_CAPS_VERTEX_ID.
Adds a JavaScript test covering indexed and non-indexed meshes with 0, 1, and 3 instances.
Adds a native-rendering availability flag and includes the new test in the JavaScript suite.

Reviewed changes

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

Show a summary per file

File	Description
`Plugins/NativeEngine/Source/VertexArray.cpp`	Supplies explicit instance counts for attribute-less draws.
`Apps/UnitTests/Source/Tests.JavaScript.cpp`	Exposes whether native rendering tests should run.
`Apps/UnitTests/JavaScript/src/tests.nativeEngine.attributeLessInstancing.ts`	Adds the GPU regression coverage.
`Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts`	Registers the new test suite.
`Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js`	Includes the compiled test bundle changes.

💡 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 4 out of 5 changed files in this pull request and generated 4 comments.

Copilot

🟡 Changes recommended

Vulkan location assignment incorrectly matches interface blocks by instance name, potentially breaking valid cross-stage linkage.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 8/9 changed files
Comments generated: 1
Review effort level: Balanced

Copilot

🔵 Needs a closer look

Low-level Vulkan shader-interface rewriting and backend-specific rendering behavior warrant final human review.

Review details

Files reviewed: 8/9 changed files
Comments generated: 0 new
Review effort level: Balanced

Regenerate the checked-in JavaScript test bundle to include the attribute-less instancing regressions. This supplies the runnable artifact for the source tests on the code branch without mixing generated JavaScript into the native implementation commit. Review-Group: E1 Source: PR BabylonJS#1888 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae Split-From: a79dba316c9e38f9597fa4bf16f652551a20a864 Split-Part: support

Tell bgfx the explicit count when a draw derives its instance data from gl_InstanceID instead of an instance buffer. Preserve the existing buffered-instance path and gate the call on BGFX_CAPS_VERTEX_ID. Add indexed/non-indexed GPU controls for ordinary draws and forced counts of one and three, using distinct colors for all three instances. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae

Require BGFX_CAPS_VERTEX_ID at runtime in addition to the existing no-op Metal and disabled-render-test configuration guards. Wait for the render target's own pass readiness before its single draw, with an explicit timeout and no warmup renders. Report all three pixel centers together when a draw fails. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae

Mark the installation compile fixture explicitly so its shared JavaScript test source does not include private bgfx headers. Internal UnitTests keep the runtime capability check and GPU regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae

Assign linked varying locations by name while preserving explicit locations and reserving complete array/matrix ranges. Assign fragment output locations and translate GLSL vertex/instance IDs to Vulkan index built-ins. The Linux lavapipe regression failed even for an ordinary count-zero draw because the previous SPIR-V lacked varying locations and used OpenGL-only ID decorations. Keep ordinary GLSL parsing and stock dependency pins. Add direct emitted-SPIR-V regressions for reversed declaration order, one-sided explicit locations, matrix arrays, vertex-only outputs, fragment outputs, and vertex/instance index built-ins. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae

Use the linked block type name as the location key, matching glslang's getAccessName semantics while retaining ordinary varying names. Vertex and fragment stages can use different block instance names. Add a regression with different instance names, a matrix member, an ordinary varying, and an explicit location on only the fragment block. Exercise it with the existing BABYLON_NATIVE_DISABLE_WEBMIN option; the default minified parser does not support shader IO blocks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae

Regenerate the checked-in JavaScript test bundle to include the attribute-less instancing regressions. This supplies the runnable artifact for the source tests on the code branch without mixing generated JavaScript into the native implementation commit. Review-Group: E1 Source: PR BabylonJS#1888 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae Split-From: a79dba316c9e38f9597fa4bf16f652551a20a864 Split-Part: support

Submit the requested instance count when a draw derives its per-instance data from gl_InstanceID instead of an instance buffer. Map vertex/instance built-ins to Vulkan's index built-ins and assign matching shader interface locations, including explicitly reserved locations. Add compiler and rendering regressions for these paths. Run the indexed and unindexed rendering regression on every real renderer, including D3D12 and installed-SDK consumers. Skip it only when the test build uses a no-op Metal device. Review-Group: E1 Source: PR BabylonJS#1888 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae Split-From: a79dba316c9e38f9597fa4bf16f652551a20a864 Split-Part: code

bkaradzic-microsoft · 2026-10-07T04:44:02Z

Superseded by #1913, now merged. The attribute-less instance count handling, Vulkan shader fixes, and regression coverage are upstream.

Copilot AI lite review requested due to automatic review settings September 17, 2026 20:34

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts Outdated

Copilot started reviewing on behalf of bkaradzic-microsoft September 17, 2026 21:35

View session

bkaradzic-microsoft requested a balanced review from Copilot September 17, 2026 22:19

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Copilot started reviewing on behalf of bkaradzic-microsoft September 17, 2026 23:34

View session

bkaradzic-microsoft requested review from CedricGuillemet, bghgary and ryantrem and a balanced review from Copilot September 18, 2026 00:56

Copilot started reviewing on behalf of bkaradzic-microsoft September 18, 2026 01:00

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread Plugins/ShaderCompiler/Source/ShaderCompilerVulkan.cpp Outdated

bkaradzic-microsoft requested a balanced review from Copilot September 18, 2026 01:24

Copilot started reviewing on behalf of bkaradzic-microsoft September 18, 2026 01:34

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

bkaradzic-microsoft force-pushed the pr/native-attribute-less-instancing branch 2 times, most recently from d469df6 to 52a0a81 Compare September 22, 2026 20:05

bkaradzic-microsoft force-pushed the pr/native-attribute-less-instancing branch from 52a0a81 to 5ab1dde Compare October 1, 2026 16:42

bkaradzic-microsoft and others added 5 commits October 5, 2026 12:26

bkaradzic-microsoft force-pushed the pr/native-attribute-less-instancing branch from 5ab1dde to 4a60437 Compare October 5, 2026 19:28

bkaradzic-microsoft closed this Oct 7, 2026

Navigation Menu

Repository navigation

Honor instance counts without per-instance vertex attributes - #1888

Honor instance counts without per-instance vertex attributes#1888
bkaradzic-microsoft wants to merge 5 commits into
BabylonJS:masterfrom
bkaradzic-microsoft:pr/native-attribute-less-instancing

bkaradzic-microsoft commented Sep 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

bkaradzic-microsoft commented Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bkaradzic-microsoft commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Independence

CI follow-up

Validation

Interface-block review follow-up

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

bkaradzic-microsoft commented Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bkaradzic-microsoft commented Sep 17, 2026 •

edited

Loading