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/19928
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Corentin Wallez <cwallez@google.com>
diff --git a/src/Common/Version.h b/src/Common/Version.h
index 4dbc92f..eae755d 100644
--- a/src/Common/Version.h
+++ b/src/Common/Version.h
@@ -15,7 +15,7 @@
#define MAJOR_VERSION 4
#define MINOR_VERSION 1
#define BUILD_VERSION 0
-#define BUILD_REVISION 0
+#define BUILD_REVISION 1
#define STRINGIFY(x) #x
#define MACRO_STRINGIFY(x) STRINGIFY(x)
diff --git a/src/Main/Config.hpp b/src/Main/Config.hpp
index 764bfed..017e38b 100644
--- a/src/Main/Config.hpp
+++ b/src/Main/Config.hpp
@@ -80,7 +80,7 @@
TEXTURE_IMAGE_UNITS = 16,
VERTEX_TEXTURE_IMAGE_UNITS = 16,
TOTAL_IMAGE_UNITS = TEXTURE_IMAGE_UNITS + VERTEX_TEXTURE_IMAGE_UNITS,
- FRAGMENT_UNIFORM_VECTORS = 227,
+ FRAGMENT_UNIFORM_VECTORS = 264,
VERTEX_UNIFORM_VECTORS = 259,
MAX_VERTEX_INPUTS = 32,
MAX_VERTEX_OUTPUTS = 34,
diff --git a/src/OpenGL/libGLESv2/Program.cpp b/src/OpenGL/libGLESv2/Program.cpp
index d2cff33..9b10a7f 100644
--- a/src/OpenGL/libGLESv2/Program.cpp
+++ b/src/OpenGL/libGLESv2/Program.cpp
@@ -71,14 +71,6 @@
data = new unsigned char[bytes];
memset(data, 0, bytes);
}
- else
- {
- data = nullptr;
- }
- dirty = true;
-
- psRegisterIndex = -1;
- vsRegisterIndex = -1;
}
Uniform::~Uniform()
@@ -368,24 +360,20 @@
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;
std::string baseName = es2::ParseUniformName(name, &subscript);
- size_t numUniforms = uniformIndex.size();
- for(size_t location = 0; location < numUniforms; location++)
+ for(size_t index = 0; index < uniforms.size(); index++)
{
- const unsigned int index = uniformIndex[location].index;
- if((uniformIndex[location].name == baseName) && ((index == GL_INVALID_INDEX) ||
- ((uniforms[index]->isArray() && uniformIndex[location].element == subscript) ||
- (subscript == GL_INVALID_INDEX))))
+ if(uniforms[index]->name == baseName)
{
- return true;
+ return uniforms[index];
}
}
- return false;
+ return nullptr;
}
GLint Program::getUniformLocation(const std::string &name) const
@@ -393,15 +381,26 @@
unsigned int subscript = GL_INVALID_INDEX;
std::string baseName = es2::ParseUniformName(name, &subscript);
- size_t numUniforms = uniformIndex.size();
- for(size_t location = 0; location < numUniforms; location++)
+ for(size_t location = 0; location < uniformIndex.size(); location++)
{
- const unsigned int index = uniformIndex[location].index;
- if((index != GL_INVALID_INDEX) && (uniformIndex[location].name == baseName) &&
- ((uniforms[index]->isArray() && uniformIndex[location].element == subscript) ||
- (subscript == GL_INVALID_INDEX)))
+ if(uniformIndex[location].name == baseName)
{
- 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 @@
blockIndex = getUniformBlockIndex(activeUniformBlocks[uniform.blockId].name);
ASSERT(blockIndex != GL_INVALID_INDEX);
}
+
if(!defineUniform(shader->getType(), uniform, Uniform::BlockInfo(uniform, blockIndex)))
{
return false;
@@ -1737,7 +1737,7 @@
bool Program::defineUniform(GLenum shader, const glsl::Uniform &glslUniform, const Uniform::BlockInfo& blockInfo)
{
if(IsSamplerUniform(glslUniform.type))
- {
+ {
int index = glslUniform.registerIndex;
do
@@ -1819,15 +1819,24 @@
index++;
}
while(index < glslUniform.registerIndex + static_cast<int>(glslUniform.arraySize));
- }
+ }
- Uniform *uniform = 0;
- GLint location = getUniformLocation(glslUniform.name);
+ Uniform *uniform = getUniform(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)
{
appendToInfoLog("Types for uniform %s do not match between the vertex and fragment shader", uniform->name.c_str());
@@ -1845,15 +1854,6 @@
return false;
}
}
- else
- {
- uniform = new Uniform(glslUniform, blockInfo);
- }
-
- if(!uniform)
- {
- return false;
- }
if(shader == GL_VERTEX_SHADER)
{
@@ -1865,17 +1865,6 @@
}
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(glslUniform.registerIndex + uniform->registerCount() > MAX_VERTEX_UNIFORM_VECTORS)
diff --git a/src/OpenGL/libGLESv2/Program.h b/src/OpenGL/libGLESv2/Program.h
index 4d89d7a..6f9940b 100644
--- a/src/OpenGL/libGLESv2/Program.h
+++ b/src/OpenGL/libGLESv2/Program.h
@@ -64,11 +64,11 @@
const BlockInfo blockInfo;
std::vector<glsl::ShaderVariable> fields;
- unsigned char *data;
- bool dirty;
+ unsigned char *data = nullptr;
+ bool dirty = true;
- short psRegisterIndex;
- short vsRegisterIndex;
+ short psRegisterIndex = -1;
+ short vsRegisterIndex = -1;
};
// Helper struct representing a single shader uniform block
@@ -145,7 +145,6 @@
GLuint getUniformBlockBinding(GLuint uniformBlockIndex) const;
void getActiveUniformBlockiv(GLuint uniformBlockIndex, GLenum pname, GLint *params) const;
- bool isUniformDefined(const std::string &name) const;
GLint getUniformLocation(const std::string &name) const;
bool setUniform1fv(GLint location, GLsizei count, const GLfloat *v);
bool setUniform2fv(GLint location, GLsizei count, const GLfloat *v);
@@ -231,6 +230,7 @@
bool linkAttributes();
int getAttributeBinding(const glsl::Attribute &attribute);
+ Uniform *getUniform(const std::string &name) const;
bool linkUniforms(const Shader *shader);
bool linkUniformBlocks(const Shader *vertexShader, const Shader *fragmentShader);
bool areMatchingUniformBlocks(const glsl::UniformBlock &block1, const glsl::UniformBlock &block2, const Shader *shader1, const Shader *shader2);
@@ -275,7 +275,6 @@
static unsigned int issueSerial();
- private:
FragmentShader *fragmentShader;
VertexShader *vertexShader;
