From 4c705a6fba590514bc3fab2a324c24fddc30cac6 Mon Sep 17 00:00:00 2001
From: Mikko Rasa <tdb@tdb.fi>
Date: Tue, 30 Mar 2021 14:49:26 +0300
Subject: [PATCH] Refactor adding and compiling shader stages in Program

SPIR-V stages don't need to be compiled so it makes more sense to compile
when adding stages from GLSL.
---
 source/core/program.cpp | 87 ++++++++++++++++++++++-------------------
 source/core/program.h   |  8 +++-
 2 files changed, 54 insertions(+), 41 deletions(-)

diff --git a/source/core/program.cpp b/source/core/program.cpp
index 7fff5859..6cad69c4 100644
--- a/source/core/program.cpp
+++ b/source/core/program.cpp
@@ -73,11 +73,28 @@ void Program::init()
 
 Program::~Program()
 {
-	for(vector<unsigned>::iterator i=shader_ids.begin(); i!=shader_ids.end(); ++i)
+	for(vector<unsigned>::iterator i=stage_ids.begin(); i!=stage_ids.end(); ++i)
 		glDeleteShader(*i);
 	glDeleteProgram(id);
 }
 
+unsigned Program::add_stage(GLenum type)
+{
+	switch(type)
+	{
+	case GL_VERTEX_SHADER: { static Require _req(ARB_vertex_shader); } break;
+	case GL_GEOMETRY_SHADER: { static Require _req(ARB_geometry_shader4); } break;
+	case GL_FRAGMENT_SHADER: { static Require _req(ARB_fragment_shader); } break;
+	default: throw invalid_argument("Program::add_stage");
+	}
+
+	unsigned stage_id = glCreateShader(type);
+	stage_ids.push_back(stage_id);
+	glAttachShader(id, stage_id);
+
+	return stage_id;
+}
+
 void Program::add_stages(const Module &mod, const map<string, int> &spec_values)
 {
 	module = &mod;
@@ -95,29 +112,15 @@ void Program::add_stages(const Module &mod, const map<string, int> &spec_values)
 	vector<SL::Stage::Type> stages = compiler.get_stages();
 	for(vector<SL::Stage::Type>::const_iterator i=stages.begin(); i!=stages.end(); ++i)
 	{
-		GLenum type;
+		unsigned stage_id = 0;
 		switch(*i)
 		{
-		case SL::Stage::VERTEX:
-			{ static Require _req(ARB_vertex_shader); }
-			type = GL_VERTEX_SHADER;
-			break;
-		case SL::Stage::GEOMETRY:
-			{ static Require _req(ARB_geometry_shader4); }
-			type = GL_GEOMETRY_SHADER;
-			break;
-		case SL::Stage::FRAGMENT:
-			{ static Require _req(ARB_fragment_shader); }
-			type = GL_FRAGMENT_SHADER;
-			break;
-		default:
-			throw invalid_operation("Program::add_stages");
+		case SL::Stage::VERTEX: stage_id = add_stage(GL_VERTEX_SHADER); break;
+		case SL::Stage::GEOMETRY: stage_id = add_stage(GL_GEOMETRY_SHADER); break;
+		case SL::Stage::FRAGMENT: stage_id = add_stage(GL_FRAGMENT_SHADER); break;
+		default: throw invalid_operation("Program::add_glsl_stages");
 		}
 
-		unsigned stage_id = glCreateShader(type);
-		shader_ids.push_back(stage_id);
-		glAttachShader(id, stage_id);
-
 		string stage_src = compiler.get_stage_glsl(*i);
 		const char *src_ptr = stage_src.data();
 		int src_len = stage_src.size();
@@ -136,9 +139,31 @@ void Program::add_stages(const Module &mod, const map<string, int> &spec_values)
 			for(map<string, unsigned>::const_iterator j=frag_outs.begin(); j!=frag_outs.end(); ++j)
 				glBindFragDataLocation(id, j->second, j->first.c_str());
 		}
+
+		compile_stage(stage_id);
 	}
 }
 
+void Program::compile_stage(unsigned stage_id)
+{
+	glCompileShader(stage_id);
+	bool compiled = get_shader_i(stage_id, GL_COMPILE_STATUS);
+
+	GLsizei info_log_len = get_shader_i(stage_id, GL_INFO_LOG_LENGTH);
+	string info_log(info_log_len+1, 0);
+	glGetShaderInfoLog(stage_id, info_log_len+1, &info_log_len, &info_log[0]);
+	info_log.erase(info_log_len);
+	if(module)
+		info_log = module->get_source_map().translate_errors(info_log);
+
+	if(!compiled)
+		throw compile_error(info_log);
+#ifdef DEBUG
+	if(!info_log.empty())
+		IO::print("Shader compile info log:\n%s", info_log);
+#endif
+}
+
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 void Program::attach_shader(Shader &shader)
@@ -146,7 +171,8 @@ void Program::attach_shader(Shader &shader)
 	unsigned shader_id = shader.steal_id();
 	if(!shader_id)
 		throw invalid_argument("Program::attach_shader");
-	shader_ids.push_back(shader_id);
+	stage_ids.push_back(shader_id);
+	compile_stage(shader_id);
 }
 
 void Program::attach_shader_owned(Shader *shader)
@@ -185,25 +211,6 @@ void Program::bind_fragment_data(unsigned index, const string &name)
 
 void Program::link()
 {
-	for(vector<unsigned>::const_iterator i=shader_ids.begin(); i!=shader_ids.end(); ++i)
-	{
-		glCompileShader(*i);
-		bool compiled = get_shader_i(*i, GL_COMPILE_STATUS);
-
-		GLsizei info_log_len = get_shader_i(*i, GL_INFO_LOG_LENGTH);
-		string info_log(info_log_len+1, 0);
-		glGetShaderInfoLog(*i, info_log_len+1, &info_log_len, &info_log[0]);
-		info_log.erase(info_log_len);
-		info_log = module->get_source_map().translate_errors(info_log);
-
-		if(!compiled)
-			throw compile_error(info_log);
-#ifdef DEBUG
-		if(!info_log.empty())
-			IO::print("Shader compile info log:\n%s", info_log);
-#endif
-	}
-
 	uniforms.clear();
 
 	glLinkProgram(id);
diff --git a/source/core/program.h b/source/core/program.h
index f318c606..7038468b 100644
--- a/source/core/program.h
+++ b/source/core/program.h
@@ -92,7 +92,7 @@ public:
 
 private:
 	unsigned id;
-	std::vector<unsigned> shader_ids;
+	std::vector<unsigned> stage_ids;
 	const Module *module;
 	bool linked;
 	UniformBlockMap uniform_blocks;
@@ -118,8 +118,14 @@ private:
 public:
 	virtual ~Program();
 
+private:
+	unsigned add_stage(GLenum);
+public:
 	void add_stages(const Module &, const std::map<std::string, int> & = std::map<std::string, int>());
+private:
+	void compile_stage(unsigned);
 
+public:
 	DEPRECATED void attach_shader(Shader &shader);
 	DEPRECATED void attach_shader_owned(Shader *shader);
 	DEPRECATED void detach_shader(Shader &shader);
-- 
2.43.0