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 352bce9..0d3e9f6 100644 (file)
--- a/automated-tests/src/dali-internal/utc-Dali-Internal-PipelineCache.cpp
+++ b/automated-tests/src/dali-internal/utc-Dali-Internal-PipelineCache.cpp
@@ -153,7 +153,7 @@ int UtcDaliCorePipelineCacheTest(void)
   {
     for(auto& iterLevel1 : iterLevel0.level1nodes)
     {
-      if(iterLevel1.noBlend.pipeline != nullptr)
+      if(!iterLevel1.noBlends.empty() && iterLevel1.noBlends.begin()->pipeline != nullptr)
       {
         noBlendFoundCount++;
       }

diff --git a/dali/internal/render/renderers/pipeline-cache.cpp b/dali/internal/render/renderers/pipeline-cache.cpp
index 92fe22b..9839d95 100644 (file)
--- a/dali/internal/render/renderers/pipeline-cache.cpp
+++ b/dali/internal/render/renderers/pipeline-cache.cpp
@@ -192,11 +192,9 @@ constexpr Graphics::BlendOp ConvertBlendEquation(DevelBlendEquation::Type blendE
 }
 } // namespace
 
-PipelineCacheL0* PipelineCache::GetPipelineCacheL0(std::size_t hash, Program* program, Render::Geometry* geometry)
+PipelineCacheL0Ptr PipelineCache::GetPipelineCacheL0(std::size_t hash, Program* program, Render::Geometry* geometry)
 {
-  auto it = std::find_if(level0nodes.begin(), level0nodes.end(), [hash, program, geometry](PipelineCacheL0& item) {
-    return ((item.hash == hash && item.program == program && item.geometry == geometry));
-  });
+  auto it = std::find_if(level0nodes.begin(), level0nodes.end(), [hash, program, geometry](PipelineCacheL0& item) { return ((item.hash == hash && item.program == program && item.geometry == geometry)); });
 
   // Add new node to cache
   if(it == level0nodes.end())
@@ -249,14 +247,14 @@ PipelineCacheL0* PipelineCache::GetPipelineCacheL0(std::size_t hash, Program* pr
     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;
@@ -301,11 +299,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;
@@ -314,14 +309,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()
@@ -339,29 +331,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();
@@ -370,20 +367,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);
@@ -414,11 +411,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);
@@ -429,11 +425,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)
@@ -446,6 +437,11 @@ bool PipelineCacheL1::ClearUnusedCache()
     }
   }
 
+  if(!noBlends.empty() && noBlends.begin()->referenceCount > 0)
+  {
+    return false;
+  }
+
   return level2nodes.empty();
 }
 
@@ -502,9 +498,10 @@ PipelineResult PipelineCache::GetPipeline(const PipelineCacheQueryInfo& queryInf
     return mLatestResult[latestUsedCacheIndex];
   }
 
-  auto* level0 = GetPipelineCacheL0(queryInfo.hash, 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.hash, 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)
@@ -573,12 +570,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 659cecc..c153716 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.
@@ -82,7 +92,7 @@ struct PipelineCacheL0 // L0 cache
   Program*                   program{};
   Graphics::VertexInputState inputState;
 
-  std::vector<PipelineCacheL1> level1nodes;
+  PipelineCacheL1Container level1nodes;
 };
 
 struct PipelineCacheQueryInfo
@@ -115,7 +125,7 @@ struct PipelineCacheQueryInfo
 struct PipelineResult
 {
   Graphics::Pipeline* pipeline;
-  PipelineCacheL2*    level2;
+  PipelineCachePtr    level2;
 };
 
 /**
@@ -133,7 +143,7 @@ public:
   /**
    * Retrieves next cache level
    */
-  PipelineCacheL0* GetPipelineCacheL0(std::size_t hash, Program* program, Render::Geometry* geometry);
+  PipelineCacheL0Ptr GetPipelineCacheL0(std::size_t hash, Program* program, Render::Geometry* geometry);
 
   /**
    * Retrieves pipeline matching queryInfo struct
@@ -162,7 +172,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:
   /**
@@ -181,8 +191,8 @@ private:
   void ClearUnusedCache();
 
 private:
-  Graphics::Controller*        graphicsController{nullptr};
-  std::vector<PipelineCacheL0> level0nodes;
+  Graphics::Controller*    graphicsController{nullptr};
+  PipelineCacheL0Container level0nodes;
 
   // Cache latest queries whether blend enabled or not.
   // (Since most UI case (like Text and Image) enable blend, and most 3D case disable blend.)

diff --git a/dali/internal/render/renderers/render-renderer.cpp b/dali/internal/render/renderers/render-renderer.cpp
index 486eb2f..1ccfc43 100644 (file)
--- a/dali/internal/render/renderers/render-renderer.cpp
+++ b/dali/internal/render/renderers/render-renderer.cpp
@@ -226,7 +226,8 @@ Renderer::Renderer(SceneGraph::RenderDataProvider* dataProvider,
   mDepthWriteMode(depthWriteMode),
   mDepthTestMode(depthTestMode),
   mPremultipliedAlphaEnabled(preMultipliedAlphaEnabled),
-  mShaderChanged(false)
+  mShaderChanged(false),
+  mPipelineCached(false)
 {
   if(blendingBitmask != 0u)
   {
@@ -248,7 +249,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::operator delete(void* ptr)
@@ -972,12 +977,17 @@ Graphics::Pipeline& Renderer::PrepareGraphicsPipeline(
   queryInfo.GenerateHash();
 
   // 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 202277f..2c6b806 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>
@@ -61,6 +62,9 @@ class PipelineCacheL2;
 class UniformBufferManager;
 class Renderer;
 
+using PipelineCacheL2Container = std::list<PipelineCacheL2>;
+using PipelineCachePtr         = PipelineCacheL2Container::iterator;
+
 using RendererKey = MemoryPoolKey<Render::Renderer>;
 } //namespace Render
 } //namespace Internal
@@ -596,7 +600,7 @@ private:
   std::vector<Graphics::UniformBufferBinding> mUniformBufferBindings{};
 
   Render::PipelineCache* mPipelineCache{nullptr};
-  PipelineCacheL2*       mPipeline{nullptr};
+  PipelineCachePtr       mPipeline{};
 
   using Hash = std::size_t;
 
@@ -641,6 +645,7 @@ private:
   DepthTestMode::Type   mDepthTestMode : 3;             ///< The depth test mode
   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                  mPipelineCached : 1;            ///< Flag indicating whether renderer cache valid pipeline or not.
 
   std::vector<Dali::DevelRenderer::DrawCommand> mDrawCommands; // Devel stuff
   RenderCallback*                               mRenderCallback{nullptr};