Changed handling of uniforms of arrays of structs 18/273618/1
authorDavid Steele <david.steele@samsung.com>
Fri, 8 Apr 2022 16:02:56 +0000 (17:02 +0100)
committerDavid Steele <david.steele@samsung.com>
Fri, 8 Apr 2022 16:02:56 +0000 (17:02 +0100)
A) For uniforms of the form "basename[index].element", the array index handling
didn't work.

In GL (on Ubuntu) the shader reflection for such uniforms provides
individual locations for each element of the array/struct;

B) For uniforms of the form "basename[index]", where basename is a basic
type (float/vecN, etc), the GL shader reflection produces only 1 location
for the basename, and provides the element count.

Mapping properties to such uniforms is done by having a Property per
element. For the first case, no index/array handling is needed.
For the second case, each property needs to match to the basename without
the array subscript, and also store it's array index.

Modified property setup to remove array index for properties of the first type.

Modified uniform lookup to handle the different hashes appropriately.

Modified test graphics to ensure the uniform reflection can be set up in the
same way as GL provides.

Change-Id: I4be92f3e6933ff1b9b4a7d48e97f5629930c0b4c
Signed-off-by: David Steele <david.steele@samsung.com>
automated-tests/src/dali/dali-test-suite-utils/test-gl-abstraction.h
automated-tests/src/dali/dali-test-suite-utils/test-graphics-reflection.cpp
automated-tests/src/dali/utc-Dali-Renderer.cpp
dali/internal/render/renderers/render-renderer.cpp
dali/internal/render/renderers/render-renderer.h
dali/internal/render/shaders/program.cpp
dali/internal/render/shaders/program.h
dali/internal/update/common/uniform-map.h

index cc7fac4..a1f8405 100644 (file)
@@ -997,10 +997,17 @@ public:
       {
         name            = uniform.name.substr(0, iter);
         auto arrayCount = std::stoi(uniform.name.substr(iter + 1));
+        iter            = uniform.name.find("]");
+        std::string suffix;
+        if(iter != std::string::npos && iter + 1 != uniform.name.length())
+        {
+          suffix = uniform.name.substr(iter + 1); // If there is a suffix, it means its an element of an array of struct
+        }
+
         for(int i = 0; i < arrayCount; ++i)
         {
           std::stringstream nss;
-          nss << name << "[" << i << "]";
+          nss << name << "[" << i << "]" << suffix;
           GetUniformLocation(program, nss.str().c_str()); // Generate a GL loc per element
         }
       }
