[Tizen] Fix invalidated PipelineCacheL2 pointer problem

Let we keep std::list<PipelineCacheL2>::iterator instead of PipelineCacheL2*.

So, the pointer of pipeline what render-renderer has should be valid even
if cache container size changed.

And also, for safe issue, let we keep the pipeline container as std::list.

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

diff --git a/automated-tests/src/dali-internal/utc-Dali-Internal-PipelineCache.cpp b/automated-tests/src/dali-internal/utc-Dali-Internal-PipelineCache.cpp
index 34591ea..9ebd463 100644 (file)
--- a/automated-tests/src/dali-internal/utc-Dali-Internal-PipelineCache.cpp
+++ b/automated-tests/src/dali-internal/utc-Dali-Internal-PipelineCache.cpp
@@ -148,7 +148,20 @@ int UtcDaliCorePipelineCacheTest(void)
   application.Render();
 
   // Test whether noBlend pipeline is set in cache
-  DALI_TEST_EQUALS(gPipelineCache->level0nodes[0].level1nodes[0].noBlend.pipeline != nullptr, true, TEST_LOCATION);
+  uint32_t noBlendFoundCount = 0u;
+  for(auto& iterLevel0 : gPipelineCache->level0nodes)
+  {
+    for(auto& iterLevel1 : iterLevel0.level1nodes)
+    {
+      if(!iterLevel1.noBlends.empty() && iterLevel1.noBlends.begin()->pipeline != nullptr)
+      {
+        noBlendFoundCount++;
+      }
+    }
+  }
+
+  DALI_TEST_EQUALS(gPipelineCache->level0nodes.size(), 2, TEST_LOCATION);
+  DALI_TEST_EQUALS(noBlendFoundCount, 1u, TEST_LOCATION);
 
   // Remove renderer to test whether old pipeline is removed
   application.GetScene().Remove(actor1);

diff --git a/dali/internal/render/renderers/pipeline-cache.cpp b/dali/internal/render/renderers/pipeline-cache.cpp
index 4335022..4f71b91 100644 (file)
--- a/dali/internal/render/renderers/pipeline-cache.cpp
+++ b/dali/internal/render/renderers/pipeline-cache.cpp
@@ -192,7 +192,7 @@ constexpr Graphics::BlendOp ConvertBlendEquation(DevelBlendEquation::Type blendE
 }
 } // namespace
 
