Support precompiled shaders such that it's possible to not include the shader compiler by bghgary · Pull Request #1598 · BabylonJS/BabylonNative

bghgary · 2026-02-04T23:43:49Z

No description provided.

Copilot

Pull request overview

This pull request adds support for precompiled shaders to allow removing the shader compiler from the runtime. The changes introduce three new plugin components and refactor the existing shader compilation and caching infrastructure.

Changes:

Introduces ShaderTool: A command-line tool for precompiling GLSL shaders offline
Refactors shader compilation into a separate ShaderCompiler plugin with optional runtime inclusion
Refactors shader caching into a separate ShaderCache plugin with improved hash-based storage
Adds ShaderProvider abstraction in NativeEngine to support both runtime compilation and precompiled shaders
Moves BgfxShaderInfo to Core/Graphics for shared access across components

Reviewed changes

Copilot reviewed 48 out of 54 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
Plugins/ShaderTool/*	New command-line tool for offline shader compilation
Plugins/ShaderCompiler/*	Extracted shader compiler as optional plugin with public/internal APIs
Plugins/ShaderCache/*	Refactored shader cache with xxHash-based hashing and improved serialization
Plugins/NativeEngine/*	Refactored to use ShaderProvider abstraction supporting both compilation modes
Plugins/ShaderProvider/Source/ShaderProvider.cpp	Incomplete/corrupted file (critical issue)
Core/Graphics/*	Added BgfxShaderInfo structure and updated include paths
Apps/*	Updated tests and Playground app to use new ShaderCache API
CMakeLists.txt files	Added build options for new plugins and updated dependencies

Comments suppressed due to low confidence (2)

Plugins/ShaderCompiler/Source/ShaderCompilerD3D.cpp:12

The file uses std::array on line 18 but does not include or <arcana/experimental/array.h>. This will likely cause a compilation error. The other platform-specific shader compiler files (Metal, OpenGL, Vulkan) include <arcana/experimental/array.h> for this purpose.
Plugins/ShaderCompiler/Source/ShaderCompilerD3D.cpp:12
The file uses gsl::narrow_cast, gsl::make_span, and gsl::span but does not include the GSL header. While ShaderCompilerCommon.h includes <gsl/gsl>, this file includes ShaderCompilerCommon.h after other headers. If ShaderCompilerCommon.h doesn't provide these definitions (for instance if it's protected by include guards that were already satisfied), this could cause compilation errors. Consider adding an explicit #include <gsl/gsl> directive or ensuring the include order is correct.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-05T01:28:05Z

Plugins/ShaderCompiler/Include/Babylon/Plugins/ShaderCompiler.h

+#pragma once
+
+#include <string_view>
+#include <Babylon/Plugins/BgfxShaderInfo.h>


The include path appears to be incorrect. This file is trying to include <Babylon/Plugins/BgfxShaderInfo.h>, but based on the directory structure shown in other files, the correct path should be <Babylon/Graphics/BgfxShaderInfo.h>. This will cause a compilation error.

Copilot · 2026-02-05T01:28:06Z

Plugins/ShaderCompiler/Include/Babylon/Plugins/ShaderCompiler.h

+        ShaderCompiler();
+        ~ShaderCompiler();
+
+        Babylon::Graphics::BgfxShaderInfo Compile(std::string_view vertexSource, std::string_view fragmentSource);


There is an API inconsistency between the public header and the internal implementation. The public header (ShaderCompiler.h) declares the Compile method as taking std::string_view parameters, while the internal header (ShaderCompilerInternal.h) and implementation declare it as taking std::string parameters by value. This signature mismatch will cause linker errors. The signatures should be consistent across both headers.

Copilot · 2026-02-05T01:28:06Z

Plugins/ShaderCache/Source/ShaderCache.cpp

+    uint32_t Save(std::ofstream& stream)
+    {
+        return ShaderCacheImpl::Instance->Save(stream);
+    }
+
+    uint32_t Load(std::ifstream& stream)
+    {
+        return ShaderCacheImpl::Instance->Load(stream);
+    }


The Save and Load functions do not check if ShaderCacheImpl::Instance is null before dereferencing it. If the cache is not enabled when these functions are called, this will cause a null pointer dereference and crash. These functions should either check for null and throw an exception like the other functions do, or the documentation should clearly state that the cache must be enabled before calling these functions.

Copilot · 2026-02-05T01:28:06Z

Plugins/ShaderProvider/Source/ShaderProvider.cpp

@@ -0,0 +1 @@
+#include <


This file appears to be incomplete or corrupted. It only contains "#include <" without a header name and closing bracket. This will cause a compilation error.

Copilot · 2026-02-05T01:28:07Z

Plugins/NativeEngine/Source/NativeEngine.cpp

+        typedef std::shared_ptr<Graphics::BgfxShaderInfo> BgfxShaderInfoPtr;
+        static auto deleteShaderInfo = [](void*, void* userData) {
+            delete reinterpret_cast<BgfxShaderInfoPtr*>(userData);
+        };

        std::unique_ptr<ProgramData> program = std::make_unique<ProgramData>(m_deviceContext);
-        auto vertexShader = bgfx::createShader(bgfx::copy(shaderInfo->VertexBytes.data(), static_cast<uint32_t>(shaderInfo->VertexBytes.size())));
+
+        auto vertexShader = bgfx::createShader(
+            bgfx::makeRef(
+                shaderInfo->VertexBytes.data(), static_cast<uint32_t>(shaderInfo->VertexBytes.size()),
+                deleteShaderInfo, new BgfxShaderInfoPtr{shaderInfo}));
        InitUniformInfos(vertexShader, shaderInfo->UniformStages, program->UniformInfos, program->UniformNameToIndex);

-        auto fragmentShader = bgfx::createShader(bgfx::copy(shaderInfo->FragmentBytes.data(), static_cast<uint32_t>(shaderInfo->FragmentBytes.size())));
+        auto fragmentShader = bgfx::createShader(
+            bgfx::makeRef(
+                shaderInfo->FragmentBytes.data(), static_cast<uint32_t>(shaderInfo->FragmentBytes.size()),
+                deleteShaderInfo, new BgfxShaderInfoPtr{shaderInfo}));
        InitUniformInfos(fragmentShader, shaderInfo->UniformStages, program->UniformInfos, program->UniformNameToIndex);


The memory management approach here is correct but could be more efficient. Two separate heap allocations are made for shared_ptr wrappers (lines 983 and 989), which will both be deleted when bgfx releases the shaders. Since both shared_ptrs point to the same BgfxShaderInfo, the reference count correctly goes from 2 to 0. However, consider storing the shaderInfo directly in the ProgramData structure and using a simpler lifetime management strategy, which would be clearer and avoid the need for these heap allocations and custom deleters.

Copilot · 2026-02-05T01:28:07Z

Plugins/NativeEngine/Source/ShaderProvider.cpp

+    std::shared_ptr<Graphics::BgfxShaderInfo> ShaderProvider::Get(std::string vertexSource, std::string fragmentSource)
+    {
+#ifdef SHADER_CACHE
+        if (Plugins::ShaderCache::IsEnabled())
+        {
+            const auto shaderInfo = Plugins::ShaderCache::GetShader(vertexSource, fragmentSource);
+            if (shaderInfo)
+            {
+                return shaderInfo;
+            }
+        }
+#endif
+
+#ifdef SHADER_COMPILER
+        CheckShaderCompilerAssumptions();
+
+#ifdef SHADER_CACHE
+        if (Plugins::ShaderCache::IsEnabled())
+        {
+            auto compiledShaderInfo = m_shaderCompiler.Compile(vertexSource, fragmentSource);
+            return Plugins::ShaderCache::AddShader(std::move(vertexSource), std::move(fragmentSource), compiledShaderInfo);
+        }
+#endif
+
+        return std::make_shared<Graphics::BgfxShaderInfo>(m_shaderCompiler.Compile(std::move(vertexSource), std::move(fragmentSource)));


There is a logic issue with the conditional compilation and control flow. When both SHADER_CACHE and SHADER_COMPILER are defined, if the cache is enabled and a cache hit occurs (line 39-42), the function returns early. If there's a cache miss, the code continues to line 47-55 where it compiles the shader and adds it to the cache, which moves the vertexSource and fragmentSource. However, line 58 is reached only when SHADER_CACHE is enabled but the cache itself is disabled at runtime (IsEnabled() returns false). In this case, the vertexSource and fragmentSource would not have been moved on line 54, so line 58 is actually safe. Upon closer inspection, this appears to be correct, though the logic flow is complex and could benefit from clearer structure.

bghgary added 4 commits January 30, 2026 15:40

Factor out shader cache and shader compiler into their own plugins

1d43aa1

Add shader tool + code cleanup

6034e96

Merge branch 'master' into support-no-shader-compiler

c63e004

Fix build

34a109e

bghgary requested a review from Copilot February 5, 2026 01:21

Copilot started reviewing on behalf of bghgary February 5, 2026 01:22 View session

bghgary changed the title ~~Support precompiled shaders such that the runtime no longer includes the shader compiler~~ Support precompiled shaders such that it's possible to not include the shader compiler Feb 5, 2026

Copilot AI reviewed Feb 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support precompiled shaders such that it's possible to not include the shader compiler#1598

Support precompiled shaders such that it's possible to not include the shader compiler#1598
bghgary wants to merge 4 commits intoBabylonJS:masterfrom
bghgary:support-no-shader-compiler

bghgary commented Feb 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		@@ -0,0 +1 @@
		#include < No newline at end of file

Conversation

bghgary commented Feb 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant