NativeEngine: route instanced generic attributes landing on TexCoord3..TexCoord15

Core/Graphics/InternalInclude/Babylon/Graphics/BgfxShaderInfo.h

@@ -23,6 +23,16 @@ namespace Babylon::Graphics
     inline constexpr uint32_t TEXCOORD0_ATTRIBUTE_LOCATION{10};
     inline constexpr uint32_t INSTANCE_DATA_FIRST_LOCATION{TEXCOORD0_ATTRIBUTE_LOCATION + INSTANCE_DATA_FIRST_TEXCOORD};
 
+    /// The built-in per-instance attributes occupy the top BUILTIN_INSTANCE_DATA_SLOT_COUNT i_data
+    /// slots: world0-3 and splatIndex0-3 map to i_data0..i_data3, instanceColor to i_data4 (see
+    /// ShaderCompilerTraversers.cpp's attribute table). BUILTIN_INSTANCE_DATA_LAST_LOCATION is the
+    /// lowest synthetic location any of them can occupy; it is the boundary NativeEngine::Draw's
+    /// "< bgfx::Attrib::Count means a real per-vertex attribute that needs rerouting" guard rests
+    /// on, so it -- not just INSTANCE_DATA_FIRST_LOCATION -- must stay >= bgfx::Attrib::Count.
+    /// Keep in sync when adding a built-in per-instance attribute on a lower i_data slot.
+    inline constexpr uint32_t BUILTIN_INSTANCE_DATA_SLOT_COUNT{5};
+    inline constexpr uint32_t BUILTIN_INSTANCE_DATA_LAST_LOCATION{INSTANCE_DATA_FIRST_LOCATION - (BUILTIN_INSTANCE_DATA_SLOT_COUNT - 1)};
+
     struct BgfxShaderInfo
     {
         std::vector<uint8_t> VertexBytes{};

Plugins/NativeEngine/Source/NativeEngine.cpp

@@ -2413,7 +2413,7 @@ namespace Babylon
         }
 
         // 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
+        // real per-vertex bgfx location was compiled to a per-vertex slot. bgfx can only feed
         // per-instance data into i_data slots (the top TEXCOORD semantics), 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 -> i_data0 (TEXCOORD31),
@@ -2431,7 +2431,16 @@ namespace Babylon
                 for (const auto& instance : instances)
                 {
                     const bgfx::Attrib::Enum attrib = instance.first;
-                    if (attrib < bgfx::Attrib::TexCoord3)
+                    // "Real per-vertex slot" means Position..TexCoord15, i.e. < Attrib::Count. The
+                    // built-in instanced attributes (world0-3, splatIndex0-3, instanceColor) are
+                    // assigned synthetic locations at/above INSTANCE_DATA_FIRST_LOCATION - 4, which
+                    // is >= Attrib::Count, so they compare false here and are correctly skipped:
+                    // they already arrive as instance data.
+                    // The previous TexCoord3 boundary silently dropped generic instanced attributes
+                    // landing on TexCoord3..TexCoord15 (e.g. sprite cellInfo -> TexCoord3), leaving
+                    // them reading per-vertex garbage even though BuildInstanceDataBuffer had
+                    // already packed them into the instance data buffer.
+                    if (attrib < bgfx::Attrib::Count)
                     {
                         const size_t rank = count - 1 - ascendingIndex;
                         const uint32_t targetLocation = Babylon::Graphics::INSTANCE_DATA_FIRST_LOCATION - static_cast<uint32_t>(rank);

Plugins/ShaderCompiler/Source/ShaderCompilerCommon.cpp

@@ -19,6 +19,12 @@ namespace Babylon::ShaderCompilerCommon
     // attribute table below).
     static_assert(Babylon::Graphics::TEXCOORD0_ATTRIBUTE_LOCATION == static_cast<uint32_t>(bgfx::Attrib::TexCoord0));
     static_assert(Babylon::Graphics::INSTANCE_DATA_FIRST_LOCATION >= static_cast<uint32_t>(bgfx::Attrib::Count));
+    // The assert above only covers i_data0, the *highest* instance-data location. NativeEngine::Draw
+    // reroutes any attribute whose location is < bgfx::Attrib::Count, so what that guard actually
+    // depends on is the *lowest* built-in one (instanceColor, on i_data4). Were bgfx::Attrib::Count
+    // to grow past it, the assert above would still pass while instanceColor started being rerouted
+    // as if it were per-vertex data -- the same silent-garbage failure the guard exists to prevent.
+    static_assert(Babylon::Graphics::BUILTIN_INSTANCE_DATA_LAST_LOCATION >= static_cast<uint32_t>(bgfx::Attrib::Count));
 
     // Patching shader code to append clip space coordinates for the current rendering API.
     // Can be done with glslang shader traversal. Done with string patching for now.

Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.cpp

@@ -873,7 +873,8 @@ namespace Babylon::ShaderCompilerTraversers
                 // combined with BuildInstanceDataBuffer's descending-key packing, world3 lands on i_data0
                 // (TEXCOORD31) and world0 on i_data3. instanceColor follows at i_data4. The i_data name is
                 // cosmetic on D3D (binding is by TEXCOORD semantic, resolved from the location via the
-                // HLSLVertexAttributeRemap table).
+                // HLSLVertexAttributeRemap table). Adding one on a lower slot means bumping
+                // BUILTIN_INSTANCE_DATA_SLOT_COUNT in BgfxShaderInfo.h.
                 IF_NAME_RETURN_ATTRIB("instanceColor", Babylon::Graphics::INSTANCE_DATA_FIRST_LOCATION - 4, "i_data4")
                 IF_NAME_RETURN_ATTRIB("world0", Babylon::Graphics::INSTANCE_DATA_FIRST_LOCATION - 3, "i_data3")
                 IF_NAME_RETURN_ATTRIB("world1", Babylon::Graphics::INSTANCE_DATA_FIRST_LOCATION - 2, "i_data2")