Fix Vulkan >16 sampler test false-pass and add Linux Vulkan CI by bkaradzic-microsoft · Pull Request #1859 · BabylonJS/BabylonNative

bkaradzic-microsoft · 2026-09-01T22:06:03Z

Summary

Follow-up to #1857 addressing @bghgary:

False pass on unexpected success — ShaderCompilation.VulkanRejectsMoreThanSixteenSamplers no longer treats unexpected success as the expected rejection.
Runnable Vulkan CI — Ubuntu_Clang_Vulkan builds the full Vulkan configuration and runs 16-sampler acceptance and 17-sampler rejection under Xvfb with lavapipe. The Xlib crash is fixed by supplying bgfx with a device-owned display connection that survives renderer shutdown. The XML report must contain both completed, passing tests; missing or skipped tests fail the lane.
Minimal Vulkan stubs so the tree links: RendererType/Vulkan, DeviceImpl_Vulkan, ExternalTexture_Vulkan (throws / not implemented), Helpers.Vulkan. ExternalTexture tests skipped via SKIP_EXTERNAL_TEXTURE_TESTS.
Compile fixes surfaced by the new job: SPIRV-Cross GLSL for Vulkan, glslang IR using namespace, ExternalTexture Shared.h stub pattern.

Test plan

Local full Linux Vulkan build and the exact CI sampler-boundary step pass
Existing OpenGL shader regressions pass locally
All 33 GitHub CI jobs passed, including both Vulkan runtime boundary tests: https://github.com/BabylonJS/BabylonNative/actions/runs/34547542730
Existing Linux OpenGL / Win32 jobs passed
Unexpected-success mutation correctly fails the rejection test
Workflow lint and missing/skipped/failed-report guard checks pass

bkaradzic-microsoft · 2026-09-01T22:06:18Z

Addresses @bghgary’s comment on #1857:

False pass: VulkanRejectsMoreThanSixteenSamplers uses explicit status values (unexpected_success / threw / not_ready); unexpected success fails outside the catch path.
CI: Ubuntu_Clang_Vulkan with GRAPHICS_API=Vulkan + lavapipe runs that test.

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

Updates Vulkan coverage by fixing a gtest that could falsely pass on unexpected shader compile success, and by introducing a Linux Vulkan CI lane (lavapipe) plus minimal Vulkan stubs to allow configure/link.

Changes:

Fix ShaderCompilation.VulkanRejectsMoreThanSixteenSamplers to distinguish expected rejection from unexpected success.
Add Ubuntu_Clang_Vulkan CI job and extend Linux workflow to support GRAPHICS_API=Vulkan with Mesa lavapipe.
Add minimal Vulkan backend stubs (renderer types, device impl, and unimplemented helpers/external texture) and skip incompatible tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
Plugins/ExternalTexture/Source/ExternalTexture_Vulkan.cpp	Adds Vulkan stub implementation that throws (not supported yet) to allow linking.
Core/Graphics/Source/DeviceImpl_Vulkan.cpp	Adds Vulkan `DeviceImpl` renderer type + platform info plumbing.
Core/Graphics/Include/RendererType/Vulkan/Babylon/Graphics/RendererType.h	Introduces Vulkan renderer-type typedefs for build plumbing.
Apps/UnitTests/Source/Tests.ShaderCompilation.cpp	Fixes false-pass logic in Vulkan “>16 samplers” shader compilation test.
Apps/UnitTests/Source/Helpers.Vulkan.cpp	Adds Vulkan stubs for unit test helpers (throws for unimplemented paths).
Apps/UnitTests/CMakeLists.txt	Defines Vulkan build flags and skips ExternalTexture tests for Vulkan.
.github/workflows/ci.yml	Adds a Linux Vulkan CI job entry.
.github/workflows/build-linux.yml	Adds `graphics-api` input and Vulkan/lavapipe setup + conditional test execution.

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

bghgary

[Reviewed by Copilot on behalf of @bghgary]

Concerns inline.

- Unexpected success is no longer thrown inside the try/catch that treats any "16" message as the expected rejection. - Add Ubuntu_Clang_Vulkan job (GRAPHICS_API=Vulkan + lavapipe) that builds UnitTests and runs ShaderCompilation.VulkanRejectsMoreThanSixteenSamplers. - Minimal Vulkan Graphics/ExternalTexture/UnitTest helpers so GRAPHICS_API=Vulkan can configure and link (ExternalTexture skipped). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601

…ture. The workflow YAML from the cherry-pick had broken nesting under inputs/steps (parse failure). Rewrite with correct alignment matching the prior layout. Also throw from Helpers::DestroyTexture on non-null Vulkan handles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601

