Skip to content

Commit

Permalink
Make GL_CHROMIUM_consistent_uniform_locations slighty more robust
Browse files Browse the repository at this point in the history
Added a program argument so that at least in debug we can
verify locations are correct. It also means we could fallback
to actually calling GetUniformLocation if need be.

TEST=unit tests
BUG=132844


Review URL: https://chromiumcodereview.appspot.com/10581029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143126 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
gman@chromium.org committed Jun 20, 2012
1 parent ad5d305 commit 991564b
Show file tree
Hide file tree
Showing 13 changed files with 132 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,13 @@ Overview
This extension guarantees uniforms always have the same locations.

This allows the client program to know the locations of uniforms
without having to compile the shaders, link the program and query
the locations and therefore have no blocking calls when initializing
programs.
without having to wait for shaders to compile or GLSL programs to
link to query the locations and therefore have no blocking calls
when initializing programs.

To be forward compatible the locations are provided through the
function GetUniformLocationsCHROMIUM but they can be provided
even before linking a program and therefore do not have to wait
for compile or link completion to return results.
function GetUniformLocationsCHROMIUM. LinkProgram must be called
before calling GetUniformLocationsCHROMIUM.

Issues

Expand All @@ -40,7 +39,8 @@ New Tokens

New Procedures and Functions

void GetUniformLocationsCHROMIUM (const GLUniformDefinitionCHROMIUM* uniforms,
void GetUniformLocationsCHROMIUM (GLuint program,
const GLUniformDefinitionCHROMIUM* uniforms,
GLsizei count, GLsizei max_locations,
GLint* locations);

Expand Down Expand Up @@ -84,3 +84,5 @@ New State
Revision History

7/17/2012 Documented the extension
7/19/2012 Added <program> argument.

4 changes: 2 additions & 2 deletions gpu/command_buffer/client/gles2_c_lib_autogen.h
Original file line number Diff line number Diff line change
Expand Up @@ -652,10 +652,10 @@ void GLES2ConsumeTextureCHROMIUM(GLenum target, const GLbyte* mailbox) {
gles2::GetGLContext()->ConsumeTextureCHROMIUM(target, mailbox);
}
void GLES2GetUniformLocationsCHROMIUM(
const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count,
GLuint program, const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count,
GLsizei max_locations, GLint* locations) {
gles2::GetGLContext()->GetUniformLocationsCHROMIUM(
uniforms, count, max_locations, locations);
program, uniforms, count, max_locations, locations);
}

#endif // GPU_COMMAND_BUFFER_CLIENT_GLES2_C_LIB_AUTOGEN_H_
Expand Down
21 changes: 20 additions & 1 deletion gpu/command_buffer/client/gles2_implementation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3295,13 +3295,18 @@ class GLUniformDefinitionComparer {
const GLUniformDefinitionCHROMIUM* uniforms_;
};



} // anonymous namespace.

