From 134f3953f910f1ac26c714e2f48f64f339bf8e24 Mon Sep 17 00:00:00 2001 From: "Eunki, Hong" Date: Fri, 12 May 2023 13:15:13 +0900 Subject: [PATCH] [Tizen] Fix invalidated PipelineCacheL2 pointer problem Let we keep std::list::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 --- .../utc-Dali-Internal-PipelineCache.cpp | 15 +++- dali/internal/render/renderers/pipeline-cache.cpp | 83 +++++++++++----------- dali/internal/render/renderers/pipeline-cache.h | 40 +++++++---- dali/internal/render/renderers/render-renderer.cpp | 18 +++-- dali/internal/render/renderers/render-renderer.h | 7 +- 5 files changed, 99 insertions(+), 64 deletions(-) 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 --- 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 --- 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 --- a/dali/internal/render/renderers/pipeline-cache.h +++ b/dali/internal/render/renderers/pipeline-cache.h @@ -22,9 +22,7 @@ #include #include #include - -// EXTERNAL INCLUDES -#include +#include namespace Dali::Internal { @@ -34,6 +32,18 @@ namespace Render class Renderer; class Geometry; +struct PipelineCacheL2; +struct PipelineCacheL1; +struct PipelineCacheL0; +using PipelineCacheL2Container = std::list; +using PipelineCacheL1Container = std::list; +using PipelineCacheL0Container = std::list; +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 pipeline; + Graphics::ColorBlendState colorBlendState{}; + Graphics::UniquePtr 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 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 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 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 --- 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 --- a/dali/internal/render/renderers/render-renderer.h +++ b/dali/internal/render/renderers/render-renderer.h @@ -19,6 +19,7 @@ */ // INTERNAL INCLUDES +#include #include #include #include @@ -60,6 +61,9 @@ class PipelineCache; class PipelineCacheL2; class UniformBufferManager; +using PipelineCacheL2Container = std::list; +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 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 mDrawCommands; // Devel stuff RenderCallback* mRenderCallback{nullptr}; -- 2.7.4