From: Mikko Rasa <tdb@tdb.fi>
Date: Sat, 29 Jan 2022 12:09:01 +0000 (+0200)
Subject: Simplify Program by removing transient data
X-Git-Url: http://git.tdb.fi/?a=commitdiff_plain;h=89347a620294a1136ee111edeadec68390654f78;p=libs%2Fgl.git

Simplify Program by removing transient data

After some earlier refactorings it was only used on one code path, so
those variables are now directly declared where they're needed.
---

diff --git a/source/backends/opengl/program_backend.cpp b/source/backends/opengl/program_backend.cpp
index a9328488..e83a9484 100644
--- a/source/backends/opengl/program_backend.cpp
+++ b/source/backends/opengl/program_backend.cpp
@@ -99,7 +99,7 @@ unsigned OpenGLProgram::add_stage(Stage type)
 	return stage_id;
 }
 
-void OpenGLProgram::add_glsl_stages(const GlslModule &mod, const map<string, int> &spec_values, TransientData &transient)
+void OpenGLProgram::add_glsl_stages(const GlslModule &mod, const map<string, int> &spec_values)
 {
 	SL::Compiler compiler(Device::get_current().get_info().glsl_features);
 	compiler.set_source(mod.get_prepared_source(), "<module>");
@@ -146,11 +146,44 @@ void OpenGLProgram::add_glsl_stages(const GlslModule &mod, const map<string, int
 		compile_glsl_stage(mod, stage_id);
 	}
 
-	transient.textures = compiler.get_texture_bindings();
-	transient.blocks = compiler.get_uniform_block_bindings();
-
 	ReflectData &rd = static_cast<Program *>(this)->reflect_data;
 	rd.n_clip_distances = compiler.get_n_clip_distances();
+
+	link(mod);
+	query_uniforms();
+	query_attributes();
+
+	const map<string, unsigned> &block_bindings = compiler.get_uniform_block_bindings();
+	if(!block_bindings.empty())
+	{
+		for(unsigned i=0; i<rd.uniform_blocks.size(); ++i)
+		{
+			auto j = block_bindings.find(rd.uniform_blocks[i].name);
+			if(j!=block_bindings.end())
+			{
+				glUniformBlockBinding(id, i, j->second);
+				rd.uniform_blocks[i].bind_point = j->second;
+			}
+		}
+	}
+
+	const map<string, unsigned> &tex_bindings = compiler.get_texture_bindings();
+	if(!tex_bindings.empty())
+	{
+		if(!ARB_separate_shader_objects)
+			glUseProgram(id);
+		for(const auto &kvp: tex_bindings)
+		{
+			int location = static_cast<const Program *>(this)->get_uniform_location(kvp.first);
+			if(location>=0)
+			{
+				if(ARB_separate_shader_objects)
+					glProgramUniform1i(id, location, kvp.second);
+				else
+					glUniform1i(location, kvp.second);
+			}
+		}
+	}
 }
 
 void OpenGLProgram::compile_glsl_stage(const GlslModule &mod, unsigned stage_id)
@@ -220,9 +253,11 @@ void OpenGLProgram::add_spirv_stages(const SpirVModule &mod, const map<string, i
 	for(unsigned i=0; i<MAX_STAGES; ++i)
 		if(stage_ids[i])
 			glSpecializeShader(stage_ids[i], j->name.c_str(), spec_id_array.size(), &spec_id_array[0], &spec_value_array[0]);
+
+	link(mod);
 }
 
-void OpenGLProgram::finalize(const Module &mod, TransientData &transient)
+void OpenGLProgram::link(const Module &mod)
 {
 	glLinkProgram(id);
 	int status = 0;
@@ -243,13 +278,6 @@ void OpenGLProgram::finalize(const Module &mod, TransientData &transient)
 	if(!info_log.empty())
 		IO::print("Program link info log:\n%s", info_log);
 #endif
-
-	if(mod.get_format()==Module::GLSL)
-	{
-		query_uniforms();
-		query_attributes();
-		apply_bindings(transient);
-	}
 }
 
 void OpenGLProgram::query_uniforms()
@@ -418,35 +446,6 @@ void OpenGLProgram::query_attributes()
 	}
 }
 