-PipelineCacheL0* PipelineCache::GetPipelineCacheL0(Program* program, Render::Geometry* geometry)
+PipelineCacheL0Ptr PipelineCache::GetPipelineCacheL0(Program* program, Render::Geometry* geometry)
 {
   auto it = std::find_if(level0nodes.begin(), level0nodes.end(), [program, geometry](PipelineCacheL0& item) {
     return ((item.program == program && item.geometry == geometry));
@@ -242,14 +242,14 @@ PipelineCacheL0* PipelineCache::GetPipelineCacheL0(Program* program, Render::Geo
     level0.program    = program;
     level0.geometry   = geometry;
     level0.inputState = vertexInputState;
-    level0nodes.emplace_back(std::move(level0));
-    it = level0nodes.end() - 1;
+
+    it = level0nodes.insert(level0nodes.end(), std::move(level0));
   }
 
-  return &*it;
+  return it;
 }
 
-PipelineCacheL1* PipelineCacheL0::GetPipelineCacheL1(Render::Renderer* renderer, bool usingReflection)
+PipelineCacheL1Ptr PipelineCacheL0::GetPipelineCacheL1(Render::Renderer* renderer, bool usingReflection)
 {
   // hash must be collision free
   uint32_t hash = 0;
@@ -294,11 +294,8 @@ PipelineCacheL1* PipelineCacheL0::GetPipelineCacheL1(Render::Renderer* renderer,
   hash = (topo & 0xffu) | ((cull << 8u) & 0xff00u) | ((uint32_t(poly) << 16u) & 0xff0000u);
 
   // If L1 not found by hash, create rasterization state describing pipeline and store it
-  auto it = std::find_if(level1nodes.begin(), level1nodes.end(), [hash](PipelineCacheL1& item) {
-    return item.hashCode == hash;
-  });
+  auto it = std::find_if(level1nodes.begin(), level1nodes.end(), [hash](PipelineCacheL1& item) { return item.hashCode == hash; });
 
-  PipelineCacheL1* retval = nullptr;
   if(it == level1nodes.end())
   {
     PipelineCacheL1 item;
@@ -307,14 +304,11 @@ PipelineCacheL1* PipelineCacheL0::GetPipelineCacheL1(Render::Renderer* renderer,
     item.rs.frontFace   = Graphics::FrontFace::COUNTER_CLOCKWISE;
     item.rs.polygonMode = poly; // not in use
     item.ia.topology    = geometry->GetTopology();
-    level1nodes.emplace_back(std::move(item));
-    retval = &level1nodes.back();
-  }
-  else
-  {
-    retval = &*it;
+
+    it = level1nodes.insert(level1nodes.end(), std::move(item));
   }
-  return retval;
+
+  return it;
 }
 
 void PipelineCacheL0::ClearUnusedCache()
@@ -332,29 +326,34 @@ void PipelineCacheL0::ClearUnusedCache()
   }
 }
 
-PipelineCacheL2* PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, BlendingOptions& blendingOptions)
+PipelineCacheL2Ptr PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, BlendingOptions& blendingOptions)
 {
   // early out
   if(!blend)
   {
+    if(DALI_UNLIKELY(noBlends.empty()))
+    {
+      noBlends.emplace_back(PipelineCacheL2{});
+    }
+
+    auto& noBlend = *noBlends.begin();
+
     if(noBlend.pipeline == nullptr)
     {
       // reset all before returning if pipeline has never been created for that case
       noBlend.hash = 0;
       memset(&noBlend.colorBlendState, 0, sizeof(Graphics::ColorBlendState));
     }
-    return &noBlend;
+    return noBlends.begin();
   }
 
   auto bitmask = uint32_t(blendingOptions.GetBitmask());
 
   // Find by bitmask (L2 entries must be sorted by bitmask)
-  auto it = std::find_if(level2nodes.begin(), level2nodes.end(), [bitmask](PipelineCacheL2& item) {
-    return item.hash == bitmask;
-  });
+  auto it = std::find_if(level2nodes.begin(), level2nodes.end(), [bitmask](PipelineCacheL2& item) { return item.hash == bitmask; });
 
   // TODO: find better way of blend constants lookup
-  PipelineCacheL2* retval = nullptr;
+  PipelineCacheL2Ptr retval = level2nodes.end();
   if(it != level2nodes.end())
   {
     bool hasBlendColor = blendingOptions.GetBlendColor();
@@ -363,20 +362,20 @@ PipelineCacheL2* PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, Bl
       Vector4 v(it->colorBlendState.blendConstants);
       if(v == *blendingOptions.GetBlendColor())
       {
-        retval = &*it;
+        retval = it;
       }
       ++it;
     }
     if(!hasBlendColor)
     {
-      retval = &*it;
+      retval = it;
     }
   }
 
-  if(!retval)
+  if(retval == level2nodes.end())
   {
     // create new entry and return it with null pipeline
-    PipelineCacheL2 l2;
+    PipelineCacheL2 l2{};
     l2.pipeline           = nullptr;
     auto& colorBlendState = l2.colorBlendState;
     colorBlendState.SetBlendEnable(true);
@@ -407,11 +406,10 @@ PipelineCacheL2* PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, Bl
     }
 
     l2.hash = blendingOptions.GetBitmask();
-    level2nodes.emplace_back(std::move(l2));
 
