Refactor VaryingPacking
Move all packing related logic from Program to VaryingPacking.
Do linkValidateTransformFeedback check first to simplify packing.
Make getPackedVaryings collect all required VaryingPackings in one
run pass, so that packUserVaryings can focus on packing merely.
BUG=angleproject:2241
Change-Id: I5e84e6e4688c80ba7a77a265b8ffaf79f4ddbb87
Reviewed-on: https://chromium-review.googlesource.com/764821
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Geoff Lang <geofflang@chromium.org>
diff --git a/src/libANGLE/Program.cpp b/src/libANGLE/Program.cpp
index 3b0d842..ce40175 100644
--- a/src/libANGLE/Program.cpp
+++ b/src/libANGLE/Program.cpp
@@ -106,37 +106,6 @@
}
}
-// true if varying x has a higher priority in packing than y
-bool ComparePackedVarying(const PackedVarying &x, const PackedVarying &y)
-{
- // If the PackedVarying 'x' or 'y' to be compared is an array element, this clones an equivalent
- // non-array shader variable 'vx' or 'vy' for actual comparison instead.
- sh::ShaderVariable vx, vy;
- const sh::ShaderVariable *px, *py;
- if (x.isArrayElement())
- {
- vx = *x.varying;
- vx.arraySize = 0;
- px = &vx;
- }
- else
- {
- px = x.varying;
- }
-
- if (y.isArrayElement())
- {
- vy = *y.varying;
- vy.arraySize = 0;
- py = &vy;
- }
- else
- {
- py = y.varying;
- }
-
- return gl::CompareShaderVar(*px, *py);
-}
template <typename VarT>
GLuint GetResourceIndexFromName(const std::vector<VarT> &list, const std::string &name)
@@ -888,9 +857,6 @@
linkOutputVariables(context);
- // Validate we can pack the varyings.
- std::vector<PackedVarying> packedVaryings = getPackedVaryings(mergedVaryings);
-
// Map the varyings to the register file
// In WebGL, we use a slightly different handling for packing variables.
auto packMode = data.getExtensions().webglCompatibility ? PackMode::WEBGL_STRICT
@@ -904,13 +870,13 @@
InitUniformBlockLinker(context, mState, &resources.uniformBlockLinker);
InitShaderStorageBlockLinker(context, mState, &resources.shaderStorageBlockLinker);
- if (!resources.varyingPacking.packUserVaryings(mInfoLog, packedVaryings,
- mState.getTransformFeedbackVaryingNames()))
+ if (!linkValidateTransformFeedback(context, mInfoLog, mergedVaryings, caps))
{
return NoError();
}
- if (!linkValidateTransformFeedback(context, mInfoLog, mergedVaryings, caps))
+ if (!resources.varyingPacking.collectAndPackUserVaryings(
+ mInfoLog, mergedVaryings, mState.getTransformFeedbackVaryingNames()))
{
return NoError();
}
@@ -2643,9 +2609,12 @@
infoLog << "Capture of array elements is undefined and not supported.";
return false;
}
- // All transform feedback varyings are expected to exist since packUserVaryings checks for
- // them.
- ASSERT(found);
+ if (!found)
+ {
+ infoLog << "Transform feedback varying " << tfVaryingName
+ << " does not exist in the vertex shader.";
+ return false;
+ }
}
if (mState.mTransformFeedbackBufferMode == GL_INTERLEAVED_ATTRIBS &&
@@ -2733,85 +2702,6 @@
return merged;
}
-std::vector<PackedVarying> Program::getPackedVaryings(
- const Program::MergedVaryings &mergedVaryings) const
-{
- const std::vector<std::string> &tfVaryings = mState.getTransformFeedbackVaryingNames();
- std::vector<PackedVarying> packedVaryings;
- std::set<std::string> uniqueFullNames;
-
- for (const auto &ref : mergedVaryings)
- {
- const sh::Varying *input = ref.second.vertex;
- const sh::Varying *output = ref.second.fragment;
-
- // Only pack varyings that have a matched input or output, plus special builtins.
- if ((input && output) || (output && output->isBuiltIn()))
- {
- // Will get the vertex shader interpolation by default.
- auto interpolation = ref.second.get()->interpolation;
-
- // Note that we lose the vertex shader static use information here. The data for the
- // variable is taken from the fragment shader.
- if (output->isStruct())
- {
- ASSERT(!output->isArray());
- for (const auto &field : output->fields)
- {
- ASSERT(!field.isStruct() && !field.isArray());
- packedVaryings.push_back(PackedVarying(field, interpolation, output->name));
- }
- }
- else
- {
- packedVaryings.push_back(PackedVarying(*output, interpolation));
- }
- continue;
- }
-
- // Keep Transform FB varyings in the merged list always.
- if (!input)
- {
- continue;
- }
-
- for (const std::string &tfVarying : tfVaryings)
- {
- std::vector<unsigned int> subscripts;
- std::string baseName = ParseResourceName(tfVarying, &subscripts);
- size_t subscript = GL_INVALID_INDEX;
- if (!subscripts.empty())
- {
- subscript = subscripts.back();
- }
- if (uniqueFullNames.count(tfVarying) > 0)
- {
- continue;
- }
- if (baseName == input->name)
- {
- // Transform feedback for varying structs is underspecified.
- // See Khronos bug 9856.
- // TODO(jmadill): Figure out how to be spec-compliant here.
- if (!input->isStruct())
- {
- packedVaryings.push_back(PackedVarying(*input, input->interpolation));
- packedVaryings.back().vertexOnly = true;
- packedVaryings.back().arrayIndex = static_cast<GLuint>(subscript);
- uniqueFullNames.insert(tfVarying);
- }
- if (subscript == GL_INVALID_INDEX)
- {
- break;
- }
- }
- }
- }
-
- std::sort(packedVaryings.begin(), packedVaryings.end(), ComparePackedVarying);
-
- return packedVaryings;
-}
void Program::linkOutputVariables(const Context *context)
{