diff --git a/MobileGL/MG_Backend/DirectGLES/ManagersPrograms.cpp b/MobileGL/MG_Backend/DirectGLES/ManagersPrograms.cpp index 0dd3b267..061b0c1a 100644 --- a/MobileGL/MG_Backend/DirectGLES/ManagersPrograms.cpp +++ b/MobileGL/MG_Backend/DirectGLES/ManagersPrograms.cpp @@ -1606,7 +1606,6 @@ namespace MobileGL::MG_Backend::DirectGLES { #endif // Before either early return below, so Use() can name the GL program of a no-op draw. m_frontendProgramId = src.GetExternalIndex(); - BeginNarrowIntegerSeedBuild(m_narrowIntegerSeeds); // GetSpirvStatus() as well as GetLinkStatus(): a program whose phase-B job was // cancelled (teardown) or whose optimizer run failed is fully linked and fully @@ -1619,6 +1618,8 @@ namespace MobileGL::MG_Backend::DirectGLES { src.GetExternalIndex()); return; } + // After the return above, which keeps the previous driver program and the widths it holds. + BeginNarrowIntegerSeedBuild(m_narrowIntegerSeeds); MGLOG_D("Syncing program to backend. State program ID: %u, Backend ID: %u", src.GetExternalIndex(), m_backendProgramId); @@ -2650,10 +2651,7 @@ namespace MobileGL::MG_Backend::DirectGLES { } // A capture program with integer inputs is armed at its first build, so arming never // relinks a program in the middle of a capture (NarrowIntegerSeeds.h). - const IntegerVertexInputMasks integerInputs = IntegerVertexInputsOf(*record); - if (!record->Archive->Link.xfbVaryings.empty() && - (integerInputs.signedInputs | integerInputs.unsignedInputs)) - m_narrowIntegerSeeds.wanted = true; + PrepareNarrowIntegerSeedBuild(m_narrowIntegerSeeds, *record); SyncToBackendFromSource(ProgramArchiveSource::FromRecord(cso, *record)); // The two server-owned keys the draw path's clean condition reads instead of // GetLinkVersion()/GetImageUnitVersion(). BindingsSerial is the second because a diff --git a/MobileGL/MG_Backend/DirectGLES/NarrowIntegerSeeds.cpp b/MobileGL/MG_Backend/DirectGLES/NarrowIntegerSeeds.cpp index e4305d73..e27b8370 100644 --- a/MobileGL/MG_Backend/DirectGLES/NarrowIntegerSeeds.cpp +++ b/MobileGL/MG_Backend/DirectGLES/NarrowIntegerSeeds.cpp @@ -24,6 +24,13 @@ namespace MobileGL::MG_Backend::DirectGLES { return slot != nullptr ? slot->get() : nullptr; } + // Whether the program's build for `record`'s link refused the seeds; a link not built yet + // is presumed to take them. + Bool SeedsRefused(MG_Pipe::MGPipeHandle program, const MG_Pipe::MGPipeShaderCsoRecord& record) { + auto* twin = ExistingProgramTwin(program); + return twin != nullptr && twin->NarrowIntegerSeeds().RefusedAt(record.Serial); + } + String UniformDeclaration(const char* name) { return String("uniform highp uint ") + name + ";"; } @@ -70,18 +77,25 @@ namespace MobileGL::MG_Backend::DirectGLES { if (attrib.Enabled && attrib.IsInteger && !attrib.IsLong) data.Note(location, static_cast(attrib.Type)); } - auto* twin = ExistingProgramTwin(program); - return ResolveIntegerAttributePlan(inputs, data, twin == nullptr || !twin->NarrowIntegerSeeds().Refused()); + // The twin is looked up only for a plan that needs seeds; most integer draws need none. + const IntegerAttributePlan plan = ResolveIntegerAttributePlan(inputs, data, true); + return plan.NeedsSeeds() && SeedsRefused(program, *record) ? plan.WithoutSeeds() : plan; } Bool DropRefusedNarrowIntegerSeeds(IntegerAttributePlan& plan, MG_Pipe::MGPipeHandle program) { if (!plan.NeedsSeeds()) return false; - auto* twin = ExistingProgramTwin(program); - if (twin == nullptr || !twin->NarrowIntegerSeeds().Refused()) return false; + const MG_Pipe::MGPipeShaderCsoRecord* const record = PipeShaderCsoRecordForHandle(program); + if (record == nullptr || !SeedsRefused(program, *record)) return false; plan = plan.WithoutSeeds(); return true; } + void PrepareNarrowIntegerSeedBuild(NarrowIntegerSeedState& state, const MG_Pipe::MGPipeShaderCsoRecord& record) { + const IntegerVertexInputMasks inputs = IntegerVertexInputsOf(record); + state.PrepareBuild(record.Serial, (inputs.signedInputs | inputs.unsignedInputs) != 0, + record.Archive && !record.Archive->Link.xfbVaryings.empty()); + } + void PushNarrowIntegerWidthsForDraw(const IntegerAttributePlan& plan, MG_Pipe::MGPipeHandle program) { // Only the twin this draw's sync stashed, and only while it is the program Use() bound: a // uniform call with program 0, or on another program, is an error or a wrong write. @@ -121,7 +135,7 @@ namespace MobileGL::MG_Backend::DirectGLES { void FinishNarrowIntegerSeedBuild(NarrowIntegerSeedState& state, GLuint program, Bool programUsable) { if (!state.built) return; if (!programUsable) { - // The seeds may be why the build failed; the next build goes without them. + // The seeds may be why the build failed; the next build of this link goes without them. state.disabled = true; MGLOG_E_ONCE("DirectGLES: a program built with narrow integer seeds did not link; it is rebuilt " "without them, and its 8/16-bit integer attributes keep the driver's own value"); diff --git a/MobileGL/MG_Backend/DirectGLES/NarrowIntegerSeeds.h b/MobileGL/MG_Backend/DirectGLES/NarrowIntegerSeeds.h index 83ac07de..e14cbe53 100644 --- a/MobileGL/MG_Backend/DirectGLES/NarrowIntegerSeeds.h +++ b/MobileGL/MG_Backend/DirectGLES/NarrowIntegerSeeds.h @@ -25,12 +25,14 @@ // // Every failure falls back to the values the driver gives today: a build whose ESSL lacks either // uniform, or whose link dropped one, is "refused" and its draws keep the application's type for -// 8/16-bit data; a build that fails outright disables arming for the object, and it is rebuilt -// without the seeds. +// 8/16-bit data; a build that fails outright disables arming until the application relinks the +// object, and it is rebuilt without the seeds. A relink is presumed to arm until its build says no. namespace MobileGL::MG_Backend::DirectGLES { struct NarrowIntegerSeedState { Bool wanted = false; Bool disabled = false; + // The record Serial (the application's link) the last build was for. + Uint64 buildSerial = 0; // What the last build ran with and what it verified. Bool built = false; Bool declared = false; @@ -44,8 +46,21 @@ namespace MobileGL::MG_Backend::DirectGLES { Bool Target() const { return wanted && !disabled; } // The program's next build has to change its arming. Bool Stale() const { return Target() != built; } - // An armed build that could not give the program working seeds. - Bool Refused() const { return disabled || (built && !available); } + // An armed build of the link at `serial` that could not give the program working seeds. + Bool RefusedAt(Uint64 serial) const { return serial == buildSerial && (disabled || (built && !available)); } + + // Before a build of the link at `serial`: a failure of an earlier link says nothing about + // this one, a link without integer inputs has nothing to arm, and a capturing one is armed. + void PrepareBuild(Uint64 serial, Bool hasIntegerInputs, Bool captures) { + if (serial != buildSerial) { + disabled = false; + buildSerial = serial; + } + if (!hasIntegerInputs) + wanted = false; + else if (captures) + wanted = true; + } }; // The program's 32-bit int and uint vertex inputs, memoized on the record. @@ -60,6 +75,8 @@ namespace MobileGL::MG_Backend::DirectGLES { // After Use(): the two widths on the program the draw runs, when they differ from what it holds. void PushNarrowIntegerWidthsForDraw(const IntegerAttributePlan& plan, MG_Pipe::MGPipeHandle program); + // PrepareBuild from the record the build is for. + void PrepareNarrowIntegerSeedBuild(NarrowIntegerSeedState& state, const MG_Pipe::MGPipeShaderCsoRecord& record); // The build's three steps: reset at its start, note each stage's final ESSL, verify after the link. void BeginNarrowIntegerSeedBuild(NarrowIntegerSeedState& state); void NoteNarrowIntegerSeedSource(NarrowIntegerSeedState& state, const String& source, GLenum shaderType); diff --git a/MobileGL/MG_Test/Backend/DirectGLES/IntegerAttributePlanTest.cpp b/MobileGL/MG_Test/Backend/DirectGLES/IntegerAttributePlanTest.cpp index 201edb2c..0a870e62 100644 --- a/MobileGL/MG_Test/Backend/DirectGLES/IntegerAttributePlanTest.cpp +++ b/MobileGL/MG_Test/Backend/DirectGLES/IntegerAttributePlanTest.cpp @@ -14,6 +14,7 @@ #include #include +#include using namespace MobileGL; using MobileGL::MG_Backend::DirectGLES::HeldIntegerRetype; @@ -21,6 +22,7 @@ using MobileGL::MG_Backend::DirectGLES::IntegerAttribTypeForInput; using MobileGL::MG_Backend::DirectGLES::IntegerAttributeDataMasks; using MobileGL::MG_Backend::DirectGLES::IntegerAttributePlan; using MobileGL::MG_Backend::DirectGLES::IntegerVertexInputMasks; +using MobileGL::MG_Backend::DirectGLES::NarrowIntegerSeedState; using MobileGL::MG_Backend::DirectGLES::ResolveIntegerAttributePlan; namespace { @@ -112,3 +114,36 @@ TEST(IntegerAttributePlan, TheIssuedTypeOnlyEverSwapsSignedness) { } EXPECT_EQ(IntegerAttribTypeForInput(GL_FLOAT, true), static_cast(GL_FLOAT)); } + +// A program's arming across builds: a failed armed build is retried without the seeds for that +// link only, and an application relink starts over from what the new link declares. +TEST(NarrowIntegerSeedState, AFailedArmedBuildLastsUntilTheNextLink) { + using MobileGL::MG_Backend::DirectGLES::BeginNarrowIntegerSeedBuild; + using MobileGL::MG_Backend::DirectGLES::FinishNarrowIntegerSeedBuild; + NarrowIntegerSeedState state; + state.PrepareBuild(1, true, false); + EXPECT_FALSE(state.wanted) << "a program that does not capture is armed by a draw, not a build"; + state.wanted = true; + BeginNarrowIntegerSeedBuild(state); + FinishNarrowIntegerSeedBuild(state, 0, false); + EXPECT_TRUE(state.RefusedAt(1)); + EXPECT_TRUE(state.Stale()) << "the failed link is rebuilt without the seeds"; + + state.PrepareBuild(1, true, false); + BeginNarrowIntegerSeedBuild(state); + EXPECT_FALSE(state.built); + EXPECT_TRUE(state.RefusedAt(1)); + EXPECT_FALSE(state.Stale()); + EXPECT_FALSE(state.RefusedAt(2)) << "a link not built yet is presumed to take the seeds"; + + state.PrepareBuild(2, true, false); + EXPECT_FALSE(state.RefusedAt(2)); + EXPECT_TRUE(state.Target()) << "a relink arms again"; + EXPECT_TRUE(state.Stale()); + + state.PrepareBuild(3, false, false); + EXPECT_FALSE(state.wanted) << "a link without integer inputs has nothing to arm"; + EXPECT_FALSE(state.Target()); + state.PrepareBuild(4, true, true); + EXPECT_TRUE(state.Target()) << "a capturing link is armed at its build"; +}