Commit 0d8993c6 by Nicolas Capens Committed by Nicolas Capens

Fix leaking uniforms.

We were leaking memory for uniforms that were previously defined but don't have a location, e.g. structures. This change also verifies that such uniforms have the same type in both shaders. Also, simplify uniform lookup. Bug chromium:863682 Change-Id: I468aace4df6f5329dc7bb9f33bf9bf533a743ae1 Reviewed-on: https://swiftshader-review.googlesource.com/19928Tested-by: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarCorentin Wallez <cwallez@google.com>
parent 8fb6f6a1
...@@ -15,7 +15,7 @@ ...@@ -15,7 +15,7 @@
#define MAJOR_VERSION 4 #define MAJOR_VERSION 4
#define MINOR_VERSION 1 #define MINOR_VERSION 1
#define BUILD_VERSION 0 #define BUILD_VERSION 0
#define BUILD_REVISION 0 #define BUILD_REVISION 1
#define STRINGIFY(x) #x #define STRINGIFY(x) #x
#define MACRO_STRINGIFY(x) STRINGIFY(x) #define MACRO_STRINGIFY(x) STRINGIFY(x)
......
...@@ -80,7 +80,7 @@ namespace sw ...@@ -80,7 +80,7 @@ namespace sw
TEXTURE_IMAGE_UNITS = 16, TEXTURE_IMAGE_UNITS = 16,
VERTEX_TEXTURE_IMAGE_UNITS = 16, VERTEX_TEXTURE_IMAGE_UNITS = 16,
TOTAL_IMAGE_UNITS = TEXTURE_IMAGE_UNITS + VERTEX_TEXTURE_IMAGE_UNITS, TOTAL_IMAGE_UNITS = TEXTURE_IMAGE_UNITS + VERTEX_TEXTURE_IMAGE_UNITS,
FRAGMENT_UNIFORM_VECTORS = 227, FRAGMENT_UNIFORM_VECTORS = 264,
VERTEX_UNIFORM_VECTORS = 259, VERTEX_UNIFORM_VECTORS = 259,
MAX_VERTEX_INPUTS = 32, MAX_VERTEX_INPUTS = 32,
MAX_VERTEX_OUTPUTS = 34, MAX_VERTEX_OUTPUTS = 34,
......
...@@ -71,14 +71,6 @@ namespace es2 ...@@ -71,14 +71,6 @@ namespace es2
data = new unsigned char[bytes]; data = new unsigned char[bytes];
memset(data, 0, bytes); memset(data, 0, bytes);
} }
else
{
data = nullptr;
}
dirty = true;
psRegisterIndex = -1;
vsRegisterIndex = -1;
} }
Uniform::~Uniform() Uniform::~Uniform()
...@@ -368,24 +360,20 @@ namespace es2 ...@@ -368,24 +360,20 @@ namespace es2
return TEXTURE_2D; return TEXTURE_2D;
} }
bool Program::isUniformDefined(const std::string &name) const Uniform *Program::getUniform(const std::string &name) const
{ {
unsigned int subscript = GL_INVALID_INDEX; unsigned int subscript = GL_INVALID_INDEX;
std::string baseName = es2::ParseUniformName(name, &subscript); std::string baseName = es2::ParseUniformName(name, &subscript);
size_t numUniforms = uniformIndex.size(); for(size_t index = 0; index < uniforms.size(); index++)
for(size_t location = 0; location < numUniforms; location++)
{ {
const unsigned int index = uniformIndex[location].index; if(uniforms[index]->name == baseName)
if((uniformIndex[location].name == baseName) && ((index == GL_INVALID_INDEX) ||
((uniforms[index]->isArray() && uniformIndex[location].element == subscript) ||
(subscript == GL_INVALID_INDEX))))
{ {
return true; return uniforms[index];
} }
} }
return false; return nullptr;
} }
GLint Program::getUniformLocation(const std::string &name) const GLint Program::getUniformLocation(const std::string &name) const
...@@ -393,15 +381,26 @@ namespace es2 ...@@ -393,15 +381,26 @@ namespace es2
unsigned int subscript = GL_INVALID_INDEX; unsigned int subscript = GL_INVALID_INDEX;
std::string baseName = es2::ParseUniformName(name, &subscript); std::string baseName = es2::ParseUniformName(name, &subscript);
size_t numUniforms = uniformIndex.size(); for(size_t location = 0; location < uniformIndex.size(); location++)
for(size_t location = 0; location < numUniforms; location++)
{ {
const unsigned int index = uniformIndex[location].index; if(uniformIndex[location].name == baseName)
if((index != GL_INVALID_INDEX) && (uniformIndex[location].name == baseName) &&
((uniforms[index]->isArray() && uniformIndex[location].element == subscript) ||
(subscript == GL_INVALID_INDEX)))
{ {
return (GLint)location; const unsigned int index = uniformIndex[location].index;
if(index != GL_INVALID_INDEX)
{
if(subscript == GL_INVALID_INDEX)
{
return (GLint)location;
}
else if(uniforms[index]->isArray())
{
if(uniformIndex[location].element == subscript)
{
return (GLint)location;
}
}
}
} }
} }
...@@ -1717,6 +1716,7 @@ namespace es2 ...@@ -1717,6 +1716,7 @@ namespace es2
blockIndex = getUniformBlockIndex(activeUniformBlocks[uniform.blockId].name); blockIndex = getUniformBlockIndex(activeUniformBlocks[uniform.blockId].name);
ASSERT(blockIndex != GL_INVALID_INDEX); ASSERT(blockIndex != GL_INVALID_INDEX);
} }
if(!defineUniform(shader->getType(), uniform, Uniform::BlockInfo(uniform, blockIndex))) if(!defineUniform(shader->getType(), uniform, Uniform::BlockInfo(uniform, blockIndex)))
{ {
return false; return false;
...@@ -1737,7 +1737,7 @@ namespace es2 ...@@ -1737,7 +1737,7 @@ namespace es2
bool Program::defineUniform(GLenum shader, const glsl::Uniform &glslUniform, const Uniform::BlockInfo& blockInfo) bool Program::defineUniform(GLenum shader, const glsl::Uniform &glslUniform, const Uniform::BlockInfo& blockInfo)
{ {
if(IsSamplerUniform(glslUniform.type)) if(IsSamplerUniform(glslUniform.type))
{ {
int index = glslUniform.registerIndex; int index = glslUniform.registerIndex;
do do
...@@ -1819,15 +1819,24 @@ namespace es2 ...@@ -1819,15 +1819,24 @@ namespace es2
index++; index++;
} }
while(index < glslUniform.registerIndex + static_cast<int>(glslUniform.arraySize)); while(index < glslUniform.registerIndex + static_cast<int>(glslUniform.arraySize));
} }
Uniform *uniform = 0; Uniform *uniform = getUniform(glslUniform.name);
GLint location = getUniformLocation(glslUniform.name);
if(location >= 0) // Previously defined, types must match if(!uniform)
{ {
uniform = uniforms[uniformIndex[location].index]; uniform = new Uniform(glslUniform, blockInfo);
uniforms.push_back(uniform);
unsigned int index = (blockInfo.index == -1) ? static_cast<unsigned int>(uniforms.size() - 1) : GL_INVALID_INDEX;
for(int i = 0; i < uniform->size(); i++)
{
uniformIndex.push_back(UniformLocation(glslUniform.name, i, index));
}
}
else // Previously defined, types must match
{
if(uniform->type != glslUniform.type) if(uniform->type != glslUniform.type)
{ {
appendToInfoLog("Types for uniform %s do not match between the vertex and fragment shader", uniform->name.c_str()); appendToInfoLog("Types for uniform %s do not match between the vertex and fragment shader", uniform->name.c_str());
...@@ -1845,15 +1854,6 @@ namespace es2 ...@@ -1845,15 +1854,6 @@ namespace es2
return false; return false;
} }
} }
else
{
uniform = new Uniform(glslUniform, blockInfo);
}
if(!uniform)
{
return false;
}
if(shader == GL_VERTEX_SHADER) if(shader == GL_VERTEX_SHADER)
{ {
...@@ -1865,17 +1865,6 @@ namespace es2 ...@@ -1865,17 +1865,6 @@ namespace es2
} }
else UNREACHABLE(shader); else UNREACHABLE(shader);
if(!isUniformDefined(glslUniform.name))
{
uniforms.push_back(uniform);
unsigned int index = (blockInfo.index == -1) ? static_cast<unsigned int>(uniforms.size() - 1) : GL_INVALID_INDEX;
for(int i = 0; i < uniform->size(); i++)
{
uniformIndex.push_back(UniformLocation(glslUniform.name, i, index));
}
}
if(shader == GL_VERTEX_SHADER) if(shader == GL_VERTEX_SHADER)
{ {
if(glslUniform.registerIndex + uniform->registerCount() > MAX_VERTEX_UNIFORM_VECTORS) if(glslUniform.registerIndex + uniform->registerCount() > MAX_VERTEX_UNIFORM_VECTORS)
......
...@@ -64,11 +64,11 @@ namespace es2 ...@@ -64,11 +64,11 @@ namespace es2
const BlockInfo blockInfo; const BlockInfo blockInfo;
std::vector<glsl::ShaderVariable> fields; std::vector<glsl::ShaderVariable> fields;
unsigned char *data; unsigned char *data = nullptr;
bool dirty; bool dirty = true;
short psRegisterIndex; short psRegisterIndex = -1;
short vsRegisterIndex; short vsRegisterIndex = -1;
}; };
// Helper struct representing a single shader uniform block // Helper struct representing a single shader uniform block
...@@ -145,7 +145,6 @@ namespace es2 ...@@ -145,7 +145,6 @@ namespace es2
GLuint getUniformBlockBinding(GLuint uniformBlockIndex) const; GLuint getUniformBlockBinding(GLuint uniformBlockIndex) const;
void getActiveUniformBlockiv(GLuint uniformBlockIndex, GLenum pname, GLint *params) const; void getActiveUniformBlockiv(GLuint uniformBlockIndex, GLenum pname, GLint *params) const;
bool isUniformDefined(const std::string &name) const;
GLint getUniformLocation(const std::string &name) const; GLint getUniformLocation(const std::string &name) const;
bool setUniform1fv(GLint location, GLsizei count, const GLfloat *v); bool setUniform1fv(GLint location, GLsizei count, const GLfloat *v);
bool setUniform2fv(GLint location, GLsizei count, const GLfloat *v); bool setUniform2fv(GLint location, GLsizei count, const GLfloat *v);
...@@ -231,6 +230,7 @@ namespace es2 ...@@ -231,6 +230,7 @@ namespace es2
bool linkAttributes(); bool linkAttributes();
int getAttributeBinding(const glsl::Attribute &attribute); int getAttributeBinding(const glsl::Attribute &attribute);
Uniform *getUniform(const std::string &name) const;
bool linkUniforms(const Shader *shader); bool linkUniforms(const Shader *shader);
bool linkUniformBlocks(const Shader *vertexShader, const Shader *fragmentShader); bool linkUniformBlocks(const Shader *vertexShader, const Shader *fragmentShader);
bool areMatchingUniformBlocks(const glsl::UniformBlock &block1, const glsl::UniformBlock &block2, const Shader *shader1, const Shader *shader2); bool areMatchingUniformBlocks(const glsl::UniformBlock &block1, const glsl::UniformBlock &block2, const Shader *shader1, const Shader *shader2);
...@@ -275,7 +275,6 @@ namespace es2 ...@@ -275,7 +275,6 @@ namespace es2
static unsigned int issueSerial(); static unsigned int issueSerial();
private:
FragmentShader *fragmentShader; FragmentShader *fragmentShader;
VertexShader *vertexShader; VertexShader *vertexShader;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment