Fix attribute cache bug

1. It is possible that GL_ACTIVE_ATTRIBUTES value is smaller than
location of attributes. If it happend, crashed due to mVertexInputAttributes.insert call.

To fix this crash issue, make mVertexInputAttributes size relative with location value,
not relative with GL_ACTIVE_ATTRIBUTES.
Also, It will guarantee that attribute's location can be index of it's real attrubutes.
(Since vector.insert operation shift the index, it was depend on the location input order.
and also slow)

2. Ensure the enable of vertex attributes.
Since  vertexInputState.attributes generated by Geometry in pipeline-cache system,
Change the attribute's list of geometry at the same shader was not worked.

TODO : Make VAO caching system works well. Currently, VAO doesn't useful.
We should found a way to EnableVertexAttribArray only with shader's information,
not depend on Geometry information.
or, we might need to calculate vertex attribute location list's hash, and use it as key.

Change-Id: I9f2c15617d279cfefbe1f8d72e5a9cbe43a7c564

diff --git a/automated-tests/src/dali-adaptor/dali-test-suite-utils/test-gl-abstraction.h b/automated-tests/src/dali-adaptor/dali-test-suite-utils/test-gl-abstraction.h
index 5e5fde3..7ba7834 100644 (file)
--- a/automated-tests/src/dali-adaptor/dali-test-suite-utils/test-gl-abstraction.h
+++ b/automated-tests/src/dali-adaptor/dali-test-suite-utils/test-gl-abstraction.h
@@ -885,6 +885,9 @@ public:
       case GL_ACTIVE_ATTRIBUTE_MAX_LENGTH:
         *params = 100;
         break;
+      case GL_ACTIVE_ATTRIBUTES:
+        *params = static_cast<GLint>(mAttribLocs.size());
+        break;
     }
   }
 

diff --git a/dali/internal/graphics/gles-impl/gles-context.cpp b/dali/internal/graphics/gles-impl/gles-context.cpp
index 1d981bc..b75b08b 100644 (file)
--- a/dali/internal/graphics/gles-impl/gles-context.cpp
+++ b/dali/internal/graphics/gles-impl/gles-context.cpp
@@ -64,6 +64,13 @@ struct Context::Impl
         mProgramVAOCurrentState = iter->second;
         gl.BindVertexArray(iter->second);
       }
+
+      // We should re-check enable attribute usage because geometry might be changed.
+      // @todo : We can remove this loop if we enable vertex attrib by shader's information.
+      for(const auto& attr : vertexInputState.attributes)
+      {
+        gl.EnableVertexAttribArray(attr.location);
+      }
       return;
     }
 
@@ -71,6 +78,9 @@ struct Context::Impl
     gl.GenVertexArrays(1, &vao);
     gl.BindVertexArray(vao);
     mProgramVAOMap[program] = vao;
+
+    // @todo : Enable vertex attrib only by shader's information, not with Geometry.
+    // Currently, vertexInputState.attributes depend on Geometry's VertexBuffer.
     for(const auto& attr : vertexInputState.attributes)
     {
       gl.EnableVertexAttribArray(attr.location);

diff --git a/dali/internal/graphics/gles-impl/gles-graphics-reflection.cpp b/dali/internal/graphics/gles-impl/gles-graphics-reflection.cpp
index 05776c2..abb066b 100644 (file)
--- a/dali/internal/graphics/gles-impl/gles-graphics-reflection.cpp
+++ b/dali/internal/graphics/gles-impl/gles-graphics-reflection.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -220,6 +220,8 @@ void Reflection::BuildVertexAttributeReflection()
   mVertexInputAttributes.clear();
   mVertexInputAttributes.resize(nAttribs);
 
+  int maximumLocation = nAttribs - 1;
+
   name = new GLchar[maxLength];
   for(int i = 0; i < nAttribs; i++)
   {
@@ -228,13 +230,21 @@ void Reflection::BuildVertexAttributeReflection()
 
     if(location >= 0)
     {
+      if(maximumLocation < location)
+      {
+        maximumLocation = location;
+        // Increate continer size s.t. we can use maximumLocation as index.
+        mVertexInputAttributes.resize(maximumLocation + 1u);
+      }
+
       AttributeInfo attributeInfo;
-      attributeInfo.location = location;
-      attributeInfo.name     = name;
-      attributeInfo.format   = GetVertexAttributeTypeFormat(type);
-      mVertexInputAttributes.insert(mVertexInputAttributes.begin() + location, attributeInfo);
+      attributeInfo.location           = location;
+      attributeInfo.name               = name;
+      attributeInfo.format             = GetVertexAttributeTypeFormat(type);
+      mVertexInputAttributes[location] = std::move(attributeInfo);
     }
   }
+
   delete[] name;
 }
 

diff --git a/dali/internal/graphics/gles-impl/gles-graphics-reflection.h b/dali/internal/graphics/gles-impl/gles-graphics-reflection.h
index 06cfba1..d1bca95 100644 (file)
--- a/dali/internal/graphics/gles-impl/gles-graphics-reflection.h
+++ b/dali/internal/graphics/gles-impl/gles-graphics-reflection.h
@@ -2,7 +2,7 @@
 #define DALI_GRAPHICS_GLES_REFLECTION_H
 
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -254,7 +254,7 @@ private:
 
   struct AttributeInfo
   {
-    uint32_t                                   location{};
+    uint32_t                                   location{ERROR_ATTRIBUTE_NOT_FOUND};
     std::string                                name{};
     Dali::Graphics::VertexInputAttributeFormat format{};
   };