Structs and struct arrays seem to work in GLSL

CamelotClient/CamelotClient.cpp

@@ -152,15 +152,27 @@ int CALLBACK WinMain(
 	HighLevelGpuProgramHandle fragProgRef = HighLevelGpuProgram::create(fragShaderCode, "main", "glsl", GPT_FRAGMENT_PROGRAM, GPP_PS_2_0);
 
 	// TODO - Make sure to document the strict input parameter naming. (Exact supported names are in GLSLParamParser)
-	String vertShaderCode = "#version 400 \n \
-							 uniform mainFragBlock { mat4 matViewProjection; }; \
+	String vertShaderCode = "#version 400 \n									\
+							struct InputStruct									\
+							{													\
+								float matMultiplier;							\
+								float uvMultiplier;								\
+							};													\
+																				\
+							 uniform mainFragBlock								\
+							 {													\
+							 float test1;										\
+							 InputStruct input[2];								\
+							 mat4 matViewProjection;							\
+							 float test2;										\
+							 };													\
 							 in vec4 cm_position; \
 							 in vec2 cm_texcoord0; \
 							 out vec2 texcoord0; \
 							 void main() \
 							 { \
-								texcoord0 = cm_texcoord0; \
-								gl_Position = cm_position * matViewProjection; \
+								texcoord0 = cm_texcoord0 * input[1].uvMultiplier; \
+								gl_Position = cm_position * (matViewProjection * input[1].matMultiplier); \
 							 }";
 
 	HighLevelGpuProgramHandle vertProgRef= HighLevelGpuProgram::create(vertShaderCode, "main", "glsl", GPT_VERTEX_PROGRAM, GPP_VS_2_0);
@@ -178,7 +190,7 @@ int CALLBACK WinMain(
 
 	testShader->addParameter("matViewProjection", "matViewProjection", GPDT_MATRIX_4X4);
 
-#ifdef DX11
+#if defined DX11 || defined GL
 	testShader->addParameter("input", "input", GPDT_STRUCT, 2, 8);
 #endif
 
@@ -209,7 +221,7 @@ int CALLBACK WinMain(
 
 	testMaterial->setMat4("matViewProjection", Matrix4::IDENTITY);
 
-#ifdef DX11
+#if defined DX11 || defined GL
 	float dbgMultipliers1[2];
 	dbgMultipliers1[0] = 0.0f;
 	dbgMultipliers1[1] = 0.0f;

CamelotD3D11RenderSystem/Source/CmD3D11HLSLParamParser.cpp

@@ -224,7 +224,12 @@ namespace CamelotEngine
 		memberDesc.arrayElementStride = memberDesc.elementSize;
 		memberDesc.gpuMemOffset = varDesc.StartOffset;
 		
+		// Calculate buffer offset and element size
+		// This is based on "Packing Rules for Constant Variables (DX11)":
+		// http://msdn.microsoft.com/en-us/library/windows/desktop/bb509632(v=vs.85).aspx
+
 		// Elements in array always start at 16 byte (4 float) boundaries so we handle them specially
+		// (Determining individual element size in an array also takes some additional work)
 		if(memberDesc.arraySize == 1)
 		{
 			UINT32 freeSlotsInRegister = (((paramBlock.blockSize / 4) + 1) * 4) - paramBlock.blockSize;
@@ -249,7 +254,7 @@ namespace CamelotEngine
 			int totalSlotsUsedByArray = (memberDesc.elementSize / 4 + 1) * 4;
 			int unusedSlotsInArray = totalSlotsUsedByArray - memberDesc.elementSize;
 
-			memberDesc.arrayElementStride = totalSlotsUsedByArray / memberDesc.arraySize; // TODO - Array element stride should be stored in ParamDataDesc and used for determining size
+			memberDesc.arrayElementStride = totalSlotsUsedByArray / memberDesc.arraySize;
 			memberDesc.elementSize = memberDesc.arrayElementStride - unusedSlotsInArray;
 
 			int freeSlotsInRegister = (((paramBlock.blockSize / 4) + 1) * 4) - paramBlock.blockSize;

CamelotGLRenderer/Source/GLSL/include/CmGLSLParamParser.h

@@ -164,6 +164,7 @@ namespace CamelotEngine
 		glGetProgramiv(glProgram, GL_ACTIVE_UNIFORM_BLOCKS, &uniformBlockCount);
 
 		map<UINT32, String>::type blockSlotToName;
+		set<String>::type blockNames;
 		for (GLuint index = 0; index < (GLuint)uniformBlockCount; index++)
 		{
 			GLsizei unusedSize = 0;
@@ -177,8 +178,11 @@ namespace CamelotEngine
 
 			returnParamDesc.paramBlocks[newBlockDesc.name] = newBlockDesc;
 			blockSlotToName.insert(std::make_pair(newBlockDesc.slot, newBlockDesc.name));
+			blockNames.insert(newBlockDesc.name);
 		}
 
+		map<String, GpuParamDataDesc>::type foundStructs;
+
 		// get the number of active uniforms
 		GLint uniformCount = 0;
 		glGetProgramiv(glProgram, GL_ACTIVE_UNIFORMS, &uniformCount);
@@ -192,15 +196,67 @@ namespace CamelotEngine
 
 			String paramName = String(uniformName);
 
-			// If the uniform name has a "[" in it then its an array element uniform.
-			String::size_type arrayStart = paramName.find("[");
-			if (arrayStart != String::npos)
+			// Naming rules and packing rules used here are described in
+			// OpenGL Core Specification 2.11.4
+
+			// Check if parameter is a part of a struct
+			vector<String>::type nameElements = StringUtil::tokenise(paramName, ".");
+
+			bool inStruct = false;
+			String structName;
+			if(nameElements.size() > 1)
 			{
-				// if not the first array element then skip it and continue to the next uniform
-				if (paramName.compare(arrayStart, paramName.size() - 1, "[0]") != 0)
-					CM_EXCEPT(NotImplementedException, "No support for array parameters yet.");
+				auto uniformBlockFind = blockNames.find(nameElements[0]);
 
-				paramName = paramName.substr(0, arrayStart);
+				// Check if the name is not a struct, and instead a Uniform block namespace
+				if(uniformBlockFind != blockNames.end())
+				{
+					// Possibly it's a struct inside a named uniform block
+					if(nameElements.size() > 2)
+					{
+						inStruct = true;
+						structName = nameElements[1];
+					}
+				}
+				else
+				{
+					inStruct = true;
+					structName = nameElements[0];
+				}
+			}
+			
+			String cleanParamName = nameElements[nameElements.size() - 1]; // Param name without namespaces or array indexes
+
+			// Check if the parameter is in an array
+			UINT32 arrayIdx = 0;
+			bool isInArray = false;
+			if(inStruct)
+			{
+				// If the uniform name has a "[" in it then its an array element uniform.
+				String::size_type arrayStart = structName.find("[");
+				String::size_type arrayEnd = structName.find("]");
+				if (arrayStart != String::npos)
+				{
+					String strArrIdx = structName.substr(arrayStart + 1, arrayEnd - (arrayStart + 1));
+					arrayIdx = parseUnsignedInt(strArrIdx, 0);
+					isInArray = true;
+
+					structName = structName.substr(0, arrayStart);
+				}
+			}
+			else
+			{
+				// If the uniform name has a "[" in it then its an array element uniform.
+				String::size_type arrayStart = cleanParamName.find("[");
+				String::size_type arrayEnd = cleanParamName.find("]");
+				if (arrayStart != String::npos)
+				{
+					String strArrIdx = cleanParamName.substr(arrayStart + 1, arrayEnd - (arrayStart + 1));
+					arrayIdx = parseUnsignedInt(strArrIdx, 0);
+					isInArray = true;
+
+					cleanParamName = cleanParamName.substr(0, arrayStart);
+				}
 			}
 
 			GLint uniformType;
@@ -251,38 +307,28 @@ namespace CamelotEngine
 			}
 			else
 			{
-				GpuParamDataDesc gpuParam;
-				gpuParam.name = paramName;
-
 				GLint blockIndex;
 				glGetActiveUniformsiv(glProgram, 1, &index, GL_UNIFORM_BLOCK_INDEX, &blockIndex);
 
+				GpuParamDataDesc gpuParam;
+				gpuParam.name = paramName;
 				determineParamInfo(gpuParam, paramName, glProgram, index);
 
 				if(blockIndex != -1)
 				{
 					GLint blockOffset;
 					glGetActiveUniformsiv(glProgram, 1, &index, GL_UNIFORM_OFFSET, &blockOffset);
-					gpuParam.gpuMemOffset = blockOffset;
-
-					GLint arrayStride;
-					glGetActiveUniformsiv(glProgram, 1, &index, GL_UNIFORM_ARRAY_STRIDE, &arrayStride);
-
-					if(arrayStride != 0)
-					{
-						assert (arrayStride % 4 == 0);
+					blockOffset = blockOffset / 4;
 
-						gpuParam.elementSize = arrayStride / 4;
-						gpuParam.arrayElementStride = gpuParam.elementSize;
-					}
+					gpuParam.gpuMemOffset = blockOffset;
 
 					gpuParam.paramBlockSlot = blockIndex + 1; // 0 is reserved for globals
 
 					String& blockName = blockSlotToName[gpuParam.paramBlockSlot];
 					GpuParamBlockDesc& curBlockDesc = returnParamDesc.paramBlocks[blockName];
 
-					gpuParam.cpuMemOffset = curBlockDesc.blockSize;
-					curBlockDesc.blockSize += gpuParam.elementSize * gpuParam.arraySize;
+					gpuParam.cpuMemOffset = blockOffset;
+					curBlockDesc.blockSize = std::max(curBlockDesc.blockSize, gpuParam.cpuMemOffset + gpuParam.arrayElementStride * gpuParam.arraySize);
 				}
 				else
 				{
@@ -290,13 +336,57 @@ namespace CamelotEngine
 					gpuParam.paramBlockSlot = 0;
 					gpuParam.cpuMemOffset = globalBlockDesc.blockSize;
 
-					globalBlockDesc.blockSize += gpuParam.elementSize * gpuParam.arraySize;
+					globalBlockDesc.blockSize = std::max(globalBlockDesc.blockSize, gpuParam.cpuMemOffset + gpuParam.arrayElementStride * gpuParam.arraySize);
+				}
+
+				// If parameter is not a part of a struct we're done
+				if(!inStruct)
+				{
+					returnParamDesc.params.insert(std::make_pair(gpuParam.name, gpuParam));
+					continue;
 				}
 
-				returnParamDesc.params.insert(std::make_pair(gpuParam.name, gpuParam));
+				// If the parameter is part of a struct, then we need to update the struct definition
+				auto findExistingStruct = foundStructs.find(structName);
+
+				// Create new definition if one doesn't exist
+				if(findExistingStruct == foundStructs.end())
+				{
+					foundStructs[structName] = GpuParamDataDesc();
+					GpuParamDataDesc& structDesc = foundStructs[structName];
+					structDesc.type = GPDT_STRUCT;
+					structDesc.name = structName;
+					structDesc.arraySize = 1;
+					structDesc.elementSize = 0;
+					structDesc.arrayElementStride = 0;
+					structDesc.gpuMemOffset = gpuParam.gpuMemOffset;
+					structDesc.cpuMemOffset = gpuParam.cpuMemOffset;
+					structDesc.paramBlockSlot = gpuParam.paramBlockSlot;
+				}
+
+				// Update struct with size of the new parameter
+				GpuParamDataDesc& structDesc = foundStructs[structName];
+				structDesc.arraySize = std::max(structDesc.arraySize, arrayIdx + 1);
+
+				assert(gpuParam.cpuMemOffset >= structDesc.cpuMemOffset);
+				if(arrayIdx == 0)
+				{
+					structDesc.elementSize = std::max(structDesc.elementSize, (gpuParam.cpuMemOffset - structDesc.cpuMemOffset) + gpuParam.arrayElementStride * gpuParam.arraySize);
+					structDesc.arrayElementStride = structDesc.elementSize; // TODO - Possibly aligned to 64 byte boundary?
+				}
 			}
 		}
 
+		for(auto iter = foundStructs.begin(); iter != foundStructs.end(); ++iter)
+			returnParamDesc.params.insert(std::make_pair(iter->first, iter->second));
+
+		// Param blocks alway needs to be a multiple of 4, so make it so
+		for(auto iter = returnParamDesc.paramBlocks.begin(); iter != returnParamDesc.paramBlocks.end(); ++iter)
+		{
+			GpuParamBlockDesc& blockDesc = iter->second;
+			blockDesc.blockSize += (4 - (blockDesc.blockSize % 4));
+		}
+
 #if CM_DEBUG_MODE
 		// Check if manually calculated and OpenGL buffer sizes match
 		for(auto iter = returnParamDesc.paramBlocks.begin(); iter != returnParamDesc.paramBlocks.end(); ++iter)
@@ -405,6 +495,19 @@ namespace CamelotEngine
 			CM_EXCEPT(InternalErrorException, "Invalid shader parameter type: " + toString(uniformType) + " for parameter " + paramName);
 		}
 
-		desc.arrayElementStride = desc.elementSize;
+		if(arraySize > 1)
+		{
+			GLint arrayStride;
+			glGetActiveUniformsiv(programHandle, 1, &uniformIndex, GL_UNIFORM_ARRAY_STRIDE, &arrayStride);
+
+			if(arrayStride != 0)
+			{
+				assert (arrayStride % 4 == 0);
+
+				desc.arrayElementStride = arrayStride / 4;
+			}
+		}
+		else
+			desc.arrayElementStride = desc.elementSize;
 	}
 }

