Refactoring ShaderProgram

include/anki/gl/ShaderProgram.h

@@ -15,14 +15,17 @@
 namespace anki {
 
 class ShaderProgram;
+class ShaderProgramUniformBlock;
 class Texture;
 
 /// @addtogroup gl
 /// @{
 
 /// Shader program variable. The type is attribute or uniform
-class ShaderProgramVariable: public NonCopyable
+class ShaderProgramVariable
 {
+	friend class ShaderProgram;
+
 public:
 	/// Shader var types
 	enum ShaderProgramVariableType
@@ -31,14 +34,8 @@ public:
 		SPVT_UNIFORM
 	};
 
-	ShaderProgramVariable(
-		GLint loc,
-		const char* name,
-		GLenum glDataType, 
-		size_t size,
-		ShaderProgramVariableType type, 
-		const ShaderProgram* fatherSProg);
-
+	ShaderProgramVariable()
+	{}
 	virtual ~ShaderProgramVariable()
 	{}
 
@@ -75,6 +72,17 @@ public:
 	}
 	/// @}
 
+	ShaderProgramVariable& operator=(const ShaderProgramVariable& b)
+	{
+		loc = b.loc;
+		name = b.name;
+		glDataType = b.glDataType;
+		size = b.size;
+		type = b.type;
+		fatherSProg = b.fatherSProg;
+		return *this;
+	}
+
 private:
 	GLint loc; ///< GL location
 	std::string name; ///< The name inside the shader program
@@ -88,30 +96,14 @@ private:
 };
 
 /// Uniform shader variable
-class ShaderProgramUniformVariable: public ShaderProgramVariable, 
-	public Flags<U32>
+class ShaderProgramUniformVariable: public ShaderProgramVariable
 {
 	friend class ShaderProgramUniformBlock;
+	friend class ShaderProgram;
 
 public:
-	enum ShaderProgramUniformVariableFlag
-	{
-		SPUVF_NONE = 0,
-		SPUVF_DIRTY = (1 << 0) ///< This means that a setter was called
-	};
-
-	ShaderProgramUniformVariable(
-		GLint loc,
-		const char* name,
-		GLenum glDataType,
-		size_t size,
-		const ShaderProgram* fatherSProg,
-		GLuint index_)
-		: ShaderProgramVariable(loc, name, glDataType, size, SPVT_UNIFORM, 
-			fatherSProg), index(index_)
-	{
-		enableFlag(SPUVF_DIRTY);
-	}
+	ShaderProgramUniformVariable()
+	{}
 
 	/// @name Set the var
 	/// @{
@@ -151,6 +143,16 @@ public:
 	}
 	/// @}
 
+	ShaderProgramUniformVariable& operator=(
+		const ShaderProgramUniformVariable& b)
+	{
+		ShaderProgramVariable::operator=(b);
+		index = b.index;
+		offset = b.offset;
+		arrayStride = b.arrayStride;
+		return *this;
+	}
+
 private:
 	GLuint index;
 
@@ -179,18 +181,14 @@ private:
 /// Attribute shader program variable
 class ShaderProgramAttributeVariable: public ShaderProgramVariable
 {
-public:
-	ShaderProgramAttributeVariable(
-		int loc_, const char* name_, GLenum glDataType_, size_t size,
-		const ShaderProgram* fatherSProg_)
-		: ShaderProgramVariable(loc_, name_, glDataType_, size, SPVT_ATTRIBUTE,
-			fatherSProg_)
-	{}
+	friend class ShaderProgram;
 };
 
 /// Uniform shader block
 class ShaderProgramUniformBlock
 {
+	friend class ShaderProgram;
+
 public:
 	ShaderProgramUniformBlock()
 	{}
@@ -233,8 +231,6 @@ public:
 
 	ShaderProgramUniformBlock& operator=(const ShaderProgramUniformBlock& b);
 
-	void init(ShaderProgram& prog, const char* blockName);
-
 private:
 	Vector<ShaderProgramUniformVariable*> uniforms;
 	GLuint index = GL_INVALID_INDEX;
@@ -242,18 +238,16 @@ private:
 	std::string name;
 	/// Ask the program to get you the binding point
 	mutable GLuint bindingPoint;
-	GLuint progId;
+	GLuint progId; ///< Needed for binding
 };
 
 /// Shader program object
 class ShaderProgram: public NonCopyable
 {
 public:
-	typedef Vector<std::shared_ptr<ShaderProgramVariable>>
-		VariablesContainer;
-	typedef Vector<ShaderProgramUniformVariable*>
+	typedef Vector<ShaderProgramUniformVariable>
 		UniformVariablesContainer;
-	typedef Vector<ShaderProgramAttributeVariable*>
+	typedef Vector<ShaderProgramAttributeVariable>
 		AttributeVariablesContainer;
 	typedef Vector<ShaderProgramUniformBlock>
 		UniformBlocksContainer;
@@ -291,12 +285,6 @@ public:
 		return glId;
 	}
 
