Remove redundant computations on combined interface block counts
This patch intends to remove redundant computations on combined
interface block counts in ValidateGraphicsInterfaceBlocks.
In this patch, we compute and check the number of combined interface
blocks by the result of ValidateInterfaceBlocksCount directly instead
of re-compute it, and if the check on max combined interface blocks
failed, Program::link() can early return without entering
ValidateGraphicsInterfaceBlocks.
BUG=angleproject:2345
Change-Id: I7573f7c645993b4d75230a8471203a305127f2a3
Reviewed-on: https://chromium-review.googlesource.com/1031852
Reviewed-by: Jiajia Qin <jiajia.qin@intel.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
diff --git a/src/libANGLE/Program.cpp b/src/libANGLE/Program.cpp
index 52dddc9..bf1d3e1 100644
--- a/src/libANGLE/Program.cpp
+++ b/src/libANGLE/Program.cpp
@@ -259,14 +259,15 @@
const std::vector<sh::InterfaceBlock> &interfaceBlocks,
ShaderType shaderType,
sh::BlockType blockType,
+ GLuint *combinedInterfaceBlocksCount,
InfoLog &infoLog)
{
GLuint blockCount = 0;
for (const sh::InterfaceBlock &block : interfaceBlocks)
{
- if (block.active || block.layout != sh::BLOCKLAYOUT_PACKED)
+ if (IsActiveInterfaceBlock(block))
{
- blockCount += (block.arraySize ? block.arraySize : 1);
+ blockCount += std::max(block.arraySize, 1u);
if (blockCount > maxInterfaceBlocks)
{
LogInterfaceBlocksExceedLimit(infoLog, shaderType, blockType, maxInterfaceBlocks);
@@ -274,6 +275,18 @@
}
}
}
+
+ // [OpenGL ES 3.1] Chapter 7.6.2 Page 105:
+ // If a uniform block is used by multiple shader stages, each such use counts separately
+ // against this combined limit.
+ // [OpenGL ES 3.1] Chapter 7.8 Page 111:
+ // If a shader storage block in a program is referenced by multiple shaders, each such
+ // reference counts separately against this combined limit.
+ if (combinedInterfaceBlocksCount)
+ {
+ *combinedInterfaceBlocksCount += blockCount;
+ }
+
return true;
}
@@ -496,18 +509,13 @@
void InitializeInterfaceBlockMap(const std::vector<sh::InterfaceBlock> &interfaceBlocks,
ShaderType shaderType,
- InterfaceBlockMap *linkedInterfaceBlocks,
- GLuint *blockCount)
+ InterfaceBlockMap *linkedInterfaceBlocks)
{
- ASSERT(linkedInterfaceBlocks && blockCount);
+ ASSERT(linkedInterfaceBlocks);
for (const sh::InterfaceBlock &interfaceBlock : interfaceBlocks)
{
(*linkedInterfaceBlocks)[interfaceBlock.name] = std::make_pair(shaderType, &interfaceBlock);
- if (IsActiveInterfaceBlock(interfaceBlock))
- {
- *blockCount += std::max(interfaceBlock.arraySize, 1u);
- }
}
}
@@ -516,10 +524,9 @@
ShaderType shaderType,
bool webglCompatibility,
InterfaceBlockMap *linkedBlocks,
- GLuint *combinedInterfaceBlockCount,
InfoLog &infoLog)
{
- ASSERT(linkedBlocks && combinedInterfaceBlockCount);
+ ASSERT(linkedBlocks);
for (const sh::InterfaceBlock &block : interfaceBlocksToLink)
{
@@ -542,17 +549,6 @@
{
(*linkedBlocks)[block.name] = std::make_pair(shaderType, &block);
}
-
- // [OpenGL ES 3.1] Chapter 7.6.2 Page 105:
- // If a uniform block is used by multiple shader stages, each such use counts separately
- // against this combined limit.
- // [OpenGL ES 3.1] Chapter 7.8 Page 111:
- // If a shader storage block in a program is referenced by multiple shaders, each such
- // reference counts separately against this combined limit.
- if (IsActiveInterfaceBlock(block))
- {
- *combinedInterfaceBlockCount += std::max(block.arraySize, 1u);
- }
}
return true;
@@ -561,14 +557,11 @@
bool ValidateGraphicsInterfaceBlocks(
const ShaderMap<const std::vector<sh::InterfaceBlock> *> &shaderInterfaceBlocks,
InfoLog &infoLog,
- bool webglCompatibility,
- sh::BlockType blockType,
- GLuint maxCombinedInterfaceBlocks)
+ bool webglCompatibility)
{
// Check that interface blocks defined in the graphics shaders are identical
InterfaceBlockMap linkedInterfaceBlocks;
- GLuint blockCount = 0u;
bool interfaceBlockMapInitialized = false;
for (ShaderType shaderType : kAllGraphicsShaderTypes)
@@ -581,36 +574,17 @@
if (!interfaceBlockMapInitialized)
{
InitializeInterfaceBlockMap(*shaderInterfaceBlocks[shaderType], shaderType,
- &linkedInterfaceBlocks, &blockCount);
+ &linkedInterfaceBlocks);
interfaceBlockMapInitialized = true;
}
- else if (!ValidateGraphicsInterfaceBlocksPerShader(
- *shaderInterfaceBlocks[shaderType], shaderType, webglCompatibility,
- &linkedInterfaceBlocks, &blockCount, infoLog))
+ else if (!ValidateGraphicsInterfaceBlocksPerShader(*shaderInterfaceBlocks[shaderType],
+ shaderType, webglCompatibility,
+ &linkedInterfaceBlocks, infoLog))
{
return false;
}
}
- if (blockCount > maxCombinedInterfaceBlocks)
- {
- switch (blockType)
- {
- case sh::BlockType::BLOCK_UNIFORM:
- infoLog << "The sum of the number of active uniform blocks exceeds "
- "MAX_COMBINED_UNIFORM_BLOCKS ("
- << maxCombinedInterfaceBlocks << ").";
- break;
- case sh::BlockType::BLOCK_BUFFER:
- infoLog << "The sum of the number of active shader storage blocks exceeds "
- "MAX_COMBINED_SHADER_STORAGE_BLOCKS ("
- << maxCombinedInterfaceBlocks << ").";
- break;
- default:
- UNREACHABLE();
- }
- return false;
- }
return true;
}
@@ -2816,7 +2790,7 @@
if (!ValidateInterfaceBlocksCount(caps.maxComputeUniformBlocks, computeUniformBlocks,
ShaderType::Compute, sh::BlockType::BLOCK_UNIFORM,
- infoLog))
+ nullptr, infoLog))
{
return false;
}
@@ -2824,7 +2798,7 @@
const auto &computeShaderStorageBlocks = computeShader.getShaderStorageBlocks(context);
if (!ValidateInterfaceBlocksCount(caps.maxComputeShaderStorageBlocks,
computeShaderStorageBlocks, ShaderType::Compute,
- sh::BlockType::BLOCK_BUFFER, infoLog))
+ sh::BlockType::BLOCK_BUFFER, nullptr, infoLog))
{
return false;
}
@@ -2836,6 +2810,7 @@
maxShaderUniformBlocks[gl::ShaderType::Fragment] = caps.maxFragmentUniformBlocks;
maxShaderUniformBlocks[gl::ShaderType::Geometry] = caps.maxGeometryUniformBlocks;
+ GLuint combinedUniformBlocksCount = 0u;
ShaderMap<const std::vector<sh::InterfaceBlock> *> graphicsShaderUniformBlocks = {};
for (ShaderType shaderType : kAllGraphicsShaderTypes)
{
@@ -2847,7 +2822,8 @@
const auto &uniformBlocks = mState.mAttachedShaders[shaderType]->getUniformBlocks(context);
if (!ValidateInterfaceBlocksCount(maxShaderUniformBlocks[shaderType], uniformBlocks,
- shaderType, sh::BlockType::BLOCK_UNIFORM, infoLog))
+ shaderType, sh::BlockType::BLOCK_UNIFORM,
+ &combinedUniformBlocksCount, infoLog))
{
return false;
}
@@ -2855,10 +2831,16 @@
graphicsShaderUniformBlocks[shaderType] = &uniformBlocks;
}
+ if (combinedUniformBlocksCount > caps.maxCombinedUniformBlocks)
+ {
+ infoLog << "The sum of the number of active uniform blocks exceeds "
+ "MAX_COMBINED_UNIFORM_BLOCKS ("
+ << caps.maxCombinedUniformBlocks << ").";
+ return false;
+ }
+
bool webglCompatibility = context->getExtensions().webglCompatibility;
- if (!ValidateGraphicsInterfaceBlocks(graphicsShaderUniformBlocks, infoLog, webglCompatibility,
- sh::BlockType::BLOCK_UNIFORM,
- caps.maxCombinedUniformBlocks))
+ if (!ValidateGraphicsInterfaceBlocks(graphicsShaderUniformBlocks, infoLog, webglCompatibility))
{
return false;
}
@@ -2870,6 +2852,7 @@
maxShaderStorageBlocks[ShaderType::Fragment] = caps.maxFragmentShaderStorageBlocks;
maxShaderStorageBlocks[ShaderType::Geometry] = caps.maxGeometryShaderStorageBlocks;
+ GLuint combinedShaderStorageBlocksCount = 0u;
ShaderMap<const std::vector<sh::InterfaceBlock> *> graphicsShaderStorageBlocks = {};
for (ShaderType shaderType : kAllGraphicsShaderTypes)
{
@@ -2880,9 +2863,9 @@
}
const auto &shaderStorageBlocks = shader->getShaderStorageBlocks(context);
- if (!ValidateInterfaceBlocksCount(maxShaderStorageBlocks[shaderType],
- shaderStorageBlocks, shaderType,
- sh::BlockType::BLOCK_BUFFER, infoLog))
+ if (!ValidateInterfaceBlocksCount(
+ maxShaderStorageBlocks[shaderType], shaderStorageBlocks, shaderType,
+ sh::BlockType::BLOCK_BUFFER, &combinedShaderStorageBlocksCount, infoLog))
{
return false;
}
@@ -2890,9 +2873,16 @@
graphicsShaderStorageBlocks[shaderType] = &shaderStorageBlocks;
}
+ if (combinedShaderStorageBlocksCount > caps.maxCombinedShaderStorageBlocks)
+ {
+ infoLog << "The sum of the number of active shader storage blocks exceeds "
+ "MAX_COMBINED_SHADER_STORAGE_BLOCKS ("
+ << caps.maxCombinedShaderStorageBlocks << ").";
+ return false;
+ }
+
if (!ValidateGraphicsInterfaceBlocks(graphicsShaderStorageBlocks, infoLog,
- webglCompatibility, sh::BlockType::BLOCK_BUFFER,
- caps.maxCombinedShaderStorageBlocks))
+ webglCompatibility))
{
return false;
}