mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-07 01:53:24 +09:00
[Refactor] (docs/ci, MG_Impl/Pipe, MG_Remote/Client, MG_Test/Wire, scripts/data): drop the E2 drop knobs
- MOBILEGL_IPC_E2_DROP_DRAW and MOBILEGL_IPC_E2_DROP_CLEAR leave the shipped emitter with their per-draw and per-clear branches, setters, counters and per-frame evidence line; nothing sets them since the dropped-draws control left CI - the wire-volume assertion on the OpenRA split legs replaces the control (758 DrawVbo records and 758/788 server draws locally, both backends, inproc and spawn) - retire the unit case RemoteEmitTable.TheE2DropSwitchStartsDisarmed, which only toggled the clear setter; skip census: removed from junit-integration-split-strict/unit-strict (test/unit and unit-split borrow that list)
This commit is contained in:
@@ -45,7 +45,6 @@
|
||||
#include <algorithm>
|
||||
#include <cstdint>
|
||||
#include <cstdio>
|
||||
#include <cstdlib>
|
||||
#include <cstring>
|
||||
#include <limits>
|
||||
|
||||
@@ -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<Uint64>(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<unsigned long long>(g_droppedDrawEmissions + g_droppedClearEmissions),
|
||||
static_cast<unsigned long long>(g_droppedDrawEmissions),
|
||||
static_cast<unsigned long long>(g_droppedClearEmissions),
|
||||
static_cast<unsigned long long>(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);
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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
|
||||
// =====================================================================================
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -9120,7 +9120,6 @@
|
||||
"RemoteEmitTable.EachXfbAndPatchSlotIsClassBAndDemandsASessionByItsOwnName",
|
||||
"RemoteEmitTable.NoSlotIsNull",
|
||||
"RemoteEmitTable.SetSwapIntervalIsTheServerForwarderAndNotAFatal",
|
||||
"RemoteEmitTable.TheE2DropSwitchStartsDisarmed",
|
||||
"RemoteEmitTable.TheFiveEmittersAreTheOnesTheCensusMeasured",
|
||||
"RemoteEmitTable.TheNineteenDrawSlotsAreEmittersDistinctFromEveryStub",
|
||||
"RemoteEmitTable.TheSevenI1SlotsAreClassBAndAreNotTheFatalThunk",
|
||||
|
||||
Reference in New Issue
Block a user