Skip to content

Commit

Permalink
Clean up some of the shader compilation code.
Browse files Browse the repository at this point in the history
This cL contains some micro optimizations as well as cleaning up some
variables that are left over from old refactors.

One variable in particular that was deleted is the "compiler_options_"
member within the Shader class. This member was used to hold extra
compiler options such as extensions for cache key purposes. However,
at some point this mechanism has been replaced by querying the angle
compiler directly for a string which represents the resources.

BUG=453543
TEST=trybots

Review URL: https://codereview.chromium.org/900543004

Cr-Commit-Position: refs/heads/master@{#314642}
  • Loading branch information
dyen authored and Commit bot committed Feb 4, 2015
1 parent a3fa541 commit 7b0b4ba
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 12 deletions.
7 changes: 5 additions & 2 deletions gpu/command_buffer/service/gles2_cmd_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7051,14 +7051,17 @@ void GLES2DecoderImpl::DoShaderSource(
GLuint client_id, GLsizei count, const char** data, const GLint* length) {
std::string str;
for (GLsizei ii = 0; ii < count; ++ii) {
str.append(data[ii]);
if (length && length[ii] > 0)
str.append(data[ii], length[ii]);
else
str.append(data[ii]);
}
Shader* shader = GetShaderInfoNotProgram(client_id, "glShaderSource");
if (!shader) {
return;
}
// Note: We don't actually call glShaderSource here. We wait until
// the call to glCompileShader.
// we actually compile the shader.
shader->set_source(str);
}

Expand Down
17 changes: 9 additions & 8 deletions gpu/command_buffer/service/shader_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,14 @@ void Shader::DoCompile(ShaderTranslatorInterface* translator,
glGetShaderiv(service_id_,
GL_TRANSLATED_SHADER_SOURCE_LENGTH_ANGLE,
&max_len);
scoped_ptr<char[]> buffer(new char[max_len]);
translated_source_.resize(max_len);
GLint len = 0;
glGetTranslatedShaderSourceANGLE(
service_id_, max_len, &len, buffer.get());
service_id_, translated_source_.size(),
&len, &translated_source_.at(0));
DCHECK(max_len == 0 || len < max_len);
DCHECK(len == 0 || buffer[len] == '\0');
translated_source_ = std::string(buffer.get(), len);
DCHECK(len == 0 || translated_source_[len] == '\0');
translated_source_.resize(len);
}

GLint status = GL_FALSE;
Expand All @@ -78,13 +79,13 @@ void Shader::DoCompile(ShaderTranslatorInterface* translator,
// All translated shaders must compile.
GLint max_len = 0;
glGetShaderiv(service_id_, GL_INFO_LOG_LENGTH, &max_len);
scoped_ptr<char[]> buffer(new char[max_len]);
log_info_.resize(max_len);
GLint len = 0;
glGetShaderInfoLog(service_id_, max_len, &len, buffer.get());
glGetShaderInfoLog(service_id_, log_info_.size(), &len, &log_info_.at(0));
DCHECK(max_len == 0 || len < max_len);
DCHECK(len == 0 || buffer[len] == '\0');
DCHECK(len == 0 || log_info_[len] == '\0');
valid_ = false;
log_info_ = std::string(buffer.get(), len);
log_info_.resize(len);
LOG_IF(ERROR, translator)
<< "Shader translator allowed/produced an invalid shader "
<< "unless the driver is buggy:"
Expand Down
1 change: 0 additions & 1 deletion gpu/command_buffer/service/shader_translator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ bool ShaderTranslator::Init(
compiler_ = ShConstructCompiler(
shader_type, shader_spec, shader_output, resources);
}
compiler_options_ = *resources;
implementation_is_glsl_es_ = (glsl_implementation_type == kGlslES);
driver_bug_workarounds_ = driver_bug_workarounds;
return compiler_ != NULL;
Expand Down
1 change: 0 additions & 1 deletion gpu/command_buffer/service/shader_translator.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ class GPU_EXPORT ShaderTranslator
int GetCompileOptions() const;

ShHandle compiler_;
ShBuiltInResources compiler_options_;
bool implementation_is_glsl_es_;
ShCompileOptions driver_bug_workarounds_;
ObserverList<DestructionObserver> destruction_observers_;
Expand Down
29 changes: 29 additions & 0 deletions gpu/command_buffer/tests/gl_program_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,35 @@ TEST_F(GLProgramTest, NewShaderInCurrentProgram) {
GLTestHelper::CheckGLError("no errors", __LINE__);
}

TEST_F(GLProgramTest, ShaderLengthSpecified) {
const std::string valid_shader_str = SHADER(
attribute vec4 a_position;
void main()
{
gl_Position = a_position;
}
);

const std::string invalid_shader = valid_shader_str + "invalid suffix";

// Compiling invalid program should fail.
const char* invalid_shader_strings[] = { invalid_shader.c_str() };
GLuint vs = glCreateShader(GL_VERTEX_SHADER);
glShaderSource(vs, 1, invalid_shader_strings, NULL);
glCompileShader(vs);

GLint compile_state = 0;
glGetShaderiv(vs, GL_COMPILE_STATUS, &compile_state);
EXPECT_EQ(GL_FALSE, compile_state);

// Compiling program cutting off invalid parts should succeed.
const GLint lengths[] = { valid_shader_str.length() };
glShaderSource(vs, 1, invalid_shader_strings, lengths);
glCompileShader(vs);
glGetShaderiv(vs, GL_COMPILE_STATUS, &compile_state);
EXPECT_EQ(GL_TRUE, compile_state);
}

TEST_F(GLProgramTest, UniformsInCurrentProgram) {
static const char* v_shader_str = SHADER(
attribute vec4 a_position;
Expand Down

0 comments on commit 7b0b4ba

Please sign in to comment.