void GLES2Implementation::GetUniformLocationsCHROMIUM(
GLuint program,
const GLUniformDefinitionCHROMIUM* uniforms,
GLsizei count,
GLsizei max_locations,
GLint* locations) {
(void)program; // To keep the compiler happy as it's unused in release.

GPU_CLIENT_SINGLE_THREAD_CHECK();
GPU_CLIENT_LOG("[" << this << "] glGenUniformLocationsCHROMIUM("
<< static_cast<const void*>(uniforms) << ", " << count << ", "
Expand Down Expand Up @@ -3342,9 +3347,23 @@ void GLES2Implementation::GetUniformLocationsCHROMIUM(
if (max_locations <= 0) {
return;
}
*locations++ = GLES2Util::SwizzleLocation(
GLint location = GLES2Util::SwizzleLocation(
GLES2Util::MakeFakeLocation(base_location, jj));
*locations++ = location;
#if defined(GPU_CLIENT_DEBUG)
std::string name(def.name);
if (jj > 0) {
char buf[20];
sprintf(buf, "%d", jj);
name = name + "[" + buf + "]";
}
GPU_DCHECK_EQ(
location,
share_group_->program_info_manager()->GetUniformLocation(
this, program, name.c_str()));
#endif
--max_locations;

}
}
}
Expand Down
2 changes: 1 addition & 1 deletion gpu/command_buffer/client/gles2_implementation_autogen.h
Original file line number Diff line number Diff line change
Expand Up @@ -1560,7 +1560,7 @@ void ConsumeTextureCHROMIUM(GLenum target, const GLbyte* mailbox) {
}

void GetUniformLocationsCHROMIUM(
const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count,
GLuint program, const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count,
GLsizei max_locations, GLint* locations);

#endif // GPU_COMMAND_BUFFER_CLIENT_GLES2_IMPLEMENTATION_AUTOGEN_H_
Expand Down
85 changes: 80 additions & 5 deletions gpu/command_buffer/client/gles2_implementation_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <GLES2/gl2ext.h>
#include "gpu/command_buffer/client/client_test_helper.h"
#include "gpu/command_buffer/client/program_info_manager.h"
#include "gpu/command_buffer/client/transfer_buffer.h"
#include "gpu/command_buffer/common/command_buffer.h"
#include "gpu/command_buffer/common/compiler_specific.h"
Expand Down Expand Up @@ -445,6 +446,12 @@ class GLES2ImplementationTest : public testing::Test {
return transfer_buffer_->GetExpectedResultMemory(size);
}

// Sets the ProgramInfoManager. The manager will be owned
// by the ShareGroup.
void SetProgramInfoManager(ProgramInfoManager* manager) {
gl_->share_group()->set_program_info_manager(manager);
}

int CheckError() {
ExpectedMemoryInfo result =
GetExpectedResultMemory(sizeof(GetError::Result));
Expand Down Expand Up @@ -2558,7 +2565,43 @@ TEST_F(GLES2ImplementationTest, BeginEndQueryEXT) {
EXPECT_EQ(0u, available);
}

namespace {

class MockProgramInfoManager : public ProgramInfoManager {
public:
virtual ~MockProgramInfoManager() {};

MOCK_METHOD1(CreateInfo, void(GLuint program));

MOCK_METHOD1(DeleteInfo, void(GLuint program));

MOCK_METHOD4(GetProgramiv, bool(
GLES2Implementation* gl, GLuint program, GLenum pname, GLint* params));

MOCK_METHOD3(GetAttribLocation, GLint(
GLES2Implementation* gl, GLuint program, const char* name));

MOCK_METHOD3(GetUniformLocation, GLint(
GLES2Implementation* gl, GLuint program, const char* name));

MOCK_METHOD8(GetActiveAttrib, bool(
GLES2Implementation* gl,
GLuint program, GLuint index, GLsizei bufsize, GLsizei* length,
GLint* size, GLenum* type, char* name));

MOCK_METHOD8(GetActiveUniform, bool(
GLES2Implementation* gl,
GLuint program, GLuint index, GLsizei bufsize, GLsizei* length,
GLint* size, GLenum* type, char* name));
};

} // anonymous namespace

TEST_F(GLES2ImplementationTest, GetUniformLocationsCHROMIUM) {
MockProgramInfoManager* manager = new MockProgramInfoManager();
SetProgramInfoManager(manager);

const GLuint kProgramId = 123;
static const GLUniformDefinitionCHROMIUM good_defs[] = {
{ GL_FLOAT_VEC4, 1, "moo", },
{ GL_FLOAT_VEC4, 4, "bar", },
Expand All @@ -2573,19 +2616,30 @@ TEST_F(GLES2ImplementationTest, GetUniformLocationsCHROMIUM) {

// Test bad count
GLint locations[50] = { -1, };
gl_->GetUniformLocationsCHROMIUM(bad_defs, 0, 1, locations);
gl_->GetUniformLocationsCHROMIUM(kProgramId, bad_defs, 0, 1, locations);
EXPECT_EQ(GL_INVALID_VALUE, CheckError());
EXPECT_EQ(-1, locations[0]);

// Test bad size.
gl_->GetUniformLocationsCHROMIUM(
bad_defs, arraysize(bad_defs), 1, locations);
kProgramId, bad_defs, arraysize(bad_defs), 1, locations);
EXPECT_EQ(GL_INVALID_VALUE, CheckError());
EXPECT_EQ(-1, locations[0]);

#if defined(GPU_CLIENT_DEBUG)
EXPECT_CALL(*manager, GetUniformLocation(_, kProgramId, _))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(2, 0))))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 0))))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 1))))
.RetiresOnSaturation();
#endif