CamelotRenderer/TODO.txt

@@ -13,15 +13,16 @@
 Pass
  - A way to bind buffers to a Pass, while specifying buffer range
  - GpuParams support for buffers, structs
+ - Add GL Texture buffers (They're equivalent to DX11 buffers) - http://www.opengl.org/wiki/Buffer_Texture
 
 Shader import:
  - Make sure none of the current shaders in CamelotClient are defined in code anymore
  - Add include files to ImportOptions
 
-  arrays in GLSL should work fine if I increase locations sequentially based on the first element (so I don't need to introduce names like name[0])
+Add a better way of determining GLSL cpuMemOffset and block size (seems to be using DX11 padding)
+ - I probably need to determine struct sizes before assigning cpuMemOffset
+ - I probably also need a preprocessing step to determine array sizes
 
-Test shader arrays
-Implement structs in GLSL
 
 Seems there is a possible deadlock when starting the render thread, while waiting for the thread to be started
 

TODODoc.txt

@@ -4,4 +4,11 @@
     - Classes that are safe to use outside of the render thread: RenderSystem, Mesh, Texture, HighLevelGpuProgram, Material, Shader, Technique
 	- Classes that are only accessible from the render thread: GpuProgram, HardwarePixelBuffer, HardwareVertexBuffer, HardwareIndexBuffer
   - Make sure the user knows resources are shared between contexts. All resource updates are executed 
-     before rendering a frame, so whichever context updated the resource last, was the version that will be used.
+     before rendering a frame, so whichever context updated the resource last, was the version that will be used.
+  - HLSL11 limitations
+     - packoffset on parameters is not supported and will likely result in invalid constant buffer size and/or element offsets
+  - GLSL limitations
+     - layout(row_major) is not supported and will likely result in incorrect matrices
+	 - Only std140 packing method is supported, others will likely result in invalid constant buffer size and/or element offsets
+  - Texture limitations: Only 1D, 2D, 3D and Cube textures (and their samplers) are supported. Support for multisampled textures
+     is included where necessary to implement render targets. Support for texture arrays and is not included.