summaryrefslogtreecommitdiffstats
path: root/src/gui
diff options
context:
space:
mode:
authorLaszlo Agocs <laszlo.agocs@qt.io>2020-09-12 22:02:05 +0200
committerLaszlo Agocs <laszlo.agocs@qt.io>2020-09-14 20:41:42 +0200
commitd7d0f4d5a09d23488d4905bd5ae6b3e0f04d17a9 (patch)
tree684cded7f3fdcffd46b8ba1a0d42b78b7fff6462 /src/gui
parent02d5566d681697844206c6f035d8fecc9c31e748 (diff)
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>
Diffstat (limited to 'src/gui')
-rw-r--r--src/gui/rhi/qrhigles2.cpp28
-rw-r--r--src/gui/rhi/qrhigles2_p_p.h6
2 files changed, 25 insertions, 9 deletions
diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp
index a06cec2b38..72a8cc539b 100644
--- a/src/gui/rhi/qrhigles2.cpp
+++ b/src/gui/rhi/qrhigles2.cpp
@@ -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);
diff --git a/src/gui/rhi/qrhigles2_p_p.h b/src/gui/rhi/qrhigles2_p_p.h
index 93c8f82f8d..9392254a78 100644
--- a/src/gui/rhi/qrhigles2_p_p.h
+++ b/src/gui/rhi/qrhigles2_p_p.h
@@ -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;