-    std::sort(level2nodes.begin(), level2nodes.end(), [](PipelineCacheL2& lhs, PipelineCacheL2& rhs) {
-      return lhs.hash < rhs.hash;
-    });
+    auto upperBound = std::upper_bound(level2nodes.begin(), level2nodes.end(), l2, [](const PipelineCacheL2& lhs, const PipelineCacheL2& rhs) { return lhs.hash < rhs.hash; });
+
+    level2nodes.insert(upperBound, std::move(l2));
 
     // run same function to retrieve retval
     retval = GetPipelineCacheL2(blend, premul, blendingOptions);
@@ -422,11 +420,6 @@ PipelineCacheL2* PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, Bl
 
 bool PipelineCacheL1::ClearUnusedCache()
 {
-  if(noBlend.referenceCount > 0)
-  {
-    return false;
-  }
-
   for(auto iter = level2nodes.begin(); iter != level2nodes.end();)
   {
     if(iter->referenceCount == 0)
@@ -439,6 +432,11 @@ bool PipelineCacheL1::ClearUnusedCache()
     }
   }
 
+  if(!noBlends.empty() && noBlends.begin()->referenceCount > 0)
+  {
+    return false;
+  }
+
   return level2nodes.empty();
 }
 
@@ -449,9 +447,10 @@ PipelineCache::PipelineCache(Graphics::Controller& controller)
 
 PipelineResult PipelineCache::GetPipeline(const PipelineCacheQueryInfo& queryInfo, bool createNewIfNotFound)
 {
-  auto* level0 = GetPipelineCacheL0(queryInfo.program, queryInfo.geometry);
-  auto* level1 = level0->GetPipelineCacheL1(queryInfo.renderer, queryInfo.cameraUsingReflection);
-  auto* level2 = level1->GetPipelineCacheL2(queryInfo.blendingEnabled, queryInfo.alphaPremultiplied, *queryInfo.blendingOptions);
+  auto level0 = GetPipelineCacheL0(queryInfo.program, queryInfo.geometry);
+  auto level1 = level0->GetPipelineCacheL1(queryInfo.renderer, queryInfo.cameraUsingReflection);
+
+  PipelineCachePtr level2 = level1->GetPipelineCacheL2(queryInfo.blendingEnabled, queryInfo.alphaPremultiplied, *queryInfo.blendingOptions);
 
   // Create new pipeline at level2 if requested
   if(level2->pipeline == nullptr && createNewIfNotFound)
@@ -509,12 +508,10 @@ void PipelineCache::ClearUnusedCache()
   }
 }
 
-void PipelineCache::ResetPipeline(PipelineCacheL2* pipelineCache)
+void PipelineCache::ResetPipeline(PipelineCachePtr pipelineCache)
 {
-  if(pipelineCache)
-  {
-    pipelineCache->referenceCount--;
-  }
+  // TODO : Can we always assume that pipelineCache input is valid iterator?
+  pipelineCache->referenceCount--;
 }
 
 } // namespace Dali::Internal::Render

diff --git a/dali/internal/render/renderers/pipeline-cache.h b/dali/internal/render/renderers/pipeline-cache.h
index ef55951..34a3a51 100644 (file)
--- a/dali/internal/render/renderers/pipeline-cache.h
+++ b/dali/internal/render/renderers/pipeline-cache.h
@@ -22,9 +22,7 @@
 #include <dali/graphics-api/graphics-pipeline.h>
 #include <dali/graphics-api/graphics-types.h>
 #include <dali/internal/common/blending-options.h>
