diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp index 5e389b15..b38dffa1 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp @@ -396,13 +396,25 @@ namespace MobileGL { return; } - if (location >= programObject->GetUniformCount() || programObject->GetUniformName(location).empty()) { + // Check if location is valid + if (location < 0 || location > programObject->GetMaxUniformLocation()) { MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, MakeShared("MG_Impl/GLImpl", __func__, "`location` does not correspond to a valid uniform variable location for the specified program object.")); return; } + + // Check if the location corresponds to an active uniform + const auto& uniformName = programObject->GetUniformName(location); + if (uniformName.empty()) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeShared("MG_Impl/GLImpl", __func__, + "`location` does not correspond to a valid uniform variable location for the specified program object.")); + return; + } + auto isOpaque = programObject->IsUniformOpaqueAtLocation(location); if (!isOpaque) { // TODO: probably handle int/float differences @@ -513,6 +525,74 @@ namespace MobileGL { } } + // Helper function to transpose a 2x2 matrix + void TransposeMatrix2x2(const GLfloat* input, GLfloat* output) { + // Input matrix is in column-major order (OpenGL default) + // [0 2] + // [1 3] + // + // Output matrix should be in row-major order if transpose is true + // [0 1] + // [2 3] + output[0] = input[0]; // 0,0 element stays the same + output[1] = input[2]; // 0,1 element becomes 1,0 + output[2] = input[1]; // 1,0 element becomes 0,1 + output[3] = input[3]; // 1,1 element stays the same + } + + // Helper function to transpose a 3x3 matrix + void TransposeMatrix3x3(const GLfloat* input, GLfloat* output) { + // Input matrix is in column-major order (OpenGL default) + // [0 3 6] + // [1 4 7] + // [2 5 8] + // + // Output matrix should be in row-major order if transpose is true + // [0 1 2] + // [3 4 5] + // [6 7 8] + output[0] = input[0]; // 0,0 element stays the same + output[1] = input[3]; // 0,1 element becomes 1,0 + output[2] = input[6]; // 0,2 element becomes 2,0 + output[3] = input[1]; // 1,0 element becomes 0,1 + output[4] = input[4]; // 1,1 element stays the same + output[5] = input[7]; // 1,2 element becomes 2,1 + output[6] = input[2]; // 2,0 element becomes 0,2 + output[7] = input[5]; // 2,1 element becomes 1,2 + output[8] = input[8]; // 2,2 element stays the same + } + + // Helper function to transpose a 4x4 matrix + void TransposeMatrix4x4(const GLfloat* input, GLfloat* output) { + // Input matrix is in column-major order (OpenGL default) + // [0 4 8 12] + // [1 5 9 13] + // [2 6 10 14] + // [3 7 11 15] + // + // Output matrix should be in row-major order if transpose is true + // [0 1 2 3] + // [4 5 6 7] + // [8 9 10 11] + // [12 13 14 15] + output[0] = input[0]; // 0,0 element stays the same + output[1] = input[4]; // 0,1 element becomes 1,0 + output[2] = input[8]; // 0,2 element becomes 2,0 + output[3] = input[12]; // 0,3 element becomes 3,0 + output[4] = input[1]; // 1,0 element becomes 0,1 + output[5] = input[5]; // 1,1 element stays the same + output[6] = input[9]; // 1,2 element becomes 2,1 + output[7] = input[13]; // 1,3 element becomes 3,1 + output[8] = input[2]; // 2,0 element becomes 0,2 + output[9] = input[6]; // 2,1 element becomes 1,2 + output[10] = input[10]; // 2,2 element stays the same + output[11] = input[14]; // 2,3 element becomes 3,2 + output[12] = input[3]; // 3,0 element becomes 0,3 + output[13] = input[7]; // 3,1 element becomes 1,3 + output[14] = input[11]; // 3,2 element becomes 2,3 + output[15] = input[15]; // 3,3 element stays the same + } + void Uniform1fv_State(GLint location, GLsizei count, const GLfloat* value) { Uniformv_State<1>(location, count, value); } @@ -560,7 +640,7 @@ namespace MobileGL { return; } - if (location >= programObject->GetUniformCount() || location < -1) { + if (location > programObject->GetMaxUniformLocation() || location < -1) { MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, MakeShared("MG_Impl/GLImpl", __func__, @@ -570,12 +650,15 @@ namespace MobileGL { // For matrix uniforms, we handle each matrix individually for (GLint i = 0; i < count; i++) { - // Note: In this implementation, we're not actually transposing the matrix data - // as we're directly copying to UBO. The transpose parameter is typically used - // in OpenGL to indicate whether the matrix should be transposed before being - // loaded into the uniform variable. In our case, we assume the shader compiler - // has handled the appropriate matrix layout. - Uniform_State<4>(*programObject, location + i, value + i * 4); + if (transpose == GL_TRUE) { + // Transpose the matrix before uploading + GLfloat transposedMatrix[4]; + TransposeMatrix2x2(value + i * 4, transposedMatrix); + Uniform_State<4>(*programObject, location + i, transposedMatrix); + } else { + // No transpose needed, directly copy the matrix data + Uniform_State<4>(*programObject, location + i, value + i * 4); + } } } @@ -594,7 +677,7 @@ namespace MobileGL { return; } - if (location >= programObject->GetUniformCount() || location < -1) { + if (location > programObject->GetMaxUniformLocation() || location < -1) { MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, MakeShared("MG_Impl/GLImpl", __func__, @@ -604,12 +687,15 @@ namespace MobileGL { // For matrix uniforms, we handle each matrix individually for (GLint i = 0; i < count; i++) { - // Note: In this implementation, we're not actually transposing the matrix data - // as we're directly copying to UBO. The transpose parameter is typically used - // in OpenGL to indicate whether the matrix should be transposed before being - // loaded into the uniform variable. In our case, we assume the shader compiler - // has handled the appropriate matrix layout. - Uniform_State<9>(*programObject, location + i, value + i * 9); + if (transpose == GL_TRUE) { + // Transpose the matrix before uploading + GLfloat transposedMatrix[9]; + TransposeMatrix3x3(value + i * 9, transposedMatrix); + Uniform_State<9>(*programObject, location + i, transposedMatrix); + } else { + // No transpose needed, directly copy the matrix data + Uniform_State<9>(*programObject, location + i, value + i * 9); + } } } @@ -628,7 +714,7 @@ namespace MobileGL { return; } - if (location >= programObject->GetUniformCount() || location < -1) { + if (location > programObject->GetMaxUniformLocation() || location < -1) { MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, MakeShared("MG_Impl/GLImpl", __func__, @@ -638,12 +724,15 @@ namespace MobileGL { // For matrix uniforms, we handle each matrix individually for (GLint i = 0; i < count; i++) { - // Note: In this implementation, we're not actually transposing the matrix data - // as we're directly copying to UBO. The transpose parameter is typically used - // in OpenGL to indicate whether the matrix should be transposed before being - // loaded into the uniform variable. In our case, we assume the shader compiler - // has handled the appropriate matrix layout. - Uniform_State<16>(*programObject, location + i, value + i * 16); + if (transpose == GL_TRUE) { + // Transpose the matrix before uploading + GLfloat transposedMatrix[16]; + TransposeMatrix4x4(value + i * 16, transposedMatrix); + Uniform_State<16>(*programObject, location + i, transposedMatrix); + } else { + // No transpose needed, directly copy the matrix data + Uniform_State<16>(*programObject, location + i, value + i * 16); + } } } diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp index 9278ba28..951faaad 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp @@ -70,8 +70,8 @@ namespace MobileGL { return; } - auto uniformCount = m_program->getNumUniformVariables(); - for (int i = 0; i < uniformCount; i++) { + m_activeUniformCount = m_program->getNumUniformVariables(); + for (int i = 0; i < m_activeUniformCount; i++) { auto& uniform = m_program->getUniform(i); auto location = uniform.layoutLocation(); m_maxUniformLocation = std::max(m_maxUniformLocation, location); @@ -85,7 +85,7 @@ namespace MobileGL { m_uniformOffsets.resize(m_maxUniformLocation + 1); m_uniformArraySizes.resize(m_maxUniformLocation + 1); - for (int i = 0; i < uniformCount; i++) { + for (int i = 0; i < m_activeUniformCount; i++) { auto& uniform = m_program->getUniform(i); auto location = uniform.layoutLocation(); m_uniformNames[location] = uniform.name; @@ -168,16 +168,17 @@ namespace MobileGL { auto srcResult = ShaderCompiler::DecompileShader(session); assert(srcResult); auto src = srcResult.value(); + printf("decompiled src: \n%s\n", src.c_str()); auto& meta = session.GetMetadata(); auto size = meta.uboSize; m_uboScratch.resize(size); - m_uniformOffsets.resize(meta.plainUniformOffsetsInUBO.size()); + m_uniformOffsets.resize(m_maxUniformLocation + 1); for (const auto& [name, offset] : meta.plainUniformOffsetsInUBO) { if (m_uniformLocations.find(name) != m_uniformLocations.end()) m_uniformOffsets[m_uniformLocations[name]] = offset; } - m_uniformSizesInBytes.resize(meta.plainUniformMemberSizesInBytes.size()); + m_uniformSizesInBytes.resize(m_maxUniformLocation + 1); for (const auto& [name, size] : meta.plainUniformMemberSizesInBytes) { if (m_uniformLocations.find(name) != m_uniformLocations.end()) m_uniformSizesInBytes[m_uniformLocations[name]] = size; diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index dec3b366..afdcca0a 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -1,6 +1,7 @@ #pragma once #include #include "ShaderObject.h" +#include "MG_Util/Metrics/BufferMetrics.h" #include "MG_Util/ShaderTranspiler/SpvcSession.h" namespace MobileGL { @@ -20,7 +21,8 @@ namespace MobileGL { Vector>& GetAttachedShaders(); const String& GetInfoLog() const { return m_infoLog; } Int GetUniformMaxLength() const { return m_uniformNameMaxLength; } - Uint GetUniformCount() { return m_uniformNames.size(); } + Uint GetUniformCount() { return m_activeUniformCount; } + Uint GetMaxUniformLocation() const { return m_maxUniformLocation; } Int GetUniformLocation(const String& name) { const auto it = m_uniformLocations.find(name); return (it == m_uniformLocations.end()) ? -1 : (Int)it->second; @@ -40,7 +42,7 @@ namespace MobileGL { return m_uniformOffsets[location]; } Uint GetUniformSizesInBytes(Uint location) const { - return m_uniformSizesInBytes[location]; + return MG_Util::GetGLTypeSize(m_uniformTypes[location]); } Int GetAttributeLocation(const String& name) { @@ -101,6 +103,7 @@ namespace MobileGL { Vector m_uniformSizesInBytes; Vector m_uboScratch; + Uint m_activeUniformCount = 0; Uint m_maxUniformLocation = 0; Int m_uniformNameMaxLength = 0; Int m_attribInNameMaxLength = 0; diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index 1a3d6b87..3eb9d853 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -32,6 +32,8 @@ in float fIn1; in float fIn3; layout(location = 0) uniform mat4 ProjMat; +layout(location = 10) uniform mat3 TestMat3; +layout(location = 20) uniform mat2 TestMat2; uniform vec2 InSize; uniform vec2 OutSize; @@ -42,6 +44,10 @@ void main(){ vec4 outPos = ProjMat * vec4(Position.xy, 0.0, 1.0); gl_Position = vec4(outPos.xy, 0.2, 1.0); + // Use TestMat2 and TestMat3 to prevent optimization + vec2 dummy2 = TestMat2[0]; + vec3 dummy3 = TestMat3[0]; + oneTexel = (1.0 * (fIn1 * fIn2 * fIn3 * fIn4 * fIn5 * fIn6)) / InSize; texCoord = Position.xy / OutSize; @@ -140,7 +146,7 @@ TEST_F(ProgramTest, CompileAndLink) { ASSERT_EQ(GetUniformLocation(program, "Saturation"), 6); GLint uniformCount = 0; GetProgramiv(program, GL_ACTIVE_UNIFORMS, &uniformCount); - ASSERT_EQ(uniformCount, 12); + ASSERT_EQ(uniformCount, 14); GLint uniformNameMaxLength = 0; GetProgramiv(program, GL_ACTIVE_UNIFORM_MAX_LENGTH, &uniformNameMaxLength); ASSERT_EQ(uniformNameMaxLength, 12); @@ -210,7 +216,7 @@ TEST_F(ProgramTest, UniformMatrixFunctions) { auto locProjMat = GetUniformLocation(program, "ProjMat"); ASSERT_NE(locProjMat, -1); - // 4x4 matrix (16 elements) + // 4x4 matrix (16 elements) - identity matrix GLfloat matrix4x4[16] = { 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, @@ -218,9 +224,200 @@ TEST_F(ProgramTest, UniformMatrixFunctions) { 0.0f, 0.0f, 0.0f, 1.0f }; - // Test UniformMatrix4fv with count = 1 + // Test UniformMatrix4fv with count = 1 and transpose = GL_FALSE UniformMatrix4fv(locProjMat, 1, GL_FALSE, matrix4x4); + // Test UniformMatrix4fv with count = 1 and transpose = GL_TRUE + UniformMatrix4fv(locProjMat, 1, GL_TRUE, matrix4x4); + + // Test with a non-identity matrix + GLfloat nonIdentityMatrix[16] = { + 1.0f, 2.0f, 3.0f, 4.0f, + 5.0f, 6.0f, 7.0f, 8.0f, + 9.0f, 10.0f, 11.0f, 12.0f, + 13.0f, 14.0f, 15.0f, 16.0f + }; + + // Test with transpose = GL_FALSE + UniformMatrix4fv(locProjMat, 1, GL_FALSE, nonIdentityMatrix); + + // Test with transpose = GL_TRUE + UniformMatrix4fv(locProjMat, 1, GL_TRUE, nonIdentityMatrix); + + // Test UniformMatrix3fv with a 3x3 matrix + // We would need a 3x3 uniform in the shader for this test + + // Test UniformMatrix2fv with a 2x2 matrix + // We would need a 2x2 uniform in the shader for this test + // Test with multiple matrices (count > 1) // For this test, we would need uniforms that are arrays of matrices } + +TEST_F(ProgramTest, UniformMatrixTranspose) { + char infoLog[1024] = ""; + + GLuint vs = CreateShader(GL_VERTEX_SHADER); + ShaderSource(vs, 1, &vsSrc, NULL); + printf("Compiling vertex shader: %s\n", vsSrc); + CompileShader(vs); + GLint vsStatus = GL_FALSE; + GetShaderiv(vs, GL_COMPILE_STATUS, &vsStatus); + GetShaderInfoLog(vs, 1024, nullptr, infoLog); + ASSERT_EQ(vsStatus, GL_TRUE) << infoLog; + printf("Compiled vertex shader.\n"); + + GLuint fs = CreateShader(GL_FRAGMENT_SHADER); + ShaderSource(fs, 1, &fsSrc, NULL); + printf("Compiling fragment shader: %s\n", fsSrc); + CompileShader(fs); + GLint fsStatus = GL_FALSE; + GetShaderiv(fs, GL_COMPILE_STATUS, &fsStatus); + GetShaderInfoLog(fs, 1024, nullptr, infoLog); + ASSERT_EQ(fsStatus, GL_TRUE) << infoLog; + printf("Compiled fragment shader.\n"); + + GLuint program = CreateProgram(); + AttachShader(program, vs); + AttachShader(program, fs); + + BindAttribLocation(program, 1, "fIn1"); + BindAttribLocation(program, 3, "fIn3"); + BindAttribLocation(program, 5, "fIn5"); + printf("Linking program...\n"); + LinkProgram(program); + printf("Program linked.\n"); + + UseProgram(program); + + // Test 2x2 matrix transpose + auto locMat2 = GetUniformLocation(program, "TestMat2"); + if (locMat2 != -1) { + // Test matrix (column-major as expected by OpenGL): + // [1 3] + // [2 4] + GLfloat matrix2x2[4] = { + 1.0f, 2.0f, // First column + 3.0f, 4.0f // Second column + }; + + // Expected values when transpose = GL_FALSE (no transpose): + // [1 3] + // [2 4] + GLfloat expected2x2_no_transpose[4] = {1.0f, 2.0f, 3.0f, 4.0f}; + + // Expected values when transpose = GL_TRUE (transposed): + // [1 2] + // [3 4] + // Stored in column-major order: [1, 3, 2, 4] + GLfloat expected2x2_transpose[4] = {1.0f, 3.0f, 2.0f, 4.0f}; + + // Test with transpose = GL_FALSE + UniformMatrix2fv(locMat2, 1, GL_FALSE, matrix2x2); + GLfloat result2x2_no_transpose[4]; + GetUniformfv(program, locMat2, result2x2_no_transpose); + for (int i = 0; i < 4; i++) { + EXPECT_FLOAT_EQ(result2x2_no_transpose[i], expected2x2_no_transpose[i]); + } + + // Test with transpose = GL_TRUE + UniformMatrix2fv(locMat2, 1, GL_TRUE, matrix2x2); + GLfloat result2x2_transpose[4]; + GetUniformfv(program, locMat2, result2x2_transpose); + for (int i = 0; i < 4; i++) { + EXPECT_FLOAT_EQ(result2x2_transpose[i], expected2x2_transpose[i]); + } + } + + // Test 3x3 matrix transpose + auto locMat3 = GetUniformLocation(program, "TestMat3"); + if (locMat3 != -1) { + // Test matrix (column-major as expected by OpenGL): + // [1 4 7] + // [2 5 8] + // [3 6 9] + GLfloat matrix3x3[9] = { + 1.0f, 2.0f, 3.0f, // First column + 4.0f, 5.0f, 6.0f, // Second column + 7.0f, 8.0f, 9.0f // Third column + }; + + // Expected values when transpose = GL_FALSE (no transpose): + // [1 4 7] + // [2 5 8] + // [3 6 9] + GLfloat expected3x3_no_transpose[9] = {1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f, 7.0f, 8.0f, 9.0f}; + + // Expected values when transpose = GL_TRUE (transposed): + // [1 2 3] + // [4 5 6] + // [7 8 9] + // Stored in column-major order: [1, 4, 7, 2, 5, 8, 3, 6, 9] + GLfloat expected3x3_transpose[9] = {1.0f, 4.0f, 7.0f, 2.0f, 5.0f, 8.0f, 3.0f, 6.0f, 9.0f}; + + // Test with transpose = GL_FALSE + UniformMatrix3fv(locMat3, 1, GL_FALSE, matrix3x3); + GLfloat result3x3_no_transpose[9]; + GetUniformfv(program, locMat3, result3x3_no_transpose); + for (int i = 0; i < 9; i++) { + EXPECT_FLOAT_EQ(result3x3_no_transpose[i], expected3x3_no_transpose[i]); + } + + // Test with transpose = GL_TRUE + UniformMatrix3fv(locMat3, 1, GL_TRUE, matrix3x3); + GLfloat result3x3_transpose[9]; + GetUniformfv(program, locMat3, result3x3_transpose); + for (int i = 0; i < 9; i++) { + EXPECT_FLOAT_EQ(result3x3_transpose[i], expected3x3_transpose[i]); + } + } + + // Test 4x4 matrix transpose + auto locProjMat = GetUniformLocation(program, "ProjMat"); + ASSERT_NE(locProjMat, -1); + + // Test matrix (column-major as expected by OpenGL): + // [1 5 9 13] + // [2 6 10 14] + // [3 7 11 15] + // [4 8 12 16] + GLfloat matrix4x4[16] = { + 1.0f, 2.0f, 3.0f, 4.0f, // First column + 5.0f, 6.0f, 7.0f, 8.0f, // Second column + 9.0f, 10.0f, 11.0f, 12.0f, // Third column + 13.0f, 14.0f, 15.0f, 16.0f // Fourth column + }; + + // Expected values when transpose = GL_FALSE (no transpose): + // [1 5 9 13] + // [2 6 10 14] + // [3 7 11 15] + // [4 8 12 16] + GLfloat expected4x4_no_transpose[16] = {1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f, 7.0f, 8.0f, + 9.0f, 10.0f, 11.0f, 12.0f, 13.0f, 14.0f, 15.0f, 16.0f}; + + // Expected values when transpose = GL_TRUE (transposed): + // [1 2 3 4] + // [5 6 7 8] + // [9 10 11 12] + // [13 14 15 16] + // Stored in column-major order + GLfloat expected4x4_transpose[16] = {1.0f, 5.0f, 9.0f, 13.0f, 2.0f, 6.0f, 10.0f, 14.0f, + 3.0f, 7.0f, 11.0f, 15.0f, 4.0f, 8.0f, 12.0f, 16.0f}; + + // Test with transpose = GL_FALSE + UniformMatrix4fv(locProjMat, 1, GL_FALSE, matrix4x4); + GLfloat result4x4_no_transpose[16]; + GetUniformfv(program, locProjMat, result4x4_no_transpose); + for (int i = 0; i < 16; i++) { + EXPECT_FLOAT_EQ(result4x4_no_transpose[i], expected4x4_no_transpose[i]); + } + + // Test with transpose = GL_TRUE + UniformMatrix4fv(locProjMat, 1, GL_TRUE, matrix4x4); + GLfloat result4x4_transpose[16]; + GetUniformfv(program, locProjMat, result4x4_transpose); + for (int i = 0; i < 16; i++) { + EXPECT_FLOAT_EQ(result4x4_transpose[i], expected4x4_transpose[i]); + } +}