-	/// Get all variables container
-	const VariablesContainer& getVariables() const
-	{
-		return vars;
-	}
-
 	const UniformVariablesContainer& getUniformVariables() const
 	{
 		return unis;
@@ -346,8 +334,6 @@ public:
 	/// Used to find and return the variable. They return nullptr if the 
 	/// variable is not found
 	/// @{
-	const ShaderProgramVariable* tryFindVariable(const char* varName) const;
-	const ShaderProgramVariable& findVariable(const char* varName) const;
 	const ShaderProgramUniformVariable* tryFindUniformVariable(
 		const char* varName) const;
 	const ShaderProgramUniformVariable& findUniformVariable(
@@ -361,9 +347,6 @@ public:
 	const ShaderProgramUniformBlock& findUniformBlock(const char* name) const;
 	/// @}
 
-	/// For all uniforms set the SPUVF_DIRTY bit to 0
-	void cleanAllUniformsDirtyFlags();
-
 	static GLuint getCurrentProgramGlId()
 	{
 		int i;
@@ -376,9 +359,6 @@ public:
 		const ShaderProgram& x);
 
 private:
-	typedef ConstCharPtrHashMap<ShaderProgramVariable*>::Type
-		NameToVarHashMap;
-
 	typedef ConstCharPtrHashMap<ShaderProgramUniformVariable*>::Type
 		NameToUniVarHashMap;
 
@@ -402,11 +382,9 @@ private:
 
 	/// @name Containers
 	/// @{
-	VariablesContainer vars; ///< All the vars. Does garbage collection
 	UniformVariablesContainer unis;
 	AttributeVariablesContainer attribs;
 
-	NameToVarHashMap nameToVar; ///< Variable searching
 	NameToUniVarHashMap nameToUniVar; ///< Uniform searching
 	NameToAttribVarHashMap nameToAttribVar; ///< Attribute searching
 
@@ -416,7 +394,7 @@ private:
 
 	/// Query the driver to get the vars. After the linking of the shader
 	/// prog is done gather all the vars in custom containers
-	void getUniAndAttribVars();
+	void initUniAndAttribVars();
 
 	/// Get info about the uniform blocks
 	void initUniformBlocks();

src/gl/ShaderProgram.cpp

@@ -3,7 +3,9 @@
 #include "anki/math/Math.h"
 #include "anki/util/Exception.h"
 #include "anki/gl/Texture.h"
+#include "anki/core/Logger.h"
 #include "anki/util/StringList.h"
+#include "anki/util/Array.h"
 #include <sstream>
 #include <iomanip>
 
@@ -14,34 +16,6 @@ namespace anki {
 static const char* padding = "======================================="
                              "=======================================";
 
-//==============================================================================
-// ShaderProgramVariable                                                       =
-//==============================================================================
-
-//==============================================================================
-ShaderProgramVariable::ShaderProgramVariable(
-	GLint loc_, 
-	const char* name_,
-	GLenum glDataType_, 
-	size_t size_,
-	ShaderProgramVariableType type_, 
-	const ShaderProgram* fatherSProg_)
-	: loc(loc_), name(name_), glDataType(glDataType_), size(size_), 
-		type(type_), fatherSProg(fatherSProg_)
-{
-	name.shrink_to_fit();
-	if(type == SPVT_ATTRIBUTE)
-	{
-		ANKI_ASSERT(loc ==
-			glGetAttribLocation(fatherSProg->getGlId(), name.c_str()));
-	}
-	else
-	{
-		ANKI_ASSERT(loc ==
-			glGetUniformLocation(fatherSProg->getGlId(), name.c_str()));
-	}
-}
-
 //==============================================================================
 // ShaderProgramUniformVariable                                                =
 //==============================================================================
@@ -165,30 +139,6 @@ ShaderProgramUniformBlock& ShaderProgramUniformBlock::operator=(
 	return *this;
 }
 
-//==============================================================================
-void ShaderProgramUniformBlock::init(ShaderProgram& prog, 
-	const char* blockName)
-{
-	GLint gli;
-	name = blockName;
-	progId = prog.getGlId();
-
-	index = glGetUniformBlockIndex(progId, blockName);
-	if(index == GL_INVALID_INDEX)
-	{
-		throw ANKI_EXCEPTION("glGetUniformBlockIndex() returned "
-			"GL_INVALID_INDEX");
-	}
-
-	glGetActiveUniformBlockiv(progId, index, GL_UNIFORM_BLOCK_DATA_SIZE, &gli);
-	size = gli;
-
-	glGetActiveUniformBlockiv(progId, index, GL_UNIFORM_BLOCK_BINDING, &gli);
-	bindingPoint = gli;
-
-	// XXX Init uniforms
-}
-
 //==============================================================================
 // ShaderProgram                                                               =
 //==============================================================================
@@ -293,7 +243,7 @@ void ShaderProgram::create(const char* vertSource, const char* tcSource,
 
 	// init the rest
 	bind();
-	getUniAndAttribVars();
+	initUniAndAttribVars();
 	initUniformBlocks();
 }
 
@@ -418,24 +368,52 @@ void ShaderProgram::link() const
 }
 
 //==============================================================================
-void ShaderProgram::getUniAndAttribVars()
+void ShaderProgram::initUniAndAttribVars()
 {
-	int num;
-	std::array<char, 256> name_;
+	GLint num;
+	Array<char, 256> name_;
 	GLsizei length;
 	GLint size;
 	GLenum type;
 
+	//
 	// attrib locations
+	//
+	U32 attribsCount = (U32)num;
 	glGetProgramiv(glId, GL_ACTIVE_ATTRIBUTES, &num);
+
+	// Count the _useful_ attribs
+	for(GLint i = 0; i < num; i++)
+	{
+		// Name
+		glGetActiveAttrib(glId, i, sizeof(name_), &length, 
+			&size, &type, &name_[0]);
+		name_[length] = '\0';
+
+		// check if its FFP location
+		GLint loc = glGetAttribLocation(glId, &name_[0]);
+
+		if(loc == -1)
+		{
+			// if -1 it means that its an FFP var or a weird crap like
+			// gl_InstanceID
+			--attribsCount;
+		}
+	}
+
+	attribs.resize(attribsCount);
+	attribs.shrink_to_fit();
 	for(int i = 0; i < num; i++) // loop all attributes
 	{
-		glGetActiveAttrib(glId, i, sizeof(name_) / sizeof(char), &length,
+		ShaderProgramAttributeVariable& var = attribs[i];
+
+		// Name
+		glGetActiveAttrib(glId, i, sizeof(name_), &length,
 			&size, &type, &name_[0]);
 		name_[length] = '\0';
 
 		// check if its FFP location
-		int loc = glGetAttribLocation(glId, &name_[0]);
+		GLint loc = glGetAttribLocation(glId, &name_[0]);
 		if(loc == -1)
 		{
 			// if -1 it means that its an FFP var or a weird crap like
@@ -443,40 +421,44 @@ void ShaderProgram::getUniAndAttribVars()
 			continue;
 		}
 
-		ShaderProgramAttributeVariable* var =
-			new ShaderProgramAttributeVariable(loc, &name_[0], type, 
-			size, this);
+		var.loc = loc;
+		var.name = &name_[0];
+		var.glDataType = type;
+		var.size = size;
+		var.type = ShaderProgramVariable::SPVT_ATTRIBUTE;
+		var.fatherSProg = this;
 
-		vars.push_back(std::shared_ptr<ShaderProgramVariable>(var));
-		attribs.push_back(var);
-		nameToVar[var->getName().c_str()] = var;
-		nameToAttribVar[var->getName().c_str()] = var;
+		nameToAttribVar[var.name.c_str()] = &var;
 	}
 
+	//
 	// uni locations
+	//
 	glGetProgramiv(glId, GL_ACTIVE_UNIFORMS, &num);
-	for(U i = 0; i < (U)num; i++) // loop all uniforms
+	unis.resize(num);
+	unis.shrink_to_fit();
+	for(GLint i = 0; i < num; i++) // loop all uniforms
 	{
-		glGetActiveUniform(glId, i, sizeof(name_) / sizeof(char), &length,
+		ShaderProgramUniformVariable& var = unis[i];
+
+		glGetActiveUniform(glId, i, sizeof(name_), &length,
 			&size, &type, &name_[0]);
 		name_[length] = '\0';
 
 		// -1 means in uniform block
 		GLint loc = glGetUniformLocation(glId, &name_[0]);
 
-		ShaderProgramUniformVariable* var =
-			new ShaderProgramUniformVariable(loc, &name_[0], type, 
-			size, this, i);
+		var.loc = loc;
+		var.name = &name_[0];
+		var.glDataType = type;
+		var.size = size;
+		var.type = ShaderProgramVariable::SPVT_UNIFORM;
+		var.fatherSProg = this;
 
-		vars.push_back(std::shared_ptr<ShaderProgramVariable>(var));
-		unis.push_back(var);
-		nameToVar[var->getName().c_str()] = var;
-		nameToUniVar[var->getName().c_str()] = var;
-	}
+		var.index = (GLuint)i;
 
-	vars.shrink_to_fit();
-	unis.shrink_to_fit();
-	attribs.shrink_to_fit();
+		nameToUniVar[var.name.c_str()] = &var;
+	}
 }
 
 //==============================================================================
@@ -488,35 +470,54 @@ void ShaderProgram::initUniformBlocks()
 	blocks.resize(blocksCount);
 	blocks.shrink_to_fit();
 
-	for(GLint i = 0; i < blocksCount; i++)
+	GLuint i = 0;
+	for(ShaderProgramUniformBlock& block : blocks)
 	{
+		GLint gli; // General purpose int
+
+		// Name
 		char name[256];
 		GLsizei len;
 		glGetActiveUniformBlockName(glId, i, sizeof(name), &len, name);
 
-		blocks[i].init(*this, name);
-		nameToBlock[blocks[i].getName().c_str()] = &blocks[i];
-	}
-}
+		block.name = name;
+		block.name.shrink_to_fit();
 
-//==============================================================================
-const ShaderProgramVariable* ShaderProgram::tryFindVariable(
-	const char* name) const
-{
-	NameToVarHashMap::const_iterator it = nameToVar.find(name);
-	return (it == nameToVar.end()) ? nullptr : it->second;
-}
+		// Index
+		ANKI_ASSERT(glGetUniformBlockIndex(glId, name) == i);
+		block.index = i;
 
-//==============================================================================
-const ShaderProgramVariable& ShaderProgram::findVariable(
-	const char* name) const
-{
-	const ShaderProgramVariable* var = tryFindVariable(name);
-	if(var == nullptr)
+		// Size
+		glGetActiveUniformBlockiv(glId, i, GL_UNIFORM_BLOCK_DATA_SIZE, &gli);
+		block.size = gli;
+
+		// Binding point
+		glGetActiveUniformBlockiv(glId, i, GL_UNIFORM_BLOCK_BINDING, &gli);
+		block.bindingPoint = gli;
+
+		// Prog id
+		block.progId = glId;
+
+		// Other update
+		nameToBlock[block.name.c_str()] = &block;
+		++i;
+	}
+
+	// Connect uniforms and blocks
+	for(ShaderProgramUniformVariable& uni : unis)
 	{
-		throw ANKI_EXCEPTION("Variable not found: " + name);
+		GLint blockIndex;
+		glGetActiveUniformsiv(glId, 1, &(uni.index),  GL_UNIFORM_BLOCK_INDEX, 
+			&blockIndex);
+
+		if(blockIndex == -1)
+		{
+			continue;
+		}
+
+		uni.block = &blocks[blockIndex];
+		blocks[blockIndex].uniforms.push_back(&uni);
 	}
-	return *var;
 }
 
 //==============================================================================
@@ -583,25 +584,19 @@ const ShaderProgramUniformBlock& ShaderProgram::findUniformBlock(
 	return *block;
 }
 
-//==============================================================================
-void ShaderProgram::cleanAllUniformsDirtyFlags()
-{
-	for(ShaderProgramUniformVariable* uni : unis)
-	{
-		uni->disableFlag(ShaderProgramUniformVariable::SPUVF_DIRTY);
-	}
-}
-
 //==============================================================================
 std::ostream& operator<<(std::ostream& s, const ShaderProgram& x)
 {
 	s << "ShaderProgram\n";
-	s << "Variables:\n";
-	for(auto var : x.vars)
+	s << "Uniform variables:\n";
+	for(auto var : x.unis)
+	{
+		s << var.getName() << " " << var.getLocation() <<  '\n';
+	}
+	s << "Attrib variables:\n";
+	for(auto var : x.attribs)
 	{
-		s << var->getName() << " " << var->getLocation() << " "
-			<< (var->getType() == ShaderProgramVariable::SPVT_ATTRIBUTE
-			? "[A]" : "[U]") <<  '\n';
+		s << var.getName() << " " << var.getLocation() <<  '\n';
 	}
 	return s;
 }

src/resource/Material.cpp

@@ -259,10 +259,10 @@ void Material::populateVariables(const XmlElement& shaderProgramEl)
 
 	for(const ShaderProgramResourcePointer* sProg : sProgs)
 	{
-		for(const ShaderProgramUniformVariable* v :
+		for(const ShaderProgramUniformVariable& v :
 			(*sProg)->getUniformVariables())
 		{
-			allVarNames[v->getName()] = v->getGlDataType();
+			allVarNames[v.getName()] = v.getGlDataType();
 		}
 	}