diff --git a/MobileGL/MG_Impl/Pipe/Verb/VerbPort.cpp b/MobileGL/MG_Impl/Pipe/Verb/VerbPort.cpp index a3bba8fc..4033ca2d 100644 --- a/MobileGL/MG_Impl/Pipe/Verb/VerbPort.cpp +++ b/MobileGL/MG_Impl/Pipe/Verb/VerbPort.cpp @@ -45,7 +45,6 @@ #include #include #include -#include #include #include @@ -69,52 +68,6 @@ namespace MobileGL::MG_Record { namespace { - Bool g_dropClearEmission = false; - Uint64 g_droppedClearEmissions = 0; - Bool g_dropDrawEmission = false; - Uint64 g_droppedDrawEmissions = 0; - - // E2's control has to be armable from OUTSIDE the process that runs the replay, because - // the statement it makes is about a trace lane and not about a unit case: "drop an - // emission and OpenRA's SSIM falls below 0.99". A recompile would make the control - // arm against source text, which is ID-22(a)'s defect. - // - // READ WITH getenv RATHER THAN THROUGH MG_Config, DELIBERATELY AND TEMPORARILY. Config.h - // is c0's and a new MOBILEGL_IPC_* knob goes through the integrator; these are - // NEGATIVE-CONTROL switches no operator may ever set, and they announce themselves at - // warning level every time they arm so they cannot be on by accident. Flagged for - // adoption into IpcTable if the integrator wants them there. - Bool ReadControlKnob(const char* name) { - const char* value = std::getenv(name); - return value != nullptr && value[0] == '1' && value[1] == '\0'; - } - - // Two negative controls. MOBILEGL_IPC_E2_DROP_DRAW drops every DrawVbo record, so a - // frame that really crosses the wire can show only the clear colour; it is the OpenRA - // retrace's control. MOBILEGL_IPC_E2_DROP_CLEAR cannot be: OpenRA's last frame clears, - // then covers the whole 640x480 surface four times before the snapshot, so dropping the - // clear leaves SSIM at 1.0 (measured). It stays for scenarios whose picture depends on - // the clear, such as ClearThenReadPixelsScenario. - void ArmControlKnobs() { - g_dropClearEmission = ReadControlKnob("MOBILEGL_IPC_E2_DROP_CLEAR"); - g_dropDrawEmission = ReadControlKnob("MOBILEGL_IPC_E2_DROP_DRAW"); - if (g_dropClearEmission) { - MGLOG_W("MG_Remote client: MOBILEGL_IPC_E2_DROP_CLEAR=1 - a NEGATIVE CONTROL is " - "armed and every glClear will be DROPPED on the wire. It is observable " - "only where the picture depends on the clear: OpenRA overdraws its whole " - "surface every frame, so this knob does NOT redden the E2 retrace " - "(measured, joint-v1.md 3) - MOBILEGL_IPC_E2_DROP_DRAW is the one that " - "does. The dropped count is published on the 'E2 control armed' line"); - } - if (g_dropDrawEmission) { - MGLOG_W("MG_Remote client: MOBILEGL_IPC_E2_DROP_DRAW=1 - E2's NEGATIVE CONTROL is " - "armed and every DrawVbo record will be DROPPED on the wire. The surface " - "can then only carry the clear colour, so this arm is expected to fail its " - "SSIM threshold; a lane that stays green with it set is not going through " - "the wire at all"); - } - } - // ID-49's two halves, in one place so the emitter and its control read the same // arithmetic. // @@ -376,16 +329,6 @@ namespace MobileGL::MG_Record { VerbChannel session = ChannelFor("Clear"); BeforeReadOnlyVerb(); - if (g_dropClearEmission) { - // A negative control. Everything above still ran, so the only difference - // between this arm and the live one is the record - which is exactly the - // statement "the picture comes from the wire" that E2 exists to prove. Its - // OBSERVABILITY is a property of the workload, not of this branch: see - // ArmControlKnobs for the measurement that took E2's retrace off this knob. - ++g_droppedClearEmissions; - return; - } - MG_Pipe::MGPClear record{}; // The DRAW framebuffer is whatever the server's own SyncRenderState resolves from // gPipeInputs, which the client's MGP_FILL(Clear) at GL_Drawing.cpp:534 has just @@ -592,8 +535,8 @@ namespace MobileGL::MG_Record { // All nineteen slots emit one draw_vbo record, the GL call verbatim beside the handles, // which the server's sink turns into the backend call the monolith makes. Each entry // point reads the bindings, plans the head and ranges, then calls EmitDrawRecord: the - // pre-verb hooks in EmitDrawArrays' order (push, mark walk, record), the drop-draw - // control, client arrays and indices as owned buffers. Refused by name: +CLIENT_COMMANDS + // pre-verb hooks in EmitDrawArrays' order (push, mark walk, record), then client arrays + // and indices as owned buffers. Refused by name: +CLIENT_COMMANDS // (no indirect buffer), +UNBOUND_PARAMETER (no parameter buffer; the sink dereferences // it) and +INDEX_OFFSET (a byte offset that is not whole indices; Start counts indices). @@ -676,14 +619,6 @@ namespace MobileGL::MG_Record { } MarkGpuWritesForDraw(); - if (g_dropDrawEmission) { - // E2's negative control covers EVERY draw record, not only DrawArrays: the - // Minecraft traces are DrawElements frames, and a control that dropped only the - // one P5 entry point would leave those lanes green with the wire disarmed. - ++g_droppedDrawEmissions; - return; - } - info.NumDraws = numDraws; MG_Pipe::MGPipeVerbTail tails[2] = {{ranges, static_cast(numDraws) * sizeof(MG_Pipe::MGPDrawRange)}, {nullptr, 0}}; @@ -1582,27 +1517,6 @@ namespace MobileGL::MG_Record { } // namespace - // THE CONTROL'S OWN EVIDENCE LINE, and it is emitted per frame rather than at teardown - // on purpose: a retrace that is killed by its own timeout, or whose library never runs - // MobileGL::Destroy, would leave a teardown-only line absent and the control would then - // have to accept a bare threshold failure - which is the thing R-16 forbids. One line - // per Present, only while a knob is armed, is bounded by the frame count and present - // whatever happens afterwards. - void LogE2ControlLine(Uint64 frameOrdinal) { - if (!g_dropClearEmission && !g_dropDrawEmission) return; - MGLOG_W("MGPipe: E2 control armed - drop-draw=%d drop-clear=%d, %llu records dropped " - "on the wire (draw=%llu clear=%llu), frame %llu", - g_dropDrawEmission ? 1 : 0, g_dropClearEmission ? 1 : 0, - static_cast(g_droppedDrawEmissions + g_droppedClearEmissions), - static_cast(g_droppedDrawEmissions), - static_cast(g_droppedClearEmissions), - static_cast(frameOrdinal)); - } - - void ArmVerbControlKnobs() { - ArmControlKnobs(); - } - #if MOBILEGL_PIPE_VERIFY Bool MonolithPortApplyingForVerify() { return t_portApplying; @@ -1742,20 +1656,6 @@ namespace MobileGL::MG_Record { return previous; } - void SetDropClearEmissionForNegativeControl(Bool drop) { - g_dropClearEmission = drop; - } - Uint64 DroppedClearEmissions() { - return g_droppedClearEmissions; - } - - void SetDropDrawEmissionForNegativeControl(Bool drop) { - g_dropDrawEmission = drop; - } - Uint64 DroppedDrawEmissions() { - return g_droppedDrawEmissions; - } - Bool ReadbackPackStateIsTightForTest(GLsizei width, Uint64 bytesPerPixel, const PixelStoreParameters& pack) { return ReadbackPackStateIsTight(width, bytesPerPixel, pack); } diff --git a/MobileGL/MG_Impl/Pipe/Verb/VerbPort.h b/MobileGL/MG_Impl/Pipe/Verb/VerbPort.h index 07ede310..c349f23d 100644 --- a/MobileGL/MG_Impl/Pipe/Verb/VerbPort.h +++ b/MobileGL/MG_Impl/Pipe/Verb/VerbPort.h @@ -53,11 +53,6 @@ namespace MobileGL::MG_Record { // persistent-map push and the deferred destroys, before its record. void BeforeReadOnlyVerb(); - // Arms the E2 negative controls from their environment knobs (once, at table build). - void ArmVerbControlKnobs(); - // The E2 control's per-frame evidence line, emitted only while a control is armed. - void LogE2ControlLine(Uint64 frameOrdinal); - #if MOBILEGL_PIPE_VERIFY // True while the monolith verb port applies a record on this thread. The verify read hook // answers a pack-state read inside that window from the applied pack payload, as it does for a @@ -81,17 +76,6 @@ namespace MobileGL::MG_Record { // Returns the previous routing. Bool SetMonolithVerbPortRoutingForTesting(Bool viaPort); - // Negative controls for the retrace lanes. When set, the named emitter skips its record but - // still runs the pre-verb hooks, so a replay that really goes through the wire loses the - // verb while one that fell through to the driver does not. Functions rather than Config.h - // knobs: a running test process sets them, and no operator may. The counts let a control - // assert that it fired. The OpenRA retrace uses the draw drop: OpenRA overdraws every pixel - // it clears, so dropping the clear leaves SSIM at 1.0 (see ArmControlKnobs in VerbPort.cpp). - void SetDropClearEmissionForNegativeControl(Bool drop); - Uint64 DroppedClearEmissions(); - void SetDropDrawEmissionForNegativeControl(Bool drop); - Uint64 DroppedDrawEmissions(); - // ID-47. The CLIENT refuses a readback whose answer would not fit a reply slot, BEFORE it // emits the record, and names the read. THE REFUSAL ITSELF IS s1's - // ClientSession::RequireReadPixelsReplyFits, forwarding to @@ -177,7 +161,7 @@ namespace MobileGL::MG_Record { // over a snapshot of the bindings the emitter read from the GL context, so a unit case can // drive the PRODUCTION derivation with synthetic bindings (R-16) while the integration lane // proves the bindings are read from the right slots. The emitters compose these and add - // nothing but the hooks, the E2 draw-drop control and the staging of a client index array. + // nothing but the hooks and the staging of a client index array. // What the emitter reads from the GL context before it plans a draw. struct RemoteDrawBindings { diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index e4b3ecfa..dd84b6a7 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -62,9 +62,8 @@ namespace MobileGL::MG_Remote::Client { - // P13 W5: the pre-verb hook and the E2 evidence line are the verb port's (VerbPort.h). + // The pre-verb hook is the verb port's (VerbPort.h). using MG_Record::BeforeReadOnlyVerb; - using MG_Record::LogE2ControlLine; // The slot arithmetic, asserted rather than commented. GlobalBackendFunctionsTable is // GLFunctionsTable plus Present plus SetSwapInterval; GLFunctionsTable is 71 function @@ -80,7 +79,6 @@ namespace MobileGL::MG_Remote::Client { namespace { - Uint64 g_presentOrdinal = 0; Uint64 g_publishedMaxRecordBytes = 0; // ---- the session, demanded rather than assumed -------------------------------- @@ -216,9 +214,6 @@ namespace MobileGL::MG_Remote::Client { // keyed on pActiveBackendObject.get() (Core.cpp:34) and that pointer never changes // under split. session.PumpControlPlane(); - - ++g_presentOrdinal; - LogE2ControlLine(g_presentOrdinal); } // ============================================================================= @@ -583,7 +578,6 @@ namespace MobileGL::MG_Remote::Client { "the three classes no longer partition the 73 slots"); MG_Backend::GlobalBackendFunctionsTable BuildRemoteEmitTable() { - MG_Record::ArmVerbControlKnobs(); MG_Backend::GlobalBackendFunctionsTable table{}; // ---- class C first, so that a slot forgotten below stays Fatal rather than null. diff --git a/MobileGL/MG_Remote/Client/EmitTables.h b/MobileGL/MG_Remote/Client/EmitTables.h index 777d0de8..22292196 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.h +++ b/MobileGL/MG_Remote/Client/EmitTables.h @@ -43,12 +43,10 @@ namespace MobileGL::MG_Remote::Client { // A reference to a never-destroyed block, like every other MG_Remote singleton (ID-8). const MG_Backend::GlobalBackendFunctionsTable& RemoteEmitTable(); - // P13 W5: the verb emitters, the monolith verb port, the E2 controls and the readback / draw - // planners are MG_Record's (MG_Impl/Pipe/Verb/VerbPort.h) - the record arm runs them in a + // The verb emitters, the monolith verb port and the readback / draw planners are + // MG_Record's (MG_Impl/Pipe/Verb/VerbPort.h) - the record arm runs them in a // library without a transport. Re-exported here under the names the remote table's users // and its unit cases already spell. - using MG_Record::DroppedClearEmissions; - using MG_Record::DroppedDrawEmissions; using MG_Record::ForEachReadbackBand; using MG_Record::InstallMonolithVerbPort; using MG_Record::MonolithVerbPortInstalled; @@ -64,8 +62,6 @@ namespace MobileGL::MG_Remote::Client { using MG_Record::RemoteIndexSizeFor; using MG_Record::ScatterReadbackBandIntoPackState; using MG_Record::ScatterTightReadbackIntoPackState; - using MG_Record::SetDropClearEmissionForNegativeControl; - using MG_Record::SetDropDrawEmissionForNegativeControl; using MG_Record::TightReadbackByteCount; using MG_Record::UnmigratedVerbFatal; diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index 0135c200..bf08e273 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -898,10 +898,6 @@ TEST(CapsMirrorTest, TheConsumerBlockDoesNotCollideWithTheFeatureBits) { EXPECT_FALSE(CapsMirrorInstance().ServerConsumes(kMGPipeSubsystemSamplers)); } -// ===================================================================================== -// E2's emitter-drop control: the switch itself, driven through the real emitter's own counter -// ===================================================================================== - // ===================================================================================== // P5b d1: the draw family's record plan (docs/archive/contracts/CONTRACT-P5B.md ยง2 d1). These drive the // PRODUCTION derivation the nineteen emitters call - PlanDrawInfo / PlanDrawRange / @@ -1073,17 +1069,6 @@ TEST(RemoteEmitTable, TheNineteenDrawSlotsAreEmittersDistinctFromEveryStub) { } } -TEST(RemoteEmitTable, TheE2DropSwitchStartsDisarmed) { - // The half a unit case can state. E2's statement - "drop one Clear emission and OpenRA's - // SSIM falls below 0.99" - is a TRACE LANE's, because the picture is the thing it is about; - // what belongs here is that the control is off unless someone armed it, so a lane that - // forgot to disarm cannot look like a lane that was never armed. - EXPECT_EQ(DroppedClearEmissions(), 0u); - SetDropClearEmissionForNegativeControl(true); - SetDropClearEmissionForNegativeControl(false); - EXPECT_EQ(DroppedClearEmissions(), 0u) << "arming and disarming the control must not, by itself, drop anything"; -} - // ===================================================================================== // ID-47: a readback larger than a reply slot is refused at the CLIENT, by name // ===================================================================================== diff --git a/docs/ci/ci-cleanup-log.md b/docs/ci/ci-cleanup-log.md index 1672da43..e3476dfb 100644 --- a/docs/ci/ci-cleanup-log.md +++ b/docs/ci/ci-cleanup-log.md @@ -39,6 +39,7 @@ them wrong. | 2026-10-17 | `Negative control (library without a transport)` (with its `Download/Unpack control runtime` steps) | Test / `retrace-split` (every leg) | a full replay rerun on all 154 legs (~60 runner-min) that, since the per-role logs, only ever proved "no client log was written"; a broken control build once reddened every leg at the download and hid every real split replay | in-test: `run_trace_case.cmake`'s transport assertions now leave `output/split-evidence.txt`, which `retrace_split_evidence.py` checks on every leg; `test_run_trace_case.py` (pipe-gates) proves each assertion red on canned logs, including the no-client-log and env-dump-line shapes this control used to rely on | | 2026-10-17 | `Negative control (dropped draws)` | Test / `retrace-split` (every leg) | a rerun of every leg with `MOBILEGL_IPC_E2_DROP_DRAW`, a production hook; its knob's own early return produced the DirectVulkan `Fatal{RoleViolation, "gPipeInputs"}` reds of 10-02 to 10-05 | in-test on the OpenRA legs (both backends, inproc and spawn): `MGTRACE_WIRE_EVIDENCE` turns on the library's counters and `run_trace_case.cmake` requires DrawVbo records on the wire and draws on the server role (758 each locally); canned reds in `test_run_trace_case.py` | | 2026-10-17 | `scripts/ci/retrace_transport_control.sh`, `scripts/ci/retrace_drop_draw_control.sh`, the retrace and drop-draw halves of `control_smoke_test.sh` and `stub_ctest.sh`, and `scripts/ci/redcheck_control_smoke_test.sh` | Test / pipe-gates (R-16) | the bodies of the two controls above and their smoke cases; the redcheck script had no caller and mutated the deleted scripts | `test_run_trace_case.py` | +| 2026-10-17 | unit `RemoteEmitTable.TheE2DropSwitchStartsDisarmed`, and the `MOBILEGL_IPC_E2_DROP_DRAW` / `MOBILEGL_IPC_E2_DROP_CLEAR` knobs with their per-draw and per-clear branches in the shipped emitter (`VerbPort.cpp`) | Test / unit (split); product code | the knobs existed only for the retired dropped-draws control (the clear knob was set by nothing); the unit case only toggled the clear setter | the wire-volume assertion in `run_trace_case.cmake` on the OpenRA split legs (DrawVbo records on the wire, draws on the server role) | ## Kept after investigation diff --git a/scripts/data/skip_census_baseline.json b/scripts/data/skip_census_baseline.json index 134380ad..7f33b652 100644 --- a/scripts/data/skip_census_baseline.json +++ b/scripts/data/skip_census_baseline.json @@ -9120,7 +9120,6 @@ "RemoteEmitTable.EachXfbAndPatchSlotIsClassBAndDemandsASessionByItsOwnName", "RemoteEmitTable.NoSlotIsNull", "RemoteEmitTable.SetSwapIntervalIsTheServerForwarderAndNotAFatal", - "RemoteEmitTable.TheE2DropSwitchStartsDisarmed", "RemoteEmitTable.TheFiveEmittersAreTheOnesTheCensusMeasured", "RemoteEmitTable.TheNineteenDrawSlotsAreEmittersDistinctFromEveryStub", "RemoteEmitTable.TheSevenI1SlotsAreClassBAndAreNotTheFatalThunk",