// Test max_locations
gl_->GetUniformLocationsCHROMIUM(
good_defs, arraysize(good_defs), 3, locations);
kProgramId, good_defs, arraysize(good_defs), 3, locations);
EXPECT_EQ(GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(2, 0)),
locations[0]);
EXPECT_EQ(GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 0)),
Expand All @@ -2594,9 +2648,31 @@ TEST_F(GLES2ImplementationTest, GetUniformLocationsCHROMIUM) {
locations[2]);
EXPECT_EQ(0, locations[3]);

#if defined(GPU_CLIENT_DEBUG)
EXPECT_CALL(*manager, GetUniformLocation(_, kProgramId, _))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(2, 0))))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 0))))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 1))))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 2))))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 3))))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(1, 0))))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(1, 1))))
.WillOnce(Return(
GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(1, 2))))
.RetiresOnSaturation();
#endif

// Test all.
gl_->GetUniformLocationsCHROMIUM(
good_defs, arraysize(good_defs), arraysize(locations), locations);
kProgramId, good_defs, arraysize(good_defs), arraysize(locations),
locations);
EXPECT_EQ(GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(2, 0)),
locations[0]);
EXPECT_EQ(GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 0)),
Expand All @@ -2621,4 +2697,3 @@ TEST_F(GLES2ImplementationTest, GetUniformLocationsCHROMIUM) {
} // namespace gles2
} // namespace gpu


3 changes: 2 additions & 1 deletion gpu/command_buffer/client/program_info_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
#define GPU_COMMAND_BUFFER_CLIENT_PROGRAM_INFO_MANAGER_H_

#include <GLES2/gl2.h>
#include "gles2_impl_export.h"

