Skip to content

Commit f3b4e6c

Browse files
ShabbyXCommit Bot
authored andcommitted
Vulkan: No inactive samplers left to cleanup in glslang wrapper
Loose inactive samplers are already removed in RemoveInactiveInterfaceVariables. In-struct samplers are always marked active by CollectVariables if the uniform is active. If the uniform is inactive, it is removed entirely by the translator. Thus no inactive samplers are left for glslang wrapper to clean up. Bug: angleproject:3394 Change-Id: Ic0fef052afa992bd612fd22ffa1e5b9aa72a17bc Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1999488 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org>
1 parent 8dfe472 commit f3b4e6c

2 files changed

Lines changed: 8 additions & 36 deletions

File tree

src/compiler/translator/TranslatorVulkan.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,9 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
730730

731731
// Remove declarations of inactive shader interface variables so glslang wrapper doesn't need to
732732
// replace them. Note: this is done before extracting samplers from structs, as removing such
733-
// inactive samplers is not yet supported.
733+
// inactive samplers is not yet supported. Note also that currently, CollectVariables marks
734+
// every field of an active uniform that's of struct type as active, i.e. no extracted sampler
735+
// is inactive.
734736
if (!RemoveInactiveInterfaceVariables(this, root, getUniforms(), getInterfaceBlocks()))
735737
{
736738
return false;

src/libANGLE/renderer/glslang_wrapper_utils.cpp

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -845,22 +845,14 @@ void AssignUniformBindings(const GlslangSourceOptions &options,
845845
void AssignResourceBinding(gl::ShaderBitSet activeShaders,
846846
const std::string &name,
847847
const std::string &bindingString,
848-
bool eraseLayoutIfInactive,
849848
gl::ShaderMap<IntermediateShaderSource> *shaderSources)
850849
{
851850
for (const gl::ShaderType shaderType : gl::AllShaderTypes())
852851
{
853852
IntermediateShaderSource &shaderSource = (*shaderSources)[shaderType];
854-
if (!shaderSource.empty())
853+
if (activeShaders[shaderType] && !shaderSource.empty())
855854
{
856-
if (activeShaders[shaderType])
857-
{
858-
shaderSource.insertLayoutSpecifier(name, bindingString);
859-
}
860-
else if (eraseLayoutIfInactive)
861-
{
862-
shaderSource.eraseLayoutAndQualifierSpecifiers(name, kInactiveVariableSubstitution);
863-
}
855+
shaderSource.insertLayoutSpecifier(name, bindingString);
864856
}
865857
}
866858
}
@@ -881,8 +873,7 @@ uint32_t AssignInterfaceBlockBindings(const GlslangSourceOptions &options,
881873
const std::string bindingString =
882874
resourcesDescriptorSet + ", binding = " + Str(bindingIndex++);
883875

884-
AssignResourceBinding(block.activeShaders(), block.name, bindingString, false,
885-
shaderSources);
876+
AssignResourceBinding(block.activeShaders(), block.name, bindingString, shaderSources);
886877
}
887878
}
888879

@@ -938,8 +929,7 @@ uint32_t AssignImageBindings(const GlslangSourceOptions &options,
938929
name = name.substr(0, name.find('['));
939930
}
940931

941-
AssignResourceBinding(imageUniform.activeShaders(), name, bindingString, false,
942-
shaderSources);
932+
AssignResourceBinding(imageUniform.activeShaders(), name, bindingString, shaderSources);
943933
}
944934

945935
return bindingIndex;
@@ -998,7 +988,7 @@ void AssignTextureBindings(const GlslangSourceOptions &options,
998988
? GetMappedSamplerNameOld(samplerUniform.name)
999989
: GlslangGetMappedSamplerName(samplerUniform.name);
1000990

1001-
AssignResourceBinding(samplerUniform.activeShaders(), samplerName, bindingString, true,
991+
AssignResourceBinding(samplerUniform.activeShaders(), samplerName, bindingString,
1002992
shaderSources);
1003993
}
1004994
}
@@ -1037,26 +1027,6 @@ void CleanupUnusedEntities(bool useOldRewriteStructSamplers,
10371027
shaderSource.eraseLayoutAndQualifierSpecifiers(varyingName, "");
10381028
}
10391029
}
1040-
1041-
// Comment out inactive samplers. This relies on the fact that the shader compiler outputs
1042-
// uniforms to a single line.
1043-
for (const gl::UnusedUniform &unusedUniform : resources.unusedUniforms)
1044-
{
1045-
if (!unusedUniform.isSampler)
1046-
{
1047-
continue;
1048-
}
1049-
1050-
std::string uniformName = useOldRewriteStructSamplers
1051-
? GetMappedSamplerNameOld(unusedUniform.name)
1052-
: GlslangGetMappedSamplerName(unusedUniform.name);
1053-
1054-
for (IntermediateShaderSource &shaderSource : *shaderSources)
1055-
{
1056-
shaderSource.eraseLayoutAndQualifierSpecifiers(uniformName,
1057-
kInactiveVariableSubstitution);
1058-
}
1059-
}
10601030
}
10611031

10621032
constexpr gl::ShaderMap<EShLanguage> kShLanguageMap = {

0 commit comments

Comments
 (0)