diff --git a/src/libANGLE/Program.cpp b/src/libANGLE/Program.cpp index 8e775478b..5caab257c 100644 --- a/src/libANGLE/Program.cpp +++ b/src/libANGLE/Program.cpp @@ -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(); + mState.mTransformFeedbackVaryingNames.resize(stream.readInt()); + for (std::string &name : mState.mTransformFeedbackVaryingNames) + { + name = stream.readString(); + } + mState.mExecutable->load(mState.mSeparable, &stream); static_assert(static_cast(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()) diff --git a/src/libANGLE/ProgramExecutable.cpp b/src/libANGLE/ProgramExecutable.cpp index d93ce3d47..2a18d6222 100644 --- a/src/libANGLE/ProgramExecutable.cpp +++ b/src/libANGLE/ProgramExecutable.cpp @@ -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 &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 &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 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 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 &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 subscripts; std::string baseName = ParseResourceName(tfVaryingName, &subscripts); diff --git a/src/libANGLE/ProgramExecutable.h b/src/libANGLE/ProgramExecutable.h index 038f6a772..57402e84c 100644 --- a/src/libANGLE/ProgramExecutable.h +++ b/src/libANGLE/ProgramExecutable.h @@ -529,22 +529,16 @@ class ProgramExecutable final : public angle::Subject const Version &clientVersion, bool webglCompatibility, const ProgramMergedVaryings &mergedVaryings, - const std::vector &transformFeedbackVaryingNames, const LinkingVariables &linkingVariables, bool isSeparable, ProgramVaryingPacking *varyingPacking); - bool linkValidateTransformFeedback( - const Caps &caps, - const Version &clientVersion, - const ProgramMergedVaryings &varyings, - ShaderType stage, - const std::vector &transformFeedbackVaryingNames); + bool linkValidateTransformFeedback(const Caps &caps, + const Version &clientVersion, + const ProgramMergedVaryings &varyings, + ShaderType stage); - void gatherTransformFeedbackVaryings( - const ProgramMergedVaryings &varyings, - ShaderType stage, - const std::vector &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 mProgramInputs; std::vector 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 mTransformFeedbackVaryingNames; // The size of the data written to each transform feedback buffer per vertex. std::vector mTransformFeedbackStrides; // Uniforms are sorted in order: diff --git a/src/libANGLE/ProgramPipeline.cpp b/src/libANGLE/ProgramPipeline.cpp index e74cd99e5..aa43cdcdc 100644 --- a/src/libANGLE/ProgramPipeline.cpp +++ b/src/libANGLE/ProgramPipeline.cpp @@ -540,12 +540,12 @@ angle::Result ProgramPipeline::link(const Context *context) tfProgram = mState.mPrograms[ShaderType::Vertex]; } - const std::vector &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; } diff --git a/src/tests/gl_tests/ProgramPipelineTest.cpp b/src/tests/gl_tests/ProgramPipelineTest.cpp index 78fd46cae..1e90d7fd4 100644 --- a/src/tests/gl_tests/ProgramPipelineTest.cpp +++ b/src/tests/gl_tests/ProgramPipelineTest.cpp @@ -151,8 +151,16 @@ void ProgramPipelineXFBTest31::bindProgramPipelineWithXFBVaryings( const std::vector &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 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 tfVaryingsBogus = {"some", "invalid[0]", "names"}; + glTransformFeedbackVaryings(mVertProg, static_cast(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(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 redColor = { + std::array redColor = { {GLColor::red, GLColor::red, GLColor::red, GLColor::red}}; - std::array greenColor = { + std::array greenColor = { {GLColor::green, GLColor::green, GLColor::green, GLColor::green}}; // Create a red texture and bind to texture unit 0