diff options
author | Laszlo Agocs <laszlo.agocs@qt.io> | 2020-09-12 22:02:05 +0200 |
---|---|---|
committer | Laszlo Agocs <laszlo.agocs@qt.io> | 2020-09-14 20:41:42 +0200 |
commit | d7d0f4d5a09d23488d4905bd5ae6b3e0f04d17a9 (patch) | |
tree | 684cded7f3fdcffd46b8ba1a0d42b78b7fff6462 | |
parent | 02d5566d681697844206c6f035d8fecc9c31e748 (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>
-rw-r--r-- | src/gui/rhi/qrhigles2.cpp | 28 | ||||
-rw-r--r-- | src/gui/rhi/qrhigles2_p_p.h | 6 |
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; |