index 5b3da8c..6cb90ba 100644 (file)
@@ -147,8 +147,6 @@ TestGraphicsReflection::TestGraphicsReflection(TestGlAbstraction& gl, uint32_t p
   for(const auto& data : mCustomUniforms)
   {
     fprintf(stderr, "\ncustom uniforms: %s\n", data.name.c_str());
-    mDefaultUniformBlock.members.emplace_back();
-    auto& item = mDefaultUniformBlock.members.back();
 
     auto iter        = data.name.find("[", 0);
     int  numElements = 1;
@@ -161,26 +159,60 @@ TestGraphicsReflection::TestGraphicsReflection(TestGlAbstraction& gl, uint32_t p
       {
         numElements = 1;
       }
-
-      item.name         = baseName;
-      item.binding      = 0;
-      item.bufferIndex  = 0;
-      item.uniformClass = Graphics::UniformClass::UNIFORM;
-      item.type         = data.type;
-      item.numElements  = numElements;
-
-      for(int i = 0; i < numElements; ++i)
+      iter = data.name.find("]");
+      std::string suffix;
+      if(iter != std::string::npos && iter + 1 != data.name.length())
       {
-        std::stringstream elementNameStream;
-        elementNameStream << baseName << "[" << i << "]";
+        suffix = data.name.substr(iter + 1); // If there is a suffix, it means it is an element of an array of struct
+      }
 
-        item.locations.push_back(gl.GetUniformLocation(programId, elementNameStream.str().c_str()));
-        item.offsets.push_back(offset);
-        offset += GetSizeForType(data.type);
+      if(!suffix.empty())
+      {
+        // Write multiple items
+        for(int i = 0; i < numElements; ++i)
+        {
+          std::stringstream elementNameStream;
+          elementNameStream << baseName << "[" << i << "]" << suffix;
+          mDefaultUniformBlock.members.emplace_back();
+          auto& item   = mDefaultUniformBlock.members.back();
+          item.name    = elementNameStream.str();
+          item.binding = 0;
+          item.offsets.push_back(offset);
+          item.locations.push_back(gl.GetUniformLocation(programId, elementNameStream.str().c_str()));
+          item.bufferIndex  = 0;
+          item.uniformClass = Graphics::UniformClass::UNIFORM;
+          item.type         = data.type;
+          offset += GetSizeForType(data.type);
+        }
+      }
+      else
+      {
+        // Write 1 item with multiple elements
+        mDefaultUniformBlock.members.emplace_back();
+        auto& item = mDefaultUniformBlock.members.back();
+
+        item.name         = baseName;
+        item.binding      = 0;
+        item.bufferIndex  = 0;
+        item.uniformClass = Graphics::UniformClass::UNIFORM;
+        item.type         = data.type;
+        item.numElements  = numElements;
+
+        for(int i = 0; i < numElements; ++i)
+        {
+          std::stringstream elementNameStream;
+          elementNameStream << baseName << "[" << i << "]";
+          item.locations.push_back(gl.GetUniformLocation(programId, elementNameStream.str().c_str()));
+          item.offsets.push_back(offset);
+          offset += GetSizeForType(data.type);
+        }
       }
     }
     else
     {
+      // Write 1 item with 1 element
+      mDefaultUniformBlock.members.emplace_back();
+      auto& item   = mDefaultUniformBlock.members.back();
       item.name    = data.name;
       item.binding = 0;
       item.offsets.push_back(offset);
index 8557d2c..ba640e0 100644 (file)
@@ -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.
@@ -3787,3 +3787,117 @@ int UtcDaliRendererPreparePipeline(void)
 
   END_TEST;
 }
+
+int UtcDaliRendererUniformArrayOfStruct(void)
+{
+  TestApplication application;
+  tet_infoline("Test that uniforms that are elements of arrays of structs can be accessed");
+
+  std::vector<UniformData> customUniforms{{"arrayof[10].color", Property::VECTOR4},
+                                          {"arrayof[10].position", Property::VECTOR2},
+                                          {"arrayof[10].normal", Property::VECTOR3}};
+
+  application.GetGraphicsController().AddCustomUniforms(customUniforms);
+
+  Geometry geometry = CreateQuadGeometry();
+  Shader   shader   = Shader::New("vertexSrc", "fragmentSrc");
+  Renderer renderer = Renderer::New(geometry, shader);
+  Actor    actor    = Actor::New();
+  actor.AddRenderer(renderer);
+  actor[Actor::Property::SIZE] = Vector2(120, 120);
+  application.GetScene().Add(actor);
+
+  // Define some properties to match the custom uniforms.
+  // Ensure they can be written & read back from the abstraction.
+
+  struct UniformIndexPair
+  {
+    Property::Index index;
+    std::string     name;
+    UniformIndexPair(Property::Index index, std::string name)
+    : index(index),
+      name(name)
+    {
+    }
+  };
+  std::vector<UniformIndexPair> uniformIndices;
+
+  std::ostringstream oss;
+  for(int i = 0; i < 10; ++i)
+  {
+    Property::Index index;
+    oss << "arrayof[" << i << "].color";
+    Vector4 color = Color::WHITE;
+    color.r       = 25.5f * i;
+    index         = renderer.RegisterProperty(oss.str(), color);
+    uniformIndices.emplace_back(index, oss.str());
+
+    oss.str("");
+    oss.clear();
+    oss << "arrayof[" << i << "].position";
+    Vector2 pos(i, 10 + i * 5);
+    index = renderer.RegisterProperty(oss.str(), pos);
+    uniformIndices.emplace_back(index, oss.str());
+
+    oss.str("");
+    oss.clear();
+    oss << "arrayof[" << i << "].normal";
+    Vector3 normal(i, i * 10, i * 100);
+    index = renderer.RegisterProperty(oss.str(), normal);
+    uniformIndices.emplace_back(index, oss.str());
+    oss.str("");
+    oss.clear();
+  }
+  auto&           gl        = application.GetGlAbstraction();
+  TraceCallStack& callStack = gl.GetSetUniformTrace();
+  gl.EnableSetUniformCallTrace(true);
+
+  application.SendNotification();
+  application.Render();
+
+  // Check that the uniforms match.
+  TraceCallStack::NamedParams params;
+  for(auto& uniformInfo : uniformIndices)
+  {
+    Property::Value value = renderer.GetProperty(uniformInfo.index);
+    switch(value.GetType())
+    {
+      case Property::VECTOR2:
+      {
+        DALI_TEST_CHECK(callStack.FindMethodAndGetParameters(uniformInfo.name, params));
+        Vector2 setValue;
+        DALI_TEST_CHECK(gl.GetUniformValue<Vector2>(uniformInfo.name.c_str(), setValue));
+        DALI_TEST_EQUALS(value.Get<Vector2>(), setValue, 0.001f, TEST_LOCATION);
+        break;
+      }
+      case Property::VECTOR3:
+      {
+        DALI_TEST_CHECK(callStack.FindMethodAndGetParameters(uniformInfo.name, params));
+        Vector3 setValue;
+        DALI_TEST_CHECK(gl.GetUniformValue<Vector3>(uniformInfo.name.c_str(), setValue));
+        DALI_TEST_EQUALS(value.Get<Vector3>(), setValue, 0.001f, TEST_LOCATION);
+        break;
+      }
+      case Property::VECTOR4:
+      {
+        DALI_TEST_CHECK(callStack.FindMethodAndGetParameters(uniformInfo.name, params));
+        Vector4 setValue;
+        DALI_TEST_CHECK(gl.GetUniformValue<Vector4>(uniformInfo.name.c_str(), setValue));
+        DALI_TEST_EQUALS(value.Get<Vector4>(), setValue, 0.001f, TEST_LOCATION);
+        break;
+      }
+      default:
+        break;
+    }
+  }
+
+  // There is a hash in the property name's uniform map: check this in debugger
+  // There is a hash in the reflection. Check this in the debugger.
+
+  // Check that the reflection contains individual locs for each array entry's struct element
+  // and that it hashes the whole string
+
+  // Ensure that the property name's hash is also for the whole string.
+
+  END_TEST;
+}
index 3a84873..afd7656 100644 (file)
@@ -759,8 +759,8 @@ void Renderer::FillUniformBuffer(Program&                                      p
       {
         auto uniformInfo  = Graphics::UniformInfo{};
         auto uniformFound = program.GetUniform(uniform.uniformName.GetCString(),
-                                               uniform.uniformNameHashNoArray ? uniform.uniformNameHashNoArray
-                                                                              : uniform.uniformNameHash,
+                                               uniform.uniformNameHash,
+                                               uniform.uniformNameHashNoArray,
                                                uniformInfo);
 
         uniform.uniformOffset   = uniformInfo.offset;
index eb7764c..9d578a2 100644 (file)
@@ -533,7 +533,7 @@ private:
 
   Render::PipelineCache* mPipelineCache{nullptr};
 
-  using Hash = unsigned long;
+  using Hash = std::size_t;
 
   typedef const float& (PropertyInputImpl::*FuncGetter)(BufferIndex) const;
 
index fca4972..1b899fe 100644 (file)
@@ -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.
@@ -117,6 +117,13 @@ void Program::BuildReflection(const Graphics::Reflection& graphicsReflection)
     // for each member store data
     for(const auto& item : uboInfo.members)
     {
+      // Add a hash for the whole name.
+      //
+      // If the name represents an array of basic types, it won't contain an index
+      // operator "[",NN,"]".
+      //
+      // If the name represents an element in an array of structs, it will contain an
+      // index operator, but should be hashed in full.
       auto hashValue = CalculateHash(item.name);
       mReflection.emplace_back(ReflectionUniformInfo{hashValue, false, item});
 
@@ -169,35 +176,51 @@ void Program::BuildReflection(const Graphics::Reflection& graphicsReflection)
 
   // Calculate size of memory for uniform blocks
   mUniformBlockRequirements.totalSizeRequired = 0;
-  mUniformBlockRequirements.blockCount = graphicsReflection.GetUniformBlockCount();
-  for (auto i = 0u; i < mUniformBlockRequirements.blockCount; ++i)
+  mUniformBlockRequirements.blockCount        = graphicsReflection.GetUniformBlockCount();
+  for(auto i = 0u; i < mUniformBlockRequirements.blockCount; ++i)
   {
     auto blockSize = GetUniformBufferDataAlignment(graphicsReflection.GetUniformBlockSize(i));
     mUniformBlockRequirements.totalSizeRequired += blockSize;
   }
 }
 
-void Program::SetGraphicsProgram( Graphics::UniquePtr<Graphics::Program>&& program )
+void Program::SetGraphicsProgram(Graphics::UniquePtr<Graphics::Program>&& program)
 {
   mGfxProgram = std::move(program);
   BuildReflection(mGfxController.GetProgramReflection(*mGfxProgram.get()));
 }
 
-
-bool Program::GetUniform(const std::string& name, size_t hashedName, Graphics::UniformInfo& out) const
+bool Program::GetUniform(const std::string& name, Hash hashedName, Hash hashedNameNoArray, Graphics::UniformInfo& out) const
 {
   if(mReflection.empty())
   {
     return false;
   }
+  DALI_ASSERT_DEBUG(hashedName != 0 && "GetUniform() hash is not set");
+
+  // If name contains a "]", but has nothing after, it's an element in an array,
+  // The reflection doesn't contain such elements, it only contains the name without square brackets
+  // Use the hash without array subscript.
+
+  // If the name contains a "]" anywhere but the end, it's a structure element. The reflection
+  // does contain such elements, so use normal hash.
+  Hash               hash  = hashedName;
+  const std::string* match = &name;
 
-  hashedName = !hashedName ? CalculateHash(name, '[') : hashedName;
+  std::string baseName;
+  if(!name.empty() && name.back() == ']')
+  {
+    hash     = hashedNameNoArray;
+    auto pos = name.rfind("[");
+    baseName = name.substr(0, pos - 1); // Remove subscript
+    match    = &baseName;
+  }
 
   for(const ReflectionUniformInfo& item : mReflection)
   {
-    if(item.hashValue == hashedName)
+    if(item.hashValue == hash)
     {
-      if(!item.hasCollision || item.uniformInfo.name == name)
+      if(!item.hasCollision || item.uniformInfo.name == *match)
       {
         out = item.uniformInfo;
         return true;
index a0e2ba9..0d22e62 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_INTERNAL_PROGRAM_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.
@@ -50,6 +50,8 @@ class ProgramCache;
 class Program
 {
 public:
+  using Hash = std::size_t;
+
   /**
    * Indices of default uniforms
    */
@@ -77,7 +79,7 @@ public:
    * @param[in]  gfxController Reference to valid graphics Controller object
    * @return pointer to the program
    */
- static Program* New(ProgramCache& cache, Internal::ShaderDataPtr shaderData, Graphics::Controller& gfxController);
 static Program* New(ProgramCache& cache, Internal::ShaderDataPtr shaderData, Graphics::Controller& gfxController);
 
   /**
    * Set the projection matrix that has currently been sent
@@ -125,7 +127,7 @@ public:
     return mGfxProgram.get();
   }
 
-  void SetGraphicsProgram( Graphics::UniquePtr<Graphics::Program>&& program );
+  void SetGraphicsProgram(Graphics::UniquePtr<Graphics::Program>&& program);
 
   /**
    * Retrieves uniform data.
@@ -134,11 +136,12 @@ public:
    *
    * @param name Name of uniform
    * @param hashedName Hash value from name or 0 if unknown
+   * @param hashedNameNoArray Hash value from name without array index & trailing string, or 0 if unknown
    * @param out Reference to output structure
    *
    * @return False when uniform is not found or due to hash collision the result is ambiguous
    */
-  bool GetUniform(const std::string& name, size_t hashedName, Graphics::UniformInfo& out) const;
+  bool GetUniform(const std::string& name, Hash hashedName, Hash hashedNameNoArray, Graphics::UniformInfo& out) const;
 
   /**
    * Retrieves default uniform
@@ -218,9 +221,9 @@ private:                           // Data
 
   using UniformReflectionContainer = std::vector<ReflectionUniformInfo>;
 
-  UniformReflectionContainer mReflection{};                ///< Contains reflection build per program
-  UniformReflectionContainer mReflectionDefaultUniforms{}; ///< Contains default uniforms
-  UniformBlockMemoryRequirements mUniformBlockRequirements{}; ///< Memory requirements for uniform blocks
+  UniformReflectionContainer     mReflection{};                ///< Contains reflection build per program
+  UniformReflectionContainer     mReflectionDefaultUniforms{}; ///< Contains default uniforms
+  UniformBlockMemoryRequirements mUniformBlockRequirements{};  ///< Memory requirements for uniform blocks
 };
 
 } // namespace Internal
index fbfb9f6..0a505de 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_INTERNAL_SCENE_GRAPH_UNIFORM_MAP_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.
@@ -53,13 +53,17 @@ public:
     arrayIndex(0u)
   {
     // Look for array index closing bracket
-    auto pos = theUniformName.GetStringView().rfind("]");
+    auto nameStringView = theUniformName.GetStringView();
+    auto pos            = nameStringView.rfind("]");
 
-    // If found, extract the array index and store it
+    // If found, extract the array index and store it, if it's an element in an array of basic types.
     if(pos != std::string::npos)
     {
-      auto pos0  = theUniformName.GetStringView().rfind("[", pos);
-      arrayIndex = atoi(theUniformName.GetCString() + pos0 + 1);
+      auto pos0 = theUniformName.GetStringView().rfind("[", pos);
+      if(pos == nameStringView.length() - 1) // if element is in struct, don't set array index.
+      {
+        arrayIndex = atoi(theUniformName.GetCString() + pos0 + 1);
+      }
       // Calculate hash from name without array index
       uniformNameHashNoArray = Dali::CalculateHash(theUniformName.GetStringView().substr(0, pos0).data(), '[');
     }