From 39d0677616916f959deb0e08c26c79e7e40574af Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 07:50:27 -0400 Subject: [PATCH] [MG_Pipe, MG_Impl, MG_Remote, MG_State, MG_Test] (Disaggregated): P5e pg - the archive becomes server-owned and the bindings get a record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - MGPipeShaderCsoRecord gains Archive, a SharedPtr the create ADOPTS under a transport. Before this the applier stored the descriptor and the two companion pointers named the frontend's own LinkArtifacts/SpirvArtifacts, which ProgramObject::Link() replaces in place - so every reflection read on the apply thread was torn or stale by construction the moment the client runs ahead (gap G-A). - MGPipeApplyCreateShaderState takes the archive as a fourth argument, undefaulted for PipeRoute.h's stated reason: a caller that has one and forgets it would compile, and the failure would be silent. Null on the monolith arm, which keeps its frontend arms token for token (ruling 1 / ID-81). - ProgramArtifactsCodec gains ProgramArchive and the FRAME. SpirvArtifacts carries no stage list and MGPProgramDesc::StageMask cannot stand in for one: it is a bit set, and GL lets two shader objects share a stage, so a list rebuilt from it can be shorter than generatedSpirv - which the backend pairs with it by one running index. - Codec completeness, as CONTRACT-P5E §5.5 asks: the skipped member is named (LinkArtifacts::program, the live glslang TProgram) and proven unread by either backend twin; the libc++ caveat at the size echo is read as what it is - a cross-toolchain check, exact for an in-process split - and both verdicts are written where they were asked for. - set_program_bindings (opcode 80) lands end to end: EmitProgramBindings on the client, the FIFTH ROUTE ESCAPE (three tails in three index spaces plus a parallel name array - no generated row can express it, so op 80's gMGPipeContext slot stays null as PipeCatalogueTest pins), the wire emitter, the decoder arm that resolves and NUL-checks every name span, and the applier body that copies the names into the record (rule C) and advances BindingsSerial. - The record is emitted AFTER create_shader_state, not before as the draft note said: a re-issued create clears all three tails because they index into the archive it just replaced, so bindings published ahead of it are wiped by the create that follows. - MGPProgramDesc::LinkStatus is filled, and ID-88 is VERIFIED rather than assumed: ProgramObject::Link()'s prologue bumps m_linkVersion and assigns m_artifacts = {} - "the complete not-linked state" - before the body runs, and every ProgramLinkTask failure arm leaves linkStatus false. A failed relink therefore reports unlinked, the re-issue carries LinkStatus = 0, and nothing needs an object_death. - ID-87's measurement gets its instrument: the archive's byte count is added to PipeStats' CsoBlobBytes where the bytes actually exist, one sample per link. --- MobileGL/MG_Impl/Pipe/ProgramEmit.h | 243 +++++++++++++++++- MobileGL/MG_Pipe/PipeApply.cpp | 98 ++++++- MobileGL/MG_Pipe/PipeApply.h | 42 ++- MobileGL/MG_Pipe/PipeRoute.cpp | 30 ++- MobileGL/MG_Pipe/PipeRoute.h | 39 ++- MobileGL/MG_Remote/Client/WireTables.cpp | 91 ++++++- MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 77 ++++-- .../ProgramState/ProgramArtifactsCodec.cpp | 59 +++++ .../ProgramState/ProgramArtifactsCodec.h | 59 +++++ .../MG_Test/Pipe/CompositeResolverTest.cpp | 16 +- MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp | 9 +- MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp | 24 +- MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp | 4 +- MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 24 +- MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 7 +- 15 files changed, 745 insertions(+), 77 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/ProgramEmit.h b/MobileGL/MG_Impl/Pipe/ProgramEmit.h index ce3d51e7..02e98b55 100644 --- a/MobileGL/MG_Impl/Pipe/ProgramEmit.h +++ b/MobileGL/MG_Impl/Pipe/ProgramEmit.h @@ -79,6 +79,25 @@ namespace MobileGL::MG_Pipe { // would describe a program that does not exist yet. GetLinkedShaderStages() is also what // indexes GetGeneratedSpirv(), so the two halves of this descriptor are guaranteed to agree // by construction rather than by care. + // P5e (pg). ONE ENTRY OF THE STORAGE-OVERRIDE SIGNATURE, and it is a VERBATIM copy of the + // mix in `MG_Backend/DirectGLES/Managers.cpp`'s ComputeShaderStorageBlockBindingSignature - + // the function whose answer this record replaces on the handle arm. The two halves cannot + // share one definition today because MG_Backend includes nothing from MG_Impl (checked at + // this head), so they are twins with a named pointer at each other and a unit case in + // ProgramEmitTest that pins the number. Order-independent by construction: the source is an + // UnorderedMap, the combine is commutative, and the entry mixes name and binding so two + // entries cannot trade halves and cancel out. + // + // OVER THE VALUES, NOT OVER A CHANGE COUNTER, which is the property the pipeline composite's + // uniform mirror depends on: it re-sets every override every draw, and re-setting a block to + // the binding it already carries must produce the same signature and force no rebuild. + inline Uint64 MGPipeStorageOverrideSignatureEntry(const String& blockName, Int binding) { + Uint64 entry = std::hash{}(blockName); + entry ^= (static_cast(static_cast(binding)) + 0x9e3779b97f4a7c15ull + + (entry << 6) + (entry >> 2)); + return entry; + } + inline Uint32 MGPipeStageMaskOf(const MG_State::GLState::ProgramObject& program) { Uint32 mask = 0; for (const ShaderStage stage : program.GetLinkedShaderStages()) { @@ -109,6 +128,17 @@ namespace MobileGL::MG_Pipe { const MGPipeHandle drawCso = drawProgram ? AcquireShaderCso(*drawProgram, bytes) : kMGPipeNullHandle; + // P5e (pg): AFTER the create and never before it. The draft note on + // MGPProgramBindings said "emitted BEFORE create_shader_state at the same validate + // point", and that ordering is wrong against the applier the same contract + // specifies: a re-issued create CLEARS all three tails, for the reason it clears + // GlobalConstants (a relink replaces the archive the tails index into), so bindings + // published ahead of it are wiped by the create that follows. What the draft was + // protecting - "a rebuild inside the verb must already see the bindings" - is + // satisfied by both records going out at the SAME validate point, ahead of the verb, + // which is what this ordering does. See MG_Remote/CONTRACT-P5E.md §5.5 and the + // record's own comment in PipeApply.h. + if (drawProgram) bytes += EmitProgramBindings(*drawProgram, drawCso); // THE COMPOSITE'S SECOND RELEASE PATH is spoken here, not in a destructor: when the // bound pipeline's draw-program signature moves, the resolver releases the slot the // previous composite held. Whichever of the two paths runs second - this one or the @@ -130,6 +160,9 @@ namespace MobileGL::MG_Pipe { dispatchProgram ? (dispatchProgram == drawProgram ? drawCso : AcquireShaderCso(*dispatchProgram, bytes)) : kMGPipeNullHandle; + if (dispatchProgram && dispatchProgram != drawProgram) { + bytes += EmitProgramBindings(*dispatchProgram, dispatchCso); + } // THE BOUND CSO IS THE DRAW ONE WHEN THERE IS ONE. bind_shader_state names what // glUseProgram selected, and when a program pipeline is bound instead that is the @@ -208,6 +241,136 @@ namespace MobileGL::MG_Pipe { return bytes + sizeof(MGPGlobalConstants) + size; } + // ---- P5e (pg): set_program_bindings, opcode 80 ----------------------------------- + // + // THE THREE POST-LINK MUTABLE REFLECTION FIELDS, and why an archive alone is not an + // answer. `uniformBlockBinding[i]`, `uniformSamplerOrImageUnitIndex[loc]` and the + // name-keyed `shaderStorageBlockBinding` map are all MEMBERS OF LinkArtifacts - so the + // archive create_shader_state carries does hold them - but glUniformBlockBinding, + // glUniform1i on a sampler uniform and glShaderStorageBlockBinding all move them AFTER + // the link that produced that archive, without relinking. A server answering a draw + // from the archive alone would bind the uniform blocks the program was LINKED with + // rather than the ones it is BOUND with. This record is the delta carrier; the applier + // overlays it on the archive, and a re-issued create drops it because the indices no + // longer mean anything. + // + // LATCHED ON THE TWO COUNTERS THAT MOVE WHEN ANY OF THE THREE DOES, plus the handle: + // m_backendStateVersion (glUniformBlockBinding and glUniform1i both bump it) and + // m_blockBindingVersion (glUniformBlockBinding and glShaderStorageBlockBinding both + // bump it, and the storage setter deliberately bumps ONLY it). Neither setter moves a + // counter on an unchanged value - both have an equality bail-out - so an application + // that re-sets the same bindings every frame emits nothing, which is what the pipeline + // composite's uniform mirror needs (it replays every override per draw). + Uint64 EmitProgramBindings(const ProgramObject& program, MGPipeHandle cso) { + if (MGPipeHandleIsNull(cso)) return 0; + const Uint32 backendStateVersion = program.GetBackendStateVersion(); + const Uint32 blockBindingVersion = program.GetBlockBindingVersion(); + Latch& latch = LatchFor(cso); + if (latch.BindingsLive && latch.BindingsGen == cso.Gen && + latch.BindingsBackendStateVersion == backendStateVersion && + latch.BindingsBlockBindingVersion == blockBindingVersion) { + return 0; + } + + m_blockBindings.clear(); + m_samplerUnits.clear(); + m_storageOverrides.clear(); + m_storageOverrideNames.clear(); + + // TAIL 1: dense, in BLOCK index order - the space GetActiveUniformBlocksCount(), + // GetUniformBlockName(i) and GetUniformBlockBinding(i) all speak, which is exactly + // the space the server's CacheResourceLocations walks. Dense because the server + // indexes it by i and a sparse form would need a second index space on the wire. + const Int blockCount = program.GetActiveUniformBlocksCount(); + if (static_cast(std::max(blockCount, 0)) > kMGPipeMaxProgramBlockBindings) { + // A COUNTED REFUSAL AND NOT A TRUNCATION (D-J3): a program with more uniform + // blocks than the record can name would have the tail of its block set bound + // to whatever the archive's link-time snapshot said, silently. The counter is + // what makes that visible; the record is not emitted at all, so the server + // keeps the archive's values for every block rather than for some of them. + ++m_bindingRefusals; + return 0; + } + m_blockBindings.reserve(static_cast(std::max(blockCount, 0))); + for (Int i = 0; i < blockCount; ++i) { + m_blockBindings.push_back(static_cast(program.GetUniformBlockBinding(static_cast(i)))); + } + + // TAIL 2: sparse, ascending by LOCATION, because the frontend's array is indexed by + // uniform location and maxUniformLocation runs into the thousands while the + // sampler count is a handful. -1 is "never assigned" and is what the server's own + // default already is, so it is not worth a wire entry. + const Uint maxUniformLoc = program.GetMaxUniformLocation(); + for (Uint loc = 0; loc <= maxUniformLoc; ++loc) { + // The same guard CacheResourceLocations uses one level down: a location with no + // uniform behind it has an empty name and must not index the type tables. + if (program.GetUniformName(loc).empty()) continue; + const Int unit = program.GetUniformSamplerOrImageUnitIndex(loc); + if (unit < 0) continue; + if (m_samplerUnits.size() >= kMGPipeMaxProgramSamplerUnits) { + ++m_bindingRefusals; + return 0; + } + MGPProgramSamplerUnit entry{}; + entry.Location = loc; + entry.Unit = static_cast(unit); + m_samplerUnits.push_back(entry); + } + + // TAIL 3: name-keyed BY DESIGN. A shader storage block has three index spaces - the + // frontend interface query, DirectVulkan's descriptor order and the real driver's - + // and the name is the only coordinate all three agree on (ProgramObject.h says so + // over the setter). Empty for the overwhelming majority of programs, which is why + // the signature below exists: the server compares one Uint64 and touches the names + // only on a rebuild. + const auto& overrides = program.GetShaderStorageBlockBindingOverrides(); + if (overrides.size() > kMGPipeMaxProgramStorageOverrides) { + ++m_bindingRefusals; + return 0; + } + Uint64 signature = 0; + m_storageOverrides.reserve(overrides.size()); + m_storageOverrideNames.reserve(overrides.size()); + for (const auto& [blockName, binding] : overrides) { + if (binding < 0) continue; // never rebound; the declared qualifier still stands + MGPProgramStorageOverride entry{}; + entry.Binding = binding; + m_storageOverrides.push_back(entry); + m_storageOverrideNames.push_back(blockName); + signature += MGPipeStorageOverrideSignatureEntry(blockName, binding); + } + m_storageOverrides.resize(m_storageOverrideNames.size()); + + m_lastBindings = MGPProgramBindings{}; + m_lastBindings.Cso = cso; + m_lastBindings.Signature = signature; + m_lastBindings.BlockBindingCount = static_cast(m_blockBindings.size()); + m_lastBindings.SamplerUnitCount = static_cast(m_samplerUnits.size()); + m_lastBindings.StorageOverrideCount = static_cast(m_storageOverrides.size()); + + // The name POINTERS, index-aligned with the override tail. Built here rather than + // inside the loop because push_back on m_storageOverrideNames may reallocate and + // every c_str() taken before that would dangle. + m_storageOverrideNamePtrs.clear(); + m_storageOverrideNamePtrs.reserve(m_storageOverrideNames.size()); + for (const String& name : m_storageOverrideNames) m_storageOverrideNamePtrs.push_back(name.c_str()); + + MGPipeRouteSetProgramBindings( + m_lastBindings, m_blockBindings.empty() ? nullptr : m_blockBindings.data(), + m_samplerUnits.empty() ? nullptr : m_samplerUnits.data(), + m_storageOverrides.empty() ? nullptr : m_storageOverrides.data(), + m_storageOverrideNamePtrs.empty() ? nullptr : m_storageOverrideNamePtrs.data()); + ++m_bindingSets; + + latch.BindingsLive = true; + latch.BindingsGen = cso.Gen; + latch.BindingsBackendStateVersion = backendStateVersion; + latch.BindingsBlockBindingVersion = blockBindingVersion; + return sizeof(MGPProgramBindings) + m_blockBindings.size() * sizeof(Int32) + + m_samplerUnits.size() * sizeof(MGPProgramSamplerUnit) + + m_storageOverrides.size() * sizeof(MGPProgramStorageOverride); + } + // D-H4's re-issue rule, and it is the CreateVertexElements shape one for one: the // record goes out again on the SAME handle whenever the link version moves, which is // legal because MGPipeHandle::Gen increments only on slot reuse and never on a @@ -232,6 +395,21 @@ namespace MobileGL::MG_Pipe { m_lastDesc.GlobalUboSize = static_cast(program.GetUBOSize()); m_lastDesc.ReservedNumSamplesOffset = static_cast(spirv.reservedNumSamplesOffset); m_lastDesc.SpirvStatus = spirv.spirvStatus ? 1 : 0; + // P5e (pg), ID-88 / ruling 9: THE FAILED RELINK OF A BOUND PROGRAM, and the ruling + // asked this package to verify the frontend's actual behaviour rather than assume + // it. VERIFIED, at ProgramObject.cpp:510-531: `Link()`'s PROLOGUE bumps + // m_linkVersion and then assigns `m_artifacts = {}` - "the complete not-linked + // state" in its own words - before the link body runs, and every failure arm of + // ProgramLinkTask leaves `linkStatus = false`. So a failed relink reports UNLINKED + // through GetLinkStatus(), the re-issue this latch triggers is the one the ruling + // names, and it carries LinkStatus = 0. + // + // AND NEVER AN object_death: the program object is alive, its handle is alive, and + // its twin must stay alive to be rebuilt by the next successful link. What the + // server does with LinkStatus = 0 is exactly what the monolith arm already does + // with GetLinkStatus() == false - glUseProgram(0), a visible no-op draw - which is + // also what GL requires, since MobileGL's frontend withdraws LINK_STATUS here. + m_lastDesc.LinkStatus = program.GetLinkStatus() ? 1 : 0; m_lastDesc.NativeFloat64 = spirv.nativeFloat64 ? 1 : 0; m_lastDesc.PointSizeDemoted = spirv.pointSizeDemoted ? 1 : 0; m_lastDesc.EnableSpirvValidation = spirv.enableSpirvValidation ? 1 : 0; @@ -260,7 +438,20 @@ namespace MobileGL::MG_Pipe { m_lastDesc.Reflection.Offset = reinterpret_cast(&link); m_lastDesc.Reflection.Size = 0; - MGPipeRouteCreateShaderState(m_lastDesc, &link, &spirv); + // P5e (pg): THE STAGE OF EACH MODULE, beside the modules and not folded into + // StageMask. StageMask is a bit SET and GL lets two shader objects of one stage be + // attached to a program, so a list rebuilt from it can be SHORTER than + // generatedSpirv - and the server pairs the two by one running index. The client + // arm frames this in front of the archive; the monolith arm discards it and reads + // the snapshot off the object it is handed. Taken from the same accessor + // MGPipeStageMaskOf walks, so the mask and the list cannot disagree. + m_linkedStageWords.clear(); + for (const ShaderStage stage : program.GetLinkedShaderStages()) { + m_linkedStageWords.push_back(static_cast(stage)); + } + + MGPipeRouteCreateShaderState(m_lastDesc, &link, &spirv, m_linkedStageWords.data(), + static_cast(m_linkedStageWords.size())); // THE CREATE WENT OUT, so the publication latch is taken here and nowhere else // (contract-v2 §3.1). MGPipeEmitShaderCsoDestroyAndFree reads it, and without it // delete_shader_state can never go out - for an ordinary program or for a @@ -283,6 +474,14 @@ namespace MobileGL::MG_Pipe { m_constantsVersion = kMGPipeGlobalConstantsNeverUploaded; } + // P5e (pg): THE BINDINGS LATCH GOES WITH THE GLOBAL-CONSTANTS ONE, and for the same + // reason stated one branch up. The applier clears all three binding tails on a + // re-issued create (the indices point into an archive that has just been replaced), + // so a latch that survived it would suppress the very record that has to re-establish + // them and the server would draw the whole program off the archive's link-time + // snapshot. Invalidated rather than emitted here: EmitShaderState sends the bindings + // immediately after this returns, which is also the ONLY order that works. + latch.BindingsLive = false; latch.RecordLive = true; latch.RecordGen = handle.Gen; latch.LinkVersion = linkVersion; @@ -304,7 +503,13 @@ namespace MobileGL::MG_Pipe { // validate-point emission and has no payload budget to report into. void EmitShaderCso(ProgramObject& program) { Uint64 bytes = 0; - AcquireShaderCso(program, bytes); + const MGPipeHandle cso = AcquireShaderCso(program, bytes); + // P5e (pg): the birth hook publishes the bindings too, for the same reason the + // validate point does. A program born here and drawn before the next validate point + // would otherwise reach the server with its archive's LINK-TIME snapshot and no + // delta, which is right only for a program nothing has rebound since - and this + // hook is exactly where a program that WAS rebound at creation time arrives. + EmitProgramBindings(program, cso); } // The emitter's OWN record memo - "have I already published a create_shader_state at @@ -378,6 +583,7 @@ namespace MobileGL::MG_Pipe { void ResetCounters() { m_creates = m_binds = m_drawSets = m_dispatchSets = m_constantSets = 0; m_moduleTruncations = 0; + m_bindingSets = m_bindingRefusals = 0; } // ---- what a unit case reads ---- @@ -399,6 +605,15 @@ namespace MobileGL::MG_Pipe { Uint64 DrawProgramSetCount() const { return m_drawSets; } Uint64 DispatchProgramSetCount() const { return m_dispatchSets; } Uint64 GlobalConstantsSetCount() const { return m_constantSets; } + // P5e (pg). The record a case reads back, and the two counters D-J3's rule asks for: + // how many set_program_bindings went out, and how many programs were REFUSED because + // one of their three sets is larger than the record can name. A refusal leaves the + // server on the archive's link-time snapshot for that program - correct for anything + // that was never rebound after the link, wrong for anything that was, and countable + // either way, which is the whole point of not truncating. + const MGPProgramBindings& LastProgramBindings() const { return m_lastBindings; } + Uint64 ProgramBindingsSetCount() const { return m_bindingSets; } + Uint64 ProgramBindingsRefusalCount() const { return m_bindingRefusals; } private: // THE ONE PLACE THE BAND CAN ENTER. An ordinary program's slot comes from the ordinary @@ -426,6 +641,15 @@ namespace MobileGL::MG_Pipe { Bool RecordLive = false; Uint32 RecordGen = 0; Uint32 LinkVersion = 0; + // P5e (pg): set_program_bindings' own key, in the SAME latch because it is keyed on + // the same handle and a slot recycle has to invalidate both at once. Kept separate + // from RecordLive because the two records have different triggers - a create fires + // on a link-version move, the bindings fire on a binding move without a relink, and + // most frames have neither. + Bool BindingsLive = false; + Uint32 BindingsGen = 0; + Uint32 BindingsBackendStateVersion = 0; + Uint32 BindingsBlockBindingVersion = 0; }; // TWO TABLES, NOT A WIDER ONE, and it is the allocator's own reason repeated where it @@ -461,6 +685,19 @@ namespace MobileGL::MG_Pipe { MGPProgramDesc m_lastDesc{}; MGPGlobalConstants m_lastConstants{}; + // P5e (pg): the three tails and their names, MEMBERS rather than locals so the steady + // frame allocates nothing - the same reason every other emitter in this directory keeps + // its scratch. Cleared and refilled per emission; none of them outlives the route call + // except as the record's own copy on the far side (rule C). + MGPProgramBindings m_lastBindings{}; + Vector m_blockBindings; + Vector m_samplerUnits; + Vector m_storageOverrides; + Vector m_storageOverrideNames; + Vector m_storageOverrideNamePtrs; + // The stage of each module of the descriptor being built, index-aligned with + // GetGeneratedSpirv(); the client arm frames it in front of the archive. + Vector m_linkedStageWords; Vector m_latch; Vector m_compositeLatch; @@ -476,6 +713,8 @@ namespace MobileGL::MG_Pipe { Uint64 m_dispatchSets = 0; Uint64 m_constantSets = 0; Uint64 m_moduleTruncations = 0; + Uint64 m_bindingSets = 0; + Uint64 m_bindingRefusals = 0; }; inline MGPipeProgramEmitter& MGPipeProgramEmitterInstance() { diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index 52f91983..578a14b8 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -2851,21 +2851,69 @@ namespace MobileGL::MG_Pipe { std::abort(); } + // P5e (pg) WRITES THE SECOND BODY. The three post-link mutable reflection fields - + // glUniformBlockBinding's block-to-point map, glUniform1i's sampler unit per uniform + // LOCATION, and glShaderStorageBlockBinding's name-keyed override set - all live INSIDE + // the frontend's LinkArtifacts and all move after the link that produced it. An archive + // snapshot alone therefore answers the wrong question: a twin built from it would bind the + // uniform blocks the program was LINKED with rather than the ones it is BOUND with. void MGPipeApplySetProgramBindings(const MGPProgramBindings& hdr, const Int32* blockBindings, const MGPProgramSamplerUnit* samplerUnits, const MGPProgramStorageOverride* storageOverrides, const char* const* storageOverrideNames) { - (void)blockBindings; - (void)samplerUnits; - (void)storageOverrides; - (void)storageOverrideNames; - MGLOG_F("MGPipe: Fatal{UnmigratedVerb, \"set_program_bindings\"} - the applier entry point " - "is declared by P5e package c0e and bodied by package pg; a record for shader CSO " - "{%u, %u} with %u/%u/%u tail entries reached it, which means a route was installed " - "ahead of its consumer", - hdr.Cso.Slot, hdr.Cso.Gen, hdr.BlockBindingCount, hdr.SamplerUnitCount, - hdr.StorageOverrideCount); - std::abort(); + // THE BOUNDS ARE THE DECODER'S AND ARE RE-ASSERTED HERE, because the monolith adapter + // reaches this entry point without passing the decoder at all: under monolith the only + // validation a record ever gets is the one the applier does itself, which is the rule + // every sibling in this file already keeps. + const char* fault = nullptr; + if (hdr.BlockBindingCount > kMGPipeMaxProgramBlockBindings) fault = "BlockBindingCount"; + else if (hdr.SamplerUnitCount > kMGPipeMaxProgramSamplerUnits) fault = "SamplerUnitCount"; + else if (hdr.StorageOverrideCount > kMGPipeMaxProgramStorageOverrides) fault = "StorageOverrideCount"; + else if (hdr.BlockBindingCount != 0 && blockBindings == nullptr) fault = "BlockBindings"; + else if (hdr.SamplerUnitCount != 0 && samplerUnits == nullptr) fault = "SamplerUnits"; + else if (hdr.StorageOverrideCount != 0 && + (storageOverrides == nullptr || storageOverrideNames == nullptr)) { + fault = "StorageOverrides"; + } + if (fault != nullptr) { + MGP_TRIP_WIRE_REPORT("MGPipe: " MGP_TRIP_WIRE_TAG("ProtocolCorruption") + " set_program_bindings {slot=%u, gen=%u}: %s is out of range or " + "its tail is absent (%u/%u/%u declared)", + hdr.Cso.Slot, hdr.Cso.Gen, fault, hdr.BlockBindingCount, + hdr.SamplerUnitCount, hdr.StorageOverrideCount); + return; + } + // P4a's belt, for MGPipeApplyBindShaderState's reason: with no consumer this applier + // holds no shader CSO record at all, so the resolution below would report the designed + // state as RefusedObjectCalls. + if (NoP4aConsumer()) return; + + MGPipeShaderCsoRecord* record = ResolveShaderCso("set_program_bindings", hdr.Cso); + if (record == nullptr) return; + + // WHOLE-SET REPLACEMENT, NOT A MERGE, in all three tails. Each one describes the + // program's complete state for its index space as the client observed it at emit; a + // merge would leave a block binding the application has since reset to its declared + // default sitting on top of the archive's value for ever, and there is no per-entry + // "cleared" token on the wire that could say otherwise. + record->BlockBindings.assign(blockBindings, blockBindings + hdr.BlockBindingCount); + record->SamplerUnits.assign(samplerUnits, samplerUnits + hdr.SamplerUnitCount); + record->StorageOverrides.clear(); + record->StorageOverrides.reserve(hdr.StorageOverrideCount); + for (Uint32 i = 0; i < hdr.StorageOverrideCount; ++i) { + MGPipeProgramStorageOverride entry; + // COPIED, NOT POINTED AT (rule C). The name rides SEG_STAGE and the staged run + // retires with the record that named it; the rebuild that reads this may be a + // frame later, and by then those bytes are somebody else's. + entry.Name = storageOverrideNames[i] != nullptr ? String(storageOverrideNames[i]) : String(); + entry.Binding = storageOverrides[i].Binding; + record->StorageOverrides.push_back(Move(entry)); + } + record->Signature = hdr.Signature; + // ADVANCED, NEVER RETURNED TO A VALUE IT HAS HANDED OUT - VertexBuffersSerial's rule. + // Both texture-unit memos and the program twin's clean condition key on it (ruling 6), + // so a serial that went backwards would make a memo match state it has never seen. + ++record->BindingsSerial; } // --------------------------------------------------------------------------------- @@ -2932,7 +2980,8 @@ namespace MobileGL::MG_Pipe { void MGPipeApplyCreateShaderState(const MGPProgramDesc& desc, const MG_State::GLState::LinkArtifacts* link, - const MG_State::GLState::SpirvArtifacts* spirv) { + const MG_State::GLState::SpirvArtifacts* spirv, + SharedPtr archive) { MOBILEGL_ASSERT(desc.Cso.Slot >= kMGPipeFirstAllocatableSlot, "create_shader_state named the reserved slot 0"); if (desc.Cso.Slot < kMGPipeFirstAllocatableSlot) return; @@ -3005,6 +3054,31 @@ namespace MobileGL::MG_Pipe { // deserialised archive is attached to. Either way the applier owns identity, extent and // order, and never content. record.Desc = desc; + // P5e (pg), gap G-A: "under split the descriptor is what the deserialised archive is + // attached to" is now literally true, and this is the attachment. ADOPTED + // UNCONDITIONALLY, INCLUDING A NULL: a monolith create must CLEAR an archive a + // previous split-arm create left behind, because the record outlives a transport + // change (MGPipeApplierReset keeps the CSO table - a program lives in a share group) + // and a stale archive is worse than none. + // + // AFTER Desc, so a reader that sees the new descriptor sees the artefacts it describes; + // the apply thread is the only writer and the only reader, so ordering here is about + // reading this function rather than about memory visibility. + record.Archive = Move(archive); + // THE RE-ISSUE CLEARS THE THREE BINDING TAILS for the reason it clears GlobalConstants + // one branch up: they are indices INTO the archive that has just been replaced, so a + // surviving tail would name block indices and uniform locations of a program that no + // longer exists. Unconditional rather than inside the else-branch above, because a + // recycled slot must not inherit its predecessor's bindings either - and a fresh record + // has empty tails, so the clear costs a live program nothing. The client re-emits + // set_program_bindings immediately after this create (ProgramEmit.h), which is the + // ordering the applier relies on: emitting them BEFORE would put them where this line + // wipes them. + record.BlockBindings.clear(); + record.SamplerUnits.clear(); + record.StorageOverrides.clear(); + record.Signature = 0; + ++record.BindingsSerial; } void MGPipeApplyBindShaderState(const MGPHandleOnly& handle) { diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index 232ef337..38f068b3 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -38,9 +38,16 @@ // this header needs their NAMES and never their definitions; the forward declaration is the // whole coupling and the closure gate is what keeps it one. The verify build is the only // place the codec runs, and it runs from PipeApply.cpp. +// P5e (pg) ADDS A THIRD NAME AND KEEPS THE RULE: ProgramArchive is what a SPLIT create carries +// - the record's OWN copy of both structs plus the stage of each module - and the record holds +// it through a SharedPtr, which needs no definition either (the deleter is captured where the +// archive is constructed, in the decoder's TU). So the coupling is still three names and no +// closure change, and MG_Backend, which does need the definition, includes +// ProgramArtifactsCodec.h for it. namespace MobileGL::MG_State::GLState { struct LinkArtifacts; struct SpirvArtifacts; + struct ProgramArchive; } // namespace MobileGL::MG_State::GLState namespace MobileGL::MG_Pipe { @@ -392,6 +399,28 @@ namespace MobileGL::MG_Pipe { Uint64 GlobalConstantsSerial = 0; Uint64 Serial = 0; + // ---- P5e's server-owned archive (CONTRACT-P5E.md §5.5, gap G-A) -------------------- + // + // FILLED BY PACKAGE pg, AND ONLY UNDER A TRANSPORT. This is the whole of rule F for + // this family: before P5e the record stored the DESCRIPTOR and the artefacts rode + // beside it as two companion pointers into the frontend's own ProgramObject, so every + // reflection question the program twin asked - forty accessors in SyncToBackend alone - + // was a read of client memory that `ProgramObject::Link()` REPLACES IN PLACE. Under + // run-ahead the client is already several records past the create, so those reads are + // torn or stale by construction and no amount of care at the read site can fix it. + // + // The client encodes the archive once per link (ProgramEmit.h -> SEG_STAGE), the + // decoder frames and deserialises it, and the record ADOPTS it here - a SharedPtr and + // not a Vector, because a program-pipeline composite and its stage programs are three + // records over one link's artefacts, and because the twin holds the pointer across the + // build it is running. It retires with the record, which is rule C: the staged run the + // bytes arrived in belongs to somebody else by the next frame. + // + // NULL UNDER MONOLITH, and that is the arm selection and not an omission (ruling 1): + // the push-monolith build keeps its frontend arms token for token, so its twin reads + // the frontend's archive as it always has and this pointer is never consulted. + SharedPtr Archive; + // ---- P5e's set_program_bindings tails (CONTRACT-P5E.md §1, §5.5) ------------------- // // DECLARED BY c0e, FILLED BY PACKAGE pg. THE THREE POST-LINK MUTABLE REFLECTION FIELDS @@ -1324,9 +1353,20 @@ namespace MobileGL::MG_Pipe { // field-compares before storing, and a mismatch is Fatal{PipeVerifyDiffer, "program-archive"}. // Splitting this record for a transport whose ring caps one record at half its capacity is // P5's problem, not this entry point's. + // + // P5e (pg) ADDS THE FOURTH ARGUMENT AND DID NOT DEFAULT IT, for the reason PipeRoute.h + // gives about the three byte counts it also refused to default: a caller that HAS the + // archive and forgets to pass it would compile, and the failure would be a program twin + // reading the frontend's artefacts on the apply thread - silent, and exactly what this + // package exists to end. `archive` is null under monolith (the two companion pointers are + // the frontend's own and the handle arm is not taken there) and non-null under a + // transport, where `link` and `spirv` point INTO it: the record adopts it, so every later + // reflection read is of memory the server owns. A non-null archive whose Link/Spirv are + // not the two pointers passed beside it is a caller bug this entry point asserts on. void MGPipeApplyCreateShaderState(const MGPProgramDesc& desc, const MG_State::GLState::LinkArtifacts* link, - const MG_State::GLState::SpirvArtifacts* spirv); + const MG_State::GLState::SpirvArtifacts* spirv, + SharedPtr archive); void MGPipeApplyBindShaderState(const MGPHandleOnly& handle); void MGPipeApplyDeleteShaderState(const MGPHandleOnly& handle); void MGPipeApplySetDrawProgram(const MGPHandleOnly& handle); diff --git a/MobileGL/MG_Pipe/PipeRoute.cpp b/MobileGL/MG_Pipe/PipeRoute.cpp index a9b4e359..e8f38c0a 100644 --- a/MobileGL/MG_Pipe/PipeRoute.cpp +++ b/MobileGL/MG_Pipe/PipeRoute.cpp @@ -310,8 +310,33 @@ namespace MobileGL::MG_Pipe { void Mono_Escape_CreateShaderState(const MGPProgramDesc* desc, const MG_State::GLState::LinkArtifacts* link, - const MG_State::GLState::SpirvArtifacts* spirv) { - MGPipeApplyCreateShaderState(*desc, link, spirv); + const MG_State::GLState::SpirvArtifacts* spirv, + const Uint32* linkedStages, Uint32 linkedStageCount) { + // The stage list is the CLIENT ARM's framing input and this arm has no use for it: + // the monolith twin reads GetLinkedShaderStages() off the frontend object it is + // handed. Named and discarded rather than left out of the signature, so the two + // arms stay one row. + (void)linkedStages; + (void)linkedStageCount; + // P5e (pg): NO ARCHIVE ON THIS ARM, and the null is the arm selection rather than a + // hole. Under monolith the two companion pointers ARE the frontend's own archive, + // the backend twin reads it through its frontend overload (ruling 1 keeps that arm + // token for token), and serialising here to deserialise into the record would put + // EncodeProgramArtifacts on the monolith path - which PipeRoute.h's escape note + // and PipeApply.h both promise it is not. + MGPipeApplyCreateShaderState(*desc, link, spirv, nullptr); + } + + // P5e (pg), the fifth escape. A pass-through like the four above: the caller already + // holds the three tails and the parallel name array, and under monolith there is no + // segment to resolve - the names ARE the frontend's own String::c_str()s, alive for + // the duration of the call, which is exactly the lifetime the applier's copy-in needs. + void Mono_Escape_SetProgramBindings(const MGPProgramBindings* hdr, const Int32* blockBindings, + const MGPProgramSamplerUnit* samplerUnits, + const MGPProgramStorageOverride* storageOverrides, + const char* const* storageOverrideNames) { + MGPipeApplySetProgramBindings(*hdr, blockBindings, samplerUnits, storageOverrides, + storageOverrideNames); } } // namespace @@ -357,6 +382,7 @@ namespace MobileGL::MG_Pipe { gMGPipeRouteEscapes.ResourceFlushRange = &Mono_Escape_ResourceFlushRange; gMGPipeRouteEscapes.MapPersistent = &Mono_Escape_MapPersistent; gMGPipeRouteEscapes.CreateShaderState = &Mono_Escape_CreateShaderState; + gMGPipeRouteEscapes.SetProgramBindings = &Mono_Escape_SetProgramBindings; // KEPT, not merely installed. The client arm overwrites the three tables above; these // three copies are what it forwards to when it finds itself on the server's own thread. diff --git a/MobileGL/MG_Pipe/PipeRoute.h b/MobileGL/MG_Pipe/PipeRoute.h index 7e794bf9..6344d5fc 100644 --- a/MobileGL/MG_Pipe/PipeRoute.h +++ b/MobileGL/MG_Pipe/PipeRoute.h @@ -144,6 +144,16 @@ namespace MobileGL::MG_Pipe { // `EncodeProgramArtifacts` on the monolith path, which `PipeApply.h` // explicitly promises it is not ("zero serialisation cost on the // monolith path"). + // SetProgramBindings THREE tails in three index spaces plus a PARALLEL NAME ARRAY + // (P5e, MG_Remote/CONTRACT-P5E.md §1/§5.5). One `varTail` + + // `varTailCount` pair cannot express three runs at all, and the + // fourth argument is not a run: the storage-override key is a + // `MGHostSpan` on the wire and a plain `const char*` under monolith, + // so the applier takes the resolved names beside the tail rather + // than resolving segments itself (rule C - a span retires with the + // record that named it, and the applier copies the bytes it keeps). + // The generated `gMGPipeContext.SetProgramBindings` row therefore + // STAYS NULL, which is what PipeCatalogueTest already pins. // // OVERTURN CONDITIONS, one per row: give `MGPResourceDesc` and `MGPFlushRange` a blobref // (overturns R-13.2/R-13.3, and c0 owns `MGPipeTypes.h`); give `MGPHandleOnly` a size @@ -154,9 +164,19 @@ namespace MobileGL::MG_Pipe { const MGPRespecifiedLevel* level); void (*ResourceFlushRange)(const MGPFlushRange* record, const void* bytes); void* (*MapPersistent)(const MGPHandleOnly* handle, Uint64 size, const void* seedBytes); + // P5e (pg) grew this row by the STAGE LIST, and it belongs on the route rather than in + // the two structs: `SpirvArtifacts::generatedSpirv` is one module per shader object and + // the stage of each lives in `ProgramObject::m_linkedShaderSnapshot`, which is GL-thread + // state and not an artifact. The client arm frames it in front of the archive; the + // monolith arm ignores it, because its twin reads the snapshot directly. void (*CreateShaderState)(const MGPProgramDesc* desc, const MG_State::GLState::LinkArtifacts* link, - const MG_State::GLState::SpirvArtifacts* spirv); + const MG_State::GLState::SpirvArtifacts* spirv, + const Uint32* linkedStages, Uint32 linkedStageCount); + void (*SetProgramBindings)(const MGPProgramBindings* hdr, const Int32* blockBindings, + const MGPProgramSamplerUnit* samplerUnits, + const MGPProgramStorageOverride* storageOverrides, + const char* const* storageOverrideNames); }; inline MGPipeRouteEscapes gMGPipeRouteEscapes{}; @@ -384,8 +404,9 @@ namespace MobileGL::MG_Pipe { // ---- programs ---------------------------------------------------------------------- inline void MGPipeRouteCreateShaderState(const MGPProgramDesc& desc, const MG_State::GLState::LinkArtifacts* link, - const MG_State::GLState::SpirvArtifacts* spirv) { - gMGPipeRouteEscapes.CreateShaderState(&desc, link, spirv); + const MG_State::GLState::SpirvArtifacts* spirv, + const Uint32* linkedStages, Uint32 linkedStageCount) { + gMGPipeRouteEscapes.CreateShaderState(&desc, link, spirv, linkedStages, linkedStageCount); } inline void MGPipeRouteBindShaderState(const MGPHandleOnly& handle) { MGP_BindShaderState(&handle); @@ -403,6 +424,18 @@ namespace MobileGL::MG_Pipe { Uint64 byteCount) { MGP_SetGlobalConstants(&record, bytes, byteCount); } + // P5e (pg), opcode 80. The fifth escape; see MGPipeRouteEscapes above for why this row has + // no generated shape. `storageOverrideNames` is index-aligned with `storageOverrides` and + // is what the applier copies into the record - the MGHostSpan inside each override element + // describes where the name lives on the WIRE and is resolved by the decoder, never here. + inline void MGPipeRouteSetProgramBindings(const MGPProgramBindings& hdr, + const Int32* blockBindings, + const MGPProgramSamplerUnit* samplerUnits, + const MGPProgramStorageOverride* storageOverrides, + const char* const* storageOverrideNames) { + gMGPipeRouteEscapes.SetProgramBindings(&hdr, blockBindings, samplerUnits, storageOverrides, + storageOverrideNames); + } } // namespace MobileGL::MG_Pipe diff --git a/MobileGL/MG_Remote/Client/WireTables.cpp b/MobileGL/MG_Remote/Client/WireTables.cpp index 947fb771..05a70ea2 100644 --- a/MobileGL/MG_Remote/Client/WireTables.cpp +++ b/MobileGL/MG_Remote/Client/WireTables.cpp @@ -44,6 +44,9 @@ #include #include #include +// P5e (pg): ID-87 asks this package for bytes-per-link measured rather than argued, and the +// archive's byte count exists exactly once - here, where it is serialised. +#include #include #include @@ -487,9 +490,11 @@ namespace MobileGL::MG_Remote::Client { // Spirv[i] is Fatal on the far side rather than ignored. void Wire_Escape_CreateShaderState(const MG_Pipe::MGPProgramDesc* desc, const MG_State::GLState::LinkArtifacts* link, - const MG_State::GLState::SpirvArtifacts* spirv) { + const MG_State::GLState::SpirvArtifacts* spirv, + const Uint32* linkedStages, Uint32 linkedStageCount) { if (RunsAsTheServerRole()) { - MG_Pipe::MGPipeMonolithEscapes().CreateShaderState(desc, link, spirv); + MG_Pipe::MGPipeMonolithEscapes().CreateShaderState(desc, link, spirv, linkedStages, + linkedStageCount); return; } ClientSession& session = RequireSession("CreateShaderState"); @@ -501,11 +506,23 @@ namespace MobileGL::MG_Remote::Client { "code"); std::abort(); } - // EncodeProgramArtifacts APPENDS and never fails - everything it walks is owned + // EncodeProgramArchive APPENDS and never fails - everything it walks is owned // plain data - so an empty archive means the two structs themselves were empty, // which is a linked program with no artefacts and is not a codec question. + // + // P5e (pg): THE FRAMED form, so the record's own copy on the far side can pair each + // module with its stage. The bare EncodeProgramArtifacts is still what the verify + // build's round-trip pin uses; this is the one that crosses. Vector archive; - MG_State::GLState::EncodeProgramArtifacts(*link, *spirv, archive); + Vector stages(linkedStages, linkedStages + linkedStageCount); + MG_State::GLState::EncodeProgramArchive(*link, *spirv, stages, archive); + if (MG_Util::PipeStats::Enabled()) { + // ID-87's measurement, taken where the bytes actually exist rather than + // estimated: one sample per link, so the steady-state cost is (links this + // frame) x (bytes per link) and both halves are countable from the stats line. + MG_Util::PipeStats::AddBytes(MG_Util::PipeStats::ByteClass::CsoBlobBytes, + static_cast(archive.size())); + } if (archive.empty()) { MGLOG_F("MGPipe: Fatal{ArchiveEmpty, \"create_shader_state\"} - the program's " "artefacts serialised to nothing"); @@ -519,6 +536,71 @@ namespace MobileGL::MG_Remote::Client { ++g_emitted; } + // set_program_bindings (P5e, pg). THE FIFTH ESCAPE, for PipeRoute.h's reason: three + // tails in three index spaces plus a parallel name array, which no generated + // (payload, varTail, varTailCount) row can express. + // + // THE NAMES ARE STAGED ONE BY ONE and each element's own MGHostSpan names its run, so + // the third tail is self-describing the way set_storage_block_binding's single name + // already is. Staged rather than packed into one run with offsets, because the honesty + // pass the decoder runs is per span and a packed run would have to be re-split there + // against arithmetic nothing on the wire declares. + void Wire_Escape_SetProgramBindings(const MG_Pipe::MGPProgramBindings* hdr, + const Int32* blockBindings, + const MG_Pipe::MGPProgramSamplerUnit* samplerUnits, + const MG_Pipe::MGPProgramStorageOverride* storageOverrides, + const char* const* storageOverrideNames) { + if (RunsAsTheServerRole()) { + MG_Pipe::MGPipeMonolithEscapes().SetProgramBindings(hdr, blockBindings, samplerUnits, + storageOverrides, + storageOverrideNames); + return; + } + ClientSession& session = RequireSession("SetProgramBindings"); + MG_Pipe::MGPProgramBindings record = *hdr; + + // The override tail is COPIED before it is emitted, because the span is the one + // field the client must write after the caller is done with it - the same reason + // MGP_WIRE_BLOB copies its payload. + Vector overrides; + overrides.reserve(record.StorageOverrideCount); + for (Uint32 i = 0; i < record.StorageOverrideCount; ++i) { + MG_Pipe::MGPProgramStorageOverride entry = storageOverrides[i]; + const char* const name = storageOverrideNames[i]; + // Size = strlen + 1: THE NUL TRAVELS, exactly as set_storage_block_binding's + // name does (contract table 0's block-name row), and the decoder refuses a run + // whose last byte is not NUL. + const Uint64 nameBytes = static_cast(std::strlen(name)) + 1ull; + const MG_Pipe::MGPBlobRef staged = session.Encoder().StageBytes(name, nameBytes); + // A HOST SPAN AND NOT A BLOBREF, which is what the row's kHostSpan flag means: + // the same run, described in the shape the decoder's honesty pass reads. Ptr + // stays null - rule B, "the encoder writes nullptr and names SEG_STAGE" - so + // the name is unreachable on a second process except through the segment table, + // which is the whole point of the flag. + entry.Name = MG_Pipe::MGHostSpan{}; + entry.Name.Ptr = nullptr; + entry.Name.Seg = staged.Seg; + entry.Name.Offset = staged.Offset; + entry.Name.Size = staged.Size; + overrides.push_back(entry); + } + + const Wire::WireTail tails[3] = { + {blockBindings, static_cast(record.BlockBindingCount) * sizeof(Int32)}, + {samplerUnits, static_cast(record.SamplerUnitCount) * + sizeof(MG_Pipe::MGPProgramSamplerUnit)}, + {overrides.empty() ? nullptr : overrides.data(), + static_cast(record.StorageOverrideCount) * + sizeof(MG_Pipe::MGPProgramStorageOverride)}, + }; + // ALWAYS THREE, even when a count is 0 - the decoder derives the third tail's + // offset from the first two, so declaring fewer would put the same bytes somewhere + // else (PipeWireCodec.cpp's layout arm says this from the other side). + session.EmitAndWaitTails(MGPWireOp::SetProgramBindings, &record, sizeof(record), tails, 3, + nullptr, 0, nullptr); + ++g_emitted; + } + } // namespace // ===================================================================================== @@ -664,6 +746,7 @@ namespace MobileGL::MG_Remote::Client { gMGPipeRouteEscapes.ResourceFlushRange = &Wire_Escape_ResourceFlushRange; gMGPipeRouteEscapes.MapPersistent = &Wire_Escape_MapPersistent; gMGPipeRouteEscapes.CreateShaderState = &Wire_Escape_CreateShaderState; + gMGPipeRouteEscapes.SetProgramBindings = &Wire_Escape_SetProgramBindings; MGPipeNoteInstalledArm(MGPipeRouteArm::kClientWire); } diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 342b3e6e..d8c69a99 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -1756,17 +1756,28 @@ namespace MobileGL::MG_Remote::Wire { std::abort(); } } - const void* archive = ResolveOrFatal(op, desc.Reflection); - MG_State::GLState::LinkArtifacts link; - MG_State::GLState::SpirvArtifacts spirv; - if (!MG_State::GLState::DecodeProgramArtifacts(static_cast(archive), - static_cast(desc.Reflection.Size), - link, spirv)) { + const void* archiveBytes = ResolveOrFatal(op, desc.Reflection); + // P5e (pg), gap G-A: DECODED ONTO THE HEAP AND HANDED TO THE RECORD, not onto this + // stack. Before P5e these were two locals that died at the closing brace and the + // applier kept only the descriptor, so every reflection question the program twin + // later asked went back to the frontend's own ProgramObject - which Link() replaces + // in place, and which a run-ahead client is several records past by then. The + // record adopts the archive here and answers from it for the rest of its life. + // + // AND THE FRAME, not the bare codec stream: ProgramArchive carries the stage of + // each module beside the modules, because SpirvArtifacts does not and StageMask + // cannot stand in for it (two shader objects may share a stage). See + // ProgramArtifactsCodec.h. + auto archive = MakeShared(); + if (!MG_State::GLState::DecodeProgramArchive(static_cast(archiveBytes), + static_cast(desc.Reflection.Size), + *archive)) { WireProtocolFatal("CreateShaderState.Reflection", - "DecodeProgramArtifacts refused the archive - a truncated " - "stream, a codec version mismatch or a struct-size mismatch"); + "DecodeProgramArchive refused the archive - a truncated " + "stream, a codec version mismatch, a struct-size mismatch, or " + "a stage list that does not match the modules it frames"); } - MGPipeApplyCreateShaderState(desc, &link, &spirv); + MGPipeApplyCreateShaderState(desc, &archive->Link, &archive->Spirv, archive); return true; } @@ -1861,12 +1872,11 @@ namespace MobileGL::MG_Remote::Wire { return false; case MGPWireOp::SetProgramBindings: { - // P5e, opcode 80 (MG_Remote/CONTRACT-P5E.md §1). NO APPLIER ENTRY POINT EXISTS YET - // and c0e does not invent one: the record's consumer is the program twin's - // rebuild, which package pg writes. What the record needs NOW and cannot get later - // is its shape - the three declared counts against their bounds and the three-tail - // arithmetic, both of which MGPipeWireRecordLayout above has already run - plus the - // honesty of every name span it carries. + // P5e, opcode 80 (MG_Remote/CONTRACT-P5E.md §1). c0e wrote the shape check and left + // the arm declining; PACKAGE pg COMPLETES IT. What the record needs before its + // consumer sees it is its shape - the three declared counts against their bounds + // and the three-tail arithmetic, both of which MGPipeWireRecordLayout above has + // already run - plus the honesty of every name span it carries. // // THE SPANS ARE WALKED HERE RATHER THAN BY THE ENCODER'S BLANKET PASS because they // are MEMBERS of the third tail's 40-byte elements, not a bare MGHostSpan array: @@ -1874,17 +1884,48 @@ namespace MobileGL::MG_Remote::Wire { // pad} would read one span and a quarter of garbage. All four arms of // CheckHostSpanIsHonest run, the segment-range one included, so an override name // pointing outside SEG_STAGE is caught on the way in rather than at the rebuild. + const auto& bindings = *static_cast(payload); + // RESOLVED HERE AND VALID ONLY FOR THIS CALL (rule C): the names live in the staged + // run this record named, and the applier copies them into the record before it + // returns. A Vector of resolved pointers rather than a second pass inside the + // applier, because segment resolution is the decoder's job and the applier has no + // segment table - it is reached by the monolith adapter too, where the names are + // the frontend's own c_str()s and there is nothing to resolve. + Vector overrideNames; + const MGPProgramStorageOverride* overrides = nullptr; if (layout.TailCount == 3 && layout.TailBytes[2] != 0 && m_segments != nullptr) { - const auto* overrides = tailAt(2); + overrides = reinterpret_cast(tailAt(2)); const Uint64 count = layout.TailBytes[2] / sizeof(MGPProgramStorageOverride); + overrideNames.reserve(static_cast(count)); for (Uint64 i = 0; i < count; ++i) { MGPProgramStorageOverride entry{}; - std::memcpy(&entry, overrides + i * sizeof(MGPProgramStorageOverride), + std::memcpy(&entry, reinterpret_cast(overrides) + + i * sizeof(MGPProgramStorageOverride), sizeof(MGPProgramStorageOverride)); CheckHostSpanIsHonest(entry.Name, *m_segments); + // THE NUL IS THE CLIENT'S AND IS CHECKED, not assumed: the emitter stages + // the name with its terminator (ProgramEmit.h), and a span whose last byte + // is not 0 would make the applier's String(const char*) walk off the end of + // SEG_STAGE. Honest-but-unterminated is the one shape CheckHostSpanIsHonest + // cannot see. + const auto* name = static_cast( + m_segments->Resolve(entry.Name.Seg, entry.Name.Offset, entry.Name.Size)); + if (entry.Name.Size == 0 || name == nullptr || + name[entry.Name.Size - 1] != '\0') { + WireProtocolFatal("SetProgramBindings.StorageOverrides.Name", + "an override name span is empty or is not " + "NUL-terminated; the applier copies it as a C string"); + } + overrideNames.push_back(name); } } - return false; + MGPipeApplySetProgramBindings( + bindings, + layout.TailBytes[0] != 0 ? reinterpret_cast(tailAt(0)) : nullptr, + layout.TailBytes[1] != 0 ? reinterpret_cast(tailAt(1)) + : nullptr, + overrides, overrideNames.empty() ? nullptr : overrideNames.data()); + return true; } case MGPWireOp::SetGlobalConstants: { diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.cpp index 945cf6cb..8d5c31e0 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.cpp @@ -242,6 +242,14 @@ namespace MobileGL::MG_State::GLState { // (ProgramArtifacts.h's libc++ branch until the integrator fills it in) this is 0, // which still round-trips within one build - the echo compares what THIS build wrote // against what THIS build expects - and stops mattering the moment the pin lands. + // + // P5e (pg) READ THIS LINE AS THE CONTRACT ASKED (CONTRACT-P5E §5.5) and its verdict is + // in ProgramArtifactsCodec.h's header comment: the caveat is about the ECHO WORD, not + // about a member the codec skips - the one member VisitFields omits is + // `LinkArtifacts::program`, and no backend twin reads it. A 0 echo is exact for an + // in-process split, where the writer and the reader are one build, and becomes a real + // cross-toolchain check when P6 puts a client and a server in different processes + // built by different compilers. #ifdef MGL_LINKARTIFACTS_SIZE inline constexpr Uint64 kLinkArtifactsSizeEcho = MGL_LINKARTIFACTS_SIZE; #else @@ -300,4 +308,55 @@ namespace MobileGL::MG_State::GLState { link.program = nullptr; return true; } + + // ---- P5e (pg): the frame ------------------------------------------------------------- + // + // ONE MORE LENGTH-PREFIXED RUN IN FRONT, and deliberately in front rather than behind: + // DecodeProgramArtifacts refuses trailing bytes ("the format accounts for every byte it + // writes"), which is a rule worth keeping, so the stage list is consumed BEFORE the + // codec's own stream is handed the exact remainder. The version word inside that stream + // still governs the archive proper; the frame has no version of its own because it is one + // count and one run of Uint32 and there is nothing about it a future reader could + // misinterpret without the count already disagreeing. + void EncodeProgramArchive(const LinkArtifacts& link, const SpirvArtifacts& spirv, + const Vector& linkedStages, Vector& out) { + PutRaw(out, static_cast(linkedStages.size())); + for (const Uint32 stage : linkedStages) PutRaw(out, stage); + EncodeProgramArtifacts(link, spirv, out); + } + + Bool DecodeProgramArchive(const Uint8* bytes, SizeT size, ProgramArchive& out) { + out = ProgramArchive{}; + if (bytes == nullptr) return false; + if (size < sizeof(Uint32)) return false; + + Uint32 stageCount = 0; + std::memcpy(&stageCount, bytes, sizeof(stageCount)); + // BOUNDED BEFORE IT IS MULTIPLIED, the same rule the codec's own TakeCount keeps: a + // corrupt count must not become a four-billion-element resize, and a count past the + // declared maximum is a program this frame cannot describe rather than one to truncate. + if (static_cast(stageCount) > kProgramArchiveMaxStages) return false; + const SizeT framed = sizeof(Uint32) + static_cast(stageCount) * sizeof(Uint32); + if (size < framed) return false; + + out.LinkedStages.resize(stageCount); + for (Uint32 i = 0; i < stageCount; ++i) { + std::memcpy(&out.LinkedStages[i], bytes + sizeof(Uint32) + i * sizeof(Uint32), + sizeof(Uint32)); + } + if (!DecodeProgramArtifacts(bytes + framed, size - framed, out.Link, out.Spirv)) { + out = ProgramArchive{}; + return false; + } + // THE TWO HALVES MUST AGREE, and this is the only place that can say so: the backend + // pairs linkedStages[i] with generatedSpirv[i] and indexes both by one running index, + // so a frame whose count disagrees with the decoded module count would read off the + // end of one of them. The frontend builds both from one snapshot loop, so a mismatch + // here is a wire fault and not a program shape. + if (out.LinkedStages.size() != out.Spirv.generatedSpirv.size()) { + out = ProgramArchive{}; + return false; + } + return true; + } } // namespace MobileGL::MG_State::GLState diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.h b/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.h index 0517d16c..d02980ba 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.h @@ -42,6 +42,26 @@ // * and `LinkArtifacts::program` is NEVER visited. It is the live glslang TProgram, it is // null for every archived instance by construction, and VisitFields deliberately omits it // (57 of the 58 members). Decode leaves it null. +// +// P5e (pg), CONTRACT-P5E §5.5 "codec completeness": THE SKIPPED MEMBER IS NAMED, AND IT IS +// `LinkArtifacts::program`. The contract asks this package either to carry it or to prove the +// twin never reads it, and the proof is a closed grep rather than an argument: every read of +// that member in the tree is inside `ProgramLinkTask.cpp` (`DoReflection`, which is the only +// code that may dereference it at all) and the three sites that `reset()` it - +// `ProgramLinkTask.cpp:678,764,877` and `ProgramObject::ResetLinkArtifacts`. `MG_Backend/` +// contains NO read of it, in either backend; `ProgramTranslationCache` asserts it is null at +// insert. So the archive answers every reflection question the program twin asks, and the one +// member it does not carry is one no twin has ever asked for. It could not be carried in any +// case: it points into a glslang arena that no archived instance owns. +// +// AND THE OTHER HALF OF THAT CONTRACT LINE - `ProgramArtifactsCodec.cpp`'s libc++ caveat - is +// about the STRUCT-SIZE ECHO, not about a member: under a toolchain whose `MGL_LINKARTIFACTS_SIZE` +// is not pinned yet (the NDK's libc++ branch of `ProgramArtifacts.h`, inert until the integrator +// fills it in) the echo word is 0. That is SAFE FOR P5e AND NOT A GAP: the echo compares what +// this build wrote against what this build expects, so a round trip within one build is exact, +// and P5e is an in-process split where the writer and the reader ARE one build. It becomes a +// real check the day a client and a server built by different toolchains share a wire, which is +// P6's boundary and P6's problem - stated here so it cannot be discovered instead. namespace MobileGL::MG_State::GLState { // Bumped whenever the byte format changes in a way a previous reader would misread. A @@ -60,6 +80,45 @@ namespace MobileGL::MG_State::GLState { Bool DecodeProgramArtifacts(const Uint8* bytes, SizeT size, LinkArtifacts& link, SpirvArtifacts& spirv); + // ---- P5e (pg): the archive as the SERVER OWNS it ------------------------------------- + // + // WHAT THE TWO STRUCTS ABOVE DO NOT CARRY, and why a package that makes the server answer + // every reflection question out of them had to notice: THE STAGE OF EACH MODULE. + // `SpirvArtifacts::generatedSpirv` is one module per SHADER OBJECT the link consumed, in + // snapshot order, and the stage of each lives in `ProgramObject::m_linkedShaderSnapshot` - + // GL-thread state, not an artifact. `MGPProgramDesc::StageMask` cannot stand in for it: it + // is a bit SET, and GL lets two shader objects of the same stage be attached to one + // program, so a list rebuilt from the mask would be shorter than `generatedSpirv` and the + // backend's "linked stages and modules must agree" refusal would fire on a perfectly good + // program. + // + // So the frame is [stage count][stage words][the codec's own bytes], written and read by + // this package on both sides, and `ProgramArchive` is what a decode produces: the record's + // OWN copy, with the lifetime of the record rather than of the staged run that carried it + // (rule C). Stages travel as Uint32 rather than as `ShaderStage` so this header keeps its + // include closure - `ProgramArtifacts.h` beside it may not reach `ShaderStage.h`. + struct ProgramArchive { + LinkArtifacts Link; + SpirvArtifacts Spirv; + // ShaderStage, one per Spirv.generatedSpirv entry, at the same index. + Vector LinkedStages; + }; + + // The bound a decode refuses past: six is what MGPProgramDesc::Spirv[] can name and what + // ProgramEmit.h's own truncation counter already watches, doubled so that a program this + // stack can build is never refused here for a reason the emitter did not already count. + inline constexpr SizeT kProgramArchiveMaxStages = 12; + + // Appends the framed archive to `out` (not cleared, so a caller may frame it further). + // `linkedStages` must be index-aligned with `spirv.generatedSpirv`. + void EncodeProgramArchive(const LinkArtifacts& link, const SpirvArtifacts& spirv, + const Vector& linkedStages, Vector& out); + + // Replaces `out` with what `bytes` describes. Same refusal terms as + // DecodeProgramArtifacts, plus: a stage count past kProgramArchiveMaxStages, and a stage + // count that does not match the decoded module count. + Bool DecodeProgramArchive(const Uint8* bytes, SizeT size, ProgramArchive& out); + // How many fields a type's VisitFields table actually visits. The codec walks exactly that // table, so this is what pins "the codec did not quietly grow an arm of its own" - most of // all for LinkArtifacts, whose 58th member is the live TProgram the table omits. It is a diff --git a/MobileGL/MG_Test/Pipe/CompositeResolverTest.cpp b/MobileGL/MG_Test/Pipe/CompositeResolverTest.cpp index b93c9db0..3fa4729e 100644 --- a/MobileGL/MG_Test/Pipe/CompositeResolverTest.cpp +++ b/MobileGL/MG_Test/Pipe/CompositeResolverTest.cpp @@ -279,7 +279,7 @@ TEST(CompositeResolver, ACompositeRecordLandsInTheBandsOwnTableAndNeverGrowsTheO const MGPipeHandle composite{kMGPipeShaderCsoCompositeSlotBase + 2, 1}; ASSERT_TRUE(MGPipeIsCompositeShaderSlot(composite.Slot)); - MGPipeApplyCreateShaderState(CompositeDesc(composite, 0x3u), &link, &spirv); + MGPipeApplyCreateShaderState(CompositeDesc(composite, 0x3u), &link, &spirv, nullptr); EXPECT_TRUE(MGPipeApplier().ShaderCsos.empty()) << "one composite grew the ordinary table to the band's base - that is the 236 MB spike"; ASSERT_EQ(MGPipeApplier().CompositeShaderCsos.size(), 3u) @@ -298,7 +298,7 @@ TEST(CompositeResolver, ACompositeRecordLandsInTheBandsOwnTableAndNeverGrowsTheO // An ordinary program lands in the other table, and the two do not see each other even // though the composite's record is at index 2 of its own. - MGPipeApplyCreateShaderState(CompositeDesc(MGPipeHandle{2, 1}, 0x7u), &link, &spirv); + MGPipeApplyCreateShaderState(CompositeDesc(MGPipeHandle{2, 1}, 0x7u), &link, &spirv, nullptr); ASSERT_GT(MGPipeApplier().ShaderCsos.size(), 2u); EXPECT_EQ(MGPipeApplier().ShaderCsos[2].Desc.StageMask, 0x7u); EXPECT_EQ(MGPipeApplier().CompositeShaderCsos[2].Desc.StageMask, 0x3u) @@ -319,7 +319,7 @@ TEST(CompositeResolver, ASecondDeleteOfACompositeIsARefusedNoOpRatherThanASecond const SpirvArtifacts spirv; const MGPipeHandle composite{kMGPipeShaderCsoCompositeSlotBase, 3}; - MGPipeApplyCreateShaderState(CompositeDesc(composite, 0x3u), &link, &spirv); + MGPipeApplyCreateShaderState(CompositeDesc(composite, 0x3u), &link, &spirv, nullptr); MGPipeApplySetDrawProgram(ProgramHandle(composite)); ASSERT_EQ(MGPipeApplier().DrawProgram, composite); @@ -337,7 +337,7 @@ TEST(CompositeResolver, ASecondDeleteOfACompositeIsARefusedNoOpRatherThanASecond // And the band's slot is re-usable afterwards: a recycled composite is a new identity and // starts its record over. - MGPipeApplyCreateShaderState(CompositeDesc(MGPipeHandle{composite.Slot, 4}, 0x1u), &link, &spirv); + MGPipeApplyCreateShaderState(CompositeDesc(MGPipeHandle{composite.Slot, 4}, 0x1u), &link, &spirv, nullptr); EXPECT_TRUE(MGPipeApplier().CompositeShaderCsos[0].Live); EXPECT_EQ(MGPipeApplier().CompositeShaderCsos[0].Gen, 4u); EXPECT_EQ(MGPipeApplier().CompositeShaderCsos[0].Serial, 0u); @@ -358,7 +358,7 @@ TEST(CompositeResolver, ASlotAtTheShaderCsoLimitIsRefusedWhileTheLastBandSlotIsN // The positive control: the LAST slot of the band is a legal composite handle. const MGPipeHandle last{kMGPipeShaderCsoSlotLimit - 1, 1}; ASSERT_TRUE(MGPipeIsCompositeShaderSlot(last.Slot)); - MGPipeApplyCreateShaderState(CompositeDesc(last, 0x3u), &link, &spirv); + MGPipeApplyCreateShaderState(CompositeDesc(last, 0x3u), &link, &spirv, nullptr); ASSERT_EQ(MGPipeApplier().CompositeShaderCsos.size(), static_cast(kMGPipeShaderCsoSlotLimit - kMGPipeShaderCsoCompositeSlotBase)); EXPECT_TRUE(MGPipeApplier().CompositeShaderCsos.back().Live); @@ -367,7 +367,7 @@ TEST(CompositeResolver, ASlotAtTheShaderCsoLimitIsRefusedWhileTheLastBandSlotIsN const MGPProgramDesc past = CompositeDesc(MGPipeHandle{kMGPipeShaderCsoSlotLimit, 1}, 0x3u); ExpectRefusedNaming("create_shader_state {slot=1048576, gen=1}: the slot is outside the record table's " "bound", - [&past, &link, &spirv]() { MGPipeApplyCreateShaderState(past, &link, &spirv); }); + [&past, &link, &spirv]() { MGPipeApplyCreateShaderState(past, &link, &spirv, nullptr); }); // And an ORDINARY slot at or above the band's base is out of range by definition: the // allocator refuses the band for an ordinary program, so nothing legal can name one. @@ -684,7 +684,7 @@ void main() { o_color = vec4(0.5); } const MGPipeHandle handleB = MGPipeSlots().AllocateComposite(compositeB->GetLifetimeId()); ASSERT_NE(handleA, handleB); for (const MGPipeHandle handle : {handleA, handleB}) { - MGPipeApplyCreateShaderState(CompositeDesc(handle, 0x3u), &link, &spirv); + MGPipeApplyCreateShaderState(CompositeDesc(handle, 0x3u), &link, &spirv, nullptr); MGPipeNoteHandlePublished(MGPipeKind::ShaderCso, handle); } const Uint64 releasesBefore = resolver.GetCounters().Releases; @@ -736,7 +736,7 @@ void main() { o_color = vec4(0.5); } const ProgramPipelineObject pipeline{kPipelineName}; SharedPtr composite = MakeShared(0u); const MGPipeHandle handle = MGPipeSlots().AllocateComposite(composite->GetLifetimeId()); - MGPipeApplyCreateShaderState(CompositeDesc(handle, 0x3u), &link, &spirv); + MGPipeApplyCreateShaderState(CompositeDesc(handle, 0x3u), &link, &spirv, nullptr); MGPipeNoteHandlePublished(MGPipeKind::ShaderCso, handle); ASSERT_EQ(resolver.Observe(kDoomedContext, pipeline, *composite, handle), handle); ASSERT_EQ(resolver.Size(), sizeBefore + 1u); diff --git a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp index 359dd707..dbff069f 100644 --- a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp +++ b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp @@ -255,9 +255,14 @@ TEST(PipeCatalogue, ExactlyTheRoutedRowsAreInstalledAndTheRestAreStillNull) { for (SizeT i = 0; i < sizeof(MGPipeRouteEscapes) / sizeof(void*); ++i) { if (escapes[i] != nullptr) ++escapesInstalled; } - EXPECT_EQ(escapesInstalled, 4u) << "an escape row is null; its call site would take a null " + // P5e (pg) MADE IT FIVE: set_program_bindings carries three tails in three index spaces + // plus a parallel name array, which no generated (payload, varTail, varTailCount) row can + // express - so opcode 80's `gMGPipeContext` slot stays null (pinned below, unchanged) and + // its adapter lives in the escape table beside create_shader_state's. The applier's entry + // points are 39 with it. + EXPECT_EQ(escapesInstalled, 5u) << "an escape row is null; its call site would take a null " "pointer rather than fall back to anything"; - EXPECT_EQ(installed + escapesInstalled, 38u) + EXPECT_EQ(installed + escapesInstalled, 39u) << "the two tables plus the escapes must be exactly PipeApply.h's entry points"; // And the rows that MUST still be null, named rather than counted: these are calls with no diff --git a/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp b/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp index 4f63b495..b9b6d774 100644 --- a/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp @@ -258,7 +258,7 @@ TEST(ProgramEmit, ACreateStoresTheDescriptorAndARelinkCountsUpAndDropsTheBlockKe const MGPipeHandle cso{5, 2}; const Uint8 block[64] = {}; - MGPipeApplyCreateShaderState(ProgramDesc(cso, 0x3u, 64), &link, &spirv); + MGPipeApplyCreateShaderState(ProgramDesc(cso, 0x3u, 64), &link, &spirv, nullptr); EXPECT_TRUE(ProgramRecordOf(5).Live); EXPECT_EQ(ProgramRecordOf(5).Gen, 2u); EXPECT_EQ(ProgramRecordOf(5).Serial, 0u) << "a create is not a mutation"; @@ -274,7 +274,7 @@ TEST(ProgramEmit, ACreateStoresTheDescriptorAndARelinkCountsUpAndDropsTheBlockKe const Uint64 blockSerial = ProgramRecordOf(5).GlobalConstantsSerial; // The relink. - MGPipeApplyCreateShaderState(ProgramDesc(cso, 0x7u, 32), &link, &spirv); + MGPipeApplyCreateShaderState(ProgramDesc(cso, 0x7u, 32), &link, &spirv, nullptr); EXPECT_EQ(ProgramRecordOf(5).Serial, 1u); EXPECT_EQ(ProgramRecordOf(5).Desc.StageMask, 0x7u); EXPECT_TRUE(ProgramRecordOf(5).GlobalConstants.empty()) @@ -285,7 +285,7 @@ TEST(ProgramEmit, ACreateStoresTheDescriptorAndARelinkCountsUpAndDropsTheBlockKe // A RECYCLED SLOT STARTS OVER: inheriting one field of the previous occupant is how a // program at a recycled slot inherits its predecessor's reflection. - MGPipeApplyCreateShaderState(ProgramDesc(MGPipeHandle{5, 3}, 0x1u, 16), &link, &spirv); + MGPipeApplyCreateShaderState(ProgramDesc(MGPipeHandle{5, 3}, 0x1u, 16), &link, &spirv, nullptr); EXPECT_EQ(ProgramRecordOf(5).Gen, 3u); EXPECT_EQ(ProgramRecordOf(5).Serial, 0u) << "a recycled slot kept its predecessor's serial"; EXPECT_EQ(ProgramRecordOf(5).Desc.StageMask, 0x1u); @@ -305,23 +305,23 @@ TEST(ProgramEmit, ACreateWithNoArtefactsAnOversizedBlockOrACorruptSlotIsRefusedN const MGPProgramDesc desc = ProgramDesc(MGPipeHandle{4, 1}, 0x3u, 0); ExpectRefusedNaming("create_shader_state {slot=4, gen=1}: the record declares no blobs and carries no " "artefacts", - [&desc, &spirv]() { MGPipeApplyCreateShaderState(desc, nullptr, &spirv); }); + [&desc, &spirv]() { MGPipeApplyCreateShaderState(desc, nullptr, &spirv, nullptr); }); ExpectRefusedNaming("create_shader_state {slot=4, gen=1}: the record declares no blobs and carries no " "artefacts", - [&desc, &link]() { MGPipeApplyCreateShaderState(desc, &link, nullptr); }); + [&desc, &link]() { MGPipeApplyCreateShaderState(desc, &link, nullptr, nullptr); }); EXPECT_TRUE(MGPipeApplier().ShaderCsos.empty()); const MGPProgramDesc huge = ProgramDesc(MGPipeHandle{4, 1}, 0x3u, kMGPipeMaxGlobalConstantsBytes + 1); ExpectRefusedNaming("create_shader_state {slot=4, gen=1}: the default uniform block is larger than any " "program may declare", - [&huge, &link, &spirv]() { MGPipeApplyCreateShaderState(huge, &link, &spirv); }); + [&huge, &link, &spirv]() { MGPipeApplyCreateShaderState(huge, &link, &spirv, nullptr); }); EXPECT_TRUE(MGPipeApplier().ShaderCsos.empty()) << "the table was grown by a refused record"; const MGPProgramDesc pastTheBound = ProgramDesc(MGPipeHandle{kMGPipeMaxShaderCsoSlots, 1}, 0x3u, 0); ExpectRefusedNaming("create_shader_state {slot=1048576, gen=1}: the slot is outside the record table's " "bound", [&pastTheBound, &link, &spirv]() { - MGPipeApplyCreateShaderState(pastTheBound, &link, &spirv); + MGPipeApplyCreateShaderState(pastTheBound, &link, &spirv, nullptr); }); EXPECT_TRUE(MGPipeApplier().ShaderCsos.empty()); EXPECT_TRUE(MGPipeApplier().CompositeShaderCsos.empty()); @@ -340,8 +340,8 @@ TEST(ProgramEmit, TheThreeBindingsFollowTheirOwnHandleAndADeadOneLeavesThePrevio const SpirvArtifacts spirv; const MGPipeHandle draw{2, 1}; const MGPipeHandle dispatch{3, 1}; - MGPipeApplyCreateShaderState(ProgramDesc(draw, 0x3u, 0), &link, &spirv); - MGPipeApplyCreateShaderState(ProgramDesc(dispatch, 0x20u, 0), &link, &spirv); + MGPipeApplyCreateShaderState(ProgramDesc(draw, 0x3u, 0), &link, &spirv, nullptr); + MGPipeApplyCreateShaderState(ProgramDesc(dispatch, 0x20u, 0), &link, &spirv, nullptr); const Uint64 serialBefore = MGPipeApplier().ProgramBindingSerial; MGPipeApplyBindShaderState(ProgramHandle(draw)); @@ -383,7 +383,7 @@ TEST(ProgramEmit, ADeleteDropsTheRecordAndClearsEveryBindingThatNamedIt) { const LinkArtifacts link; const SpirvArtifacts spirv; const MGPipeHandle cso{6, 4}; - MGPipeApplyCreateShaderState(ProgramDesc(cso, 0x3u, 0), &link, &spirv); + MGPipeApplyCreateShaderState(ProgramDesc(cso, 0x3u, 0), &link, &spirv, nullptr); MGPipeApplyBindShaderState(ProgramHandle(cso)); MGPipeApplySetDrawProgram(ProgramHandle(cso)); MGPipeApplySetDispatchProgram(ProgramHandle(cso)); @@ -420,7 +420,7 @@ TEST(ProgramEmit, TheDefaultUniformBlockLandsOnTheProgramsRecordAndTheSentinelIs const LinkArtifacts link; const SpirvArtifacts spirv; const MGPipeHandle cso{7, 1}; - MGPipeApplyCreateShaderState(ProgramDesc(cso, 0x3u, 8), &link, &spirv); + MGPipeApplyCreateShaderState(ProgramDesc(cso, 0x3u, 8), &link, &spirv, nullptr); Uint8 block[8] = {1, 2, 3, 4, 5, 6, 7, 8}; MGPipeApplySetGlobalConstants(GlobalConstants(cso, 11), block); @@ -473,7 +473,7 @@ TEST(ProgramEmit, TheProgramRecordSurvivesAMakeCurrentWhileTheThreeBindingsDoNot const SpirvArtifacts spirv; const MGPipeHandle cso{8, 1}; const Uint8 block[4] = {9, 9, 9, 9}; - MGPipeApplyCreateShaderState(ProgramDesc(cso, 0x3u, 4), &link, &spirv); + MGPipeApplyCreateShaderState(ProgramDesc(cso, 0x3u, 4), &link, &spirv, nullptr); MGPipeApplySetGlobalConstants(GlobalConstants(cso, 21), block); MGPipeApplyBindShaderState(ProgramHandle(cso)); MGPipeApplySetDrawProgram(ProgramHandle(cso)); diff --git a/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp b/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp index dbd576ee..af60083f 100644 --- a/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp @@ -1522,7 +1522,7 @@ namespace { MGPipeApplySetSamplerViews(viewSet, viewTail); MGPipeApplyBindSamplerStates(stateSet, stateTail); MGPipeApplySetShaderImages(imageSet, imageTail); - MGPipeApplyCreateShaderState(program, &link, &spirv); + MGPipeApplyCreateShaderState(program, &link, &spirv, nullptr); MGPipeApplyBindShaderState(csoHandle); MGPipeApplySetDrawProgram(csoHandle); MGPipeApplySetDispatchProgram(csoHandle); @@ -1583,7 +1583,7 @@ namespace { MGPipeApplySetSamplerViews(viewSet, viewTail); MGPipeApplyBindSamplerStates(stateSet, stateTail); MGPipeApplySetShaderImages(imageSet, imageTail); - MGPipeApplyCreateShaderState(program, &link, &spirv); + MGPipeApplyCreateShaderState(program, &link, &spirv, nullptr); MGPipeApplyBindShaderState(csoHandle); MGPipeApplySetDrawProgram(csoHandle); MGPipeApplySetDispatchProgram(csoHandle); diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 47ca3189..8f6f8340 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1557,8 +1557,14 @@ TEST_F(PipeWireCodecTest, CreateShaderStateCrossesAsOneArchiveAndSixUndeclaredRu } spirv.globalUboScratch.assign(32, 0xAB); + // P5e (pg): THE FRAMED archive is what crosses now - the codec's own stream with the stage + // of each module in front of it. SpirvArtifacts does not carry the stages and StageMask + // cannot stand in for them (two shader objects may share a stage, so a list rebuilt from + // the mask can be shorter than generatedSpirv), and the server pairs the two by one running + // index. See ProgramArtifactsCodec.h. + const Vector stages{0, 1, 2, 3, 4, 5}; Vector archive; - MG_State::GLState::EncodeProgramArtifacts(link, spirv, archive); + MG_State::GLState::EncodeProgramArchive(link, spirv, stages, archive); ASSERT_FALSE(archive.empty()); MGPProgramDesc desc{}; @@ -1582,16 +1588,16 @@ TEST_F(PipeWireCodecTest, CreateShaderStateCrossesAsOneArchiveAndSixUndeclaredRu EXPECT_TRUE(applied); // And the archive really did carry the six modules: decode it the way the arm does. - MG_State::GLState::LinkArtifacts back; - MG_State::GLState::SpirvArtifacts backSpirv; - ASSERT_TRUE(MG_State::GLState::DecodeProgramArtifacts(archive.data(), archive.size(), back, - backSpirv)); - ASSERT_EQ(backSpirv.generatedSpirv.size(), 6u); + MG_State::GLState::ProgramArchive back; + ASSERT_TRUE(MG_State::GLState::DecodeProgramArchive(archive.data(), archive.size(), back)); + ASSERT_EQ(back.Spirv.generatedSpirv.size(), 6u); for (std::size_t stage = 0; stage < 6; ++stage) { - EXPECT_EQ(backSpirv.generatedSpirv[stage].size(), 4 + stage); - EXPECT_EQ(backSpirv.generatedSpirv[stage][0], 0x07230203u + stage); + EXPECT_EQ(back.Spirv.generatedSpirv[stage].size(), 4 + stage); + EXPECT_EQ(back.Spirv.generatedSpirv[stage][0], 0x07230203u + stage); } - EXPECT_TRUE(backSpirv.spirvStatus); + EXPECT_TRUE(back.Spirv.spirvStatus); + // The frame's own half: one stage word per module, at the same index. + EXPECT_EQ(back.LinkedStages, stages); } // ===================================================================================== diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index fc6699ea..3d42e2df 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -1144,6 +1144,9 @@ TEST(PipeRouting, TheInstalledClientArmIsWireAndNotMonolithAndEveryRoutedRowMove C1F_ESCAPE(ResourceFlushRange); C1F_ESCAPE(MapPersistent); C1F_ESCAPE(CreateShaderState); + // P5e (pg): set_program_bindings is the fifth escape - three tails in three index spaces + // plus a parallel name array, which no generated row can express (MG_Pipe/PipeRoute.h). + C1F_ESCAPE(SetProgramBindings); #undef C1F_ESCAPE const SizeT movedContext = CountDifferingCells(gMGPipeContext, MGPipeMonolithContext()); EXPECT_EQ(movedScreen + movedContext, 34u) @@ -1154,8 +1157,8 @@ TEST(PipeRouting, TheInstalledClientArmIsWireAndNotMonolithAndEveryRoutedRowMove "thread under split) or an unrouted row was overwritten"; const SizeT movedEscapes = CountDifferingCells(gMGPipeRouteEscapes, MGPipeMonolithEscapes()); - EXPECT_EQ(movedEscapes, 4u) - << "the four escape routes must move off the monolith escapes too"; + EXPECT_EQ(movedEscapes, 5u) + << "the five escape routes must move off the monolith escapes too"; // Restore the monolith arm for the sibling cases that assert it (and for a clean binary). MGPipeInstallMonolithTables();