Clarify function of material uniform struct packing

It's actually std140 not std430, so fix comment and GLShader member
names to reflect that. (GLUniform still has the old _std430XXX names
though.)

Since it cannot handle arrays, add an assert that the uniforms are not
arrays. Also delete WriteToBuffer implementations that assume std430
layout for arrays.

Author: slipher

src/engine/renderer/Material.cpp

@@ -1446,7 +1446,7 @@ void MaterialSystem::ProcessStage( MaterialSurface* surface, shaderStage_t* pSta
 
 	material.bspSurface = surface->bspSurface;
 	pStage->materialProcessor( &material, pStage, surface );
-	pStage->paddedSize = material.shader->GetSTD430Size();
+	pStage->paddedSize = material.shader->GetSTD140Size();
 
 	// HACK: Copy the shaderStage_t and MaterialSurface that we need into the material, so we can use it with glsl_restart
 	material.refStage = pStage;

src/engine/renderer/gl_shader.cpp

@@ -1578,7 +1578,7 @@ void GLShaderManager::GenerateUniformStructDefinesText( const std::vector<GLUnif
 
 // This will generate all the extra code for material system shaders
 std::string GLShaderManager::ShaderPostProcess( GLShader *shader, const std::string& shaderText, const uint32_t offset ) {
-	if ( !shader->std430Size ) {
+	if ( !shader->std140Size ) {
 		return shaderText;
 	}
 
@@ -1641,7 +1641,7 @@ std::string GLShaderManager::ShaderPostProcess( GLShader *shader, const std::str
 	materialStruct += "};\n\n";
 
 	// 6 kb for materials
-	const uint32_t count = ( 4096 + 2048 ) / shader->GetSTD430Size();
+	const uint32_t count = ( 4096 + 2048 ) / shader->GetSTD140Size();
 	std::string materialBlock = "layout(std140, binding = "
 		+ std::to_string( BufferBind::MATERIALS )
 		+ ") uniform materialsUBO {\n"
@@ -2084,8 +2084,8 @@ bool GLCompileMacro_USE_BSP_SURFACE::HasConflictingMacros(size_t permutation, co
 	return false;
 }
 
-uint32_t* GLUniform::WriteToBuffer( uint32_t* buffer ) {
-	return buffer;
+uint32_t* GLUniform::WriteToBuffer( uint32_t * ) {
+	Sys::Error( "WriteToBuffer not implemented for GLUniform '%s'", _name );
 }
 
 void GLShader::RegisterUniform( GLUniform* uniform ) {
@@ -2107,7 +2107,9 @@ static auto FindUniformForOffset( std::vector<GLUniform*>& uniforms, const GLuin
 	return uniforms.end();
 }
 
-// Compute std430 size/alignment and sort uniforms from highest to lowest alignment
+// Compute std140 size/alignment and sort uniforms from highest to lowest alignment
+// Note: using the std430 uniform size will give the wrong result for matrix types where
+// the number of rows is not 4
 void GLShader::PostProcessUniforms() {
 	if ( !_useMaterialSystem ) {
 		return;
@@ -2128,17 +2130,18 @@ void GLShader::PostProcessUniforms() {
 
 	// Sort uniforms from highest to lowest alignment so we don't need to pad uniforms (other than vec3s)
 	GLuint align = 4; // mininum alignment since this will be used as an std140 array element
-	std430Size = 0;
+	std140Size = 0;
 	_materialSystemUniforms.clear();
-	while ( !uniformQueue.empty() || std430Size & ( align - 1 ) ) {
-		auto iterNext = FindUniformForOffset( uniformQueue, std430Size );
+	while ( !uniformQueue.empty() || std140Size & ( align - 1 ) ) {
+		auto iterNext = FindUniformForOffset( uniformQueue, std140Size );
 		if ( iterNext == uniformQueue.end() ) {
 			// add 1 unit of padding
-			++std430Size;
+			++std140Size;
 			++_materialSystemUniforms.back()->_std430Size;
 		} else {
+			ASSERT_EQ( 0, ( *iterNext )->_components ); // array handling not implemented
 			( *iterNext )->_std430Size = ( *iterNext )->_std430BaseSize;
-			std430Size += ( *iterNext )->_std430Size;
+			std140Size += ( *iterNext )->_std430Size;
 			align = std::max( align, ( *iterNext )->_std430Alignment );
 			_materialSystemUniforms.push_back( *iterNext );
 			uniformQueue.erase( iterNext );

src/engine/renderer/gl_shader.h

@@ -185,7 +185,7 @@ class GLShader {
 	const bool hasFragmentShader;
 	const bool hasComputeShader;
 
-	GLuint std430Size = 0;
+	GLuint std140Size = 0;
 
 	const bool worldShader;
 protected:
@@ -300,8 +300,8 @@ class GLShader {
 		_vertexAttribs &= ~bit;
 	}
 
-	GLuint GetSTD430Size() const {
-		return std430Size;
+	GLuint GetSTD140Size() const {
+		return std140Size;
 	}
 
 	bool UseMaterialSystem() const {
@@ -317,6 +317,7 @@ class GLUniform {
 	const std::string _type;
 
 	// In multiples of 4 bytes
+	// FIXME: the uniform structs are actually std140 so it would be more relevant to provide std140 info
 	const GLuint _std430BaseSize;
 	GLuint _std430Size; // includes padding that depends on the other uniforms in the struct
 	const GLuint _std430Alignment;
@@ -690,11 +691,6 @@ class GLUniform1Bool : protected GLUniform {
 		return sizeof( int );
 	}
 
-	uint32_t* WriteToBuffer( uint32_t *buffer ) override {
-		memcpy( buffer, &currentValue, sizeof( bool ) );
-		return buffer + _std430Size;
-	}
-
 	private:
 	int currentValue = 0;
 };
@@ -772,11 +768,6 @@ class GLUniform1fv : protected GLUniform
 		glUniform1fv( p->uniformLocations[ _locationIndex ], numFloats, f );
 	}
 
-	uint32_t* WriteToBuffer( uint32_t* buffer ) override {
-		memcpy( buffer, currentValue.data(), currentValue.size() * sizeof( float ) );
-		return buffer + _components * _std430Size;
-	}
-
 	private:
 	std::vector<float> currentValue;
 };
@@ -1044,11 +1035,6 @@ class GLUniformMatrix32f : protected GLUniform {
 		return 6 * sizeof( float );
 	}
 
-	uint32_t* WriteToBuffer( uint32_t* buffer ) override {
-		memcpy( buffer, currentValue, 6 * sizeof( float ) );
-		return buffer + _std430Size * _components;
-	}
-
 	private:
 	vec_t currentValue[6] {};
 };

src/engine/renderer/gl_shader_test.cpp

@@ -67,7 +67,7 @@ TEST(MaterialUniformPackingTest, OneMatrix)
 
     Shader1 shader1;
     std::vector<GLUniform*> uniforms = shader1.GetUniforms();
-    EXPECT_EQ(shader1.GetSTD430Size(), 16u);
+    EXPECT_EQ(shader1.GetSTD140Size(), 16u);
     ASSERT_EQ(uniforms.size(), 1);
     EXPECT_EQ(uniforms[0], Get<u_ModelViewMatrix>(shader1));
     EXPECT_EQ(uniforms[0]->_std430Size, 16u);
@@ -85,7 +85,7 @@ TEST(MaterialUniformPackingTest, TwoFloats)
 
     Shader1 shader1;
     std::vector<GLUniform*> uniforms = shader1.GetUniforms();
-    EXPECT_EQ(shader1.GetSTD430Size(), 4u);
+    EXPECT_EQ(shader1.GetSTD140Size(), 4u);
     ASSERT_EQ(uniforms.size(), 2);
     EXPECT_EQ(uniforms[0], Get<u_DeformMagnitude>(shader1));
     EXPECT_EQ(uniforms[0]->_std430Size, 1u);
@@ -110,7 +110,7 @@ TEST(MaterialUniformPackingTest, Vec3Handling)
 
     Shader1 shader1;
     std::vector<GLUniform*> uniforms = shader1.GetUniforms();
-    EXPECT_EQ(shader1.GetSTD430Size(), 12u);
+    EXPECT_EQ(shader1.GetSTD140Size(), 12u);
     ASSERT_EQ(uniforms.size(), 4);
     EXPECT_EQ(uniforms[0], Get<u_FogColor>(shader1));
     EXPECT_EQ(uniforms[0]->_std430Size, 3u);