diff --git a/Apps/Playground/Scripts/config.json b/Apps/Playground/Scripts/config.json index e0022cf38..d3a315239 100644 --- a/Apps/Playground/Scripts/config.json +++ b/Apps/Playground/Scripts/config.json @@ -1568,9 +1568,7 @@ "title": "Particles", "playgroundId": "#G3ZYFU#7", "renderCount": 100, - "referenceImage": "particles.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles.png" }, { "title": "PBRMetallicRoughnessMaterial", @@ -1842,8 +1840,6 @@ { "title": "Instances with color buffer", "playgroundId": "#YPABS1#91", - "excludeFromAutomaticTesting": true, - "reason": "Pixel comparison fails (more than 20% pixels differ)", "referenceImage": "instancecolors.png" }, { @@ -1851,7 +1847,7 @@ "playgroundId": "#65MUMZ#47", "renderCount": 50, "excludeFromAutomaticTesting": true, - "reason": "Test crashes or hangs on Babylon Native", + "reason": "SSAO2 blur post-process shader fails to compile on desktop GL (samples uniform used as int loop bound); unrelated to instancing.", "referenceImage": "prepass-ssao-particles.png" }, { @@ -2694,8 +2690,6 @@ "title": "halo-particle-system", "playgroundId": "#2441BU#1", "renderCount": 20, - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up.", "referenceImage": "halo-particle-system.png" }, { @@ -2710,8 +2704,6 @@ "title": "particle-system-with-custom-nme-shader", "playgroundId": "#DMLLV2", "renderCount": 5, - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up.", "referenceImage": "particle-system-with-custom-nme-shader.png" }, { @@ -4106,9 +4098,7 @@ "title": "Particles - Basic Properties - Color", "playgroundId": "#0K3AQ2#3786", "renderCount": 120, - "referenceImage": "particles-basic-color.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-basic-color.png" }, { "title": "Particles - Basic Properties - Speed", @@ -4138,9 +4128,7 @@ "title": "Particles - Basic Properties - Direction", "playgroundId": "#0K3AQ2#3814", "renderCount": 120, - "referenceImage": "particles-basic-direction.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-basic-direction.png" }, { "title": "Particles - Basic Properties - Direction - Gravity", @@ -4158,9 +4146,7 @@ "title": "Particles - Basic Properties - Emit Rate - Fast", "playgroundId": "#0K3AQ2#3823", "renderCount": 120, - "referenceImage": "particles-basic-emit-rate-fast.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-basic-emit-rate-fast.png" }, { "title": "Particles - Basic Properties - Emission Limits", @@ -4184,9 +4170,7 @@ "title": "Particles - Change - Size", "playgroundId": "#0K3AQ2#3841", "renderCount": 120, - "referenceImage": "particles-basic-change-size.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-basic-change-size.png" }, { "title": "Particles - Change - Size 2", @@ -4198,17 +4182,13 @@ "title": "Particles - Change - Color", "playgroundId": "#0K3AQ2#3847", "renderCount": 120, - "referenceImage": "particles-basic-change-color.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-basic-change-color.png" }, { "title": "Particles - Change - Color 2", "playgroundId": "#0K3AQ2#3851", "renderCount": 120, - "referenceImage": "particles-basic-change-color-2.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-basic-change-color-2.png" }, { "title": "Particles - Change - Speed", @@ -4268,9 +4248,7 @@ "title": "Particles - Change - Lifetime", "playgroundId": "#0K3AQ2#3885", "renderCount": 120, - "referenceImage": "particles-basic-change-lifetime.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-basic-change-lifetime.png" }, { "title": "Particles - Change - Lifetime 2", @@ -4288,25 +4266,19 @@ "title": "Particles - Emitters - Point", "playgroundId": "#WLX2I2#7", "renderCount": 120, - "referenceImage": "particles-emitters-point.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-emitters-point.png" }, { "title": "Particles - Emitters - Box", "playgroundId": "#WLX2I2#8", "renderCount": 120, - "referenceImage": "particles-emitters-box.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-emitters-box.png" }, { "title": "Particles - Emitters - Sphere", "playgroundId": "#WLX2I2#9", "renderCount": 120, - "referenceImage": "particles-emitters-sphere.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-emitters-sphere.png" }, { "title": "Particles - Emitters - Directed Sphere", @@ -4324,9 +4296,7 @@ "title": "Particles - Emitters - Cylinder", "playgroundId": "#WLX2I2#12", "renderCount": 120, - "referenceImage": "particles-emitters-cylinder.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-emitters-cylinder.png" }, { "title": "Particles - Emitters - Directed Cylinder", @@ -4346,9 +4316,7 @@ "title": "Particles - Emitters - Directed Cone", "playgroundId": "#WLX2I2#17", "renderCount": 120, - "referenceImage": "particles-emitters-directed-cone.png", - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up." + "referenceImage": "particles-emitters-directed-cone.png" }, { "title": "Particles - Emitters - Mesh", @@ -4458,8 +4426,6 @@ "title": "Particles - Attractors", "playgroundId": "#DEZ79M#73", "renderCount": 240, - "excludeFromAutomaticTesting": true, - "reason": "Pixel diff after #1691 instance-data stride fix; not stride-related, needs follow-up.", "referenceImage": "particles-attractors.png" }, { diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 27e64ed31..72795e028 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -2,6 +2,9 @@ #include +#include +#include + #include "JsConsoleLogger.h" #include @@ -970,6 +973,7 @@ namespace Babylon Napi::Value jsProgram = Napi::Pointer::Create(info.Env(), program, Napi::NapiPointerDeleter(program)); try { + program->SetSources(vertexSource, fragmentSource); program->Initialize(m_shaderProvider.Get(vertexSource, fragmentSource)); } catch (const std::exception& ex) @@ -989,6 +993,8 @@ namespace Babylon Program* program = new Program{m_deviceContext}; Napi::Value jsProgram = Napi::Pointer::Create(info.Env(), program, Napi::NapiPointerDeleter(program)); + program->SetSources(vertexSource, fragmentSource); + arcana::make_task(arcana::threadpool_scheduler, *m_cancellationSource, [this, vertexSource = std::move(vertexSource), fragmentSource = std::move(fragmentSource), program, cancellationSource{m_cancellationSource}]() { program->Initialize(m_shaderProvider.Get(vertexSource, fragmentSource)); @@ -2335,6 +2341,47 @@ namespace Babylon encoder->setUniform({it.first}, value.Data.data(), value.ElementLength); } + // Divisor-driven instancing: a consumer-instanced attribute (divisor==1) recorded at a + // base bgfx location below TexCoord3 was compiled to a per-vertex slot. bgfx can only feed + // per-instance data into i_data slots (TexCoord3..TexCoord7), so route those attributes to + // the correct i_data slot via a lazily-compiled program variant. The target location mirrors + // BuildInstanceDataBuffer's reverse-attrib packing: highest base attrib -> TexCoord7. + bgfx::ProgramHandle programHandle = m_currentProgram->Handle(); + if (m_boundVertexArray != nullptr) + { + const auto& instances = m_boundVertexArray->GetInstances(); + if (!instances.empty()) + { + std::map genericInstancedAttributes; + const auto& attributeLocations = m_currentProgram->VertexAttributeLocations(); + const size_t count = instances.size(); + size_t ascendingIndex = 0; + for (const auto& instance : instances) + { + const bgfx::Attrib::Enum attrib = instance.first; + if (attrib < bgfx::Attrib::TexCoord3) + { + const size_t rank = count - 1 - ascendingIndex; + const uint32_t targetLocation = static_cast(bgfx::Attrib::TexCoord7) - static_cast(rank); + for (const auto& [name, location] : attributeLocations) + { + if (location == static_cast(attrib)) + { + genericInstancedAttributes.emplace(name, targetLocation); + break; + } + } + } + ++ascendingIndex; + } + + if (!genericInstancedAttributes.empty()) + { + programHandle = m_currentProgram->GetOrCreateInstancedVariant(genericInstancedAttributes, m_shaderProvider); + } + } + } + auto& boundFrameBuffer = GetBoundFrameBuffer(*encoder); if (boundFrameBuffer.HasDepth()) { @@ -2348,7 +2395,7 @@ namespace Babylon boundFrameBuffer.SetStencil(*encoder, m_stencilState); // Discard everything except textures since we keep the state of everything else. - boundFrameBuffer.Submit(*encoder, m_currentProgram->Handle(), BGFX_DISCARD_ALL & ~BGFX_DISCARD_BINDINGS); + boundFrameBuffer.Submit(*encoder, programHandle, BGFX_DISCARD_ALL & ~BGFX_DISCARD_BINDINGS); } Graphics::UpdateToken& NativeEngine::GetUpdateToken() diff --git a/Plugins/NativeEngine/Source/Program.cpp b/Plugins/NativeEngine/Source/Program.cpp index 7978af4e2..3f930993d 100644 --- a/Plugins/NativeEngine/Source/Program.cpp +++ b/Plugins/NativeEngine/Source/Program.cpp @@ -27,6 +27,17 @@ namespace uniformNameToIndex[info.name] = handleIndex; } } + + bgfx::ShaderHandle CreateShader(const std::shared_ptr& shaderInfo, gsl::span bytes) + { + using ShaderInfoPtr = std::shared_ptr; + static auto ShaderInfoReleaseFn = [](void*, void* userData) { + delete reinterpret_cast(userData); + }; + return bgfx::createShader(bgfx::makeRef( + bytes.data(), static_cast(bytes.size()), + ShaderInfoReleaseFn, new ShaderInfoPtr{shaderInfo})); + } } namespace Babylon @@ -47,31 +58,52 @@ namespace Babylon { arcana::trace_region region{"Program::Initialize"}; - using ShaderInfoPtr = std::shared_ptr; - - static auto ShaderInfoReleaseFn = [](void*, void* userData) { - delete reinterpret_cast(userData); - }; - - auto vertexShader = bgfx::createShader(bgfx::makeRef( - shaderInfo->VertexBytes.data(), static_cast(shaderInfo->VertexBytes.size()), - ShaderInfoReleaseFn, new ShaderInfoPtr{shaderInfo})); + auto vertexShader = CreateShader(shaderInfo, shaderInfo->VertexBytes); InitUniformInfos(vertexShader, shaderInfo->UniformStages, m_uniformInfos, m_uniformNameToIndex); - auto fragmentShader = bgfx::createShader(bgfx::makeRef( - shaderInfo->FragmentBytes.data(), static_cast(shaderInfo->FragmentBytes.size()), - ShaderInfoReleaseFn, new ShaderInfoPtr{shaderInfo})); + auto fragmentShader = CreateShader(shaderInfo, shaderInfo->FragmentBytes); InitUniformInfos(fragmentShader, shaderInfo->UniformStages, m_uniformInfos, m_uniformNameToIndex); m_handle = bgfx::createProgram(vertexShader, fragmentShader, true); m_vertexAttributeLocations = shaderInfo->VertexAttributeLocations; } + void Program::SetSources(std::string vertexSource, std::string fragmentSource) + { + m_vertexSource = std::move(vertexSource); + m_fragmentSource = std::move(fragmentSource); + } + + bgfx::ProgramHandle Program::GetOrCreateInstancedVariant(const std::map& instancedAttributes, ShaderProvider& shaderProvider) + { + if (instancedAttributes.empty()) + { + return m_handle; + } + + const auto it = m_instancedVariants.find(instancedAttributes); + if (it != m_instancedVariants.end()) + { + return it->second; + } + + auto shaderInfo = shaderProvider.Get(m_vertexSource, m_fragmentSource, instancedAttributes); + + auto vertexShader = CreateShader(shaderInfo, shaderInfo->VertexBytes); + auto fragmentShader = CreateShader(shaderInfo, shaderInfo->FragmentBytes); + bgfx::ProgramHandle handle = bgfx::createProgram(vertexShader, fragmentShader, true); + + m_instancedVariants.emplace(instancedAttributes, handle); + return handle; + } + void Program::Dispose() { + const bool sameDevice = m_deviceID == m_deviceContext.GetDeviceId(); + if (bgfx::isValid(m_handle)) { - if (m_deviceID == m_deviceContext.GetDeviceId()) + if (sameDevice) { bgfx::destroy(m_handle); } @@ -79,6 +111,15 @@ namespace Babylon m_handle = BGFX_INVALID_HANDLE; } + for (auto& [key, handle] : m_instancedVariants) + { + if (sameDevice && bgfx::isValid(handle)) + { + bgfx::destroy(handle); + } + } + m_instancedVariants.clear(); + m_uniforms.clear(); m_uniformNameToIndex.clear(); m_uniformInfos.clear(); diff --git a/Plugins/NativeEngine/Source/Program.h b/Plugins/NativeEngine/Source/Program.h index d04330d1b..d7e694a0d 100644 --- a/Plugins/NativeEngine/Source/Program.h +++ b/Plugins/NativeEngine/Source/Program.h @@ -55,6 +55,15 @@ namespace Babylon void Initialize(std::shared_ptr shaderInfo); void Dispose(); + // Stores the original GLSL sources so divisor-driven instanced variants can be + // recompiled lazily on the first instanced draw (see GetOrCreateInstancedVariant). + void SetSources(std::string vertexSource, std::string fragmentSource); + + // Returns a program handle whose vertex shader routes the given consumer-instanced + // attributes (name -> bgfx per-instance location) to bgfx i_data slots. Compiled and + // cached on first use. An empty map returns the base handle. + bgfx::ProgramHandle GetOrCreateInstancedVariant(const std::map& instancedAttributes, ShaderProvider& shaderProvider); + void SetUniform(bgfx::UniformHandle handle, gsl::span data, size_t elementLength = 1); const UniformInfo* GetUniformInfo(const std::string& name) const; bgfx::ProgramHandle Handle() const { return m_handle; } @@ -69,5 +78,8 @@ namespace Babylon std::map m_uniformNameToIndex; std::map m_uniformInfos; std::map m_vertexAttributeLocations; + std::string m_vertexSource; + std::string m_fragmentSource; + std::map, bgfx::ProgramHandle> m_instancedVariants; }; } diff --git a/Plugins/NativeEngine/Source/ShaderProvider.cpp b/Plugins/NativeEngine/Source/ShaderProvider.cpp index 8deb1e149..a045817d4 100644 --- a/Plugins/NativeEngine/Source/ShaderProvider.cpp +++ b/Plugins/NativeEngine/Source/ShaderProvider.cpp @@ -31,10 +31,14 @@ namespace namespace Babylon { - std::shared_ptr ShaderProvider::Get(std::string_view vertexSource, std::string_view fragmentSource) + std::shared_ptr ShaderProvider::Get(std::string_view vertexSource, std::string_view fragmentSource, const std::map& instancedAttributes) { + // The shader cache is keyed only by source, so it must be bypassed when routing instanced + // attributes (otherwise a variant would collide with the base program's cached shader). + const bool useCache = instancedAttributes.empty(); + #ifdef SHADER_CACHE - if (Plugins::ShaderCache::IsEnabled()) + if (useCache && Plugins::ShaderCache::IsEnabled()) { const auto shaderInfo = Plugins::ShaderCache::GetShader(vertexSource, fragmentSource); if (shaderInfo) @@ -48,14 +52,14 @@ namespace Babylon CheckShaderCompilerAssumptions(); #ifdef SHADER_CACHE - if (Plugins::ShaderCache::IsEnabled()) + if (useCache && Plugins::ShaderCache::IsEnabled()) { - auto compiledShaderInfo = m_shaderCompiler.Compile(vertexSource, fragmentSource); + auto compiledShaderInfo = m_shaderCompiler.Compile(vertexSource, fragmentSource, instancedAttributes); return Plugins::ShaderCache::AddShader(vertexSource, fragmentSource, compiledShaderInfo); } #endif - return std::make_shared(m_shaderCompiler.Compile(vertexSource, fragmentSource)); + return std::make_shared(m_shaderCompiler.Compile(vertexSource, fragmentSource, instancedAttributes)); #else throw std::runtime_error{"Shader compiler is not available"}; #endif diff --git a/Plugins/NativeEngine/Source/ShaderProvider.h b/Plugins/NativeEngine/Source/ShaderProvider.h index 39cb62a51..6dd2c4924 100644 --- a/Plugins/NativeEngine/Source/ShaderProvider.h +++ b/Plugins/NativeEngine/Source/ShaderProvider.h @@ -2,6 +2,10 @@ #include +#include +#include +#include + #ifdef SHADER_COMPILER #include #endif @@ -13,7 +17,7 @@ namespace Babylon class ShaderProvider { public: - std::shared_ptr Get(std::string_view vertexSource, std::string_view fragmentSource); + std::shared_ptr Get(std::string_view vertexSource, std::string_view fragmentSource, const std::map& instancedAttributes = {}); private: #ifdef SHADER_COMPILER diff --git a/Plugins/NativeEngine/Source/VertexArray.h b/Plugins/NativeEngine/Source/VertexArray.h index 8cc6bfb1c..8a162e805 100644 --- a/Plugins/NativeEngine/Source/VertexArray.h +++ b/Plugins/NativeEngine/Source/VertexArray.h @@ -24,6 +24,8 @@ namespace Babylon void SetIndexBuffer(bgfx::Encoder* encoder, uint32_t firstIndex, uint32_t numIndices); void SetVertexBuffers(bgfx::Encoder* encoder, uint32_t startVertex, uint32_t numVertices, uint32_t instanceCount = 0); + const std::map& GetInstances() const { return m_vertexBufferInstances; } + private: IndexBuffer* m_indexBuffer{}; diff --git a/Plugins/ShaderCompiler/InternalInclude/Babylon/Plugins/ShaderCompiler.h b/Plugins/ShaderCompiler/InternalInclude/Babylon/Plugins/ShaderCompiler.h index 76a468240..bccb37bdf 100644 --- a/Plugins/ShaderCompiler/InternalInclude/Babylon/Plugins/ShaderCompiler.h +++ b/Plugins/ShaderCompiler/InternalInclude/Babylon/Plugins/ShaderCompiler.h @@ -1,6 +1,9 @@ #pragma once #include +#include +#include +#include #include namespace Babylon::Plugins @@ -13,6 +16,12 @@ namespace Babylon::Plugins ShaderCompiler(); ~ShaderCompiler(); - Graphics::BgfxShaderInfo Compile(std::string_view vertexSource, std::string_view fragmentSource); + /// `instancedAttributes` maps consumer-bound per-instance vertex-attribute names + /// (in addition to the built-in instanced names) to the bgfx per-instance attribute + /// location (a top TEXCOORD semantic, TexCoord7 down) they must occupy. The location + /// is derived from the draw-time instance packing order so the shader reads each + /// attribute from the slot bgfx fills. An empty map preserves the legacy per-vertex + /// mapping for all non-built-in attributes. + Graphics::BgfxShaderInfo Compile(std::string_view vertexSource, std::string_view fragmentSource, const std::map& instancedAttributes = {}); }; } diff --git a/Plugins/ShaderCompiler/Source/ShaderCompilerDXBC.cpp b/Plugins/ShaderCompiler/Source/ShaderCompilerDXBC.cpp index d51a4ab19..0b11e534d 100644 --- a/Plugins/ShaderCompiler/Source/ShaderCompilerDXBC.cpp +++ b/Plugins/ShaderCompiler/Source/ShaderCompilerDXBC.cpp @@ -81,7 +81,7 @@ namespace Babylon::Plugins glslang::FinalizeProcess(); } - Graphics::BgfxShaderInfo ShaderCompiler::Compile(std::string_view vertexSource, std::string_view fragmentSource) + Graphics::BgfxShaderInfo ShaderCompiler::Compile(std::string_view vertexSource, std::string_view fragmentSource, const std::map& instancedAttributes) { glslang::TProgram program; @@ -105,7 +105,7 @@ namespace Babylon::Plugins auto cutScope = ShaderCompilerTraversers::ChangeUniformTypes(program, ids); auto utstScope = ShaderCompilerTraversers::MoveNonSamplerUniformsIntoStruct(program, ids); std::map vertexAttributeRenaming = {}; - ShaderCompilerTraversers::AssignLocationsAndNamesToVertexVaryingsD3D(program, ids, vertexAttributeRenaming); + ShaderCompilerTraversers::AssignLocationsAndNamesToVertexVaryingsD3D(program, ids, vertexAttributeRenaming, instancedAttributes); ShaderCompilerTraversers::SplitSamplersIntoSamplersAndTextures(program, ids); ShaderCompilerTraversers::SplitSamplerFunctionParameters(program, ids); ShaderCompilerTraversers::ZeroInitializeStructLocals(program); diff --git a/Plugins/ShaderCompiler/Source/ShaderCompilerDXIL.cpp b/Plugins/ShaderCompiler/Source/ShaderCompilerDXIL.cpp index 6f6bb83c3..88b20d410 100644 --- a/Plugins/ShaderCompiler/Source/ShaderCompilerDXIL.cpp +++ b/Plugins/ShaderCompiler/Source/ShaderCompilerDXIL.cpp @@ -157,7 +157,7 @@ namespace Babylon::Plugins glslang::FinalizeProcess(); } - Graphics::BgfxShaderInfo ShaderCompiler::Compile(std::string_view vertexSource, std::string_view fragmentSource) + Graphics::BgfxShaderInfo ShaderCompiler::Compile(std::string_view vertexSource, std::string_view fragmentSource, const std::map& instancedAttributes) { glslang::TProgram program; @@ -181,7 +181,7 @@ namespace Babylon::Plugins auto cutScope = ShaderCompilerTraversers::ChangeUniformTypes(program, ids); auto utstScope = ShaderCompilerTraversers::MoveNonSamplerUniformsIntoStruct(program, ids); std::map vertexAttributeRenaming = {}; - ShaderCompilerTraversers::AssignLocationsAndNamesToVertexVaryingsD3D(program, ids, vertexAttributeRenaming); + ShaderCompilerTraversers::AssignLocationsAndNamesToVertexVaryingsD3D(program, ids, vertexAttributeRenaming, instancedAttributes); ShaderCompilerTraversers::SplitSamplersIntoSamplersAndTextures(program, ids); ShaderCompilerTraversers::SplitSamplerFunctionParameters(program, ids); ShaderCompilerTraversers::ZeroInitializeStructLocals(program); diff --git a/Plugins/ShaderCompiler/Source/ShaderCompilerMetal.cpp b/Plugins/ShaderCompiler/Source/ShaderCompilerMetal.cpp index 8c448becd..4d7e5be1a 100644 --- a/Plugins/ShaderCompiler/Source/ShaderCompilerMetal.cpp +++ b/Plugins/ShaderCompiler/Source/ShaderCompilerMetal.cpp @@ -82,7 +82,7 @@ namespace Babylon::Plugins glslang::FinalizeProcess(); } - Graphics::BgfxShaderInfo ShaderCompiler::Compile(std::string_view vertexSource, std::string_view fragmentSource) + Graphics::BgfxShaderInfo ShaderCompiler::Compile(std::string_view vertexSource, std::string_view fragmentSource, const std::map& instancedAttributes) { glslang::TProgram program; @@ -106,7 +106,7 @@ namespace Babylon::Plugins auto cutScope = ShaderCompilerTraversers::ChangeUniformTypes(program, ids); auto utstScope = ShaderCompilerTraversers::MoveNonSamplerUniformsIntoStruct(program, ids); std::map vertexAttributeRenaming = {}; - ShaderCompilerTraversers::AssignLocationsAndNamesToVertexVaryingsMetal(program, ids, vertexAttributeRenaming); + ShaderCompilerTraversers::AssignLocationsAndNamesToVertexVaryingsMetal(program, ids, vertexAttributeRenaming, instancedAttributes); ShaderCompilerTraversers::SplitSamplersIntoSamplersAndTextures(program, ids); ShaderCompilerTraversers::SplitSamplerFunctionParameters(program, ids); ShaderCompilerTraversers::ZeroInitializeStructLocals(program); diff --git a/Plugins/ShaderCompiler/Source/ShaderCompilerOpenGL.cpp b/Plugins/ShaderCompiler/Source/ShaderCompilerOpenGL.cpp index 5be09e275..c21f1b99f 100644 --- a/Plugins/ShaderCompiler/Source/ShaderCompilerOpenGL.cpp +++ b/Plugins/ShaderCompiler/Source/ShaderCompilerOpenGL.cpp @@ -63,7 +63,7 @@ namespace Babylon::Plugins glslang::FinalizeProcess(); } - Graphics::BgfxShaderInfo ShaderCompiler::Compile(std::string_view vertexSource, std::string_view fragmentSource) + Graphics::BgfxShaderInfo ShaderCompiler::Compile(std::string_view vertexSource, std::string_view fragmentSource, const std::map& instancedAttributes) { glslang::TProgram program; @@ -86,7 +86,7 @@ namespace Babylon::Plugins ShaderCompilerTraversers::IdGenerator ids{}; auto cutScope = ShaderCompilerTraversers::ChangeUniformTypes(program, ids); std::map vertexAttributeRenaming = {}; - ShaderCompilerTraversers::AssignLocationsAndNamesToVertexVaryingsOpenGL(program, ids, vertexAttributeRenaming); + ShaderCompilerTraversers::AssignLocationsAndNamesToVertexVaryingsOpenGL(program, ids, vertexAttributeRenaming, instancedAttributes); std::string vertexGLSL(vertexSource.data(), vertexSource.size()); auto [vertexParser, vertexCompiler] = CompileShader(program, EShLangVertex, vertexGLSL); diff --git a/Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.cpp b/Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.cpp index d8a1c906a..9f34fe7e4 100644 --- a/Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.cpp +++ b/Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.cpp @@ -12,6 +12,7 @@ #include #include +#include #include using namespace glslang; @@ -552,8 +553,12 @@ namespace Babylon::ShaderCompilerTraversers replacementToOriginalName[newName] = name; } - static bool IsInstance(const char* name) + bool IsInstance(const char* name) const { + if (m_instancedAttributes != nullptr && m_instancedAttributes->count(name) != 0) + { + return true; + } return (!strcmp(name, "world0") || !strcmp(name, "world1") || !strcmp(name, "world2") || @@ -565,7 +570,27 @@ namespace Babylon::ShaderCompilerTraversers !strcmp(name, "splatIndex3")); } + // True when the name has no built-in instance mapping and must be assigned a + // generic per-instance i_data slot (top TEXCOORD semantics). + bool IsGenericInstance(const char* name) const + { + if (m_instancedAttributes == nullptr || m_instancedAttributes->count(name) == 0) + { + return false; + } + return strcmp(name, "world0") != 0 && + strcmp(name, "world1") != 0 && + strcmp(name, "world2") != 0 && + strcmp(name, "world3") != 0 && + strcmp(name, "instanceColor") != 0 && + strcmp(name, "splatIndex0") != 0 && + strcmp(name, "splatIndex1") != 0 && + strcmp(name, "splatIndex2") != 0 && + strcmp(name, "splatIndex3") != 0; + } + unsigned int m_genericAttributesRunningCount{0}; + const std::map* m_instancedAttributes{nullptr}; std::map m_varyingNameToSymbol{}; std::vector> m_symbolsToParents{}; @@ -607,18 +632,21 @@ namespace Babylon::ShaderCompilerTraversers class VertexVaryingInTraverserOpenGL final : private VertexVaryingInTraverser { public: - static void Traverse(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName) + static void Traverse(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName, const std::map& instancedAttributes) { auto intermediate{program.getIntermediate(EShLangVertex)}; VertexVaryingInTraverserOpenGL traverser{}; + traverser.m_instancedAttributes = &instancedAttributes; intermediate->getTreeRoot()->traverse(&traverser); // Pre-count instance attributes so i_data names can be assigned in reverse. // bgfx maps i_data0 to the last attribute (TEXCOORD7), so instance names - // must be assigned in reverse order, matching the Metal traverser. + // must be assigned in reverse order, matching the Metal traverser. Generic + // (consumer-declared) instanced attributes are excluded here because they are + // routed to an explicit i_data slot from their caller-supplied location. for (const auto& [name, symbol] : traverser.m_varyingNameToSymbol) { - if (IsInstance(name.c_str())) + if (traverser.IsInstance(name.c_str()) && !traverser.IsGenericInstance(name.c_str())) { traverser.m_instanceAttributeCount++; } @@ -635,6 +663,17 @@ namespace Babylon::ShaderCompilerTraversers // the first attribute encountered with the symbol bgfx uses for attribute 0 and increment for each subsequent attribute encountered. // This will cause our shader to have nonsensical naming, but will allow us to efficiently "pack" the attributes. m_genericAttributesRunningCount++; + if (IsGenericInstance(name)) + { + // Consumer-declared instanced attribute: route to the explicit bgfx i_data + // slot derived from its caller-supplied per-instance location (TEXCOORD7 == + // i_data0, descending), matching BuildInstanceDataBuffer's packing and the D3D path. + const unsigned int location = m_instancedAttributes->at(name); + const unsigned int slot = static_cast(bgfx::Attrib::TexCoord7) - location; + if (slot >= BX_COUNTOF(s_attribInstanceName)) + throw std::runtime_error(std::string{"Instanced attribute '"} + name + "' has location " + std::to_string(location) + " which does not map to a valid bgfx i_data slot (computed slot " + std::to_string(slot) + ")."); + return {static_cast(m_genericAttributesRunningCount - 1), s_attribInstanceName[slot]}; + } if (IsInstance(name)) { // Reverse: bgfx maps i_data0 to the highest semantic (TEXCOORD7), @@ -652,10 +691,11 @@ namespace Babylon::ShaderCompilerTraversers class VertexVaryingInTraverserMetal final : private VertexVaryingInTraverser { public: - static void Traverse(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName) + static void Traverse(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName, const std::map& instancedAttributes) { auto intermediate{program.getIntermediate(EShLangVertex)}; VertexVaryingInTraverserMetal traverser{}; + traverser.m_instancedAttributes = &instancedAttributes; intermediate->getTreeRoot()->traverse(&traverser); traverser.Traverse(intermediate, ids, replacementToOriginalName); } @@ -685,7 +725,10 @@ namespace Babylon::ShaderCompilerTraversers const bool isInstance = IsInstance(name.c_str()); if ((pass == 0 && isInstance) || (pass == 1 && !isInstance)) { - if (pass == 0) + // Count only built-in instance attributes for the reverse i_data + // assignment; generic (consumer-declared) instanced attributes are + // routed to an explicit i_data slot from their caller-supplied location. + if (pass == 0 && !IsGenericInstance(name.c_str())) { m_instanceAttributeCount++; } @@ -706,6 +749,17 @@ namespace Babylon::ShaderCompilerTraversers // This will cause our shader to have nonsensical naming, but will allow us to efficiently "pack" the attributes. m_genericAttributesRunningCount++; + if (IsGenericInstance(name)) + { + // Consumer-declared instanced attribute: route to the explicit bgfx i_data + // slot derived from its caller-supplied per-instance location (TEXCOORD7 == + // i_data0, descending), matching BuildInstanceDataBuffer's packing and the D3D path. + const unsigned int location = m_instancedAttributes->at(name); + const unsigned int slot = static_cast(bgfx::Attrib::TexCoord7) - location; + if (slot >= BX_COUNTOF(s_attribInstanceName)) + throw std::runtime_error(std::string{"Instanced attribute '"} + name + "' has location " + std::to_string(location) + " which does not map to a valid bgfx i_data slot (computed slot " + std::to_string(slot) + ")."); + return {static_cast(m_genericAttributesRunningCount - 1), s_attribInstanceName[slot]}; + } if (IsInstance(name)) { return {static_cast(m_genericAttributesRunningCount - 1), s_attribInstanceName[--m_instanceAttributeCount]}; @@ -722,10 +776,11 @@ namespace Babylon::ShaderCompilerTraversers class VertexVaryingInTraverserD3D final : private VertexVaryingInTraverser { public: - static void Traverse(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName) + static void Traverse(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName, const std::map& instancedAttributes) { auto intermediate{program.getIntermediate(EShLangVertex)}; VertexVaryingInTraverserD3D traverser{}; + traverser.m_instancedAttributes = &instancedAttributes; intermediate->getTreeRoot()->traverse(&traverser); // UVs are effectively a special kind of generic attribute since they both use // are implemented using texture coordinates, so we preprocess to pre-count the @@ -743,6 +798,19 @@ namespace Babylon::ShaderCompilerTraversers private: std::pair GetVaryingLocationAndNewNameForName(const char* name) { + // Consumer-declared instanced attributes with no built-in mapping (e.g. the + // fluid renderer's `position` or an instanced `color`) are routed to the bgfx + // per-instance i_data location supplied by the caller. That location is derived + // from the draw-time instance packing order (TEXCOORD7 == i_data0, descending), + // so per-instance data reaches the shader instead of the per-vertex input. + if (IsGenericInstance(name)) + { + const unsigned int location = m_instancedAttributes->at(name); + const unsigned int slot = static_cast(bgfx::Attrib::TexCoord7) - location; + if (slot >= BX_COUNTOF(s_attribInstanceName)) + throw std::runtime_error(std::string{"Instanced attribute '"} + name + "' has location " + std::to_string(location) + " which does not map to a valid bgfx i_data slot (computed slot " + std::to_string(slot) + ")."); + return {location, s_attribInstanceName[slot]}; + } #define IF_NAME_RETURN_ATTRIB(varyingName, attrib, newName) \ if (std::strcmp(name, varyingName) == 0) \ { \ @@ -1617,19 +1685,19 @@ namespace Babylon::ShaderCompilerTraversers return UniformTypeChangeTraverser::Traverse(program, ids); } - void AssignLocationsAndNamesToVertexVaryingsOpenGL(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName) + void AssignLocationsAndNamesToVertexVaryingsOpenGL(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName, const std::map& instancedAttributes) { - VertexVaryingInTraverserOpenGL::Traverse(program, ids, replacementToOriginalName); + VertexVaryingInTraverserOpenGL::Traverse(program, ids, replacementToOriginalName, instancedAttributes); } - void AssignLocationsAndNamesToVertexVaryingsMetal(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName) + void AssignLocationsAndNamesToVertexVaryingsMetal(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName, const std::map& instancedAttributes) { - VertexVaryingInTraverserMetal::Traverse(program, ids, replacementToOriginalName); + VertexVaryingInTraverserMetal::Traverse(program, ids, replacementToOriginalName, instancedAttributes); } - void AssignLocationsAndNamesToVertexVaryingsD3D(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName) + void AssignLocationsAndNamesToVertexVaryingsD3D(TProgram& program, IdGenerator& ids, std::map& replacementToOriginalName, const std::map& instancedAttributes) { - VertexVaryingInTraverserD3D::Traverse(program, ids, replacementToOriginalName); + VertexVaryingInTraverserD3D::Traverse(program, ids, replacementToOriginalName, instancedAttributes); } void SplitSamplersIntoSamplersAndTextures(TProgram& program, IdGenerator& ids) diff --git a/Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.h b/Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.h index 1f96b4373..c613ae483 100644 --- a/Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.h +++ b/Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.h @@ -2,8 +2,10 @@ #include +#include #include #include +#include namespace Babylon::ShaderCompilerTraversers { @@ -68,9 +70,17 @@ namespace Babylon::ShaderCompilerTraversers /// Changes the names and locations of varying attributes in the vertex shader to /// match bgfx's expectations. - void AssignLocationsAndNamesToVertexVaryingsOpenGL(glslang::TProgram& program, IdGenerator& ids, std::map& vertexAttributeRenaming); - void AssignLocationsAndNamesToVertexVaryingsMetal(glslang::TProgram& program, IdGenerator& ids, std::map& vertexAttributeRenaming); - void AssignLocationsAndNamesToVertexVaryingsD3D(glslang::TProgram& program, IdGenerator& ids, std::map& vertexAttributeRenaming); + /// + /// `instancedAttributes` maps vertex-attribute names that the consumer binds with a + /// per-instance divisor (e.g. the fluid renderer's `position` or an instanced `color`) + /// to the bgfx per-instance i_data location (top TEXCOORD semantic) they must occupy. + /// The location is computed from the draw-time instance packing order so the attribute + /// is read from the slot bgfx actually fills. The built-in instanced names (`world0-3`, + /// `instanceColor`, `splatIndex0-3`) keep their fixed mapping; an empty map preserves + /// legacy behavior. + void AssignLocationsAndNamesToVertexVaryingsOpenGL(glslang::TProgram& program, IdGenerator& ids, std::map& vertexAttributeRenaming, const std::map& instancedAttributes = {}); + void AssignLocationsAndNamesToVertexVaryingsMetal(glslang::TProgram& program, IdGenerator& ids, std::map& vertexAttributeRenaming, const std::map& instancedAttributes = {}); + void AssignLocationsAndNamesToVertexVaryingsD3D(glslang::TProgram& program, IdGenerator& ids, std::map& vertexAttributeRenaming, const std::map& instancedAttributes = {}); /// WebGL (and therefore Babylon.js) treats texture samplers as a single variable. /// Native platforms expect them to be two separate variables -- a texture and a diff --git a/Plugins/ShaderCompiler/Source/ShaderCompilerVulkan.cpp b/Plugins/ShaderCompiler/Source/ShaderCompilerVulkan.cpp index 9b145ec3c..e86585fa8 100644 --- a/Plugins/ShaderCompiler/Source/ShaderCompilerVulkan.cpp +++ b/Plugins/ShaderCompiler/Source/ShaderCompilerVulkan.cpp @@ -57,7 +57,7 @@ namespace Babylon::Plugins glslang::FinalizeProcess(); } - Graphics::BgfxShaderInfo ShaderCompiler::Compile(std::string_view vertexSource, std::string_view fragmentSource) + Graphics::BgfxShaderInfo ShaderCompiler::Compile(std::string_view vertexSource, std::string_view fragmentSource, const std::map& instancedAttributes) { glslang::TProgram program; @@ -81,7 +81,7 @@ namespace Babylon::Plugins auto cutScope = ShaderCompilerTraversers::ChangeUniformTypes(program, ids); auto utstScope = ShaderCompilerTraversers::MoveNonSamplerUniformsIntoStruct(program, ids); std::map vertexAttributeRenaming = {}; - ShaderCompilerTraversers::AssignLocationsAndNamesToVertexVaryingsD3D(program, ids, vertexAttributeRenaming); + ShaderCompilerTraversers::AssignLocationsAndNamesToVertexVaryingsD3D(program, ids, vertexAttributeRenaming, instancedAttributes); ShaderCompilerTraversers::SplitSamplersIntoSamplersAndTextures(program, ids); ShaderCompilerTraversers::SplitSamplerFunctionParameters(program, ids); ShaderCompilerTraversers::ZeroInitializeStructLocals(program);