-void OpenGLProgram::apply_bindings(const TransientData &transient)
-{
-	ReflectData &rd = static_cast<Program *>(this)->reflect_data;
-
-	for(unsigned i=0; i<rd.uniform_blocks.size(); ++i)
-	{
-		auto j = transient.blocks.find(rd.uniform_blocks[i].name);
-		if(j!=transient.blocks.end())
-		{
-			glUniformBlockBinding(id, i, j->second);
-			rd.uniform_blocks[i].bind_point = j->second;
-		}
-	}
-
-	if(!ARB_separate_shader_objects)
-		glUseProgram(id);
-	for(const auto &kvp: transient.textures)
-	{
-		int location = static_cast<const Program *>(this)->get_uniform_location(kvp.first);
-		if(location>=0)
-		{
-			if(ARB_separate_shader_objects)
-				glProgramUniform1i(id, location, kvp.second);
-			else
-				glUniform1i(location, kvp.second);
-		}
-	}
-}
-
 void OpenGLProgram::finalize_uniforms()
 {
 	ReflectData &rd = static_cast<Program *>(this)->reflect_data;
diff --git a/source/backends/opengl/program_backend.h b/source/backends/opengl/program_backend.h
index 2a223ff5..14650708 100644
--- a/source/backends/opengl/program_backend.h
+++ b/source/backends/opengl/program_backend.h
@@ -23,12 +23,6 @@ protected:
 		MAX_STAGES
 	};
 
-	struct TransientData
-	{
-		std::map<std::string, unsigned> textures;
-		std::map<std::string, unsigned> blocks;
-	};
-
 	struct UniformCall
 	{
 		using FuncPtr = void (*)(unsigned, unsigned, const void *);
@@ -52,15 +46,14 @@ protected:
 
 	bool has_stages() const;
 	unsigned add_stage(Stage);
-	void add_glsl_stages(const GlslModule &, const std::map<std::string, int> &, TransientData &);
+	void add_glsl_stages(const GlslModule &, const std::map<std::string, int> &);
 	void compile_glsl_stage(const GlslModule &, unsigned);
 	void add_spirv_stages(const SpirVModule &, const std::map<std::string, int> &);
 
-	void finalize(const Module &, TransientData &);
+	void link(const Module &);
 	void query_uniforms();
 	void query_uniform_blocks(const std::vector<ReflectData::UniformInfo *> &);
 	void query_attributes();
-	void apply_bindings(const TransientData &);
 	void finalize_uniforms();
 
 	void set_debug_name(const std::string &);
diff --git a/source/backends/vulkan/program_backend.cpp b/source/backends/vulkan/program_backend.cpp
index 50d77263..68ade90c 100644
--- a/source/backends/vulkan/program_backend.cpp
+++ b/source/backends/vulkan/program_backend.cpp
@@ -43,7 +43,7 @@ bool VulkanProgram::has_stages() const
 	return n_stages;
 }
 
-void VulkanProgram::add_glsl_stages(const GlslModule &, const map<string, int> &, TransientData &)
+void VulkanProgram::add_glsl_stages(const GlslModule &, const map<string, int> &)
 {
 	throw invalid_operation("VulkanProgram::add_glsl_stages");
 }
diff --git a/source/backends/vulkan/program_backend.h b/source/backends/vulkan/program_backend.h
index c01a5361..1454b40b 100644
--- a/source/backends/vulkan/program_backend.h
+++ b/source/backends/vulkan/program_backend.h
@@ -32,10 +32,9 @@ protected:
 	~VulkanProgram();
 
 	bool has_stages() const;
-	void add_glsl_stages(const GlslModule &, const std::map<std::string, int> &, TransientData &);
+	void add_glsl_stages(const GlslModule &, const std::map<std::string, int> &);
 	void add_spirv_stages(const SpirVModule &, const std::map<std::string, int> &);
 
-	void finalize(const Module &, TransientData &) { }
 	void finalize_uniforms();
 
 	void set_debug_name(const std::string &) { }
diff --git a/source/core/program.cpp b/source/core/program.cpp
index e146c7ac..88187221 100644
--- a/source/core/program.cpp
+++ b/source/core/program.cpp
@@ -33,11 +33,10 @@ void Program::add_stages(const Module &mod, const map<string, int> &spec_values)
 	reflect_data = ReflectData();
 	const Module *final_module = &mod;
 
-	TransientData transient;
 	switch(mod.get_format())
 	{
 	case Module::GLSL:
-		add_glsl_stages(static_cast<const GlslModule &>(mod), spec_values, transient);
+		add_glsl_stages(static_cast<const GlslModule &>(mod), spec_values);
 		break;
 	case Module::SPIR_V:
 		if(!spec_values.empty())
@@ -51,8 +50,6 @@ void Program::add_stages(const Module &mod, const map<string, int> &spec_values)
 		throw invalid_argument("Program::add_stages");
 	}
 
-	finalize(*final_module, transient);
-
 	if(final_module->get_format()==Module::SPIR_V)
 	{
 		const SpirVModule &spirv_mod = *static_cast<const SpirVModule *>(final_module);