Removing shader uniform map observers

Many renderers use few shaders. Currently, there is an observer list
in Shader's uniform map to inform Renderer to update it's collective
uniform map when it's updated.

In practice, shader uniforms are rare (Especially in visuals, with the
exception of PrimitiveVisual, which isn't really used much), so we
are paying the price (an array of observers) for something un-necessary.

Instead, we can change Renderer to store the last known change counter
for it's attached shader. If the shader's uniform map changes, then we
can detect this during PrepareRender, prior to rendering, by testing
the change counter.

We can therefore fully remove the uniform map observer list, improving
memory consumption and speeding up renderer creation time.

Change-Id: I96b9b5c4f7b21fda9593dc1f89f580e7ceb4513c
Signed-off-by: David Steele <david.steele@samsung.com>

diff --git a/dali/internal/update/common/property-owner.cpp b/dali/internal/update/common/property-owner.cpp
index e23831f..1111f94 100644 (file)
--- a/dali/internal/update/common/property-owner.cpp
+++ b/dali/internal/update/common/property-owner.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 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.
@@ -158,11 +158,13 @@ PropertyOwner::PropertyOwner()
 void PropertyOwner::AddUniformMapping(const UniformPropertyMapping& map)
 {
   mUniformMaps.Add(map);
+  OnMappingChanged();
 }
 
 void PropertyOwner::RemoveUniformMapping(const ConstString& uniformName)
 {
   mUniformMaps.Remove(uniformName);
+  OnMappingChanged();
 }
 
 const UniformMap& PropertyOwner::GetUniformMap() const
@@ -170,16 +172,6 @@ const UniformMap& PropertyOwner::GetUniformMap() const
   return mUniformMaps;
 }
 
-void PropertyOwner::AddUniformMapObserver(UniformMap::Observer& observer)
-{
-  mUniformMaps.AddObserver(observer);
-}
-
-void PropertyOwner::RemoveUniformMapObserver(UniformMap::Observer& observer)
-{
-  mUniformMaps.RemoveObserver(observer);
-}
-
 } // namespace SceneGraph
 
 } // namespace Internal

diff --git a/dali/internal/update/common/property-owner.h b/dali/internal/update/common/property-owner.h
index f4329fa..9eb16d0 100644 (file)
--- a/dali/internal/update/common/property-owner.h
+++ b/dali/internal/update/common/property-owner.h
@@ -2,7 +2,7 @@
 #define DALI_INTERNAL_SCENE_GRAPH_PROPERTY_OWNER_H
 
 /*
- * Copyright (c) 2022 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 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.
@@ -212,16 +212,6 @@ public:
   const UniformMap& GetUniformMap() const;
 
   /**
-   * @copydoc UniformMap::AddUniformMapObserver
-   */
-  void AddUniformMapObserver(UniformMap::Observer& observer);
-
-  /**
-   * @copydoc UniformMap::RemoveUniformMapObserver
-   */
-  void RemoveUniformMapObserver(UniformMap::Observer& observer);
-
-  /**
    * Query whether playing an animation is possible or not.
    * @return true if playing an animation is possible.
    */
@@ -236,6 +226,14 @@ protected:
    */
   PropertyOwner();
 
+  /**
+   * Method to inform derived classes when property maps have been modified.
+   */
+  virtual void OnMappingChanged()
+  {
+    // Default behaviour is to do nothing
+  }
+
 private:
   // Undefined
   PropertyOwner(const PropertyOwner&);

