rhi: gl: Eliminate duplicate glUniform calls

This can happen when there is a uniform block in the Vulkan
shader with an instance name, both in the vertex and fragment
shader. The members are mapped to members of a struct uniform
in GLSL since we do not use uniform buffers there.

Now if there is, for example, "ubuf.opacity" both in the vertex and
fragment shader reflection info, then it's still just one single
uniform in the GL program, using one location. Registering it twice
in our 'uniforms' list is harmless, but wasteful, since it means the
uniform value will be set twice upon each setShaderResources().

Change-Id: Ib646eaec333522560d631b3b81800ef610f09319
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
This commit is contained in:
Laszlo Agocs 2020-09-12 22:02:05 +02:00
parent 02d5566d68
commit d7d0f4d5a0
2 changed files with 25 additions and 9 deletions

View File

@ -3447,6 +3447,7 @@ void QRhiGles2::registerUniformIfActive(const QShaderDescription::BlockVariable
int binding,
int baseOffset,
GLuint program,
QSet<int> *activeUniformLocations,
QGles2UniformDescriptionVector *dst)
{
if (var.type == QShaderDescription::Struct) {
@ -3457,8 +3458,14 @@ void QRhiGles2::registerUniformIfActive(const QShaderDescription::BlockVariable
QGles2UniformDescription uniform;
uniform.type = var.type;
const QByteArray name = namePrefix + var.name;
// Here we expect that the OpenGL implementation has proper active uniform
// handling, meaning that a uniform that is declared but not accessed
// elsewhere in the code is reported as -1 when querying the location. If
// that is not the case, it won't break anything, but we'll generate
// unnecessary glUniform* calls then.
uniform.glslLocation = f->glGetUniformLocation(program, name.constData());
if (uniform.glslLocation >= 0) {
if (uniform.glslLocation >= 0 && !activeUniformLocations->contains(uniform.glslLocation)) {
activeUniformLocations->insert(uniform.glslLocation);
if (var.arrayDims.count() > 1) {
qWarning("Array '%s' has more than one dimension. This is not supported.",
var.name.constData());
@ -3474,6 +3481,7 @@ void QRhiGles2::registerUniformIfActive(const QShaderDescription::BlockVariable
void QRhiGles2::gatherUniforms(GLuint program,
const QShaderDescription::UniformBlock &ub,
QSet<int> *activeUniformLocations,
QGles2UniformDescriptionVector *dst)
{
QByteArray prefix = ub.structName + '.';
@ -3485,7 +3493,7 @@ void QRhiGles2::gatherUniforms(GLuint program,
if (blockMember.arrayDims.isEmpty()) {
for (const QShaderDescription::BlockVariable &structMember : blockMember.structMembers)
registerUniformIfActive(structMember, structPrefix + ".", ub.binding,
baseOffset, program, dst);
baseOffset, program, activeUniformLocations, dst);
} else {
if (blockMember.arrayDims.count() > 1) {
qWarning("Array of struct '%s' has more than one dimension. Only the first "
@ -3498,12 +3506,12 @@ void QRhiGles2::gatherUniforms(GLuint program,
for (int di = 0; di < dim; ++di) {
const QByteArray arrayPrefix = structPrefix + '[' + QByteArray::number(di) + ']' + '.';
for (const QShaderDescription::BlockVariable &structMember : blockMember.structMembers)
registerUniformIfActive(structMember, arrayPrefix, ub.binding, elemOffset, program, dst);
registerUniformIfActive(structMember, arrayPrefix, ub.binding, elemOffset, program, activeUniformLocations, dst);
elemOffset += elemSize;
}
}
} else {
registerUniformIfActive(blockMember, prefix, ub.binding, 0, program, dst);
registerUniformIfActive(blockMember, prefix, ub.binding, 0, program, activeUniformLocations, dst);
}
}
}
@ -4328,11 +4336,16 @@ bool QGles2GraphicsPipeline::create()
rhiD->trySaveToDiskCache(program, diskCacheKey);
}
// Use the same work area for the vertex & fragment stages, thus ensuring
// that we will not do superfluous glUniform calls for uniforms that are
// present in both shaders.
QSet<int> activeUniformLocations;
for (const QShaderDescription::UniformBlock &ub : vsDesc.uniformBlocks())
rhiD->gatherUniforms(program, ub, &uniforms);
rhiD->gatherUniforms(program, ub, &activeUniformLocations, &uniforms);
for (const QShaderDescription::UniformBlock &ub : fsDesc.uniformBlocks())
rhiD->gatherUniforms(program, ub, &uniforms);
rhiD->gatherUniforms(program, ub, &activeUniformLocations, &uniforms);
for (const QShaderDescription::InOutVariable &v : vsDesc.combinedImageSamplers())
rhiD->gatherSamplers(program, v, &samplers);
@ -4403,8 +4416,9 @@ bool QGles2ComputePipeline::create()
rhiD->trySaveToDiskCache(program, diskCacheKey);
}
QSet<int> activeUniformLocations;
for (const QShaderDescription::UniformBlock &ub : csDesc.uniformBlocks())
rhiD->gatherUniforms(program, ub, &uniforms);
rhiD->gatherUniforms(program, ub, &activeUniformLocations, &uniforms);
for (const QShaderDescription::InOutVariable &v : csDesc.combinedImageSamplers())
rhiD->gatherSamplers(program, v, &samplers);

View File

@ -813,9 +813,11 @@ public:
bool linkProgram(GLuint program);
void registerUniformIfActive(const QShaderDescription::BlockVariable &var,
const QByteArray &namePrefix, int binding, int baseOffset,
GLuint program, QGles2UniformDescriptionVector *dst);
GLuint program,
QSet<int> *activeUniformLocations,
QGles2UniformDescriptionVector *dst);
void gatherUniforms(GLuint program, const QShaderDescription::UniformBlock &ub,
QGles2UniformDescriptionVector *dst);
QSet<int> *activeUniformLocations, QGles2UniformDescriptionVector *dst);
void gatherSamplers(GLuint program, const QShaderDescription::InOutVariable &v,
QGles2SamplerDescriptionVector *dst);
bool isProgramBinaryDiskCacheEnabled() const;