From: Mikko Rasa <tdb@tdb.fi>
Date: Sat, 11 Aug 2012 13:36:30 +0000 (+0300)
Subject: Better way to deal with program-owned shaders
X-Git-Url: http://git.tdb.fi/?a=commitdiff_plain;h=0646b330ffa9ea7fd77e2078257499b1d58ac0f7;p=libs%2Fgl.git

Better way to deal with program-owned shaders

Track which shaders exactly are owned by the program rather than having
just a single flag.  This prevents double-free problems if a program
contains both owned and non-owned shaders, as might be the case with the
transform feature.
---

diff --git a/source/program.cpp b/source/program.cpp
index f6eaed1e..52150f3f 100644
--- a/source/program.cpp
+++ b/source/program.cpp
@@ -87,14 +87,12 @@ const char *standard_fragment_src[] =
 namespace Msp {
 namespace GL {
 
-Program::Program():
-	del_shaders(false)
+Program::Program()
 {
 	init();
 }
 
-Program::Program(const StandardFeatures &features):
-	del_shaders(true)
+Program::Program(const StandardFeatures &features)
 {
 	init();
 
@@ -102,13 +100,12 @@ Program::Program(const StandardFeatures &features):
 	link();
 }
 
-Program::Program(const string &vert, const string &frag):
-	del_shaders(true)
+Program::Program(const string &vert, const string &frag)
 {
 	init();
 
-	attach_shader(*new Shader(VERTEX_SHADER, vert));
-	attach_shader(*new Shader(FRAGMENT_SHADER, frag));
+	attach_shader_owned(new Shader(VERTEX_SHADER, vert));
+	attach_shader_owned(new Shader(FRAGMENT_SHADER, frag));
 	link();
 }
 
@@ -122,11 +119,8 @@ void Program::init()
 
 Program::~Program()
 {
-	if(del_shaders)
-	{
-		for(list<Shader *>::iterator i=shaders.begin(); i!=shaders.end(); ++i)
-			delete *i;
-	}
+	for(list<Shader *>::iterator i=owned_data.begin(); i!=owned_data.end(); ++i)
+		delete *i;
 	glDeleteObjectARB(id);
 }
 
@@ -139,6 +133,13 @@ void Program::attach_shader(Shader &shader)
 	}
 }
 
+void Program::attach_shader_owned(Shader *shader)
+{
+	attach_shader(*shader);
+	if(find(owned_data.begin(), owned_data.end(), shader)==owned_data.end())
+		owned_data.push_back(shader);
+}
+
 void Program::detach_shader(Shader &shader)
 {
 	list<Shader *>::iterator i = remove(shaders.begin(), shaders.end(), &shader);
@@ -154,8 +155,8 @@ void Program::add_standard_shaders(const StandardFeatures &features)
 	string flags = features.create_flags();
 	string vertex_src = process_standard_source(standard_vertex_src, flags);
 	string fragment_src = process_standard_source(standard_fragment_src, flags);
-	attach_shader(*new Shader(VERTEX_SHADER, vertex_src));
-	attach_shader(*new Shader(FRAGMENT_SHADER, fragment_src));
+	attach_shader_owned(new Shader(VERTEX_SHADER, vertex_src));
+	attach_shader_owned(new Shader(FRAGMENT_SHADER, fragment_src));
 }
 
 string Program::process_standard_source(const char **source, const string &flags)
@@ -195,11 +196,6 @@ string Program::process_standard_source(const char **source, const string &flags
 	return result;
 }
 
-void Program::set_del_shaders(bool ds)
-{
-	del_shaders = ds;
-}
-
 void Program::bind_attribute(unsigned index, const string &name)
 {
 	static RequireExtension _ext("GL_ARB_vertex_shader");
@@ -313,8 +309,6 @@ string Program::StandardFeatures::create_flags() const
 Program::Loader::Loader(Program &p):
 	DataFile::ObjectLoader<Program>(p)
 {
-	obj.set_del_shaders(true);
-
 	add("attribute",       &Loader::attribute);
 	add("fragment_shader", &Loader::fragment_shader);
 	add("standard",        &Loader::standard);
@@ -333,7 +327,7 @@ void Program::Loader::attribute(unsigned i, const string &n)
 
 void Program::Loader::fragment_shader(const string &src)
 {
-	obj.attach_shader(*new Shader(FRAGMENT_SHADER, src));
+	obj.attach_shader_owned(new Shader(FRAGMENT_SHADER, src));
 }
 
 void Program::Loader::standard()
@@ -345,7 +339,7 @@ void Program::Loader::standard()
 
 void Program::Loader::vertex_shader(const string &src)
 {
-	obj.attach_shader(*new Shader(VERTEX_SHADER, src));
+	obj.attach_shader_owned(new Shader(VERTEX_SHADER, src));
 }
 
 
diff --git a/source/program.h b/source/program.h
index c3c6cc03..ac8e24b5 100644
--- a/source/program.h
+++ b/source/program.h
@@ -61,7 +61,7 @@ public:
 private:
 	unsigned id;
 	std::list<Shader *> shaders;
-	bool del_shaders;
+	std::list<Shader *> owned_data;
 	bool linked;
 	std::map<std::string, UniformInfo> uniforms;
 
@@ -75,13 +75,13 @@ public:
 	virtual ~Program();
 
 	void attach_shader(Shader &shader);
+	void attach_shader_owned(Shader *shader);
 	void detach_shader(Shader &shader);
 	void add_standard_shaders(const StandardFeatures &);
 private:
 	static std::string process_standard_source(const char **, const std::string &);
 public:
 	const std::list<Shader *> &get_shaders() const { return shaders; }
-	void set_del_shaders(bool);
 	void bind_attribute(unsigned, const std::string &);
 	void link();
 	bool is_linked() const { return linked; }