namespace gpu {
namespace gles2 {

class GLES2Implementation;

// Manages info about OpenGL ES Programs.
class ProgramInfoManager {
class GLES2_IMPL_EXPORT ProgramInfoManager {
public:
virtual ~ProgramInfoManager();

Expand Down
4 changes: 4 additions & 0 deletions gpu/command_buffer/client/share_group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,10 @@ void ShareGroup::SetGLES2ImplementationForDestruction(
gles2_ = gl_impl;
}

void ShareGroup::set_program_info_manager(ProgramInfoManager* manager) {
program_info_manager_.reset(manager);
}

ShareGroup::~ShareGroup() {
for (int i = 0; i < id_namespaces::kNumIdNamespaces; ++i) {
id_handlers_[i]->Destroy(gles2_);
Expand Down
5 changes: 5 additions & 0 deletions gpu/command_buffer/client/share_group.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ namespace gpu {
namespace gles2 {

class GLES2Implementation;
class GLES2ImplementationTest;
class ProgramInfoManager;

typedef void (GLES2Implementation::*DeleteFn)(GLsizei n, const GLuint* ids);
Expand Down Expand Up @@ -72,8 +73,12 @@ class GLES2_IMPL_EXPORT ShareGroup

private:
friend class gpu::RefCountedThreadSafe<ShareGroup>;
friend class gpu::gles2::GLES2ImplementationTest;
~ShareGroup();

// Install a new program info manager. Used for testing only;
void set_program_info_manager(ProgramInfoManager* manager);

scoped_ptr<IdHandlerInterface> id_handlers_[id_namespaces::kNumIdNamespaces];
scoped_ptr<ProgramInfoManager> program_info_manager_;

Expand Down
2 changes: 1 addition & 1 deletion gpu/command_buffer/cmd_buffer_functions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -185,5 +185,5 @@ GL_APICALL void GL_APIENTRY glVertexAttribDivisorANGLE (GLuint index, GL
GL_APICALL void GL_APIENTRY glGenMailboxCHROMIUM (GLbyte* mailbox);
GL_APICALL void GL_APIENTRY glProduceTextureCHROMIUM (GLenumTextureTarget target, const GLbyte* mailbox);
GL_APICALL void GL_APIENTRY glConsumeTextureCHROMIUM (GLenumTextureTarget target, const GLbyte* mailbox);
GL_APICALL void GL_APIENTRY glGetUniformLocationsCHROMIUM (const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
GL_APICALL void GL_APIENTRY glGetUniformLocationsCHROMIUM (GLidProgram program, const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);

Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ static GLES2Util::EnumToString enum_to_string_table[] = {
{ 0x00000400, "GL_STENCIL_BUFFER_BIT", },
{ 0x800A, "GL_FUNC_SUBTRACT", },
{ 0x8E2C, "GL_DEPTH_COMPONENT16_NONLINEAR_NV", },
{ 0x8508, "GL_DECR_WRAP", },
{ 0x889F, "GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING", },
{ 0x8006, "GL_FUNC_ADD", },
{ 0x8007, "GL_MIN_EXT", },
{ 0x8004, "GL_ONE_MINUS_CONSTANT_ALPHA", },
Expand Down Expand Up @@ -501,7 +501,7 @@ static GLES2Util::EnumToString enum_to_string_table[] = {
{ 0x8CD6, "GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT", },
{ 0x8253, "GL_GUILTY_CONTEXT_RESET_EXT", },
{ 0x8872, "GL_MAX_TEXTURE_IMAGE_UNITS", },
{ 0x889F, "GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING", },
{ 0x8508, "GL_DECR_WRAP", },
{ 0x8507, "GL_INCR_WRAP", },
{ 0x8895, "GL_ELEMENT_ARRAY_BUFFER_BINDING", },
{ 0x8894, "GL_ARRAY_BUFFER_BINDING", },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,18 @@ TEST_F(ConsistenUniformLocationsTest, Basic) {
{ GL_FLOAT_VEC4, 1, "u_colorA", },
};

GLuint program = GLTestHelper::LoadProgram(v_shader_str, f_shader_str);

GLint locations[4];

glGetUniformLocationsCHROMIUM(
defs, arraysize(defs), arraysize(locations), locations);
program, defs, arraysize(defs), arraysize(locations), locations);

GLint u_colorCLocation = locations[0];
GLint u_colorB0Location = locations[1];
GLint u_colorB1Location = locations[2];
GLint u_colorALocation = locations[3];

GLuint program = GLTestHelper::LoadProgram(v_shader_str, f_shader_str);

GLint position_loc = glGetAttribLocation(program, "a_position");

GLTestHelper::SetupUnitQuad(position_loc);
Expand Down
4 changes: 2 additions & 2 deletions third_party/khronos/GLES2/gl2ext.h
Original file line number Diff line number Diff line change
Expand Up @@ -2018,10 +2018,10 @@ struct GLUniformDefinitionCHROMIUM {
#ifdef GL_GLEXT_PROTOTYPES
#define glGetUniformLocationsCHROMIUM GLES2_GET_FUN(GetUniformLocationsCHROMIUM)
#if !defined(GLES2_USE_CPP_BINDINGS)
GL_APICALL void GL_APIENTRY glGetUniformLocationsCHROMIUM (const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
GL_APICALL void GL_APIENTRY glGetUniformLocationsCHROMIUM (GLuint program, const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
#endif
#else
typedef void (GL_APIENTRYP PFNGLGETUNIFORMLOCATIONSCHROMIUM) (const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
typedef void (GL_APIENTRYP PFNGLGETUNIFORMLOCATIONSCHROMIUM) (GLuint program, const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
#endif
#endif

Expand Down
1 change: 1 addition & 0 deletions third_party/khronos/README.chromium
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ GLES2/gl2ext.h
- Added GL_CHROMIUM_command_buffer_query
- Added GL_CHROMIUM_copy_texture
- Added GL_CHROMIUM_consistent_uniform_locations
- Update GL_CHROMIUM_consistent_uniform_locations to have a program argument.
EGL/eglplatform.h
- Added EGLNative*Type for Mac.

0 comments on commit 991564b

Please sign in to comment.