diff --git a/dali/internal/update/common/uniform-map.cpp b/dali/internal/update/common/uniform-map.cpp
index 76e6081..cdd979c 100644 (file)
--- a/dali/internal/update/common/uniform-map.cpp
+++ b/dali/internal/update/common/uniform-map.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 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.
@@ -25,43 +25,9 @@ namespace Internal
 {
 namespace SceneGraph
 {
-void UniformMap::AddObserver(Observer& observer)
-{
-  bool foundObserver = false;
-  for(ObserversIter iter = mObservers.Begin(), endIter = mObservers.End(); iter != endIter; ++iter)
-  {
-    if(*iter == &observer)
-    {
-      foundObserver = true;
-      break;
-    }
-  }
-  if(!foundObserver)
-  {
-    mObservers.PushBack(&observer);
-  }
-}
-
-void UniformMap::RemoveObserver(Observer& observer)
-{
-  for(ObserversIter iter = mObservers.Begin(), endIter = mObservers.End(); iter != endIter; ++iter)
-  {
-    if(*iter == &observer)
-    {
-      mObservers.Erase(iter);
-      return;
-    }
-  }
-}
-
 void UniformMap::MappingChanged()
 {
   ++mChangeCounter;
-  for(ObserversIter iter = mObservers.Begin(), endIter = mObservers.End(); iter != endIter; ++iter)
-  {
-    Observer* observer = (*iter);
-    observer->UniformMappingsChanged(*this);
-  }
 }
 
 void UniformMap::Add(UniformPropertyMapping newMap)

diff --git a/dali/internal/update/common/uniform-map.h b/dali/internal/update/common/uniform-map.h
index f625169..015d582 100644 (file)
--- a/dali/internal/update/common/uniform-map.h
+++ b/dali/internal/update/common/uniform-map.h
@@ -2,7 +2,7 @@
 #define DALI_INTERNAL_SCENE_GRAPH_UNIFORM_MAP_H
 
 /*
- * Copyright (c) 2022 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 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.
@@ -86,39 +86,13 @@ public:
  * The UniformMap class is used to map uniform names to property values. It is available
  * in the following rendering classes: Node, Renderer, Shader.
  *
- * It can be observed for changes to the mapping table.
+ * They can test the ChangeCounter to see if the mapping has been updated.
  */
 class UniformMap
 {
 public:
   using SizeType = uint32_t;
 
-  class Observer
-  {
-  public:
-    /**
-     * Inform observer that uniform mappings have been changed
-     * @param mappings
-     */
-    virtual void UniformMappingsChanged(const UniformMap& mappings) = 0;
-
-  protected:
-    /**
-     * Virtual destructor, no deletion through this interface
-     */
-    virtual ~Observer() = default;
-  };
-
-  /**
-   * Add an observer that watches for changes in the mappings
-   */
-  void AddObserver(Observer& observer);
-
-  /**
-   * Remove an observer
-   */
-  void RemoveObserver(Observer& observer);
-
   /**
    * Add a map to the mappings table.
    */
@@ -165,11 +139,8 @@ private:
 private:
   using UniformMapContainer = Dali::Vector<UniformPropertyMapping>;
   using UniformMapIter      = UniformMapContainer::Iterator;
-  using Observers           = Dali::Vector<Observer*>;
-  using ObserversIter       = Observers::Iterator;
 
-  UniformMapContainer mUniformMaps; ///< container of uniform maps
-  Observers           mObservers;
+  UniformMapContainer mUniformMaps;       ///< container of uniform maps
   std::size_t         mChangeCounter{0u}; ///< Counter that is incremented when the map changes
 };
 

diff --git a/dali/internal/update/rendering/scene-graph-renderer.cpp b/dali/internal/update/rendering/scene-graph-renderer.cpp
index 1550a98..8c0d502 100644 (file)
--- a/dali/internal/update/rendering/scene-graph-renderer.cpp
+++ b/dali/internal/update/rendering/scene-graph-renderer.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 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.
@@ -108,17 +108,10 @@ Renderer::Renderer()
   mOpacity(1.0f),
   mDepthIndex(0)
 {
-  // Observe our own PropertyOwner's uniform map.
-  AddUniformMapObserver(*this);
 }
 
 Renderer::~Renderer()
 {
-  if(mShader)
-  {
-    mShader->RemoveUniformMapObserver(*this);
-    mShader = nullptr;
-  }
 }
 
 void Renderer::operator delete(void* ptr)
@@ -128,14 +121,23 @@ void Renderer::operator delete(void* ptr)
 
 bool Renderer::PrepareRender(BufferIndex updateBufferIndex)
 {
-  bool rendererUpdated = mResendFlag || mRenderingBehavior == DevelRenderer::Rendering::CONTINUOUSLY || mUpdateDecay > 0;
+  bool rendererUpdated        = mResendFlag || mRenderingBehavior == DevelRenderer::Rendering::CONTINUOUSLY || mUpdateDecay > 0;
+  auto shaderMapChangeCounter = mShader ? mShader->GetUniformMap().GetChangeCounter() : 0u;
+  bool shaderMapChanged       = mShader && (mShaderMapChangeCounter != shaderMapChangeCounter);
+  if(shaderMapChanged)
+  {
+    mShaderMapChangeCounter = shaderMapChangeCounter;
+  }
 
-  if(mUniformMapChangeCounter != mUniformMaps.GetChangeCounter())
+  if(mUniformMapChangeCounter != mUniformMaps.GetChangeCounter() || shaderMapChanged)
   {
     // The map has changed since the last time we checked.
-    rendererUpdated          = true;
-    mRegenerateUniformMap    = true;
-    mUpdateDecay             = Renderer::Decay::INITIAL; // Render at least twice if the map has changed/actor has been added
+    rendererUpdated       = true;
+    mRegenerateUniformMap = true;
+    mUpdateDecay          = Renderer::Decay::INITIAL; // Render at least twice if the map has changed/actor has been added
+
+    // Update local counters to identify any future changes to maps
+    // (unlikely, but allowed by API).
     mUniformMapChangeCounter = mUniformMaps.GetChangeCounter();
   }
   if(mUpdateDecay > 0)
@@ -327,14 +329,9 @@ void Renderer::SetShader(Shader* shader)
 {
   DALI_ASSERT_DEBUG(shader != NULL && "Shader pointer is NULL");
 
-  if(mShader)
-  {
-    mShader->RemoveUniformMapObserver(*this);
-  }
-
-  mShader = shader;
-  mShader->AddUniformMapObserver(*this);
-  mRegenerateUniformMap = true;
+  mShader                 = shader;
+  mShaderMapChangeCounter = 0u;
+  mRegenerateUniformMap   = true;
   mResendFlag |= RESEND_GEOMETRY | RESEND_SHADER;
   mDirtyFlag = true;
 }
@@ -713,6 +710,7 @@ void Renderer::UpdateUniformMap(BufferIndex updateBufferIndex)
       localMap.AddMappings(mShader->GetUniformMap());
     }
     localMap.UpdateChangeCounter();
+
     mRegenerateUniformMap = false;
     SetUpdated(true);
   }
@@ -756,9 +754,9 @@ uint32_t Renderer::GetMemoryPoolCapacity()
   return gRendererMemoryPool.GetCapacity();
 }
 
