Remap OpenGL vertex attributes so that skinning attributes also fit into the first 8, to fix GLES2 skinning bug on devices that only support 8 attributes (thanks to Alex Fuller.)

Ensure that a script object's DelayedStart() function is always called during the scene variable timestep update.
Fixed animation glitch in NinjaSnowWar due to player animation not being initialized before first physics update.

Bin/CoreData/Shaders/GLSL/Transform.vert

@@ -3,11 +3,11 @@ attribute vec3 iNormal;
 attribute vec4 iColor;
 attribute vec2 iTexCoord;
 attribute vec2 iTexCoord2;
-attribute vec3 iCubeTexCoord;
-attribute vec4 iCubeTexCoord2;
 attribute vec4 iTangent;
 attribute vec4 iBlendWeights;
 attribute vec4 iBlendIndices;
+attribute vec3 iCubeTexCoord;
+attribute vec4 iCubeTexCoord2;
 
 mat4 GetSkinMatrix(vec4 blendWeights, vec4 blendIndices)
 {

Bin/Data/Scripts/NinjaSnowWar/Ninja.as

@@ -52,6 +52,13 @@ class Ninja : GameObject
         SubscribeToEvent(node, "NodeCollision", "HandleNodeCollision");
         aimX = node.rotation.yaw;
     }
+    
+    void DelayedStart()
+    {
+        // Start playing the idle animation immediately, even before the first physics update
+        AnimationController@ animCtrl = node.children[0].GetComponent("AnimationController");
+        animCtrl.PlayExclusive("Models/Ninja_Idle3.ani", LAYER_MOVE, true);
+    }
 
     void SetControls(const Controls&in newControls)
     {

Engine/Graphics/OpenGL/OGLGraphics.cpp

@@ -130,10 +130,17 @@ static const unsigned glStencilOps[] =
     GL_DECR_WRAP
 };
 
-static unsigned numInstances = 0;
+// Remap vertex attributes on OpenGL so that all usually needed attributes including skinning fit to the first 8.
+// This avoids a skinning bug on GLES2 devices which only support 8.
+static const unsigned glVertexAttrIndex[] =
+{
+    0, 1, 2, 3, 4, 8, 9, 5, 6, 7, 10, 11, 12
+};
 
 static const unsigned MAX_FRAMEBUFFER_AGE = 2000;
 
+static unsigned numInstances = 0;
+
 OBJECTTYPESTATIC(Graphics);
 
 bool CheckExtension(const String& name)
