From bb386d895f5fce2f0099886fdf7e7073b18246e8 Mon Sep 17 00:00:00 2001
From: Mikko Rasa <tdb@tdb.fi>
Date: Wed, 21 Apr 2021 17:06:18 +0300
Subject: [PATCH] Use a generation number to track if ProgramData has changed

This avoids the need for an application to call flush_shader_data after
a change to uniform values in some cases.
---
 source/render/programdata.cpp | 19 ++++++++++++++-----
 source/render/programdata.h   |  4 ++++
 source/render/renderer.cpp    | 29 ++++++++++++++++++++---------
 source/render/renderer.h      | 10 +++++++++-
 4 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/source/render/programdata.cpp b/source/render/programdata.cpp
index d934b487..f3c6108c 100644
--- a/source/render/programdata.cpp
+++ b/source/render/programdata.cpp
@@ -20,6 +20,7 @@ namespace GL {
 
 ProgramData::ProgramData(const Program *p):
 	tied_program(p),
+	generation(0),
 	last_buffer_block(0),
 	buffer(0),
 	dirty(0)
@@ -29,6 +30,7 @@ ProgramData::ProgramData(const Program *p):
 ProgramData::ProgramData(const ProgramData &other):
 	tied_program(other.tied_program),
 	uniforms(other.uniforms),
+	generation(other.generation),
 	last_buffer_block(0),
 	buffer(0),
 	dirty(0)
@@ -107,7 +109,7 @@ void ProgramData::uniform(Tag tag, Uniform *uni)
 		return add_uniform(tag, uni);
 
 	uniforms[i].replace_value(uni);
-	dirty |= 1<<i;
+	mark_dirty(1<<i);
 }
 
 template<typename T, typename V>
@@ -125,7 +127,7 @@ void ProgramData::uniform(Tag tag, V value)
 	else
 		uniforms[i].replace_value(new T(value));
 
-	dirty |= 1<<i;
+	mark_dirty(1<<i);
 }
 
 template<typename T, typename V>
@@ -144,7 +146,7 @@ void ProgramData::uniform_array(Tag tag, unsigned n, V value)
 	else
 		uniforms[i].replace_value(new UniformArray<T>(n, value));
 
-	dirty |= 1<<i;
+	mark_dirty(1<<i);
 }
 
 bool ProgramData::validate_tag(Tag tag) const
@@ -185,7 +187,14 @@ void ProgramData::add_uniform(Tag tag, Uniform *uni)
 	nu.value = uni;
 	uniforms.insert(j, nu);
 
-	dirty = ALL_ONES;
+	mark_dirty(ALL_ONES);
+}
+
+void ProgramData::mark_dirty(Mask bits)
+{
+	if(!dirty)
+		++generation;
+	dirty |= bits;
 }
 
 void ProgramData::uniform(Tag tag, const Uniform &u)
@@ -428,7 +437,7 @@ void ProgramData::remove_uniform(Tag tag)
 	delete i->value;
 	uniforms.erase(i);
 
-	dirty = ALL_ONES;
+	mark_dirty(ALL_ONES);
 }
 
 vector<Tag> ProgramData::get_uniform_tags() const
diff --git a/source/render/programdata.h b/source/render/programdata.h
index f600f025..aeffae45 100644
--- a/source/render/programdata.h
+++ b/source/render/programdata.h
@@ -152,6 +152,7 @@ private:
 	// XXX All these mutables are a bit silly, but I'm out of better ideas
 	const Program *tied_program;
 	std::vector<TaggedUniform> uniforms;
+	unsigned generation;
 	mutable std::vector<SharedBlock> blocks;
 	mutable std::vector<ProgramBlock> programs;
 	mutable BufferBackedUniformBlock *last_buffer_block;
@@ -173,6 +174,7 @@ private:
 	void uniform_array(Tag, unsigned, V);
 	bool validate_tag(Tag) const;
 	void add_uniform(Tag, Uniform *);
+	void mark_dirty(Mask);
 public:
 	void uniform(Tag, const Uniform &);
 	void uniform(Tag, int);
@@ -234,6 +236,8 @@ public:
 
 	void remove_uniform(Tag);
 
+	unsigned get_generation() const { return generation; }
+
 	std::vector<Tag> get_uniform_tags() const;
 	const Uniform &get_uniform(Tag) const;
 	const Uniform *find_uniform(Tag) const;
diff --git a/source/render/renderer.cpp b/source/render/renderer.cpp
index 15e4437f..8e8e215f 100644
--- a/source/render/renderer.cpp
+++ b/source/render/renderer.cpp
@@ -187,15 +187,20 @@ void Renderer::set_shader_program(const Program *p, const ProgramData *d)
 
 void Renderer::add_shader_data(const ProgramData &d)
 {
-	if(state->shdata_count<shdata_stack.size() && shdata_stack[state->shdata_count]==&d)
-		++state->shdata_count;
-	else
+	if(state->shdata_count<shdata_stack.size())
 	{
-		flush_shader_data();
-		shdata_stack.push_back(&d);
-		state->shdata_count = shdata_stack.size();
-		changed |= SHADER_DATA;
+		const BoundProgramData &top = shdata_stack.back();
+		if(top.shdata==&d && top.generation==d.get_generation())
+		{
+			++state->shdata_count;
+			return;
+		}
 	}
+
+	flush_shader_data();
+	shdata_stack.push_back(&d);
+	state->shdata_count = shdata_stack.size();
+	changed |= SHADER_DATA;
 }
 
 void Renderer::flush_shader_data()
@@ -392,8 +397,8 @@ void Renderer::apply_state()
 	{
 		if(extra_shdata)
 			shdata_stack.erase(shdata_stack.begin()+state->shdata_count, shdata_stack.end());
-		for(vector<const ProgramData *>::const_iterator i=shdata_stack.begin(); i!=shdata_stack.end(); ++i)
-			(*i)->apply();
+		for(vector<BoundProgramData>::const_iterator i=shdata_stack.begin(); i!=shdata_stack.end(); ++i)
+			i->shdata->apply();
 		changed &= ~SHADER_DATA;
 	}
 
@@ -422,6 +427,12 @@ Renderer::BoundTexture::BoundTexture():
 { }
 
 
+Renderer::BoundProgramData::BoundProgramData(const ProgramData *d):
+	shdata(d),
+	generation(0)
+{ }
+
+
 Renderer::State::State():
 	camera(0),
 	texture_count(0),
diff --git a/source/render/renderer.h b/source/render/renderer.h
index 60386607..89f11122 100644
--- a/source/render/renderer.h
+++ b/source/render/renderer.h
@@ -75,6 +75,14 @@ private:
 		BoundTexture();
 	};
 
+	struct BoundProgramData
+	{
+		const ProgramData *shdata;
+		unsigned generation;
+
+		BoundProgramData(const ProgramData *);
+	};
+
 	struct State
 	{
 		const Camera *camera;
@@ -110,7 +118,7 @@ private:
 	State *state;
 	std::vector<BoundTexture> texture_stack;
 	ProgramData standard_shdata;
-	std::vector<const ProgramData *> shdata_stack;
+	std::vector<BoundProgramData> shdata_stack;
 	std::set<const Renderable *> excluded;
 
 public:
-- 
2.43.0