diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7cbdbe4a..80914f31 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1100,8 +1100,8 @@ jobs: ctest --output-on-failure -L integration-split --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/split-baseline.xml" python3 ../scripts/ci/junit_tally.py "${RUNNER_TEMP}/split-baseline.xml" --require-split-ran - # ID-65 supersedes broad inproc status parity: class-C aborts and named wrong-answer - # debts are recorded. Monolith, integration-split and the controls remain hard gates. + # Both full transport runs are hard gates. The per-case comparison below + # remains diagnostic because explicit mechanism controls retain their own mode. # # The entries that name MOBILEGL_TRANSPORT in their OWN property (the Split. lanes) keep # their value in both passes, which is correct: they are the split family in both arms and @@ -1123,11 +1123,8 @@ jobs: fi echo "integration-gpu entries in the split build: ${count}" MOBILEGL_TRANSPORT=monolith ctest --output-on-failure -L integration-gpu --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/arm-monolith.xml" - # ID-65: broad inproc is a recorded debt census. Reduced split + controls gate below. - inproc_rc=0 - MOBILEGL_TRANSPORT=inproc ctest --output-on-failure -L integration-gpu --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/arm-inproc.xml" || inproc_rc=$? - python3 ../scripts/ci/census_junit.py "${RUNNER_TEMP}/arm-inproc.xml" "${inproc_rc}" >> "${GITHUB_STEP_SUMMARY}" - # Retain the per-name status delta as evidence, not as the reduced-path gate. + MOBILEGL_TRANSPORT=inproc MOBILEGL_IPC_ROLE_SPLIT_STATE=1 MOBILEGL_IPC_STRICT_ERRORS=1 ctest --output-on-failure -L integration-gpu --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/arm-inproc.xml" + # Both commands above must succeed; report differences among legitimate skips. python3 - "${RUNNER_TEMP}/arm-monolith.xml" "${RUNNER_TEMP}/arm-inproc.xml" <<'PY' import sys, xml.etree.ElementTree as ET def rows(path): @@ -1145,7 +1142,7 @@ jobs: if diff: for name in diff[:40]: print(f"{name}: monolith={a.get(name, '')} inproc={b.get(name, '')}") - print(f"Recorded ID-65 census: {len(diff)} name/status differences; not a parity gate") + print(f"Transport comparison: {len(diff)} name/status differences after both full runs passed") else: print(f"the two arms agree on all {len(a)} entries, name and status") PY @@ -1666,65 +1663,31 @@ jobs: assert 'Fatal{UnmigratedVerb, "Magma:mipmap-native-format@P7"}' in log, "missing named native-format refusal" PY - # ---- P5e (ra2), ID-134 / ID-82: THE CLIENT-ARRAY LANE, EXPECTED RED UNDER RUN-AHEAD ---- - # - # The two DirectGLES.Split.ClientVertexArrayScenario entries draw from a vertex array in - # the application's OWN memory. The server uploads those bytes by dereferencing a raw - # client pointer per draw, which is exactly the read rule F forbids once the client stops - # waiting - so ID-82 makes the draw a NAMED REFUSAL under run-ahead and gives the staging - # form to P8. Aborting is therefore the DESIGNED behaviour, and a lane that demanded these - # entries green would be demanding that the refusal not fire. - # - # THE LABEL, AND WHY IT IS NOT `integration-split-clientarrays` (ID-131): `ctest -L` is a - # REGEX. That spelling is matched by `-L integration-split` as a substring, so the entries - # would still be in the lane they were moved out of while every listing showed them - # renamed. Counted, not assumed: `-L integration-split` went 181 -> 179 and - # `-L integration-gpu` 1359 -> 1357 when this label landed. - # - # IT BRANCHES ON WHETHER RUN-AHEAD IS ACTUALLY ARMED, and that is not hedging - it is the - # only formulation that is a real assertion on BOTH heads. kMGPipeP5eRunAheadReady is a - # BUILD constant (MG_Backend/Init.cpp): on a head where it is false no environment can arm - # run-ahead, the refusal is unreachable, and these entries are green - so a step that - # simply demanded red would fail for the one reason that is not a defect. The client says - # which world it is in, in its own log line, and the step reads that and then demands the - # matching outcome. When the flip lands for good, delete the green branch. - - name: Integration-clientarrays-split - expected red under run-ahead (ID-82 / ID-134) + # Client vertex/index inputs are copied into owned wire buffer resources. + # The original dedicated entries now have to execute successfully with run-ahead. + - name: Integration-clientarrays-split - owned client inputs under run-ahead working-directory: build-split env: MOBILEGL_ITEST_REQUIRE_GPU: "1" MOBILEGL_TRANSPORT: "inproc" + MOBILEGL_IPC_ROLE_SPLIT_STATE: "1" + MOBILEGL_IPC_STRICT_ERRORS: "1" + MOBILEGL_IPC_RUN_AHEAD: "1" run: | - ulimit -c unlimited - logs=MobileGL/MG_IntegrationTest/split-logs - rm -f "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null || true - rc=0 - ctest -L integration-clientarrays-split --no-tests=error -j 1 \ - > "${RUNNER_TEMP}/clientarrays.txt" 2>&1 || rc=$? - tail -5 "${RUNNER_TEMP}/clientarrays.txt" - if ! grep -rqF "run-ahead ARMED" "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null; then - # kMGPipeP5eRunAheadReady is false on this head: the client is lockstep, vi's arm is - # alive and these entries are the positive control they were registered as. - if [ "${rc}" -ne 0 ]; then - echo "::error::integration-clientarrays-split is red WITHOUT run-ahead armed. Under lockstep the client-memory array path is legal and vi keeps it working, so this is a real regression in that arm and not ID-82's refusal." - exit 1 - fi - echo "integration-clientarrays-split is green: run-ahead is not armed on this build, so ID-82's refusal is unreachable" - exit 0 - fi - if [ "${rc}" -eq 0 ]; then - echo "::error::integration-clientarrays-split went GREEN with run-ahead ARMED. ID-82 refuses a client-memory vertex array under run-ahead by name, so a green run means the refusal did not fire and the server dereferenced a client pointer from an unbarriered record. If P8 has landed the staged form, delete this step and fold the entries back into integration-split." - exit 1 - fi - marker='Fatal{UnmigratedVerb, "' - if ! grep -rhoE 'Fatal\{UnmigratedVerb, "(Multi)?DrawArrays\+CLIENT_ARRAYS"\}' \ - "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null | sort -u | grep -q CLIENT_ARRAYS; then - echo "::error::integration-clientarrays-split is red but no private log carries ${marker}...+CLIENT_ARRAYS\"} - the lane failed for some OTHER reason, which is a defect rather than the expected-red control" - grep -rhoE '(Fatal|Admitted)\{[A-Za-z]*, "[^"]*"\}' "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null | sort | uniq -c | sort -rn | head - exit 1 - fi - echo "integration-clientarrays-split is red by design on:" - grep -rhoE 'Fatal\{UnmigratedVerb, "(Multi)?DrawArrays\+CLIENT_ARRAYS"\}' \ - "${logs}"/*ClientVertexArrayScenario*.log | sort -u + ctest -L integration-clientarrays-split --show-only=json-v1 > "${RUNNER_TEMP}/clientarrays-lane.json" + ctest -L integration-clientarrays-split --no-tests=error --output-on-failure -j 1 \ + --output-junit "${RUNNER_TEMP}/clientarrays.xml" + python3 - "${GITHUB_WORKSPACE}/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py" \ + "${RUNNER_TEMP}/clientarrays-lane.json" "${RUNNER_TEMP}/clientarrays.xml" <<'PY' + import importlib.util, json, sys + spec = importlib.util.spec_from_file_location("slp", sys.argv[1]) + slp = importlib.util.module_from_spec(spec); spec.loader.exec_module(slp) + lane = json.load(open(sys.argv[2])) + names = [test["name"] for test in lane["tests"]] + if not names: + raise SystemExit("no client-array cases discovered") + slp.require_green(lane, sys.argv[3], names) + PY - name: Upload strict lane logs if: always() diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index eed0830e..fa276580 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -2260,41 +2260,16 @@ if (MOBILEGL_BUILD_DISAGGREGATED) # ---- P5e package vi (MG_Remote/CONTRACT-P5E.md §5.1, ID-82 / ruling 2) -------------------- # - # THE LANE ENTRY THAT DID NOT EXIST, and its absence was the brief's open item (a). Nothing - # else registered here draws from a vertex array in the APPLICATION's own memory: every - # other scenario binds GL_ARRAY_BUFFER first, so its glVertexAttribPointer argument is a - # byte offset. This is the one draw shape whose bytes have no wire form - the server uploads - # them by dereferencing a client pointer per draw - and it is therefore both the positive - # control for vi's decision to KEEP that arm alive under lockstep and the only entry that - # can reach the run-ahead refusal when ra publishes the caps bit. - # - # ---- P5e (ra2), ID-134: AND WITH THE CAPS BIT PUBLISHED IT IS A LANE OF ITS OWN ---------- - # - # "When ra publishes the caps bit" is now, and what these two entries reach is - # `Fatal{UnmigratedVerb, "DrawArrays+CLIENT_ARRAYS"}` / `"MultiDrawArrays+CLIENT_ARRAYS"` - - # ID-82's NAMED REFUSAL, working exactly as ruled. They cannot be green under run-ahead and - # they should not be: a lane that demanded them green would be demanding that the refusal - # not fire, which is the opposite of what vi registered them for. Staging the bytes is P8's. - # - # THE LABEL IS `integration-clientarrays-split` AND THE SPELLING IS THE POINT (ID-131, and - # gl's `integration-magma-split` above is the worked example): `ctest -L` takes a REGEX, so a - # label spelled `integration-split-clientarrays` is still MATCHED by `-L integration-split` - # and the entries would never have left the lane they were moved out of - a split that reads - # as done in every listing and is not. Counted rather than assumed: `ctest -N -L - # integration-split` is 179 after this change and was 181 before, and `-L integration-gpu` - # is 1357 against 1359. `integration-gpu` goes with it for the same reason `integration- - # magma-split` dropped it - a lane that must be green cannot contain an entry that must not. - # - # The two entries keep their private, distinct log paths (SplitLogPaths.cmake.in lists - # MGL_SPLIT_VI_CLIENT_ARRAY_TESTS by name), which is what lets the expected-red CI step - # assert the MARKER rather than merely tolerate a failure. + # Client-memory vertices and indices now cross as owned ordinary buffers. + # Keep the original names and private logs; all six scenarios join the full + # GPU and split acceptance labels as well as their focused regression label. gtest_discover_tests(MobileGLIntegrationTest TEST_PREFIX "DirectGLES.Split." TEST_LIST MGL_SPLIT_VI_CLIENT_ARRAY_TESTS TEST_FILTER "ClientVertexArrayScenario.*" DISCOVERY_TIMEOUT 30 PROPERTIES - LABELS "integration-clientarrays-split" + LABELS "integration-gpu\;integration-split\;integration-clientarrays-split" TIMEOUT ${MGL_ITEST_TIMEOUT} ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" ) diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index 156b92e5..8766a35c 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -908,29 +908,14 @@ namespace MobileGL::MG_Remote::Client { GLenum type, void* pixels) { ClientSession& session = RequireSession("ReadPixels"); - // ID-57 / M8: A PACK-PBO DESTINATION IS REFUSED BY NAME, BEFORE ANY EMISSION AND - // BEFORE `pixels` IS TOUCHED. With GL_PIXEL_PACK_BUFFER bound, the frontend permits - // `pixels` to be a byte OFFSET into that buffer, not an address (GL_Framebuffer.cpp: - // 3055 aligns it to the type size, one byte for UNSIGNED_BYTE) - and this emitter has - // no PBO branch: it sets DstOffset = 0, hands the offset to EmitAndWait as a host - // buffer, and the reply is memcpy'd to CPU address . Under monolith the - // backend maps the PBO and writes the reply into the buffer (unchanged). The real - // split form - the server writes the reply into the buffer resource and the client - // marks it GPU-written (b1's MarkReadPixelsPackBuffer becoming the producer contract - // §3 names) - is a P6 ROADMAP item. In P5 it is class C's shape (R-4), refused here. + // A bound PACK buffer makes pixels an offset. The owned reply is + // packed on this thread and only the requested rows are uploaded. const auto pbo = MG_State::pGLContext != nullptr ? MG_State::pGLContext->GetBufferBindingSlot(::MobileGL::BufferTarget::PixelPack).GetBoundObject() : SharedPtr{}; BeforeReadOnlyVerb(); - // THE PBO HALF IS b1's DESIGN AND b1 ALREADY WIRED ITS MARK, at - // GL_Framebuffer.cpp:3109 - immediately after this table call returns, inside - // ReadPixels_Backend itself. So this emitter deliberately does NOT call - // MarkReadPixelsPackBuffer(): a second call there would be the "wire it twice" - // shape, and the per-row counter b1's unit cases assert on would then count one - // read as two. (In P5 the refusal above means no PBO read reaches here at all; the - // note stays for the P6 form.) if (width <= 0 || height <= 0) return; const Uint64 bytesPerPixel = ReadbackBytesPerPixel(format, type); // ONE tight-size function for production AND the control (M3 / codex 10a). The first @@ -1268,12 +1253,7 @@ namespace MobileGL::MG_Remote::Client { GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth) { ClientSession& session = RequireSession("CopyImageSubData"); - // ID-57's SHAPE: REFUSED BY NAME, BEFORE ANY EMISSION. GL 4.6 core 18.3.2 accepts - // GL_RENDERBUFFER as either endpoint, and an endpoint is a sum type for exactly that - // reason - but no sticky forward hands out a renderbuffer object, so the sink has no - // way to rebuild one from a name, and none was measured. P7 is where the backend - // takes handles and this arm becomes ordinary. The sink refuses the same shape by - // the same name if a record ever reaches it (defence on both sides of one wire). + // The GL target selects the texture or renderbuffer handle namespace. MG_Pipe::MGPCopyRegion record{}; const auto handle = [](const MG_Backend::CopyImageEndpoint& endpoint) { return endpoint.IsRenderbuffer() @@ -1283,8 +1263,7 @@ namespace MobileGL::MG_Remote::Client { }; record.Src = handle(src); record.Dst = handle(dst); - // The GL names beside the handles: the key MGB_CTX->GetTextureObject(name) takes on - // the far side (a BARRIER-PULLED sticky forward, counted in `rsp`, retired by P7). + // GL names remain diagnostic only; backend identity comes from handles. record.SrcGlName = src.IsRenderbuffer() ? src.Renderbuffer->GetExternalIndex() : src.Texture->GetExternalIndex(); record.DstGlName = diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 13c7d133..39f82a49 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -888,14 +888,7 @@ namespace MobileGL::MG_Remote::Server { if (table == nullptr) return false; if (table->GL.CopyImageSubData == nullptr) return false; - // REFUSED BY NAME ON BOTH SIDES OF ONE WIRE. The client refuses a renderbuffer endpoint - // before it emits (ID-57's shape, EmitTables.cpp), and this is the same refusal for a - // record that reached here anyway: no sticky forward hands out a RenderbufferObject, so - // there is no honest way to build the endpoint, and guessing an empty one would copy - // nothing and say it copied. - - // P5f fe: both texture identities already travel in the record. Pass those handles - // to the backend's server-owned texture table; GL names are diagnostic data only. + // Typed handles resolve both texture and renderbuffer storage on the server. MG_Backend::CopyImageEndpoint src{}; MG_Backend::CopyImageEndpoint dst{}; if (copy.SrcTarget == GL_RENDERBUFFER) src.RenderbufferHandle = copy.Src;