@@ -695,6 +702,7 @@ bool Graphics::SetVertexBuffers(const Vector<VertexBuffer*>& buffers, const PODV
         
         for (unsigned j = 0; j < MAX_VERTEX_ELEMENTS; ++j)
         {
+            unsigned attrIndex = glVertexAttrIndex[j];
             unsigned elementBit = 1 << j;
             
             if (elementMask & elementBit)
@@ -704,12 +712,12 @@ bool Graphics::SetVertexBuffers(const Vector<VertexBuffer*>& buffers, const PODV
                 // Enable attribute if not enabled yet
                 if ((impl_->enabledAttributes_ & elementBit) == 0)
                 {
-                    glEnableVertexAttribArray(j);
+                    glEnableVertexAttribArray(attrIndex);
                     impl_->enabledAttributes_ |= elementBit;
                 }
                 
                 // Set the attribute pointer
-                glVertexAttribPointer(j, VertexBuffer::elementComponents[j], VertexBuffer::elementType[j],
+                glVertexAttribPointer(attrIndex, VertexBuffer::elementComponents[j], VertexBuffer::elementType[j],
                     VertexBuffer::elementNormalize[j], vertexSize, (const GLvoid*)(buffer->GetElementOffset((VertexElement)j)));
             }
         }
@@ -720,12 +728,13 @@ bool Graphics::SetVertexBuffers(const Vector<VertexBuffer*>& buffers, const PODV
     
     // Now check which vertex attributes should be disabled
     unsigned disableAttributes = impl_->enabledAttributes_ & (~newAttributes);
-    int disableIndex = 0;
+    unsigned disableIndex = 0;
+    
     while (disableAttributes)
     {
         if (disableAttributes & 1)
         {
-            glDisableVertexAttribArray(disableIndex);
+            glDisableVertexAttribArray(glVertexAttrIndex[disableIndex]);
             impl_->enabledAttributes_ &= ~(1 << disableIndex);
         }
         disableAttributes >>= 1;
@@ -766,6 +775,7 @@ bool Graphics::SetVertexBuffers(const Vector<SharedPtr<VertexBuffer> >& buffers,
                 elementMask = buffer->GetElementMask() & elementMasks[i];
         }
         
+        // If buffer and element mask have stayed the same, skip to the next buffer
         if (buffer == vertexBuffers_[i] && elementMask == elementMasks_[i] && !changed)
         {
             newAttributes |= elementMask;
@@ -776,6 +786,8 @@ bool Graphics::SetVertexBuffers(const Vector<SharedPtr<VertexBuffer> >& buffers,
         elementMasks_[i] = elementMask;
         changed = true;
         
+        // Beware buffers with missing OpenGL objects, as binding a zero buffer object means accessing CPU memory for vertex data,
+        // in which case the pointer will be invalid and cause a crash
         if (!buffer || !buffer->GetGPUObject())
             continue;
         
@@ -784,19 +796,22 @@ bool Graphics::SetVertexBuffers(const Vector<SharedPtr<VertexBuffer> >& buffers,
         
         for (unsigned j = 0; j < MAX_VERTEX_ELEMENTS; ++j)
         {
+            unsigned attrIndex = glVertexAttrIndex[j];
             unsigned elementBit = 1 << j;
             
             if (elementMask & elementBit)
             {
                 newAttributes |= elementBit;
                 
+                // Enable attribute if not enabled yet
                 if ((impl_->enabledAttributes_ & elementBit) == 0)
                 {
-                    glEnableVertexAttribArray(j);
+                    glEnableVertexAttribArray(attrIndex);
                     impl_->enabledAttributes_ |= elementBit;
                 }
                 
-                glVertexAttribPointer(j, VertexBuffer::elementComponents[j], VertexBuffer::elementType[j],
+                // Set the attribute pointer
+                glVertexAttribPointer(attrIndex, VertexBuffer::elementComponents[j], VertexBuffer::elementType[j],
                     VertexBuffer::elementNormalize[j], vertexSize, (const GLvoid*)(buffer->GetElementOffset((VertexElement)j)));
             }
         }
@@ -805,13 +820,15 @@ bool Graphics::SetVertexBuffers(const Vector<SharedPtr<VertexBuffer> >& buffers,
     if (!changed)
         return true;
     
+    // Now check which vertex attributes should be disabled
     unsigned disableAttributes = impl_->enabledAttributes_ & (~newAttributes);
-    int disableIndex = 0;
+    unsigned disableIndex = 0;
+    
     while (disableAttributes)
     {
         if (disableAttributes & 1)
         {
-            glDisableVertexAttribArray(disableIndex);
+            glDisableVertexAttribArray(glVertexAttrIndex[disableIndex]);
             impl_->enabledAttributes_ &= ~(1 << disableIndex);
         }
         disableAttributes >>= 1;

Engine/Graphics/OpenGL/OGLShaderProgram.cpp

@@ -94,16 +94,18 @@ bool ShaderProgram::Link()
     }
     
     // Bind vertex attribute locations to ensure they are the same in all shaders
+    // Note: this is not the same order as in VertexBuffer, instead a remapping is used to ensure everything except cube texture
+    // coordinates fit to the first 8 for better GLES2 device compatibility
     glBindAttribLocation(object_, 0, "iPos");
     glBindAttribLocation(object_, 1, "iNormal");
     glBindAttribLocation(object_, 2, "iColor");
     glBindAttribLocation(object_, 3, "iTexCoord");
     glBindAttribLocation(object_, 4, "iTexCoord2");
-    glBindAttribLocation(object_, 5, "iCubeTexCoord");
-    glBindAttribLocation(object_, 6, "iCubeTexCoord2");
-    glBindAttribLocation(object_, 7, "iTangent");
-    glBindAttribLocation(object_, 8, "iBlendWeights");
-    glBindAttribLocation(object_, 9, "iBlendIndices");
+    glBindAttribLocation(object_, 5, "iTangent");
+    glBindAttribLocation(object_, 6, "iBlendWeights");
+    glBindAttribLocation(object_, 7, "iBlendIndices");
+    glBindAttribLocation(object_, 8, "iCubeTexCoord");
+    glBindAttribLocation(object_, 9, "iCubeTexCoord2");
     glAttachShader(object_, vertexShader_->GetGPUObject());
     glAttachShader(object_, pixelShader_->GetGPUObject());
     glLinkProgram(object_);

Engine/Script/ScriptInstance.cpp

@@ -184,7 +184,7 @@ void ScriptInstance::DelayedExecute(float delay, bool repeat, const String& decl
     delayedMethodCalls_.Push(call);
     
     // Make sure we are registered to the scene update event, because delayed calls are executed there
-    if (!methods_[METHOD_UPDATE] && !HasSubscribedToEvent(E_SCENEUPDATE))
+    if (!methods_[METHOD_UPDATE] && !methods_[METHOD_DELAYEDSTART] && !HasSubscribedToEvent(E_SCENEUPDATE))
         SubscribeToEvent(GetScene(), E_SCENEUPDATE, HANDLER(ScriptInstance, HandleSceneUpdate));
 }
 
@@ -376,7 +376,7 @@ void ScriptInstance::GetSupportedMethods()
     Scene* scene = GetScene();
     if (scene)
     {
-        if (methods_[METHOD_UPDATE])
+        if (methods_[METHOD_UPDATE] || methods_[METHOD_DELAYEDSTART])
             SubscribeToEvent(scene, E_SCENEUPDATE, HANDLER(ScriptInstance, HandleSceneUpdate));
         if (methods_[METHOD_POSTUPDATE])
             SubscribeToEvent(scene, E_SCENEPOSTUPDATE, HANDLER(ScriptInstance, HandleScenePostUpdate));
@@ -448,12 +448,6 @@ void ScriptInstance::HandleScenePostUpdate(StringHash eventType, VariantMap& eve
     
     using namespace ScenePostUpdate;
     
-    if (methods_[METHOD_DELAYEDSTART] && !delayedStart_)
-    {
-        scriptFile_->Execute(scriptObject_, methods_[METHOD_DELAYEDSTART]);
-        delayedStart_ = true;
-    }
-    
     VariantVector parameters;
     parameters.Push(eventData[P_TIMESTEP]);
     scriptFile_->Execute(scriptObject_, methods_[METHOD_POSTUPDATE], parameters);
@@ -466,12 +460,6 @@ void ScriptInstance::HandlePhysicsPreStep(StringHash eventType, VariantMap& even
     
     using namespace PhysicsPreStep;
     
-    if (methods_[METHOD_DELAYEDSTART] && !delayedStart_)
-    {
-        scriptFile_->Execute(scriptObject_, methods_[METHOD_DELAYEDSTART]);
-        delayedStart_ = true;
-    }
-    
     if (!fixedUpdateFps_)
     {
         VariantVector parameters;
@@ -499,12 +487,6 @@ void ScriptInstance::HandlePhysicsPostStep(StringHash eventType, VariantMap& eve
     
     using namespace PhysicsPostStep;
     
-    if (methods_[METHOD_DELAYEDSTART] && !delayedStart_)
-    {
-        scriptFile_->Execute(scriptObject_, methods_[METHOD_DELAYEDSTART]);
-        delayedStart_ = true;
-    }
-    
     if (!fixedUpdateFps_)
     {
         VariantVector parameters;