Nested under Ubuntu_Clang_QuickJS, which made the workflow file invalid and aborted the whole CI run before any jobs started. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601

ShaderCompilerVulkan.cpp includes spirv_glsl.hpp and ShaderCompilerCommon.h includes spirv_cross.hpp; both need spirv-cross-glsl. Disabling GLSL for every non-OpenGL API left the Linux Vulkan CI job without those headers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601

TIntermNode/TIntermSymbol are not nested in namespace glslang; qualify them like ShaderCompilerTraversers so the Linux Vulkan job compiles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601

Bare TIntermSymbol/TIntermNode need namespace glslang in scope (same pattern as ShaderCompilerTraversers). Include localintermediate.h and using namespace glslang so the Ubuntu Vulkan job can compile CollectStageUniforms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601

Declare Impl/Update like other backends; Shared.h owns the out-of-line bodies. Keep GetInfo throwing so Vulkan remains unsupported without duplicate definitions that break Ubuntu_Clang_Vulkan. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601

GH runners have no GPU; lavapipe+bgfx Xlib surface init SEGV'd on the >16-sampler gtest. Keep GRAPHICS_API=Vulkan configure+link coverage so Vulkan packaging breaks cannot land unbuilt again; drop runtime steps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601

Repair the reusable Linux workflow and run both sampler boundary tests under Xvfb with lavapipe. Require both named tests to complete successfully in the XML report so missing or skipped tests cannot silently pass. Supply bgfx with an owned X11 display connection for Vulkan and keep it alive until after renderer shutdown. Exercise synchronous native program creation for 16-sampler acceptance and the specific 17-sampler rejection, with assertions outside the JavaScript exception handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 54aaa1e1-b4b3-46e7-9de4-5a56add4ac42

bghgary

[Reviewed by Copilot on behalf of @bghgary]

Concerns inline.

Run compatible Playground validation cases, the full supported unit-test set with exact sampler-result checks, and ModuleLoadTest under lavapipe. Document the measured blockers that remain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 54aaa1e1-b4b3-46e7-9de4-5a56add4ac42

Copilot AI lite review requested due to automatic review settings September 1, 2026 22:06

bkaradzic-microsoft mentioned this pull request Sep 1, 2026

Bump bgfx.cmake (TextureRegion blit/read, shader bin v12, Vulkan bindings) #1857

Merged

4 tasks

Copilot AI reviewed Sep 1, 2026

View reviewed changes

bkaradzic-microsoft requested a balanced review from Copilot September 1, 2026 22:45

Copilot AI reviewed Sep 1, 2026

View reviewed changes

Comment thread Plugins/ExternalTexture/Source/ExternalTexture_Vulkan.cpp

Comment thread Apps/UnitTests/Source/Helpers.Vulkan.cpp Outdated

Copilot started reviewing on behalf of bkaradzic-microsoft September 1, 2026 23:15 View session

Copilot started reviewing on behalf of bkaradzic-microsoft September 2, 2026 00:00 View session

bkaradzic-microsoft force-pushed the fix/vulkan-16-sampler-test-ci branch from 6346fe8 to bdeb5c1 Compare September 2, 2026 14:39

bkaradzic-microsoft requested a review from bghgary September 2, 2026 20:39

bghgary requested changes Sep 11, 2026

View reviewed changes

Comment thread .github/workflows/build-linux.yml Outdated

Comment thread .github/workflows/build-linux.yml Outdated

bkaradzic-microsoft requested a review from bghgary September 11, 2026 00:42

bkaradzic-microsoft and others added 10 commits September 14, 2026 11:52

Address Copilot nits: optional include and DestroyTexture indentation.

a609763

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601

bkaradzic-microsoft force-pushed the fix/vulkan-16-sampler-test-ci branch from 5b51a38 to 3c3cbac Compare September 14, 2026 19:02

bghgary requested changes Sep 14, 2026

View reviewed changes

Comment thread .github/workflows/build-linux.yml

Comment thread .github/workflows/build-linux.yml Outdated

Comment thread .github/workflows/build-linux.yml Outdated

This was referenced Sep 14, 2026

Make CompileComprehensiveGLSL complete on Linux Vulkan #1877

Open

Enable full Playground validation on Linux Vulkan #1878

Open

bkaradzic-microsoft requested a review from bghgary September 14, 2026 21:44

Uh oh!

Conversation

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

Uh oh!

Summary

Test plan

Uh oh!

bkaradzic-microsoft commented Sep 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

bghgary left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bghgary left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bkaradzic-microsoft commented Sep 1, 2026 •

edited

Loading