From c240557fe0ce7f557b55ea21ad61028acec892bc Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:23:57 -0400 Subject: [PATCH 1/3] [Feat] (MG_Pipe, MG_Remote, P5b/t2): add kCapBackendOwnsXfbCapture, publish it from the server table EndTransformFeedback slot, and answer the capture-ownership probe from it --- MobileGL/MG_Backend/Init.cpp | 18 +++++++++++++++- .../MG_Impl/GLImpl/Drawing/GL_Drawing.cpp | 16 +++++++++++++- MobileGL/MG_Pipe/MGPipeTypes.h | 12 +++++++++++ MobileGL/MG_Remote/CapsCodec.cpp | 8 ++++--- MobileGL/MG_Remote/Client/SlotCaps.h | 21 ++++++++++++++++++- 5 files changed, 69 insertions(+), 6 deletions(-) diff --git a/MobileGL/MG_Backend/Init.cpp b/MobileGL/MG_Backend/Init.cpp index ba4d648b..8a8fb04f 100644 --- a/MobileGL/MG_Backend/Init.cpp +++ b/MobileGL/MG_Backend/Init.cpp @@ -150,7 +150,23 @@ namespace MobileGL::MG_Backend { // kCapNeedsHostIndexBytes and kCapNeedsHostUboBytes must be 0 for the whole of P5 // by ruling - they are the only two things that ask for an MGHostSpan, and 0 is // what keeps every one of them out of the first IPC frame (contract table 0). - session.SetCapabilityBits(0); + // + // P5b t2 (CONTRACT-P5B.md §6.5) PUBLISHES THE ONE BIT P5b ADDS, and this is the + // only place that can: the question kCapBackendOwnsXfbCapture answers is "does the + // SERVER's backend own the transform-feedback capture", and the server's table is + // visible here and nowhere on the client. It is read straight off the table + // ServerLoop::CreateBackend just built - Espryt registers XfbImpl::EndTransformFeedback + // (BackendObject_DirectGLES.cpp:1458) and Magma registers no XFB slot at all - so the + // bit is a statement about THIS backend rather than about a build option, which is + // what makes it survive a backend switch. The client reads it through + // MGL_BACKEND_SLOT_CAP at GL_Drawing.cpp's FixupGsStripCaptureOrder. + Uint64 capBits = 0; + if (const MG_Backend::BackendObject* serverBackend = loop.Backend(); + serverBackend != nullptr && + serverBackend->GetBackendFunctions().GL.EndTransformFeedback != nullptr) { + capBits |= MG_Pipe::kCapBackendOwnsXfbCapture; + } + session.SetCapabilityBits(capBits); session.SetBackend(loop.Backend()); // 3. the handshake, the four segments, and - at its end - the apply thread. diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 4b289545..232d80fb 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -15,6 +15,12 @@ #if MOBILEGL_BUILD_DISAGGREGATED #include #endif +// CONTRACT-P5.md §7 / ID-14: a null check on a GLFunctionsTable slot may not survive into the +// client under split - it becomes a caps-mirror read. SlotCaps.h carries the rule and the test +// that decides which of its two spellings a site takes; in a pull build both expand to exactly +// the check they replaced. P5b t2 converts ONE site in this file - the capture-ownership probe +// in FixupGsStripCaptureOrder - for the reason CONTRACT-P5B.md §6.5 gives. +#include #include "../Getter/GL_Getter.h" namespace MobileGL::MG_Impl::GLImpl { @@ -1287,7 +1293,15 @@ namespace MobileGL::MG_Impl::GLImpl { // Only Vulkan-order captures need this. A backend that runs the capture on its // own GL/ES driver (it owns the span, hence the EndTransformFeedback entry) has // already produced GL's vertex order, and reordering it again would corrupt it. - if (MG_Backend::gBackendFunctionsTable.GL.EndTransformFeedback != nullptr) { + // + // P5b t2 (CONTRACT-P5B.md §6.5): UNDER SPLIT THE TABLE THIS USED TO ASK IS THE CLIENT'S + // EMIT TABLE, whose EndTransformFeedback slot t2 just made non-null for every server - + // so the raw null check would answer "the backend owns the capture" even against Magma, + // which registers no XFB slot at all, and would skip a reorder Magma needs. The question + // is about the SERVER's table, so it is answered from the bit the server publishes. + // Under monolith (and in a pull build) this expands to the null check it replaced, + // character for character. + if (MGL_BACKEND_SLOT_CAP(EndTransformFeedback, MG_Pipe::kCapBackendOwnsXfbCapture)) { return; } if (program == nullptr || !program->HasGsTriangleStripCaptureFixup() || inputPrimitives == 0) { diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index 0c3ec042..c64e919f 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -121,6 +121,18 @@ namespace MobileGL::MG_Pipe { // The server packs named uniform blocks into its own ring and therefore needs the // host bytes of a set_shader_buffers(Uniform) range (D-B8). kCapNeedsHostUboBytes = 1ull << 8, + // P5b t2 (CONTRACT-P5B.md §6.5), the one cap bit P5b adds. The SERVER's backend owns + // the transform-feedback capture, i.e. its own table registers EndTransformFeedback. + // FixupGsStripCaptureOrder (GL_Drawing.cpp:1290) asks that question to decide whether + // the CLIENT must reorder the captured records into GL's vertex order, and it used to + // ask it of gBackendFunctionsTable.GL.EndTransformFeedback - which under split is the + // client's EMIT table, where the slot is non-null the moment t2 installs an emitter, + // for every server. A client talking to Espryt would then be right by accident and a + // client talking to Magma (which registers no XFB slot at all, so the sink DECLINES) + // would skip a reorder Magma needs and hand the application a silently corrupt capture + // buffer. So the answer is the SERVER's table, published as a bit and read through + // MGL_BACKEND_SLOT_CAP. The first of the "XFB span family" bits SlotCaps.h predicted. + kCapBackendOwnsXfbCapture = 1ull << 9, }; struct MGPCaps { diff --git a/MobileGL/MG_Remote/CapsCodec.cpp b/MobileGL/MG_Remote/CapsCodec.cpp index 051bd501..b6d841e7 100644 --- a/MobileGL/MG_Remote/CapsCodec.cpp +++ b/MobileGL/MG_Remote/CapsCodec.cpp @@ -44,9 +44,11 @@ namespace MobileGL::MG_Remote { - // The consumer mask may not collide with the MGPCapBits below it. kCapNeedsHostUboBytes - // is 1<<8 today; this asserts the gap stays a gap rather than trusting the comment. - static_assert((static_cast(MG_Pipe::kCapNeedsHostUboBytes) & kMGCapsConsumerMask) == 0, + // The consumer mask may not collide with the MGPCapBits below it. The HIGHEST allocated + // feature bit is kCapBackendOwnsXfbCapture, 1<<9 (P5b t2 raised it from + // kCapNeedsHostUboBytes' 1<<8); this asserts the gap stays a gap rather than trusting the + // comment, so it has to name whichever bit is currently the top one. + static_assert((static_cast(MG_Pipe::kCapBackendOwnsXfbCapture) & kMGCapsConsumerMask) == 0, "an MGPCapBit has grown into CallMask's consumer block (bits 32..47)"); static_assert(MGCapsServerConsumes(MGCapsConsumerBits(MG_Pipe::kMGPipeSubsystemResources), MG_Pipe::kMGPipeSubsystemResources), diff --git a/MobileGL/MG_Remote/Client/SlotCaps.h b/MobileGL/MG_Remote/Client/SlotCaps.h index 63bcf4b0..ca50ba04 100644 --- a/MobileGL/MG_Remote/Client/SlotCaps.h +++ b/MobileGL/MG_Remote/Client/SlotCaps.h @@ -75,11 +75,30 @@ // GL_Drawing.cpp:844 PatchParameteri. "Absent" means the patch size is never set and every // tessellation draw silently uses the previous one. Class C; it aborts by name. // +// P5b t2 CLOSED THE FIRST OF THOSE TWO, AND THE OTHER SIX SITES NEEDED NOTHING (CONTRACT-P5B.md +// §2 t2, §6.5). The six SPAN sites - :1274 Begin, :1371 End, :1420 Pause, :1435 Resume, :1641 +// Delete, :1673 Bind - are guards over a slot that is now class B in the client's table, so they +// simply call the emitter; their `if (const auto f = ...)` shape is left exactly as it was, +// because under split the slot is non-null and under monolith nothing moved. THE PROBE AT :1290 +// IS THE ONE THAT HAD TO CHANGE, and it is the reason this header said the XFB span family would +// be the first to need a bit: it is not a guard on a call, it is a QUESTION ABOUT THE BACKEND +// asked of a table that under split belongs to the client. It now reads +// MGL_BACKEND_SLOT_CAP(EndTransformFeedback, kCapBackendOwnsXfbCapture), the bit the server sets +// from ITS table in MG_Backend/Init.cpp's InitSplitRoles. Espryt registers the slot and answers +// yes; Magma registers no XFB slot, answers no, and the client keeps reordering for it exactly +// as it does under monolith. GL_Drawing.cpp:844's PatchParameteri stays a plain guard for the +// same reason as the six: the slot is class B now, so "absent" never arises. +// +// The one XFB slot still class C is DeleteTransformFeedback, which CONTRACT-P5B.md gives no row +// (unmeasured); :1641's guard therefore still reaches Fatal{UnmigratedVerb} by name, which is +// the outcome R-4 asks for. +// // WHAT THIS HEADER DELIBERATELY DOES NOT DO. It does not touch the 28 unguarded slots: those // have no probe to convert, and calling one reaches Fatal{UnmigratedVerb, ""} by name, // which is R-4's intent. And it does not invent a cap bit - a new MGPCapBit is an // MGPipeTypes.h edit and that file is c0's, so a family that needs one goes through the -// integrator (the XFB span family is the first that will). +// integrator (the XFB span family is the first that will). It did: P5b's contract granted t2 +// exactly that one bit, kCapBackendOwnsXfbCapture (CONTRACT-P5B.md §6.5, §8), and t2 added it. // // G1: in a build without MOBILEGL_BUILD_DISAGGREGATED both macros expand to the null check the // site already had, so the pull build's code generation is unchanged. From dd293203ca1adbe5969d0c755abd9bc43d06180d Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:23:57 -0400 Subject: [PATCH 2/3] [Feat] (MG_Remote, P5b/t2): emit the four stream-output span rows, the XFB object bind and the patch parameter, and give each ServerVerbSink body its backend call --- MobileGL/MG_Remote/Client/EmitTables.cpp | 164 ++++++++++++++++++++-- MobileGL/MG_Remote/Server/PipeApplier.cpp | 91 ++++++++++-- MobileGL/MG_Remote/Server/PipeApplier.h | 21 ++- 3 files changed, 254 insertions(+), 22 deletions(-) diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index 6af52e8e..d04b5a2e 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -16,6 +16,12 @@ // changes class without changing the arithmetic is a build break rather than a behaviour // change nobody reviewed. // +// P5b MOVES SLOTS FROM C TO B, ONE PACKAGE AT A TIME (MG_Remote/CONTRACT-P5B.md §7). The three +// numbers above are the partition AT THE P5b CONTRACT COMMIT and they are the ones the contract +// states; the arithmetic below is what the tree currently has, and the per-package ownership +// assertions say which package moved which slot. On this head t2 has landed: class B is 5 + 6 +// and class C is 58. +// // THE PRE-VERB HOOKS RUN BEFORE THE RECORD, NEVER AFTER (b1, ID-18). PushPersistentMapsBeforeVerb // publishes the bytes an application wrote through a coherent map with no API call at all, and // MarkGpuWritesForDraw builds the conservative GPU-write set the client now owns. Both describe @@ -428,6 +434,132 @@ namespace MobileGL::MG_Remote::Client { session.PumpControlPlane(); } + // ============================================================================= + // CLASS B - P5b package t2: the transform-feedback spans, the XFB object bind and the + // tessellation patch parameter (MG_Remote/CONTRACT-P5B.md §2 t2). + // ============================================================================= + // + // SIX SLOTS, SIX ROWS, AND NOT ONE OF THEM STARTS A SHADER. Every one of the six is a + // control call - it opens, closes, pauses, resumes or re-targets a capture span, or sets + // the patch size the next tessellation draw uses - so each takes BeforeReadOnlyVerb(), + // which is CONTRACT-P5B.md §4's rule for "a verb that reads buffers but starts no + // shader" naming the XFB and patch controls by hand. The GPU-WRITE MARK FOR THE CAPTURE + // TARGETS IS NOT TAKEN HERE and that is deliberate: it belongs at the END of the span, + // after the record and before GLContext::EndTransformFeedback clears the live bindings, + // which is exactly where b1 already put it (GL_Drawing.cpp's + // MarkEndTransformFeedbackCaptureTargets). Taking it here as well would mark the same + // buffers twice and taking it INSTEAD of there would mark nothing. + // + // RULE D (CONTRACT-P5B.md §0): each record carries the GL arguments the frontend handed + // the backend slot and nothing that is a READING of them. The capture program, the + // capture-buffer bindings, the patch state and the bound XFB object all stay + // BARRIER-PULLED - the server's backend reads the client's gPipeInputs fill of the + // moment, which MGP_FILL at each call site has just written and the verb barrier holds + // still (R-1). That is why these are six two-line emitters and not an XFB protocol. + // + // WHAT MUST HAVE CROSSED BEFORE begin_stream_output, since it is the ordering question + // this package was asked: the capture buffers' own resource records (emitted at their + // own call sites through the resource family, long before this point), the program + // (the CSO/program family, likewise), and the buffer BINDINGS - which do not cross as a + // record at all, because set_stream_output_targets (39) has no producer and no consumer + // and CONTRACT-P5B.md §2 rules it NOT required for t2: under the barrier the server's + // StartPendingTransformFeedback reads them through the kXfbSpan/kDraw pulls + // (GetTransformFeedbackProgram, GetBufferBindingPoint). Producing that row is P9's. + + void EmitBeginTransformFeedback(GLenum primitiveMode) { + ClientSession& session = RequireSession("BeginTransformFeedback"); + BeforeReadOnlyVerb(); + + MG_Pipe::MGPStreamOutputBegin record{}; + // The GL token verbatim (contract table 0's "GL enums on the wire"): the sink hands + // it to the backend slot that takes it, and nothing between here and there reads it. + record.PrimitiveMode = static_cast(primitiveMode); + session.EmitAndWait(MG_Pipe::MGPWireOp::BeginStreamOutput, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitEndTransformFeedback() { + ClientSession& session = RequireSession("EndTransformFeedback"); + BeforeReadOnlyVerb(); + + MG_Pipe::MGPXfbAccounting record{}; + // THE ACCOUNTING IS THE CLIENT'S OWN AND IT IS INFORMATIONAL ON THIS SIDE OF THE + // WIRE: end_stream_output's backend call takes no arguments, and the three numbers + // are what the frontend has counted over this span (CONTRACT-P5B.md §2 t2, the + // companions row). They travel because the row has carried them since P4a and + // because they are what a server-side scatter would need when P9 lands one; the + // sink today calls GL.EndTransformFeedback() and reads none of them. Read here, + // BEFORE GLContext::EndTransformFeedback resets the counters at the call site. + const auto& context = *MG_State::pGLContext; + record.CapturedVertices = context.GetTransformFeedbackCapturedVertices(); + record.PrimitivesWritten = context.GetTransformFeedbackPrimitiveCounter(); + record.PrimitiveMode = static_cast(context.GetTransformFeedbackPrimitiveMode()); + session.EmitAndWait(MG_Pipe::MGPWireOp::EndStreamOutput, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitPauseTransformFeedback() { + ClientSession& session = RequireSession("PauseTransformFeedback"); + BeforeReadOnlyVerb(); + + // Reserved IS zero and the contract says so (MGPStreamOutputControl{Reserved = 0}). + // The row exists to BE the verb boundary - the stamp the server puts up before the + // sink runs - not to carry anything. + MG_Pipe::MGPStreamOutputControl record{}; + record.Reserved = 0; + session.EmitAndWait(MG_Pipe::MGPWireOp::PauseStreamOutput, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitResumeTransformFeedback() { + ClientSession& session = RequireSession("ResumeTransformFeedback"); + BeforeReadOnlyVerb(); + + MG_Pipe::MGPStreamOutputControl record{}; + record.Reserved = 0; + session.EmitAndWait(MG_Pipe::MGPWireOp::ResumeStreamOutput, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitBindTransformFeedback(GLuint name) { + ClientSession& session = RequireSession("BindTransformFeedback"); + BeforeReadOnlyVerb(); + + MG_Pipe::MGPStreamOutputBind record{}; + // THE GL NAME IS NOT AN IDENTITY (ARCHITECTURE 4.2.1) and is carried anyway, because + // it is the key the backend has always used: Espryt indexes its driver objects by it + // (XfbImpl::g_xfbObjects[name], DirectGLES.cpp:1401) and generates the ES object on + // first bind. Name 0 is the default object, which is why the field is not a handle. + record.GlName = static_cast(name); + // Beside it, the identity that WILL dispatch: the frontend's per-object lifetime id, + // process-wide and never reused, which is what survives glGenTransformFeedbacks + // recycling a name. Read AFTER GLContext::BindTransformFeedbackObject at the call + // site, so it is the id of the object being bound and not of the previous one. + record.LifetimeId = MG_State::pGLContext->GetBoundTransformFeedbackLifetimeId(); + session.EmitAndWait(MG_Pipe::MGPWireOp::BindStreamOutput, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitPatchParameteri(GLenum pname, GLint value) { + ClientSession& session = RequireSession("PatchParameteri"); + BeforeReadOnlyVerb(); + + MG_Pipe::MGPPatchParameter record{}; + // GL_PATCH_VERTICES is the only pname that reaches a backend slot - the frontend + // answers GL_PATCH_DEFAULT_*_LEVEL itself and bakes those into the synthesized + // control stage - and the frontend has already rejected every other pname with + // INVALID_ENUM before this call (GL_Drawing.cpp's PatchParameteri). Carried verbatim + // so the sink reproduces the call rather than a reading of it. + record.Pname = static_cast(pname); + record.Value = static_cast(value); + // set_patch_state (43) STILL TRAVELS, at the next validate, and that is not a + // duplicate: it is the applier's working-block copy, this is the driver push Espryt + // does AT THE CALL (DirectGLES.cpp:8760), and both pushes happen today on the + // monolith path too (CONTRACT-P5B.md §2 t2). + session.EmitAndWait(MG_Pipe::MGPWireOp::PatchParameter, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + // ============================================================================= // CLASS A - answered locally from the caps mirror (R-15). NO RECORD, EVER. // ============================================================================= @@ -530,13 +662,14 @@ namespace MobileGL::MG_Remote::Client { X(MemoryBarrierByRegion, void, (GLbitfield)) \ X(ShaderStorageBlockBinding, void, (GLuint, const GLchar*, GLuint)) + // t2 LANDED (CONTRACT-P5B.md §2 t2): the three measured slots - BeginTransformFeedback + // (95 lane entries), PatchParameteri (43), BindTransformFeedback (2) - and the three + // companions that share their rows are class B now and live in the block above. + // DeleteTransformFeedback is the one that stays: it has NO ROW in P5b, by ruling and + // not by omission (unmeasured; the driver object leaks on the server until P9's XFB + // namespace work, and a bind of name 0 is what the backend does on delete of the bound + // one, DirectGLES.cpp:1422). It therefore still aborts by its own name. #define MGR_UNMIGRATED_T2_SLOTS(X) \ - X(PatchParameteri, void, (GLenum, GLint)) \ - X(BeginTransformFeedback, void, (GLenum)) \ - X(EndTransformFeedback, void, ()) \ - X(PauseTransformFeedback, void, ()) \ - X(ResumeTransformFeedback, void, ()) \ - X(BindTransformFeedback, void, (GLuint)) \ X(DeleteTransformFeedback, void, (GLuint)) #define MGR_UNMIGRATED_F1_SLOTS(X) \ @@ -645,7 +778,9 @@ namespace MobileGL::MG_Remote::Client { constexpr Uint32 kEmittedSlotsP5 = 5; // Clear, DrawArrays, ReadPixels, Blit, Present constexpr Uint32 kEmittedSlotsD1 = 0; constexpr Uint32 kEmittedSlotsI1 = 0; - constexpr Uint32 kEmittedSlotsT2 = 0; + // t2: BeginTransformFeedback, EndTransformFeedback, PauseTransformFeedback, + // ResumeTransformFeedback, BindTransformFeedback, PatchParameteri. + constexpr Uint32 kEmittedSlotsT2 = 6; constexpr Uint32 kEmittedSlotsF1 = 0; constexpr Uint32 kEmittedSlots = kEmittedSlotsP5 + kEmittedSlotsD1 + kEmittedSlotsI1 + kEmittedSlotsT2 + kEmittedSlotsF1; @@ -660,7 +795,10 @@ namespace MobileGL::MG_Remote::Client { static_assert(kUnmigratedT2 + kEmittedSlotsT2 == 7, "t2 owns the 7 XFB/tessellation slots"); static_assert(kUnmigratedF1 + kEmittedSlotsF1 == 11, "f1 owns the 11 clear/copy/mip slots"); static_assert(kUnmigratedTail == 20, "the wave-3 tail is 20 slots and no P5b package owns one"); - static_assert(kUnmigratedSlots == 64, "CONTRACT-P5.md §7 class C is 64 slots at the P5b contract commit"); + // 64 at the P5b contract commit, minus the six t2 flipped. CONTRACT-P5.md §7's number + // is the one above, not this one; this is the arithmetic after t2 and it moves again + // for every package that lands. + static_assert(kUnmigratedSlots == 58, "class C is 58 slots after t2's six"); static_assert(kLocallyAnsweredSlots + kEmittedSlots + kUnmigratedSlots == kRemoteEmitSlotCount, "the three classes no longer partition the 71 slots"); @@ -697,6 +835,16 @@ namespace MobileGL::MG_Remote::Client { table.GL.ReadPixels = &EmitReadPixels; table.GL.BlitFramebuffer = &EmitBlitFramebuffer; table.Present = &EmitPresent; + // ---- class B, P5b t2. Assigned AFTER the class-C block above, which is what makes + // the flip a single-line change per slot: the Fatal thunk is overwritten, and a slot + // whose row is removed from MGR_UNMIGRATED_T2_SLOTS but not assigned here would be + // NULL and caught by RemoteEmitTable.NoSlotIsNull rather than silently skipped. + table.GL.BeginTransformFeedback = &EmitBeginTransformFeedback; + table.GL.EndTransformFeedback = &EmitEndTransformFeedback; + table.GL.PauseTransformFeedback = &EmitPauseTransformFeedback; + table.GL.ResumeTransformFeedback = &EmitResumeTransformFeedback; + table.GL.BindTransformFeedback = &EmitBindTransformFeedback; + table.GL.PatchParameteri = &EmitPatchParameteri; return table; } diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index cae264b6..0467aa60 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -382,35 +382,104 @@ namespace MobileGL::MG_Remote::Server { ServerUnmigratedVerbFatal("ShaderStorageBlockBinding"); } - // ---- t2 ---- + // ---- t2 ---- (MG_Remote/CONTRACT-P5B.md §2 t2) + // + // SIX BODIES, SIX BACKEND CALLS, NO STATE OF THEIR OWN. Rule D: the record IS the call, and + // everything the backend reads around it - the capture program, the capture-buffer + // bindings, the bound XFB object, the patch state - it reads from gPipeInputs through its + // verb class's BARRIER-PULLED fields, which the client filled at the call site and the + // verb barrier holds still (R-1). That is why none of these touches m_backend beyond + // Table() and why not one of them caches anything across records. + // + // A NULL SLOT DECLINES, AND THE DECLINE IS THE MONOLITH'S ANSWER IN THE SAME WORDS. Magma + // (DirectVulkan) registers NO XFB slot and no PatchParameteri at all + // (BackendObject_DirectVulkan.cpp), and under monolith the frontend's own + // `if (const auto f = table.GL.X)` guard simply skips the call; `return false` here is that + // same skip, reported to DecodeAndApply as "this build did not apply it" rather than as a + // crash or as a silent success. Contract §2 t2 says so for PatchParameteri by name. + Bool ServerVerbSink::OnBeginStreamOutput(const MG_Pipe::MGPStreamOutputBegin& begin) { - (void)begin; - ServerUnmigratedVerbFatal("BeginTransformFeedback"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("begin_stream_output"); + if (table == nullptr) return false; + if (table->GL.BeginTransformFeedback == nullptr) return false; + // Espryt's Begin only ARMS the span (DirectGLES.cpp:1212-1220: primitiveMode, pending, + // targets cleared); the driver glBeginTransformFeedback happens in the tail of the next + // PrepareForDraw (StartPendingTransformFeedback, :1224), where the capture program and + // the buffer bindings are read through the pulls. So this record's effect is not + // visible until a DRAW crosses - which is why an XFB scenario whose draw is still class + // C moves its first blocker to that draw rather than rendering. + table->GL.BeginTransformFeedback(static_cast(begin.PrimitiveMode)); + ++m_streamOutputSpans; + return true; } Bool ServerVerbSink::OnEndStreamOutput(const MG_Pipe::MGPXfbAccounting& accounting) { + const MG_Backend::GlobalBackendFunctionsTable* table = Table("end_stream_output"); + if (table == nullptr) return false; + if (table->GL.EndTransformFeedback == nullptr) return false; + // THE THREE ACCOUNTING FIELDS ARE NOT READ, AND THAT IS THE RULING RATHER THAN AN + // OMISSION. glEndTransformFeedback takes no arguments; the numbers are the CLIENT's own + // per-span accounting (contract §2 t2's companions row) and the client is where they are + // consumed - by the primitive queries and by the capture-capacity clamp. A server that + // second-guessed them from its own driver would be publishing a second answer to a + // question the frontend already answers, and the second answer is the one that goes + // stale. They cross because the row has carried them since P4a and because P9's + // server-side scatter is what will need them. (void)accounting; - ServerUnmigratedVerbFatal("EndTransformFeedback"); + table->GL.EndTransformFeedback(); + ++m_streamOutputSpans; + return true; } Bool ServerVerbSink::OnPauseStreamOutput(const MG_Pipe::MGPStreamOutputControl& control) { - (void)control; - ServerUnmigratedVerbFatal("PauseTransformFeedback"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("pause_stream_output"); + if (table == nullptr) return false; + if (table->GL.PauseTransformFeedback == nullptr) return false; + (void)control; // Reserved, and the contract says it is 0. + table->GL.PauseTransformFeedback(); + ++m_streamOutputControls; + return true; } Bool ServerVerbSink::OnResumeStreamOutput(const MG_Pipe::MGPStreamOutputControl& control) { + const MG_Backend::GlobalBackendFunctionsTable* table = Table("resume_stream_output"); + if (table == nullptr) return false; + if (table->GL.ResumeTransformFeedback == nullptr) return false; (void)control; - ServerUnmigratedVerbFatal("ResumeTransformFeedback"); + table->GL.ResumeTransformFeedback(); + ++m_streamOutputControls; + return true; } Bool ServerVerbSink::OnBindStreamOutput(const MG_Pipe::MGPStreamOutputBind& bind) { - (void)bind; - ServerUnmigratedVerbFatal("BindTransformFeedback"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("bind_stream_output"); + if (table == nullptr) return false; + if (table->GL.BindTransformFeedback == nullptr) return false; + // THE GL NAME IS THE ARGUMENT, NOT THE LifetimeId BESIDE IT. Espryt keys its driver + // objects by the GL name (XfbImpl::g_xfbObjects[name], DirectGLES.cpp:1401) and creates + // the ES object on first bind; passing the lifetime id would index a map that has never + // heard of it and silently create a second driver object per bind. The lifetime id + // travels as the identity P7/P9 will dispatch on once the XFB namespace has a wire + // lifetime of its own - it has no reader on this side today, and pretending otherwise + // by folding it into the key is exactly the "a GL name is never an identity" confusion + // the contract's GlName row is written against. + table->GL.BindTransformFeedback(static_cast(bind.GlName)); + ++m_streamOutputBinds; + return true; } Bool ServerVerbSink::OnPatchParameter(const MG_Pipe::MGPPatchParameter& patch) { - (void)patch; - ServerUnmigratedVerbFatal("PatchParameteri"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("patch_parameter"); + if (table == nullptr) return false; + // Magma registers no PatchParameteri: it compiles the patch size into its synthesized + // control stage from set_patch_state instead, so the DECLINE below is the whole of the + // right answer for that backend and not a gap (contract §2 t2). + if (table->GL.PatchParameteri == nullptr) return false; + // Pname is GL_PATCH_VERTICES and the frontend has already rejected every other spelling + // with INVALID_ENUM before the record was built, so this is a forward and not a switch. + table->GL.PatchParameteri(static_cast(patch.Pname), static_cast(patch.Value)); + ++m_patchParameters; + return true; } // ---- f1 ---- diff --git a/MobileGL/MG_Remote/Server/PipeApplier.h b/MobileGL/MG_Remote/Server/PipeApplier.h index 43b09c1b..0dd0d553 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.h +++ b/MobileGL/MG_Remote/Server/PipeApplier.h @@ -121,9 +121,9 @@ namespace MobileGL::MG_Remote::Server { // i1 OnLaunchGrid ("DispatchCompute"), OnMemoryBarrier, OnResourceCopyRegion // ("CopyImageSubData"), OnBindShaderImage ("BindImageTexture"), // OnSetStorageBlockBinding ("ShaderStorageBlockBinding") - // t2 OnBeginStreamOutput / OnEndStreamOutput / OnPauseStreamOutput / - // OnResumeStreamOutput ("*TransformFeedback"), OnBindStreamOutput - // ("BindTransformFeedback"), OnPatchParameter ("PatchParameteri") + // t2 LANDED. OnBeginStreamOutput / OnEndStreamOutput / OnPauseStreamOutput / + // OnResumeStreamOutput / OnBindStreamOutput / OnPatchParameter are real bodies + // now: the backend call the contract names, the null-slot DECLINE, a tally. // f1 OnGenerateMipmap, OnCopyFramebufferToTexture ("CopyTexImage2D" / // "CopyTexSubImage2D"), and OnClear's four non-Whole kinds (live already) // d1 OnDrawVbo above: the indirect tail, the user-index span, NumDraws > 1 and the @@ -158,6 +158,17 @@ namespace MobileGL::MG_Remote::Server { // DstSize is exactly the heap overflow codex 1 found, one field over. Uint64 ReadbackScratchBytes() const { return static_cast(m_readbackScratch.size()); } + // P5b t2's tallies, for the same reason the five above exist (R-16): under split "the + // scenario passed" is also what a scenario that ran entirely on the monolith path looks + // like, so a lane that wants to say the XFB spans CROSSED has to read a counter the + // server moved. Spans counts Begin and End together - they are one span and a lane that + // saw only one of them has a bug the two-counter version would have hidden behind a + // sum; controls counts Pause and Resume; binds and patch parameters count their own. + Uint64 StreamOutputSpans() const { return m_streamOutputSpans; } + Uint64 StreamOutputControls() const { return m_streamOutputControls; } + Uint64 StreamOutputBinds() const { return m_streamOutputBinds; } + Uint64 PatchParameters() const { return m_patchParameters; } + private: const MG_Backend::GlobalBackendFunctionsTable* Table(const char* verb) const; @@ -169,6 +180,10 @@ namespace MobileGL::MG_Remote::Server { Uint64 m_presents = 0; Uint64 m_lastPresentSerial = 0; Uint64 m_readbackBytes = 0; + Uint64 m_streamOutputSpans = 0; + Uint64 m_streamOutputControls = 0; + Uint64 m_streamOutputBinds = 0; + Uint64 m_patchParameters = 0; // ReadPixels' destination. The pixels go into the reply slot, but GLFunctionsTable:: // ReadPixels writes into a caller buffer, so one staging vector per session sits // between them. Grown, never shrunk, and never handed out past the call. From c831ab03902362eed39ef2a4e388be6f26d5ac92 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:23:57 -0400 Subject: [PATCH 3/3] [Test] (MG_Test, MG_IntegrationTest, P5b/t2): pin the span order and the accounting fields, the six flipped slots by name, and run the tessellation-capture and buffer-reuse scenarios under inproc --- MobileGL/MG_IntegrationTest/CMakeLists.txt | 47 ++++++++ .../Harness/SplitLogPaths.cmake.in | 5 +- MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 107 ++++++++++++++++++ MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 98 +++++++++++++++- 4 files changed, 254 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index af59f21a..fb69e532 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -1935,6 +1935,53 @@ if (MOBILEGL_BUILD_DISAGGREGATED) ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" ) + # ---- P5b t2: the transform-feedback spans, the XFB object bind, the patch parameter ------- + # + # MG_Remote/CONTRACT-P5B.md §2 t2. The three P5 lanes above are the REDUCED PATH's targets + # (a clear, a triangle, a persistent map); these two are the first lane entries whose GREEN + # DEPENDS ON A t2 RECORD HAVING CROSSED AND BEEN APPLIED BY THE BACKEND, which is the only + # statement a package flipping a verb can make that a unit case cannot: + # + # TessellationXfbCaptureScenario covers BOTH halves of t2 in one workload. Every case + # calls glPatchParameteri(GL_PATCH_VERTICES, n) and then draws GL_PATCHES into a + # transform-feedback capture, and asserts the CAPTURED BYTES. A patch_parameter (73) + # that did not reach the server tessellates at the previous patch size and the capture + # is the wrong length; a begin/end_stream_output (62/63) that did not reach it leaves + # the buffer holding the scenario's poison value, which is what those cases print. + # XfbCaptureBufferReuseScenario is the span family alone, across four buffer lifetimes + # (a buffer per span, one immutable-storage buffer, one respecified buffer, a + # respecification that changes the capture size). It is the case that would notice a + # span whose END crossed but whose BEGIN did not, because the second span's bytes + # would be the first span's. + # + # Both are DirectGLES only, deliberately, and the reason is measured rather than assumed: + # under Magma the capture is written into the server's resident slice and there is no route + # back, because MG_Backend/Init.cpp's ConsumedSubsystemsFor(DirectVulkan) withholds + # kMGPipeSubsystemResources, so BufferObject::SyncGpuWrites() emits no resource_readback. + # Every DirectVulkan XFB capture entry in the inproc census fails on exactly that (t2-v1.md + # §"the 51"), and it retires with P7 (Magma's resource family) / P9 (the readback carrier) - + # not here. A DirectVulkan arm of this lane would be red for a reason t2 cannot fix. + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectGLES.Split." + TEST_LIST MGL_SPLIT_T2_TESS_TESTS + TEST_FILTER "TessellationXfbCaptureScenario.*" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-gpu\;integration-split" + TIMEOUT ${MGL_ITEST_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" + ) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectGLES.Split." + TEST_LIST MGL_SPLIT_T2_XFB_TESTS + TEST_FILTER "XfbCaptureBufferReuseScenario.*" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-gpu\;integration-split" + TIMEOUT ${MGL_ITEST_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" + ) + # The split half of the counting pair. MGITEST_PERSISTENT_MAP_ARM=emulated is R-6: under split # the adopt tier is pinned at T2, the resource owner declines every acquisition and the client # pushes the mapping's dirty blocks - so pmap must be non-zero and mpr must be the monolith diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in b/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in index 532ba453..757addcb 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in +++ b/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in @@ -1,7 +1,10 @@ # Included by CTest after all GoogleTest discovery files (ID-53). # CTest appends ENVIRONMENT here; keep all previously discovered lane settings. file(MAKE_DIRECTORY "@CMAKE_CURRENT_BINARY_DIR@/split-logs") -foreach(entry IN LISTS MGL_SPLIT_CLEAR_TESTS MGL_SPLIT_TRIANGLE_TESTS MGL_SPLIT_PMAP_TESTS) +# P5b t2's two lanes ride the same rule: one private log path per entry, or +# SplitLogPaths.PrivateAndDistinct is red for them (ID-53). +foreach(entry IN LISTS MGL_SPLIT_CLEAR_TESTS MGL_SPLIT_TRIANGLE_TESTS MGL_SPLIT_PMAP_TESTS + MGL_SPLIT_T2_TESS_TESTS MGL_SPLIT_T2_XFB_TESTS) set_tests_properties("${entry}" PROPERTIES ENVIRONMENT "MOBILEGL_LOG_FILE_PATH=@CMAKE_CURRENT_BINARY_DIR@/split-logs/${entry}.log") endforeach() diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 7b909290..f846033c 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1204,6 +1204,113 @@ TEST_F(PipeWireCodecTest, BindStreamOutputReachesTheSink) { EXPECT_EQ(wire.Sink().StreamOutputBinds[0].LifetimeId, 0x1234567890ull); } +// ===================================================================================== +// P5b t2 (MG_Remote/CONTRACT-P5B.md §2 t2). c0b's cases above round-trip each row once; these +// pin the fields t2's EMITTERS actually fill and the one ordering property the span family has. +// ===================================================================================== + +TEST_F(PipeWireCodecTest, EndStreamOutputCarriesAllThreeAccountingFieldsAndNotJustTheVertices) { + // t2's emitter fills all three from the frontend's own per-span accounting + // (GetTransformFeedbackCapturedVertices / GetTransformFeedbackPrimitiveCounter / + // GetTransformFeedbackPrimitiveMode) and the row above asserts only CapturedVertices, so a + // codec that dropped either of the other two - or an emitter that left them zero - reads + // green there. THE THREE ARE DELIBERATELY DIFFERENT NUMBERS: with 300/300 a swap of the two + // 64-bit fields is invisible. + // + // Red once by making the recorder above push a default-constructed MGPXfbAccounting{} + // instead of the one the decoder handed it - the shape of a seam that loses the payload: + // "end_stream_output lost the primitives-written half of its accounting". + Wire2 wire; + MGPXfbAccounting end{}; + end.CapturedVertices = 21; + end.PrimitivesWritten = 7; + end.PrimitiveMode = 0x0000; // GL_POINTS - and 0 is a legal primitive mode, not "unset" + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::EndStreamOutput, &end, sizeof(end)), kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_TRUE(applied); + ASSERT_EQ(wire.Sink().Ends.size(), 1u); + EXPECT_EQ(wire.Sink().Ends[0].CapturedVertices, 21u); + EXPECT_EQ(wire.Sink().Ends[0].PrimitivesWritten, 7u) + << "end_stream_output lost the primitives-written half of its accounting"; + EXPECT_EQ(wire.Sink().Ends[0].PrimitiveMode, 0u) + << "GL_POINTS is 0 and a row that treats 0 as 'no mode' has made a legal capture " + "indistinguishable from an unfilled record"; +} + +TEST_F(PipeWireCodecTest, BindStreamOutputOfNameZeroIsTheDefaultObjectAndNotAnAbsentOne) { + // THE ONE NAME t2 CANNOT TREAT AS "NOTHING". CONTRACT-P5B.md gives DeleteTransformFeedback + // no row, and the backend's answer to deleting the bound object is a bind of NAME 0 + // (DirectGLES.cpp:1422) - so a row that folded 0 into kMGPipeNullHandle, or a sink that read + // 0 as "no object", would silently stop rebinding the default object and leave the driver + // bound to a deleted one. The lifetime id beside it is 0 too here, which is the frontend's + // seed value for the default object, so this case also pins that a wholly-zero record is + // legal and applies rather than being refused as unfilled. + // + // Red once by making the sink's OnBindStreamOutput refuse GlName == 0 (return false): the + // EXPECT_TRUE(applied) below failed, which is the shape the wrong reading would take. + Wire2 wire; + MGPStreamOutputBind bind{}; + bind.GlName = 0; + bind.LifetimeId = 0; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindStreamOutput, &bind, sizeof(bind)), kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_TRUE(applied) << "a bind of the default transform-feedback object did not apply"; + ASSERT_EQ(wire.Sink().StreamOutputBinds.size(), 1u); + EXPECT_EQ(wire.Sink().StreamOutputBinds[0].GlName, 0u); +} + +TEST_F(PipeWireCodecTest, AWholeCaptureSpanReachesTheSinkInTheOrderTheClientEmittedIt) { + // THE PROPERTY A PER-ROW ROUND TRIP CANNOT STATE. A capture span is five calls whose MEANING + // is their order - bind the object, open the span, pause it, resume it, close it - and every + // one of them rides its own row with no sequence field of its own to check. The decoder's + // ordering is the ring's, so this is a pin on the seam rather than a new guarantee: if a + // later change ever batches or reorders records per row, a capture reordered into + // bind/begin/end/pause/resume applies five records, sets `applied` five times, and leaves + // every per-row case above green while the capture is destroyed. + // + // Red once by encoding the Pause AFTER the End: the interleaved-order EXPECT below failed on + // Pauses being 0 at the point the End was seen. + Wire2 wire; + MGPStreamOutputBind bind{}; + bind.GlName = 3; + bind.LifetimeId = 0x5150ull; + MGPStreamOutputBegin begin{}; + begin.PrimitiveMode = 0x0004; // GL_TRIANGLES + MGPStreamOutputControl control{}; + MGPXfbAccounting end{}; + end.CapturedVertices = 9; + end.PrimitivesWritten = 3; + end.PrimitiveMode = 0x0004; + + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindStreamOutput, &bind, sizeof(bind)), kInvalidSeq); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BeginStreamOutput, &begin, sizeof(begin)), kInvalidSeq); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::PauseStreamOutput, &control, sizeof(control)), kInvalidSeq); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::ResumeStreamOutput, &control, sizeof(control)), kInvalidSeq); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::EndStreamOutput, &end, sizeof(end)), kInvalidSeq); + + // Pumped ONE AT A TIME with the sink read between pumps, which is what makes this a + // statement about order rather than about totals: after the third record the pause must + // have happened and the end must NOT have. + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_EQ(wire.Sink().StreamOutputBinds.size(), 1u) << "the object bind did not come first"; + EXPECT_EQ(wire.Sink().Begins.size(), 0u); + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_EQ(wire.Sink().Begins.size(), 1u) << "the span did not open second"; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_EQ(wire.Sink().Pauses, 1u) << "the pause did not arrive third"; + EXPECT_EQ(wire.Sink().Ends.size(), 0u) + << "the span closed before it was paused - the capture's order did not survive the wire"; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_EQ(wire.Sink().Resumes, 1u); + ASSERT_TRUE(wire.PumpOne(&applied)); + ASSERT_EQ(wire.Sink().Ends.size(), 1u); + EXPECT_EQ(wire.Sink().Ends[0].PrimitivesWritten, 3u); + EXPECT_EQ(wire.Decoder().AppliedSeq(), 5u); +} + TEST_F(PipeWireCodecTest, SetStorageBlockBindingCarriesItsNameAsAStagedBlob) { // i1: the ONE string on the wire. Size is strlen + 1 - the NUL travels - and the decoder // hands the sink a pointer that dies with the call. diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index acef2348..385a90bb 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -174,13 +174,107 @@ TEST(RemoteEmitTable, TheThreeClassesPartitionAllSeventyOneSlots) { // CONTRACT-P5.md §7: 2 answered locally + 5 emitted + 64 Fatal. Read from the functions the // table itself reports with - which is also what t1's arming condition reads - rather than // recomputed here, so a table that lost an emitter cannot look like one that never had it. + // + // P5b MOVES THE SECOND AND THIRD NUMBERS, ONE PACKAGE AT A TIME (CONTRACT-P5B.md §7). On + // this head t2 has landed its six XFB / patch slots, so it is 2 + 11 + 58. The SUM is what + // actually has to hold, and it is asserted separately below for that reason; the two moving + // numbers are spelled out anyway so a package that flips a slot without owning it has to + // edit this line and say so. EXPECT_EQ(LocallyAnsweredSlotCount(), 2u); - EXPECT_EQ(ImplementedVerbCount(), 5u); - EXPECT_EQ(UnmigratedSlotCount(), 64u); + EXPECT_EQ(ImplementedVerbCount(), 11u) << "P5's five class-B verbs plus t2's six"; + EXPECT_EQ(UnmigratedSlotCount(), 58u) << "64 at the P5b contract commit, minus t2's six"; EXPECT_EQ(LocallyAnsweredSlotCount() + ImplementedVerbCount() + UnmigratedSlotCount(), kRemoteEmitSlotCount); } +TEST(RemoteEmitTable, TheSixXfbAndPatchSlotsAreNonNullAndDistinct) { + // P5b t2 (CONTRACT-P5B.md §2 t2), the half that needs no fork: the six slots exist and are + // six DIFFERENT functions. Six identical pointers would be one emitter assigned six times, + // which is how a copy-paste flip loses five records and still passes every count. + // + // Red once by assigning `table.GL.PauseTransformFeedback = &EmitResumeTransformFeedback;` + // in BuildRemoteEmitTable - the exact copy-paste this guards: "t2 slots 2 and 3 are one + // function". + const MG_Backend::GlobalBackendFunctionsTable& table = RemoteEmitTable(); + const void* const six[] = { + reinterpret_cast(table.GL.BeginTransformFeedback), + reinterpret_cast(table.GL.EndTransformFeedback), + reinterpret_cast(table.GL.PauseTransformFeedback), + reinterpret_cast(table.GL.ResumeTransformFeedback), + reinterpret_cast(table.GL.BindTransformFeedback), + reinterpret_cast(table.GL.PatchParameteri), + }; + for (SizeT i = 0; i < 6; ++i) { + EXPECT_NE(six[i], nullptr) << "t2 slot " << i << " is null"; + for (SizeT j = i + 1; j < 6; ++j) { + EXPECT_NE(six[i], six[j]) << "t2 slots " << i << " and " << j << " are one function"; + } + } + // And the one XFB slot t2 does NOT flip is still there to be Fatal - CONTRACT-P5B.md gives + // DeleteTransformFeedback no row (unmeasured), so it must not have been swept up. + EXPECT_NE(table.GL.DeleteTransformFeedback, nullptr); +} + +#if MGTEST_HAVE_FORK +TEST(RemoteEmitTable, EachXfbAndPatchSlotIsClassBAndDemandsASessionByItsOwnName) { + // P5b t2, THE HALF THAT DECIDES THE CLASS. A pointer comparison cannot tell a class-B + // emitter from a class-C thunk - each unmigrated slot gets its own generated function, so + // every slot in the table is already a distinct non-null address. What distinguishes them is + // WHAT THEY SAY when called with no ClientSession: an emitter reaches RequireSession and + // dies Fatal{NoClientSession, ""}; a thunk dies Fatal{UnmigratedVerb, ""}. Both + // strings are asserted, because a case that only looked for the first would be satisfied by + // a build where every one of these had been flipped by accident. + // + // Red once by SWAPPING the Pause and Resume assignments in BuildRemoteEmitTable - the two + // emitters with the same signature, so the swap compiles and neither is orphaned (the first + // attempt redirected one slot at another's emitter and the build failed on the signature + // and on -Wunused-function, which is a control that did not run). The child called through + // PauseTransformFeedback died Fatal{NoClientSession, "ResumeTransformFeedback"} and this + // case failed with "PauseTransformFeedback did not reach the class-B emitter's session + // demand", so the string really is the slot's own name and not a shared constant. + struct Slot { + const char* Name; + void (*Call)(); + }; + static const Slot kSlots[] = { + {"BeginTransformFeedback", [] { RemoteEmitTable().GL.BeginTransformFeedback(0x0004); }}, + {"EndTransformFeedback", [] { RemoteEmitTable().GL.EndTransformFeedback(); }}, + {"PauseTransformFeedback", [] { RemoteEmitTable().GL.PauseTransformFeedback(); }}, + {"ResumeTransformFeedback", [] { RemoteEmitTable().GL.ResumeTransformFeedback(); }}, + {"BindTransformFeedback", [] { RemoteEmitTable().GL.BindTransformFeedback(0); }}, + {"PatchParameteri", [] { RemoteEmitTable().GL.PatchParameteri(0x8E72, 3); }}, + }; + for (const Slot& slot : kSlots) { + const ChildResult r = RunInChild([&slot] { slot.Call(); }); + ASSERT_TRUE(DiedOfAbort(r)) << slot.Name << ": " << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find(std::string("Fatal{NoClientSession, \"") + slot.Name + "\"}"), + std::string::npos) + << slot.Name << " did not reach the class-B emitter's session demand:\n" + << r.Log; + EXPECT_EQ(r.Log.find("Fatal{UnmigratedVerb"), std::string::npos) + << slot.Name << " is still class C:\n" + << r.Log; + } +} + +TEST(RemoteEmitTable, DeleteTransformFeedbackHasNoRowAndStillAbortsByItsOwnName) { + // CONTRACT-P5B.md §2 t2 and c0b-v1.md §6: the seventh slot in t2's ownership block gets NO + // row in P5b - it is unmeasured, the driver object leaks on the server until P9's XFB + // namespace work, and a bind of name 0 is what the backend does on delete of the bound one. + // That is a RULING, so it is pinned rather than left to be re-derived from a count: a later + // round that gives it a row has to delete this case and say why. + // + // Red once by assigning `table.GL.DeleteTransformFeedback = &EmitBindTransformFeedback;` in + // BuildRemoteEmitTable - a package sweeping the whole XFB family into class B: the child + // died Fatal{NoClientSession, "BindTransformFeedback"} and the UnmigratedVerb expectation + // failed. + const ChildResult r = RunInChild([] { RemoteEmitTable().GL.DeleteTransformFeedback(7); }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("Fatal{UnmigratedVerb, \"DeleteTransformFeedback\"}"), std::string::npos) + << r.Log; +} +#endif // MGTEST_HAVE_FORK + TEST(RemoteEmitTable, NoSlotIsNull) { // R-4's whole rule, asserted over the STRUCT rather than over the list that built it. 91 // MG_Impl sites call through this table directly; a null slot is 91 potential null calls,