Merge pull request #1690 from EdvinLndh/uniform_syntax_change

Update syntax of GlUniformBuffer to match that of GlUniformInput #1683

Author: heinezen

copying.md

@@ -154,6 +154,7 @@ _the openage authors_ are:
 | Haoyang Bi                  | AyiStar                     | ayistar à outlook dawt com                        |
 | Michael Seibt               | RoboSchmied                 | github à roboschmie dawt de                       |
 | Nikhil Ghosh                | NikhilGhosh75               | nghosh606 à gmail dawt com                        |
+| Edvin Lindholm              | EdvinLndh                   | edvinlndh à gmail dawt com                        |
 
 If you're a first-time committer, add yourself to the above list. This is not
 just for legal reasons, but also to keep an overview of all those nicknames.

libopenage/renderer/opengl/renderer.cpp

@@ -86,22 +86,21 @@ std::shared_ptr<RenderTarget> GlRenderer::get_display_target() {
 
 std::shared_ptr<UniformBuffer> GlRenderer::add_uniform_buffer(resources::UniformBufferInfo const &info) {
 	auto inputs = info.get_inputs();
-	std::unordered_map<std::string, GlInBlockUniform> uniforms{};
+	std::vector<GlInBlockUniform> uniforms{};
 	size_t offset = 0;
 	for (auto const &input : inputs) {
 		auto type = GL_UBO_INPUT_TYPE.get(input.type);
 		auto size = resources::UniformBufferInfo::get_size(input, info.get_layout());
 
 		// align offset to the size of the type
 		offset += offset % size;
-
-		uniforms.emplace(
-			std::make_pair(input.name,
-		                   GlInBlockUniform{type,
-		                                    offset,
-		                                    resources::UniformBufferInfo::get_size(input, info.get_layout()),
-		                                    resources::UniformBufferInfo::get_stride_size(input.type, info.get_layout()),
-		                                    input.count}));
+		uniforms.push_back(
+			GlInBlockUniform{type,
+		                     offset,
+		                     resources::UniformBufferInfo::get_size(input, info.get_layout()),
+		                     resources::UniformBufferInfo::get_stride_size(input.type, info.get_layout()),
+		                     input.count,
+							 input.name});
 
 		offset += size;
 	}

libopenage/renderer/opengl/shader_data.h

@@ -5,6 +5,7 @@
 #include <optional>
 #include <string>
 #include <unordered_map>
+#include <vector>
 
 #include <epoxy/gl.h>
 
@@ -61,6 +62,11 @@ struct GlInBlockUniform {
 	 * Only relevant for arrays. The number of elements in the array.
 	 */
 	size_t count;
+
+	/**
+	 * Name of the block uniform.
+	 */
+	std::string name;
 };
 
 /**
@@ -78,7 +84,7 @@ struct GlUniformBlock {
 	/**
 	 * Maps uniform names within this block to their descriptions.
 	 */
-	std::unordered_map<std::string, GlInBlockUniform> uniforms;
+	std::vector<GlInBlockUniform> uniforms;
 
 	/**
 	 * The binding point assigned to this block.

libopenage/renderer/opengl/shader_program.cpp

@@ -125,7 +125,7 @@ GlShaderProgram::GlShaderProgram(const std::shared_ptr<GlContext> &context,
 		std::vector<GLint> uniform_indices(val);
 		glGetActiveUniformBlockiv(handle, i_unif_block, GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES, uniform_indices.data());
 
-		std::unordered_map<std::string, GlInBlockUniform> uniforms;
+		std::vector<GlInBlockUniform> uniforms;
 		for (GLuint const i_unif : uniform_indices) {
 			in_block_unifs.insert(i_unif);
 
@@ -152,14 +152,14 @@ GlShaderProgram::GlShaderProgram(const std::shared_ptr<GlContext> &context,
 			// We do not need to handle sampler types here like in the uniform loop below,
 			// because named blocks cannot contain samplers.
 
-			uniforms.insert(std::make_pair(
-				name.data(),
+			uniforms.push_back(
 				GlInBlockUniform{
 					type,
 					size_t(offset),
 					size_t(count) * GL_UNIFORM_TYPE_SIZE.get(type),
 					size_t(stride),
-					size_t(count)}));
+					size_t(count),
+					std::string(name.data())});
 		}
 
 		// ENSURE(block_binding < caps.max_uniform_buffer_bindings,
@@ -257,10 +257,10 @@ GlShaderProgram::GlShaderProgram(const std::shared_ptr<GlContext> &context,
 		for (auto const &pair : this->uniform_blocks) {
 			log::log(MSG(dbg) << "(" << pair.second.index << ") " << pair.first
 			                  << " (size: " << pair.second.data_size << ") {");
-			for (auto const &unif_pair : pair.second.uniforms) {
-				log::log(MSG(dbg) << "\t+" << unif_pair.second.offset
-				                  << " " << unif_pair.first << ": "
-				                  << GLSL_TYPE_NAME.get(unif_pair.second.type));
+			for (auto const &unif : pair.second.uniforms) {
+				log::log(MSG(dbg) << "\t+" << unif.offset
+				                  << " " << unif.name << ": "
+				                  << GLSL_TYPE_NAME.get(unif.type));
 			}
 			log::log(MSG(dbg) << "}");
 		}
@@ -478,9 +478,9 @@ void GlShaderProgram::bind_uniform_buffer(const char *block_name, std::shared_pt
 	auto &block = this->uniform_blocks[block_name];
 
 	// Check if the uniform buffer matches the block definition
-	for (auto const &pair : block.uniforms) {
-		ENSURE(gl_buffer->has_uniform(pair.first.c_str()),
-		       "Uniform buffer does not contain uniform '" << pair.first << "' required by block " << block_name);
+	for (auto const &unif : block.uniforms) {
+		ENSURE(gl_buffer->has_uniform(unif.name.c_str()),
+		       "Uniform buffer does not contain uniform '" << unif.name << "' required by block " << block_name);
 	}
 
 	block.binding_point = gl_buffer->get_binding_point();

libopenage/renderer/opengl/uniform_buffer.cpp

@@ -16,7 +16,7 @@ namespace openage::renderer::opengl {
 
 GlUniformBuffer::GlUniformBuffer(const std::shared_ptr<GlContext> &context,
                                  size_t size,
-                                 std::unordered_map<std::string, GlInBlockUniform> uniforms,
+                                 std::vector<GlInBlockUniform> uniforms,
                                  GLuint binding_point,
                                  GLenum usage) :
 	GlSimpleObject(context,
@@ -33,6 +33,12 @@ GlUniformBuffer::GlUniformBuffer(const std::shared_ptr<GlContext> &context,
 	this->bind();
 	glBufferData(GL_UNIFORM_BUFFER, this->data_size, NULL, usage);
 
+	uniform_id_t unif_id = 0;
+	for (auto &uniform : uniforms) {
+		this->uniforms_by_name.insert(std::make_pair(uniform.name, unif_id));
+		unif_id += 1;
+	}
+
 	glBindBufferRange(GL_UNIFORM_BUFFER, this->binding_point, *this->handle, 0, this->data_size);
 
 	log::log(MSG(dbg) << "Created OpenGL uniform buffer (size: "
@@ -51,23 +57,35 @@ void GlUniformBuffer::set_binding_point(GLuint binding_point) {
 
 void GlUniformBuffer::update_uniforms(std::shared_ptr<UniformBufferInput> const &unif_in) {
 	auto glunif_in = std::dynamic_pointer_cast<GlUniformBufferInput>(unif_in);
-	ENSURE(glunif_in->get_buffer() == this->shared_from_this(), "Uniform input passed to different buffer than it was created with.");
+	ENSURE(glunif_in->get_buffer().get() == this, "Uniform input passed to different buffer than it was created with.");
 
 	this->bind();
 
+	const auto &update_offs = glunif_in->update_offs;
+	const auto &used_uniforms = glunif_in->used_uniforms;
+	const auto &uniforms = this->uniforms;
 	uint8_t const *data = glunif_in->update_data.data();
-	for (auto const &pair : glunif_in->update_offs) {
-		uint8_t const *ptr = data + pair.second;
-		auto unif_def = this->uniforms[pair.first];
-		auto loc = unif_def.offset;
-		auto size = unif_def.size;
+
+	size_t unif_count = used_uniforms.size();
+	for (size_t i = 0; i < unif_count; ++i) {
+		uniform_id_t unif_id = used_uniforms[i];
+		auto offset = update_offs[unif_id];
+
+		uint8_t const *ptr = data + offset.offset;
+		auto &unif = uniforms[unif_id];
+		auto loc = unif.offset;
+		auto size = unif.size;
 
 		glBufferSubData(GL_UNIFORM_BUFFER, loc, size, ptr);
 	}
 }
 
-bool GlUniformBuffer::has_uniform(const char *unif) {
-	return this->uniforms.count(unif) != 0;
+const std::vector<GlInBlockUniform> &GlUniformBuffer::get_uniforms() const {
+	return this->uniforms;
+}
+
+bool GlUniformBuffer::has_uniform(const char *name) {
+	return this->uniforms_by_name.contains(name);
 }
 
 void GlUniformBuffer::bind() const {
@@ -80,14 +98,14 @@ std::shared_ptr<UniformBufferInput> GlUniformBuffer::new_unif_in() {
 	return in;
 }
 
-void GlUniformBuffer::set_unif(std::shared_ptr<UniformBufferInput> const &in, const char *unif, void const *val, GLenum type) {
-	auto unif_in = std::dynamic_pointer_cast<GlUniformBufferInput>(in);
+void GlUniformBuffer::set_unif(UniformBufferInput &in, const char *unif, void const *val, GLenum type) {
+	auto &unif_in = dynamic_cast<GlUniformBufferInput &>(in);
 
-	auto uniform = this->uniforms.find(unif);
-	ENSURE(uniform != std::end(this->uniforms),
-	       "Tried to set uniform " << unif << " that does not exist in the shader program.");
+	auto unif_id = this->uniforms_by_name.find(unif)->second;
+	ENSURE(unif_id < this->uniforms.size(),
+	       "Tried to set uniform with invalid ID " << unif_id);
 
-	auto const &unif_data = uniform->second;
+	auto const &unif_data = this->uniforms[unif_id];
 
 	ENSURE(type == unif_data.type,
 	       "Tried to set uniform " << unif << " to a value of the wrong type.");
@@ -96,83 +114,80 @@ void GlUniformBuffer::set_unif(std::shared_ptr<UniformBufferInput> const &in, co
 	ENSURE(size == unif_data.size,
 	       "Tried to set uniform " << unif << " to a value of the wrong size.");
 
-	auto update_off = unif_in->update_offs.find(unif);
-	if (update_off != std::end(unif_in->update_offs)) [[likely]] { // always used after the uniform value is written once
-		// already wrote to this uniform since last upload
-		size_t off = update_off->second;
-		memcpy(unif_in->update_data.data() + off, val, size);
-	}
-	else {
-		// first time writing to this uniform since last upload, so
-		// extend the buffer before storing the uniform value
-		size_t prev_size = unif_in->update_data.size();
-		unif_in->update_data.resize(prev_size + size);
-		memcpy(unif_in->update_data.data() + prev_size, val, size);
-		unif_in->update_offs.emplace(unif, prev_size);
+	auto &update_off = unif_in.update_offs[unif_id];
+	auto offset = update_off.offset;
+	memcpy(unif_in.update_data.data() + offset, val, size);
+	if (not update_off.used) [[unlikely]] { // only true if the uniform value was not set before
+		auto lower_bound = std::lower_bound(
+			std::begin(unif_in.used_uniforms),
+			std::end(unif_in.used_uniforms),
+			unif_id);
+		unif_in.used_uniforms.insert(lower_bound, unif_id);
+		update_off.used = true;
 	}
 }
 
-void GlUniformBuffer::set_i32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, int32_t val) {
+void GlUniformBuffer::set_i32(UniformBufferInput &in, const char *unif, int32_t val) {
 	this->set_unif(in, unif, &val, GL_INT);
 }
 
-void GlUniformBuffer::set_u32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, uint32_t val) {
+void GlUniformBuffer::set_u32(UniformBufferInput &in, const char *unif, uint32_t val) {
 	this->set_unif(in, unif, &val, GL_UNSIGNED_INT);
 }
 
-void GlUniformBuffer::set_f32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, float val) {
+void GlUniformBuffer::set_f32(UniformBufferInput &in, const char *unif, float val) {
 	this->set_unif(in, unif, &val, GL_FLOAT);
 }
 
-void GlUniformBuffer::set_f64(std::shared_ptr<UniformBufferInput> const &in, const char *unif, double val) {
+void GlUniformBuffer::set_f64(UniformBufferInput &in, const char *unif, double val) {
 	this->set_unif(in, unif, &val, GL_DOUBLE);
 }
 
-void GlUniformBuffer::set_bool(std::shared_ptr<UniformBufferInput> const &in, const char *unif, bool val) {
+void GlUniformBuffer::set_bool(UniformBufferInput &in, const char *unif, bool val) {
 	this->set_unif(in, unif, &val, GL_BOOL);
 }
 
-void GlUniformBuffer::set_v2f32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, Eigen::Vector2f const &val) {
+void GlUniformBuffer::set_v2f32(UniformBufferInput &in, const char *unif, Eigen::Vector2f const &val) {
 	this->set_unif(in, unif, val.data(), GL_FLOAT_VEC2);
 }
 
-void GlUniformBuffer::set_v3f32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, Eigen::Vector3f const &val) {
+void GlUniformBuffer::set_v3f32(UniformBufferInput &in, const char *unif, Eigen::Vector3f const &val) {
 	this->set_unif(in, unif, val.data(), GL_FLOAT_VEC3);
 }
 
-void GlUniformBuffer::set_v4f32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, Eigen::Vector4f const &val) {
+void GlUniformBuffer::set_v4f32(UniformBufferInput &in, const char *unif, Eigen::Vector4f const &val) {
 	this->set_unif(in, unif, val.data(), GL_FLOAT_VEC4);
 }
 
-void GlUniformBuffer::set_v2i32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, Eigen::Vector2i const &val) {
+void GlUniformBuffer::set_v2i32(UniformBufferInput &in, const char *unif, Eigen::Vector2i const &val) {
 	this->set_unif(in, unif, val.data(), GL_INT_VEC2);
 }
 
-void GlUniformBuffer::set_v3i32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, Eigen::Vector3i const &val) {
+void GlUniformBuffer::set_v3i32(UniformBufferInput &in, const char *unif, Eigen::Vector3i const &val) {
 	this->set_unif(in, unif, val.data(), GL_INT_VEC3);
 }
 
-void GlUniformBuffer::set_v4i32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, Eigen::Vector4i const &val) {
+void GlUniformBuffer::set_v4i32(UniformBufferInput &in, const char *unif, Eigen::Vector4i const &val) {
 	this->set_unif(in, unif, val.data(), GL_INT_VEC4);
 }
 
-void GlUniformBuffer::set_v2ui32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, Eigen::Vector2<uint32_t> const &val) {
+void GlUniformBuffer::set_v2ui32(UniformBufferInput &in, const char *unif, Eigen::Vector2<uint32_t> const &val) {
 	this->set_unif(in, unif, val.data(), GL_UNSIGNED_INT_VEC2);
 }
 
-void GlUniformBuffer::set_v3ui32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, Eigen::Vector3<uint32_t> const &val) {
+void GlUniformBuffer::set_v3ui32(UniformBufferInput &in, const char *unif, Eigen::Vector3<uint32_t> const &val) {
 	this->set_unif(in, unif, val.data(), GL_UNSIGNED_INT_VEC3);
 }
 
-void GlUniformBuffer::set_v4ui32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, Eigen::Vector4<uint32_t> const &val) {
+void GlUniformBuffer::set_v4ui32(UniformBufferInput &in, const char *unif, Eigen::Vector4<uint32_t> const &val) {
 	this->set_unif(in, unif, val.data(), GL_UNSIGNED_INT_VEC4);
 }
 
-void GlUniformBuffer::set_m4f32(std::shared_ptr<UniformBufferInput> const &in, const char *unif, Eigen::Matrix4f const &val) {
+void GlUniformBuffer::set_m4f32(UniformBufferInput &in, const char *unif, Eigen::Matrix4f const &val) {
 	this->set_unif(in, unif, val.data(), GL_FLOAT_MAT4);
 }
 
-void GlUniformBuffer::set_tex(std::shared_ptr<UniformBufferInput> const &in, const char *unif, std::shared_ptr<Texture2d> const &val) {
+void GlUniformBuffer::set_tex(UniformBufferInput &in, const char *unif, std::shared_ptr<Texture2d> const &val) {
 	auto tex = std::dynamic_pointer_cast<GlTexture2d>(val);
 	GLuint handle = tex->get_handle();
 	this->set_unif(in, unif, &handle, GL_SAMPLER_2D);