Implement 298292a in a different way. Turns out it's a problematic ca…

…se to have a sigc::signal with thousands of slots in it, since for each disconnect() call the signal implementation checks *all* remaining slots for emptiness, which takes a *lot* of time when the map is freed. Introduce a Shader::Observer interface and store those in a std::set for fast lookup of existing observers.
codereader · Dec 30, 2017 · 95e03ee · 95e03ee
1 parent f3282b6
commit 95e03ee
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 38 deletions.
diff --git a/include/irender.h b/include/irender.h
@@ -420,6 +420,14 @@ typedef std::shared_ptr<Material> MaterialPtr;
 class Shader
 {
 public:
+	// Observer interface to get notified on (un-)realisation
+	class Observer
+	{
+	public:
+		virtual void onShaderRealised() = 0;
+		virtual void onShaderUnrealised() = 0;
+	};
+
     virtual ~Shader() {}
 
 	/**
@@ -463,12 +471,15 @@ class Shader
     virtual void incrementUsed() = 0;
     virtual void decrementUsed() = 0;
 
-	virtual sigc::signal<void>& signal_Realised() = 0;
-	virtual sigc::signal<void>& signal_Unrealised() = 0;
+	// Attach/detach an observer to this shader object.
+	// In case the shader is already realised when attachObserver() is called,
+	// the observer's onShaderRealised() method is immediately invoked.
+	// The analogous holds for detachObserver(): if the shader is realised,
+	// the observer's onShaderUnrealised() method is invoked before unregistering it.
+	virtual void attachObserver(Observer& observer) = 0;
+	virtual void detachObserver(Observer& observer) = 0;
 
 	virtual bool isRealised() = 0;
-    //virtual void attach(ModuleObserver& observer) = 0;
-    //virtual void detach(ModuleObserver& observer) = 0;
 
 	/**
      * \brief Retrieve the Material that was used to construct this shader (if

diff --git a/libs/SurfaceShader.h b/libs/SurfaceShader.h
@@ -14,7 +14,8 @@
  * shadersystem reference available.
  */
 class SurfaceShader :
-    public util::Noncopyable
+    public util::Noncopyable,
+	public Shader::Observer
 {
 private:
     // greebo: The name of the material
@@ -29,10 +30,6 @@ class SurfaceShader :
 
     bool _realised;
 
-	// Signals connected to the contained GL shader
-	sigc::connection _glShaderRealised;
-	sigc::connection _glShaderUnrealised;
-
 	// Client signals
 	sigc::signal<void> _signalRealised;
 	sigc::signal<void> _signalUnrealised;
@@ -187,18 +184,7 @@ class SurfaceShader :
             _glShader = _renderSystem->capture(_materialName);
             assert(_glShader);
 
-			_glShaderRealised = _glShader->signal_Realised().connect(
-				sigc::mem_fun(*this, &SurfaceShader::realise)
-			);
-			_glShaderUnrealised = _glShader->signal_Unrealised().connect(
-				sigc::mem_fun(*this, &SurfaceShader::unrealise)
-			);
-
-			// Realise right now if the GLShader is already in that state
-			if (_glShader->isRealised())
-			{
-				realise();
-			}
+			_glShader->attachObserver(*this);
 
             if (_inUse)
             {
@@ -209,15 +195,9 @@ class SurfaceShader :
 
     void releaseShader()
     {
-		_glShaderRealised.disconnect();
-		_glShaderUnrealised.disconnect();
-
         if (_glShader)
         {
-			if (_glShader->isRealised())
-			{
-				unrealise();
-			}
+			_glShader->detachObserver(*this);
 
             if (_inUse)
             {
@@ -227,4 +207,15 @@ class SurfaceShader :
             _glShader.reset();
         }
     }
+
+	// Inherited via Observer
+	void onShaderRealised() override
+	{
+		realise();
+	}
+
+	void onShaderUnrealised() override
+	{
+		unrealise();
+	}
 };
diff --git a/radiant/render/backend/OpenGLShader.cpp b/radiant/render/backend/OpenGLShader.cpp
@@ -148,14 +148,32 @@ void OpenGLShader::decrementUsed()
     }
 }
 
-sigc::signal<void>& OpenGLShader::signal_Realised()
+void OpenGLShader::attachObserver(Observer& observer)
 {
-	return _sigRealised;
+	std::pair<Observers::iterator, bool> result = _observers.insert(&observer);
+
+	// Prevent double-attach operations in debug mode
+	assert(result.second);
+
+	// Emit the signal immediately if we're in realised state
+	if (isRealised())
+	{
+		observer.onShaderRealised();
+	}
 }
 
-sigc::signal<void>& OpenGLShader::signal_Unrealised()
+void OpenGLShader::detachObserver(Observer& observer)
 {
-	return _sigUnrealised;
+	// Emit the signal immediately if we're in realised state
+	if (isRealised())
+	{
+		observer.onShaderUnrealised();
+	}
+
+	// Prevent invalid detach operations in debug mode
+	assert(_observers.find(&observer) != _observers.end());
+
+	_observers.erase(&observer);
 }
 
 bool OpenGLShader::isRealised()
@@ -181,7 +199,10 @@ void OpenGLShader::realise(const std::string& name)
 
     insertPasses();
 
-	signal_Realised().emit();
+	for (Observer* observer : _observers)
+	{
+		observer->onShaderRealised();
+	}
 }
 
 void OpenGLShader::insertPasses()
@@ -210,7 +231,10 @@ void OpenGLShader::removePasses()
 
 void OpenGLShader::unrealise()
 {
-	signal_Unrealised().emit();
+	for (Observer* observer : _observers)
+	{
+		observer->onShaderUnrealised();
+	}
 
     removePasses();
 

diff --git a/radiant/render/backend/OpenGLShader.h b/radiant/render/backend/OpenGLShader.h
@@ -34,8 +34,9 @@ class OpenGLShader :
 
 	std::size_t _useCount;
 
-	sigc::signal<void> _sigRealised;
-	sigc::signal<void> _sigUnrealised;
+	// Observers attached to this Shader
+	typedef std::set<Observer*> Observers;
+	Observers _observers;
 
 private:
 
@@ -92,8 +93,8 @@ class OpenGLShader :
     void incrementUsed() override;
     void decrementUsed() override;
 
-	sigc::signal<void>& signal_Realised() override;
-	sigc::signal<void>& signal_Unrealised() override;
+	void attachObserver(Observer& observer) override;
+	void detachObserver(Observer& observer) override;
 
 	bool isRealised() override;