[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

@@ -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

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()

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.

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(ImplementedVerbCount(), 11u) << "P5's five class-B verbs plus t2's six";
-    EXPECT_EQ(UnmigratedSlotCount(), 64u);
+    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<const void*>(table.GL.BeginTransformFeedback),
+        reinterpret_cast<const void*>(table.GL.EndTransformFeedback),
+        reinterpret_cast<const void*>(table.GL.PauseTransformFeedback),
+        reinterpret_cast<const void*>(table.GL.ResumeTransformFeedback),
+        reinterpret_cast<const void*>(table.GL.BindTransformFeedback),
+        reinterpret_cast<const void*>(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, "<slot>"}; a thunk dies Fatal{UnmigratedVerb, "<slot>"}. 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,