-void Renderer::UniformMappingsChanged(const UniformMap& mappings)
+void Renderer::OnMappingChanged()
 {
-  // The mappings are either from PropertyOwner base class, or the Shader
+  // Properties have been registered on the base class.
   mRegenerateUniformMap = true; // Should remain true until this renderer is added to a RenderList.
 }
 

diff --git a/dali/internal/update/rendering/scene-graph-renderer.h b/dali/internal/update/rendering/scene-graph-renderer.h
index 22ae4e8..49cd12b 100644 (file)
--- a/dali/internal/update/rendering/scene-graph-renderer.h
+++ b/dali/internal/update/rendering/scene-graph-renderer.h
@@ -2,7 +2,7 @@
 #define DALI_INTERNAL_SCENE_GRAPH_RENDERER_H
 
 /*
- * Copyright (c) 2022 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 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.
@@ -140,8 +140,7 @@ struct AnimatableDecoratedVisualProperties
 
 class Renderer : public PropertyOwner,
                  public UniformMapDataProvider,
-                 public RenderDataProvider,
-                 public UniformMap::Observer
+                 public RenderDataProvider
 {
 public:
   enum OpacityType
@@ -527,11 +526,11 @@ public:
    */
   static uint32_t GetMemoryPoolCapacity();
 
-public: // UniformMap::Observer
+public: // PropertyOwner::MappingChanged
   /**
-   * @copydoc UniformMap::Observer::UniformMappingsChanged
+   * @copydoc PropertyOwner::OnMappingChanged
    */
-  void UniformMappingsChanged(const UniformMap& mappings) override;
+  void OnMappingChanged() override;
 
 public: // PropertyOwner implementation
   /**
@@ -590,12 +589,13 @@ private:
 
   Dali::Internal::Render::Renderer::StencilParameters mStencilParameters; ///< Struct containing all stencil related options
 
-  uint64_t mUniformsHash{0};             ///< Hash of uniform map property values
-  uint32_t mIndexedDrawFirstElement;     ///< first element index to be drawn using indexed draw
-  uint32_t mIndexedDrawElementsCount;    ///< number of elements to be drawn using indexed draw
-  uint32_t mBlendBitmask;                ///< The bitmask of blending options
-  uint32_t mResendFlag;                  ///< Indicate whether data should be resent to the renderer
-  uint32_t mUniformMapChangeCounter{0u}; ///< Value to check if uniform data should be updated
+  uint64_t             mUniformsHash{0};             ///< Hash of uniform map property values
+  uint32_t             mIndexedDrawFirstElement;     ///< first element index to be drawn using indexed draw
+  uint32_t             mIndexedDrawElementsCount;    ///< number of elements to be drawn using indexed draw
+  uint32_t             mBlendBitmask;                ///< The bitmask of blending options
+  uint32_t             mResendFlag;                  ///< Indicate whether data should be resent to the renderer
+  UniformMap::SizeType mUniformMapChangeCounter{0u}; ///< Value to check if uniform data should be updated
+  UniformMap::SizeType mShaderMapChangeCounter{0u};  ///< Value to check if uniform data should be updated
 
   DepthFunction::Type            mDepthFunction : 4;     ///< Local copy of the depth function
   FaceCullingMode::Type          mFaceCullingMode : 3;   ///< Local copy of the mode of face culling