-
-// EXTERNAL INCLUDES
-#include <vector>
+#include <dali/public-api/common/list-wrapper.h>
 
 namespace Dali::Internal
 {
@@ -34,6 +32,18 @@ namespace Render
 class Renderer;
 class Geometry;
 
+struct PipelineCacheL2;
+struct PipelineCacheL1;
+struct PipelineCacheL0;
+using PipelineCacheL2Container = std::list<PipelineCacheL2>;
+using PipelineCacheL1Container = std::list<PipelineCacheL1>;
+using PipelineCacheL0Container = std::list<PipelineCacheL0>;
+using PipelineCacheL2Ptr       = PipelineCacheL2Container::iterator;
+using PipelineCacheL1Ptr       = PipelineCacheL1Container::iterator;
+using PipelineCacheL0Ptr       = PipelineCacheL0Container::iterator;
+
+using PipelineCachePtr = PipelineCacheL2Ptr;
+
 /**
  * Cache Level 2 : Last level of cache, stores actual pipeline
  */
@@ -41,8 +51,8 @@ struct PipelineCacheL2
 {
   uint32_t                                hash{};
   uint32_t                                referenceCount{0u};
-  Graphics::ColorBlendState               colorBlendState;
-  Graphics::UniquePtr<Graphics::Pipeline> pipeline;
+  Graphics::ColorBlendState               colorBlendState{};
+  Graphics::UniquePtr<Graphics::Pipeline> pipeline{};
 };
 
 /**
@@ -50,7 +60,7 @@ struct PipelineCacheL2
  */
 struct PipelineCacheL1
 {
-  PipelineCacheL2* GetPipelineCacheL2(bool blend, bool premul, BlendingOptions& blendingOptions);
+  PipelineCacheL2Ptr GetPipelineCacheL2(bool blend, bool premul, BlendingOptions& blendingOptions);
 
   /**
    * @brief Clear unused caches.
@@ -61,8 +71,8 @@ struct PipelineCacheL1
   Graphics::RasterizationState rs{};
   Graphics::InputAssemblyState ia{};
 
-  PipelineCacheL2              noBlend; // special case
-  std::vector<PipelineCacheL2> level2nodes;
+  PipelineCacheL2Container noBlends; // special case
+  PipelineCacheL2Container level2nodes;
 };
 
 /**
@@ -70,7 +80,7 @@ struct PipelineCacheL1
  */
 struct PipelineCacheL0 // L0 cache
 {
-  PipelineCacheL1* GetPipelineCacheL1(Render::Renderer* renderer, bool usingReflection);
+  PipelineCacheL1Ptr GetPipelineCacheL1(Render::Renderer* renderer, bool usingReflection);
 
   /**
    * @brief Clear unused caches.
@@ -81,7 +91,7 @@ struct PipelineCacheL0 // L0 cache
   Program*                   program{};
   Graphics::VertexInputState inputState;
 
-  std::vector<PipelineCacheL1> level1nodes;
+  PipelineCacheL1Container level1nodes;
 };
 
 struct PipelineCacheQueryInfo
@@ -105,7 +115,7 @@ struct PipelineCacheQueryInfo
 struct PipelineResult
 {
   Graphics::Pipeline* pipeline;
-  PipelineCacheL2*    level2;
+  PipelineCachePtr    level2;
 };
 
 /**
@@ -123,7 +133,7 @@ public:
   /**
    * Retrieves next cache level
    */
-  PipelineCacheL0* GetPipelineCacheL0(Program* program, Render::Geometry* geometry);
+  PipelineCacheL0Ptr GetPipelineCacheL0(Program* program, Render::Geometry* geometry);
 
   /**
    * Retrieves pipeline matching queryInfo struct
@@ -141,7 +151,7 @@ public:
    * @brief Decrease the reference count of the pipeline cache.
    * @param pipelineCache The pipeline cache to decrease the reference count
    */
-  void ResetPipeline(PipelineCacheL2* pipelineCache);
+  void ResetPipeline(PipelineCachePtr pipelineCache);
 
 private:
   /**
@@ -150,8 +160,8 @@ private:
   void ClearUnusedCache();
 
 private:
-  Graphics::Controller*        graphicsController{nullptr};
-  std::vector<PipelineCacheL0> level0nodes;
+  Graphics::Controller*    graphicsController{nullptr};
+  PipelineCacheL0Container level0nodes;
 
   uint32_t mFrameCount{0u};
 };

diff --git a/dali/internal/render/renderers/render-renderer.cpp b/dali/internal/render/renderers/render-renderer.cpp
index 803b0a3..f347198 100644 (file)
--- a/dali/internal/render/renderers/render-renderer.cpp
+++ b/dali/internal/render/renderers/render-renderer.cpp
@@ -187,7 +187,8 @@ Renderer::Renderer(SceneGraph::RenderDataProvider* dataProvider,
   mDepthTestMode(depthTestMode),
   mPremultipliedAlphaEnabled(preMultipliedAlphaEnabled),
   mShaderChanged(false),
-  mUpdated(true)
+  mUpdated(true),
+  mPipelineCached(false)
 {
   if(blendingBitmask != 0u)
   {
@@ -209,7 +210,11 @@ void Renderer::Initialize(Graphics::Controller& graphicsController, ProgramCache
 Renderer::~Renderer()
 {
   // Reset old pipeline
-  mPipelineCache->ResetPipeline(mPipeline);
+  if(DALI_LIKELY(mPipelineCached))
+  {
+    mPipelineCache->ResetPipeline(mPipeline);
+    mPipelineCached = false;
+  }
 }
 
 void Renderer::SetGeometry(Render::Geometry* geometry)
@@ -953,12 +958,17 @@ Graphics::Pipeline& Renderer::PrepareGraphicsPipeline(
   queryInfo.cameraUsingReflection = instruction.GetCamera()->GetReflectionUsed();
 
   // Reset old pipeline
-  mPipelineCache->ResetPipeline(mPipeline);
+  if(DALI_LIKELY(mPipelineCached))
+  {
+    mPipelineCache->ResetPipeline(mPipeline);
+    mPipelineCached = false;
+  }
 
   // Find or generate new pipeline.
   auto pipelineResult = mPipelineCache->GetPipeline(queryInfo, true);
 
-  mPipeline = pipelineResult.level2;
+  mPipeline       = pipelineResult.level2;
+  mPipelineCached = true;
 
   // should be never null?
   return *pipelineResult.pipeline;

diff --git a/dali/internal/render/renderers/render-renderer.h b/dali/internal/render/renderers/render-renderer.h
index 1ba37f1..a954035 100644 (file)
--- a/dali/internal/render/renderers/render-renderer.h
+++ b/dali/internal/render/renderers/render-renderer.h
@@ -19,6 +19,7 @@
  */
 
 // INTERNAL INCLUDES
+#include <dali/public-api/common/list-wrapper.h>
 #include <dali/public-api/common/vector-wrapper.h>
 #include <dali/public-api/math/matrix.h>
 #include <dali/public-api/math/vector4.h>
@@ -60,6 +61,9 @@ class PipelineCache;
 class PipelineCacheL2;
 class UniformBufferManager;
 
+using PipelineCacheL2Container = std::list<PipelineCacheL2>;
+using PipelineCachePtr         = PipelineCacheL2Container::iterator;
+
 /**
  * Renderers are used to render meshes
  * These objects are used during RenderManager::Render(), so properties modified during
@@ -547,7 +551,7 @@ private:
   std::vector<Graphics::UniformBufferBinding> mUniformBufferBindings{};
 
   Render::PipelineCache* mPipelineCache{nullptr};
-  PipelineCacheL2*       mPipeline{nullptr};
+  PipelineCachePtr       mPipeline{};
 
   using Hash = std::size_t;
 
@@ -595,6 +599,7 @@ private:
   bool                  mPremultipliedAlphaEnabled : 1; ///< Flag indicating whether the Pre-multiplied Alpha Blending is required
   bool                  mShaderChanged : 1;             ///< Flag indicating the shader changed and uniform maps have to be updated
   bool                  mUpdated : 1;
+  bool                  mPipelineCached : 1;            ///< Flag indicating whether renderer cache valid pipeline or not.
 
   std::vector<Dali::DevelRenderer::DrawCommand> mDrawCommands; // Devel stuff
   RenderCallback*                               mRenderCallback{nullptr};