Cache transform feedback varying names in the executable

Currently, ANGLE actually does a full link of the programs inside PPOs.
This was never the intention of the spec (hence why an explicit link
doesn't exist).  During this link operation, the transform feedback
varying names are used, and they are retrieved from the program itself.

This is not correct, because the transform feedback varyings may have
changed, the program may have failed to relink, and the program pipeline
is expected to continue functioning using the "installed" executable.

Bug: angleproject:5486
Bug: angleproject:8297
Change-Id: I583dbd2abcc51e8536b4c460b92211bdddebda16
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4834055
Reviewed-by: Charlie Lao <cclao@google.com>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
This commit is contained in:
Shahbaz Youssefi
2023-08-31 17:19:44 -04:00
committed by Angle LUCI CQ
parent 179bd7762f
commit ebf1e71632
5 changed files with 88 additions and 60 deletions

View File

@@ -1173,11 +1173,17 @@ angle::Result Program::linkImpl(const Context *context)
auto *platform = ANGLEPlatformCurrent();
double startTime = platform->currentTime(platform);
// Make sure the executable state is in sync with the program. Only the transform feedback
// buffer mode is duplicated in the executable as is the only link-input that is also needed at
// draw time.
// Make sure the executable state is in sync with the program.
//
// The transform feedback buffer mode is duplicated in the executable as is the only link-input
// that is also needed at draw time.
//
// The transform feedback varying names are duplicated because the program pipeline link is not
// currently able to use the link result of the program directly (and redoes the link, using
// these names).
mState.mExecutable->mPODStruct.transformFeedbackBufferMode =
mState.mTransformFeedbackBufferMode;
mState.mExecutable->mTransformFeedbackVaryingNames = mState.mTransformFeedbackVaryingNames;
// Unlink the program, but do not clear the validation-related caching yet, since we can still
// use the previously linked program if linking the shaders fails.
@@ -1335,10 +1341,9 @@ angle::Result Program::linkImpl(const Context *context)
}
mergedVaryings = GetMergedVaryingsFromLinkingVariables(linkingVariables);
if (!mState.mExecutable->linkMergedVaryings(
caps, limitations, clientVersion, isWebGL, mergedVaryings,
mState.mTransformFeedbackVaryingNames, linkingVariables, isSeparable(),
&resources.varyingPacking))
if (!mState.mExecutable->linkMergedVaryings(caps, limitations, clientVersion, isWebGL,
mergedVaryings, linkingVariables, isSeparable(),
&resources.varyingPacking))
{
return angle::Result::Continue;
}
@@ -3573,6 +3578,12 @@ angle::Result Program::serialize(const Context *context, angle::MemoryBuffer *bi
stream.writeBool(mState.mSeparable);
stream.writeInt(mState.mTransformFeedbackBufferMode);
stream.writeInt(mState.mTransformFeedbackVaryingNames.size());
for (const std::string &name : mState.mTransformFeedbackVaryingNames)
{
stream.writeString(name);
}
mState.mExecutable->save(mState.mSeparable, &stream);
// Warn the app layer if saving a binary with unsupported transform feedback.
@@ -3670,6 +3681,12 @@ angle::Result Program::deserialize(const Context *context, BinaryInputStream &st
mState.mSeparable = stream.readBool();
mState.mTransformFeedbackBufferMode = stream.readInt<GLenum>();
mState.mTransformFeedbackVaryingNames.resize(stream.readInt<size_t>());
for (std::string &name : mState.mTransformFeedbackVaryingNames)
{
name = stream.readString();
}
mState.mExecutable->load(mState.mSeparable, &stream);
static_assert(static_cast<unsigned long>(ShaderType::EnumCount) <= sizeof(unsigned long) * 8,
@@ -3686,6 +3703,7 @@ angle::Result Program::deserialize(const Context *context, BinaryInputStream &st
if (!mState.mAttachedShaders[ShaderType::Compute])
{
mState.mExecutable->updateTransformFeedbackStrides();
mState.mExecutable->mTransformFeedbackVaryingNames = mState.mTransformFeedbackVaryingNames;
}
if (context->getShareGroup()->getFrameCaptureShared()->enabled())

View File

@@ -882,21 +882,18 @@ void ProgramExecutable::saveLinkedStateInfo(const ProgramState &state)
}
}
bool ProgramExecutable::linkMergedVaryings(
const Caps &caps,
const Limitations &limitations,
const Version &clientVersion,
bool webglCompatibility,
const ProgramMergedVaryings &mergedVaryings,
const std::vector<std::string> &transformFeedbackVaryingNames,
const LinkingVariables &linkingVariables,
bool isSeparable,
ProgramVaryingPacking *varyingPacking)
bool ProgramExecutable::linkMergedVaryings(const Caps &caps,
const Limitations &limitations,
const Version &clientVersion,
bool webglCompatibility,
const ProgramMergedVaryings &mergedVaryings,
const LinkingVariables &linkingVariables,
bool isSeparable,
ProgramVaryingPacking *varyingPacking)
{
ShaderType tfStage = GetLastPreFragmentStage(linkingVariables.isShaderStageUsedBitset);
if (!linkValidateTransformFeedback(caps, clientVersion, mergedVaryings, tfStage,
transformFeedbackVaryingNames))
if (!linkValidateTransformFeedback(caps, clientVersion, mergedVaryings, tfStage))
{
return false;
}
@@ -930,28 +927,26 @@ bool ProgramExecutable::linkMergedVaryings(
}
if (!varyingPacking->collectAndPackUserVaryings(*mInfoLog, caps, packMode, activeShadersMask,
mergedVaryings, transformFeedbackVaryingNames,
mergedVaryings, mTransformFeedbackVaryingNames,
isSeparable))
{
return false;
}
gatherTransformFeedbackVaryings(mergedVaryings, tfStage, transformFeedbackVaryingNames);
gatherTransformFeedbackVaryings(mergedVaryings, tfStage);
updateTransformFeedbackStrides();
return true;
}
bool ProgramExecutable::linkValidateTransformFeedback(
const Caps &caps,
const Version &clientVersion,
const ProgramMergedVaryings &varyings,
ShaderType stage,
const std::vector<std::string> &transformFeedbackVaryingNames)
bool ProgramExecutable::linkValidateTransformFeedback(const Caps &caps,
const Version &clientVersion,
const ProgramMergedVaryings &varyings,
ShaderType stage)
{
// Validate the tf names regardless of the actual program varyings.
std::set<std::string> uniqueNames;
for (const std::string &tfVaryingName : transformFeedbackVaryingNames)
for (const std::string &tfVaryingName : mTransformFeedbackVaryingNames)
{
if (clientVersion < Version(3, 1) && tfVaryingName.find('[') != std::string::npos)
{
@@ -982,7 +977,7 @@ bool ProgramExecutable::linkValidateTransformFeedback(
// From OpneGLES spec. 11.1.2.1: A program will fail to link if:
// the count specified by TransformFeedbackVaryings is non-zero, but the
// program object has no vertex, tessellation evaluation, or geometry shader
if (transformFeedbackVaryingNames.size() > 0 &&
if (mTransformFeedbackVaryingNames.size() > 0 &&
!gl::ShaderTypeSupportsTransformFeedback(getLinkedTransformFeedbackStage()))
{
*mInfoLog << "Linked transform feedback stage " << getLinkedTransformFeedbackStage()
@@ -992,7 +987,7 @@ bool ProgramExecutable::linkValidateTransformFeedback(
// Validate against program varyings.
size_t totalComponents = 0;
for (const std::string &tfVaryingName : transformFeedbackVaryingNames)
for (const std::string &tfVaryingName : mTransformFeedbackVaryingNames)
{
std::vector<unsigned int> subscripts;
std::string baseName = ParseResourceName(tfVaryingName, &subscripts);
@@ -1068,14 +1063,12 @@ bool ProgramExecutable::linkValidateTransformFeedback(
return true;
}
void ProgramExecutable::gatherTransformFeedbackVaryings(
const ProgramMergedVaryings &varyings,
ShaderType stage,
const std::vector<std::string> &transformFeedbackVaryingNames)
void ProgramExecutable::gatherTransformFeedbackVaryings(const ProgramMergedVaryings &varyings,
ShaderType stage)
{
// Gather the linked varyings that are used for transform feedback, they should all exist.
mLinkedTransformFeedbackVaryings.clear();
for (const std::string &tfVaryingName : transformFeedbackVaryingNames)
for (const std::string &tfVaryingName : mTransformFeedbackVaryingNames)
{
std::vector<unsigned int> subscripts;
std::string baseName = ParseResourceName(tfVaryingName, &subscripts);

View File

@@ -529,22 +529,16 @@ class ProgramExecutable final : public angle::Subject
const Version &clientVersion,
bool webglCompatibility,
const ProgramMergedVaryings &mergedVaryings,
const std::vector<std::string> &transformFeedbackVaryingNames,
const LinkingVariables &linkingVariables,
bool isSeparable,
ProgramVaryingPacking *varyingPacking);
bool linkValidateTransformFeedback(
const Caps &caps,
const Version &clientVersion,
const ProgramMergedVaryings &varyings,
ShaderType stage,
const std::vector<std::string> &transformFeedbackVaryingNames);
bool linkValidateTransformFeedback(const Caps &caps,
const Version &clientVersion,
const ProgramMergedVaryings &varyings,
ShaderType stage);
void gatherTransformFeedbackVaryings(
const ProgramMergedVaryings &varyings,
ShaderType stage,
const std::vector<std::string> &transformFeedbackVaryingNames);
void gatherTransformFeedbackVaryings(const ProgramMergedVaryings &varyings, ShaderType stage);
void updateTransformFeedbackStrides();
@@ -661,6 +655,15 @@ class ProgramExecutable final : public angle::Subject
// Vertex attributes, Fragment input varyings, etc.
std::vector<ProgramInput> mProgramInputs;
std::vector<TransformFeedbackVarying> mLinkedTransformFeedbackVaryings;
// Duplicate of ProgramState::mTransformFeedbackVaryingNames. This is cached here because the
// xfb names may change, relink may fail, yet program pipeline link should be able to function
// with the last installed executable. In truth, program pipeline link should have been able to
// hoist transform feedback varyings directly from the executable, among most other things, but
// that is currently not done.
//
// This array is not serialized, it's already done by the program, and will be duplicated during
// deserialization.
std::vector<std::string> mTransformFeedbackVaryingNames;
// The size of the data written to each transform feedback buffer per vertex.
std::vector<GLsizei> mTransformFeedbackStrides;
// Uniforms are sorted in order:

View File

@@ -540,12 +540,12 @@ angle::Result ProgramPipeline::link(const Context *context)
tfProgram = mState.mPrograms[ShaderType::Vertex];
}
const std::vector<std::string> &transformFeedbackVaryingNames =
tfProgram->getState().getTransformFeedbackVaryingNames();
mState.mExecutable->mTransformFeedbackVaryingNames =
tfProgram->getExecutable().mTransformFeedbackVaryingNames;
if (!mState.mExecutable->linkMergedVaryings(caps, limitations, clientVersion, isWebGL,
mergedVaryings, transformFeedbackVaryingNames,
linkingVariables, false, &varyingPacking))
mergedVaryings, linkingVariables, false,
&varyingPacking))
{
return angle::Result::Stop;
}

View File

@@ -151,8 +151,16 @@ void ProgramPipelineXFBTest31::bindProgramPipelineWithXFBVaryings(
const std::vector<std::string> &tfVaryings,
GLenum bufferMode)
{
mVertProg = glCreateShaderProgramv(GL_VERTEX_SHADER, 1, &vertString);
ASSERT_NE(mVertProg, 0u);
GLShader vertShader(GL_VERTEX_SHADER);
mVertProg = glCreateProgram();
glShaderSource(vertShader, 1, &vertString, nullptr);
glCompileShader(vertShader);
glProgramParameteri(mVertProg, GL_PROGRAM_SEPARABLE, GL_TRUE);
glAttachShader(mVertProg, vertShader);
glLinkProgram(mVertProg);
EXPECT_GL_NO_ERROR();
mFragProg = glCreateShaderProgramv(GL_FRAGMENT_SHADER, 1, &fragString);
ASSERT_NE(mFragProg, 0u);
@@ -1245,8 +1253,6 @@ TEST_P(ProgramPipelineXFBTest31, VaryingIOBlockSeparableProgramWithXFB)
// Only the Vulkan backend supports PPOs
ANGLE_SKIP_TEST_IF(!IsVulkan());
ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_shader_io_blocks"));
// http://anglebug.com/5486
ANGLE_SKIP_TEST_IF(IsVulkan());
constexpr char kVS[] =
R"(#version 310 es
@@ -1254,12 +1260,13 @@ TEST_P(ProgramPipelineXFBTest31, VaryingIOBlockSeparableProgramWithXFB)
precision highp float;
in vec4 inputAttribute;
out Block_inout { vec4 value; } user_out;
out Block_inout { vec4 value; vec4 value2; } user_out;
void main()
{
gl_Position = inputAttribute;
user_out.value = vec4(4.0, 5.0, 6.0, 7.0);
user_out.value2 = vec4(8.0, 9.0, 10.0, 11.0);
})";
constexpr char kFS[] =
@@ -1268,7 +1275,7 @@ TEST_P(ProgramPipelineXFBTest31, VaryingIOBlockSeparableProgramWithXFB)
precision highp float;
layout(location = 0) out mediump vec4 color;
in Block_inout { vec4 value; } user_in;
in Block_inout { vec4 value; vec4 value2; } user_in;
void main()
{
@@ -1276,9 +1283,16 @@ TEST_P(ProgramPipelineXFBTest31, VaryingIOBlockSeparableProgramWithXFB)
})";
std::vector<std::string> tfVaryings;
tfVaryings.push_back("Block_inout.value");
tfVaryings.push_back("Block_inout.value2");
bindProgramPipelineWithXFBVaryings(kVS, kFS, tfVaryings, GL_INTERLEAVED_ATTRIBS);
glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, mTransformFeedbackBuffer);
// Make sure reconfiguring the vertex shader's transform feedback varyings without a link does
// not affect the pipeline. Same with changing buffer modes
std::vector<const char *> tfVaryingsBogus = {"some", "invalid[0]", "names"};
glTransformFeedbackVaryings(mVertProg, static_cast<GLsizei>(tfVaryingsBogus.size()),
tfVaryingsBogus.data(), GL_SEPARATE_ATTRIBS);
glBeginTransformFeedback(GL_TRIANGLES);
drawQuadWithPPO("inputAttribute", 0.5f, 1.0f);
glEndTransformFeedback();
@@ -1287,11 +1301,11 @@ TEST_P(ProgramPipelineXFBTest31, VaryingIOBlockSeparableProgramWithXFB)
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
void *mappedBuffer =
glMapBufferRange(GL_TRANSFORM_FEEDBACK_BUFFER, 0, sizeof(float) * 4, GL_MAP_READ_BIT);
glMapBufferRange(GL_TRANSFORM_FEEDBACK_BUFFER, 0, sizeof(float) * 8, GL_MAP_READ_BIT);
ASSERT_NE(nullptr, mappedBuffer);
float *mappedFloats = static_cast<float *>(mappedBuffer);
for (unsigned int cnt = 0; cnt < 4; ++cnt)
for (unsigned int cnt = 0; cnt < 8; ++cnt)
{
EXPECT_EQ(4 + cnt, mappedFloats[cnt]);
}
@@ -1480,9 +1494,9 @@ void main()
my_FragColor = texture(tex, texCoord);
})";
std::array<GLColor, kWidth *kHeight> redColor = {
std::array<GLColor, kWidth * kHeight> redColor = {
{GLColor::red, GLColor::red, GLColor::red, GLColor::red}};
std::array<GLColor, kWidth *kHeight> greenColor = {
std::array<GLColor, kWidth * kHeight> greenColor = {
{GLColor::green, GLColor::green, GLColor::green, GLColor::green}};
// Create a red texture and bind to texture unit 0