(Vulkan::Reflection) Append uniform blocks only 1 times per each programs + Clean...

Since we create relfections per each shader at vulkan side,
there was some problem if we try to use same uniform block at both
vertex and fragment shader.

Since current spriv don't know the uniform block's name,
let we just seperate it by binding of layout.

+

Their might be issue if VertexShader code have sampler2D (e.g. Scene3D blendshape)
It is code side bug, and we don't need to append sampler informations
whenever we call BuildReflection().

To make ensure code clean, let we clear mUniformBlocks and mUniformOpaques
at the begin of BuildReflection(), and sort + unique each of them.

TODO : Shouldn't we need to consider multi-set case?

Change-Id: Icece552a1dff9f144be580e93e7f3d65d401cf6a
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>

diff --git a/dali/internal/graphics/vulkan-impl/vulkan-reflection.cpp b/dali/internal/graphics/vulkan-impl/vulkan-reflection.cpp
index 145f243ec596b4fb05f59d869d2519f31d7c188a..a1b9ec3f582f311f24df75d9b3b9c9ec86fc32c3 100644 (file)
--- a/dali/internal/graphics/vulkan-impl/vulkan-reflection.cpp
+++ b/dali/internal/graphics/vulkan-impl/vulkan-reflection.cpp
@@ -126,7 +126,11 @@ void Reflection::BuildReflection()
 
   // initialize list of samplers
   // NOTE: We support only COMBINED_IMAGE_SAMPLER type currently (regular sampler on the GLES side)
-  std::vector<UniformInfo> samplers;
+  mUniformOpaques.clear();
+
+  // Ensure to clear the layout list
+  mVkDescriptorSetLayoutCreateInfoList.clear();
+  mVkDescriptorSetLayoutBindingList.clear();
 
   // build descriptor set layout (currently, we support only one set!)
   std::unordered_map<uint32_t, vk::DescriptorSetLayoutCreateInfo*> boundSets;
@@ -147,8 +151,7 @@ void Reflection::BuildReflection()
     }
 
     // helper lambda if we need to check more types of pipeline stages in the future
-    auto CheckStageIfDone = [stage](auto expectedStage, auto& variable, const char* stageName) -> StageCheckResult
-    {
+    auto CheckStageIfDone = [stage](auto expectedStage, auto& variable, const char* stageName) -> StageCheckResult {
       if(stage == expectedStage)
       {
         if(!variable)
@@ -258,11 +261,14 @@ void Reflection::BuildReflection()
           out.bufferIndex   = blockIndex++; // TODO: do we need this for Vulkan?
           block.size += memb.padded_size;
         }
+
+        // Sort members by offset
+        std::sort(block.members.begin(), block.members.end(), [](auto& lhs, auto& rhs) { return lhs.offset < rhs.offset; });
       }
       else if(binding->descriptor_type == SPV_REFLECT_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER)
       {
-        samplers.emplace_back();
-        auto uniformInfo          = &samplers.back();
+        mUniformOpaques.emplace_back();
+        auto uniformInfo          = &mUniformOpaques.back();
         uniformInfo->uniformClass = UniformClass::COMBINED_IMAGE_SAMPLER;
         uniformInfo->name         = binding->name;
         uniformInfo->offset       = 0;
@@ -271,19 +277,32 @@ void Reflection::BuildReflection()
       }
     }
 
-    if(!samplers.empty())
+    spvReflectDestroyShaderModule(&module);
+  }
+
+  // Merge shared sampler bindings
+  if(!mUniformOpaques.empty())
+  {
+    // sort samplers by bindings, and erase duplicated bindings.
+    std::sort(mUniformOpaques.begin(), mUniformOpaques.end(), [](auto& lhs, auto& rhs) { return lhs.binding < rhs.binding; });
+    mUniformOpaques.erase(std::unique(mUniformOpaques.begin(), mUniformOpaques.end(), [](auto& lhs, auto& rhs) { return lhs.binding == rhs.binding; }), mUniformOpaques.end());
+
+    // Apply location.
+    for(auto i = 0u; i < mUniformOpaques.size(); ++i)
     {
-      mUniformOpaques.insert(mUniformOpaques.end(), samplers.begin(), samplers.end());
-      // sort samplers by bindings
-      std::sort(mUniformOpaques.begin(), mUniformOpaques.end(), [](auto& lhs, auto& rhs)
-                { return lhs.binding < rhs.binding; });
-      for(auto i = 0u; i < mUniformOpaques.size(); ++i)
-      {
-        mUniformOpaques[i].location = i;
-      }
+      mUniformOpaques[i].location = i;
     }
+  }
 
-    spvReflectDestroyShaderModule(&module);
+  // Merge shared UBO block bindings
+  // Note that 0'th ubo is standalone block, so need to ignore it.
+  if(mUniformBlocks.size() > 1u)
+  {
+    // sort blocks by bindings, and erase duplicated bindings.
+    std::sort(mUniformBlocks.begin() + 1u, mUniformBlocks.end(), [](auto& lhs, auto& rhs) { return lhs.binding < rhs.binding; });
+
+    // @todo: Need to make compile error if uniform blocks with same binding has difference member infomations!
+    mUniformBlocks.erase(std::unique(mUniformBlocks.begin() + 1u, mUniformBlocks.end(), [](auto& lhs, auto& rhs) { return lhs.binding == rhs.binding; }), mUniformBlocks.end());
   }
 
   auto vkDevice = mController.GetGraphicsDevice().GetLogicalDevice();

diff --git a/dali/internal/graphics/vulkan-impl/vulkan-reflection.h b/dali/internal/graphics/vulkan-impl/vulkan-reflection.h
index b0a194b0b84f88497e066d235fa31afe9be0de12..c85c12d7815080c15ec6323cf3cab7bd31c8c5e2 100644 (file)
--- a/dali/internal/graphics/vulkan-impl/vulkan-reflection.h
+++ b/dali/internal/graphics/vulkan-impl/vulkan-reflection.h
@@ -2,7 +2,7 @@
 #define DALI_GRAPHICS_VULKAN_REFLECTION_H
 
 /*
- * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2025 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.
@@ -199,6 +199,11 @@ public:
   [[nodiscard]] Graphics::ShaderLanguage GetLanguage() const override;
 
 public:
+  vk::PipelineLayout GetVkPipelineLayout() const;
+
+  const std::vector<vk::DescriptorSetLayout>& GetVkDescriptorSetLayouts() const;
+
+private:
   /**
    * @brief Build the reflection of vertex attributes
    */
@@ -206,10 +211,6 @@ public:
 
   void BuildReflection();
 
-  vk::PipelineLayout GetVkPipelineLayout() const;
-
-  const std::vector<vk::DescriptorSetLayout>& GetVkDescriptorSetLayouts() const;
-
 protected:
   Reflection(Reflection&&) = default;
   Reflection& operator=(Reflection&&) = default;