Skip to content

Commit

Permalink
Fixed the bug using uniform block array
Browse files Browse the repository at this point in the history
At the beginning of Program::defineUniformBlock, getUniformBlockSize will call
getUniformBlockIndex on MacOS to check if the block is an active block with
giving blockName. However, it didn't distinguish array and non-array situations
and unified to use interfaceBlock.name as the blockName to call
getUniformBlockIndex. It would result that INVALID_INDEX was returned when
it's a block array. For example, using 'blockName' not 'blockName[0]'.

In OpenGL 4.3 spec, section 7.3.1, there are following descriptions:
If name exactly matches the name string of one of the active resources for
programInterface, the index of the matched resource is returned. Additionally, if
name would exactly match the name string of an active resource if "[0]" were
appended to name, the index of the matched resource is returned. Otherwise, name
is considered not to be the name of an active resource, and INVALID_INDEX is
returned.

So, for array block case, we use blockName appending [0] to check the activity.

BUG=angleproject:1543

Change-Id: I8189b62066b779f7d392a7dba1cf5cb02a31936d
Reviewed-on: https://chromium-review.googlesource.com/405830
Reviewed-by: Jamie Madill <[email protected]>
Reviewed-by: Geoff Lang <[email protected]>
Commit-Queue: Geoff Lang <[email protected]>
  • Loading branch information
qjia7 authored and Commit Bot committed Nov 4, 2016
1 parent 8f77560 commit 0350a64
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 23 deletions.
21 changes: 11 additions & 10 deletions src/libANGLE/Program.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2758,7 +2758,13 @@ void Program::defineUniformBlock(const sh::InterfaceBlock &interfaceBlock, GLenu
size_t blockSize = 0;

// Don't define this block at all if it's not active in the implementation.
if (!mProgram->getUniformBlockSize(interfaceBlock.name, &blockSize))
std::stringstream blockNameStr;
blockNameStr << interfaceBlock.name;
if (interfaceBlock.arraySize > 0)
{
blockNameStr << "[0]";
}
if (!mProgram->getUniformBlockSize(blockNameStr.str(), &blockSize))
{
return;
}
Expand Down Expand Up @@ -2804,15 +2810,10 @@ void Program::defineUniformBlock(const sh::InterfaceBlock &interfaceBlock, GLenu
UNREACHABLE();
}

// TODO(jmadill): Determine if we can ever have an inactive array element block.
size_t blockElementSize = 0;
if (!mProgram->getUniformBlockSize(block.nameWithArrayIndex(), &blockElementSize))
{
continue;
}

ASSERT(blockElementSize == blockSize);
block.dataSize = static_cast<unsigned int>(blockElementSize);
// Since all block elements in an array share the same active uniforms, they will all be
// active once any uniform member is used. So, since interfaceBlock.name[0] was active,
// here we will add every block element in the array.
block.dataSize = static_cast<unsigned int>(blockSize);
mState.mUniformBlocks.push_back(block);
}
}
Expand Down
51 changes: 38 additions & 13 deletions src/tests/gl_tests/UniformBufferTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,43 +358,68 @@ TEST_P(UniformBufferTest, ActiveUniformNames)
const std::string &vertexShaderSource =
"#version 300 es\n"
"in vec2 position;\n"
"out float v;\n"
"uniform blockName {\n"
" float f;\n"
"} instanceName;\n"
"out vec2 v;\n"
"uniform blockName1 {\n"
" float f1;\n"
"} instanceName1;\n"
"uniform blockName2 {\n"
" float f2;\n"
"} instanceName2[1];\n"
"void main() {\n"
" v = instanceName.f;\n"
" v = vec2(instanceName1.f1, instanceName2[0].f2);\n"
" gl_Position = vec4(position, 0, 1);\n"
"}";

const std::string &fragmentShaderSource =
"#version 300 es\n"
"precision highp float;\n"
"in float v;\n"
"in vec2 v;\n"
"out vec4 color;\n"
"void main() {\n"
" color = vec4(v, 0, 0, 1);\n"
" color = vec4(v, 0, 1);\n"
"}";

GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
ASSERT_NE(0u, program);

GLint activeUniformBlocks;
glGetProgramiv(program, GL_ACTIVE_UNIFORM_BLOCKS, &activeUniformBlocks);
ASSERT_EQ(2, activeUniformBlocks);

GLint maxLength;
GLsizei length;
glGetProgramiv(program, GL_ACTIVE_UNIFORM_BLOCK_MAX_NAME_LENGTH, &maxLength);
std::vector<GLchar> strBlockNameBuffer(maxLength + 1, 0);
glGetActiveUniformBlockName(program, 0, maxLength, &length, &strBlockNameBuffer[0]);
ASSERT_GL_NO_ERROR();
EXPECT_EQ("blockName1", std::string(&strBlockNameBuffer[0]));

glGetActiveUniformBlockName(program, 1, maxLength, &length, &strBlockNameBuffer[0]);
ASSERT_GL_NO_ERROR();
EXPECT_EQ("blockName2[0]", std::string(&strBlockNameBuffer[0]));

GLint activeUniforms;
glGetProgramiv(program, GL_ACTIVE_UNIFORMS, &activeUniforms);

ASSERT_EQ(1, activeUniforms);
ASSERT_EQ(2, activeUniforms);

GLint maxLength, size;
GLint size;
GLenum type;
GLsizei length;
glGetProgramiv(program, GL_ACTIVE_UNIFORM_MAX_LENGTH, &maxLength);
std::vector<GLchar> strBuffer(maxLength + 1, 0);
glGetActiveUniform(program, 0, maxLength, &length, &size, &type, &strBuffer[0]);
std::vector<GLchar> strUniformNameBuffer(maxLength + 1, 0);
glGetActiveUniform(program, 0, maxLength, &length, &size, &type, &strUniformNameBuffer[0]);

ASSERT_GL_NO_ERROR();
EXPECT_EQ(1, size);
EXPECT_GLENUM_EQ(GL_FLOAT, type);
EXPECT_EQ("blockName1.f1", std::string(&strUniformNameBuffer[0]));

glGetActiveUniform(program, 1, maxLength, &length, &size, &type, &strUniformNameBuffer[0]);

ASSERT_GL_NO_ERROR();
EXPECT_EQ(1, size);
EXPECT_GLENUM_EQ(GL_FLOAT, type);
EXPECT_EQ("blockName.f", std::string(&strBuffer[0]));
EXPECT_EQ("blockName2.f2", std::string(&strUniformNameBuffer[0]));
}

// Tests active uniforms and blocks when the layout is std140, shared and packed.
Expand Down

0 comments on commit 0350a64

Please sign in to comment.