From 3ba150d02120cd1e6e5aee87cbd236e58a207453 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 22 Sep 2026 13:55:47 -0400 Subject: [PATCH] [tools, android-plugin, CI] (P7): the device matrix runner selects DirectVulkan, repeats are archived, and the spawn env parse is entry-aware wave 0 item 5. Three instruments P7's device window needs, and the CI steps that can make each of them red. run_tcp_matrix.py --backend DirectVulkan end to end. --backend already existed and already filtered by each case's ci_backends; what did not exist is a per-backend GOLDEN. The two backends shared `golden` plus an optional `alternate_golden`, and alternate_golden is an OR across backends, not a per-backend key - so a DirectVulkan run that matched the DirectGLES picture passed, which is the "went green on the wrong arm" shape the exit gate exists to catch. trace_cases.json gains `backend_overrides.` ({golden, alternate_golden, ssim_threshold} only: restating target_call or a crop would stop the two backends retracing the same frame through the same window). A per-backend golden REPLACES the shared pair, matching another backend's declared image is refused BY NAME, and a --case named explicitly that does not run a requested backend is an error instead of one plan fewer and no message. The CMake emitter and the APK matrix refuse a case that declares the key rather than silently emitting one golden for both arms. DirectGLES is unchanged and that is asserted, not asserted-in-prose: a case with no override resolves to itself, and the key is dropped from the resolved case even for the backend that overrides nothing - so giving DirectVulkan its own golden does not move the DirectGLES identity fingerprint and does not invalidate its checkpoints. Each results.json row now records the golden, alternate golden and threshold it was scored against. run_android_retrace_local.py gets --matrix (the CI split subset, 39 cases, each running only its own ci_backends - not --all, which adds the rd12 workload ID-P7-4 excludes), --repeat N, and --archive-dir. The archive is the point: the result root is keyed by case and backend and by NOTHING else, so repeat 2 overwrites repeat 1 and the only artefact that survives a three-repeat run is the last one, which makes "were the three repeats the same picture?" unanswerable after the fact - in the one place where re-running to find out is expensive. It also refuses --benchmark with a non-monolith --transport, which has always produced a monolith run under a spawn label (run_benchmark_case passes args.env alone; p6/README.md records the pitfall). compare_actuals.py computes the pair result.json cannot: SSIM between two actual PNGs, plus a case x repeat x ssim-vs-golden x ssim-vs-first-repeat table over an archive. It is the retrace gate's OWN SSIM - a transcription of ComputeChannelSsim / ComputeRgbSsim (trace_replay_core.cpp:630/:671), the only SSIM in the tree - and the crop comes from each repeat's own result.json, so the two columns describe the same rectangle. PNG decode is stdlib zlib, because an unavailable Pillow in the device window would turn "the repeats diverged" into "the tool did not run". TraceReplayActivity: resolveSpawnServerPath's two decisions were String.contains() on the whole joined K=V;K=V list and neither was entry-aware. The one that bit: `--env MOBILEGL_IPC_SERVER_PATH` with no '=' is the documented UNSET spelling (trace_env_overrides_test.cpp:92) and the only way to ask for "spawn with no image" and watch LaunchServer refuse by name - it did not match "MOBILEGL_IPC_SERVER_PATH=" so a resolved path was appended AFTER it and, because ApplyEnvOverrides applies in order with setenv(...,1), the deliberate unset was silently undone. That is exactly the "silently replacing it would make that knob untestable" the method's own comment forbids. Two more: the key inside another entry's VALUE flipped both decisions the wrong way, and the transport match was case-sensitive while ConfigLoader lower-cases the value before matching (ConfigLoader.cpp:322). The parse moves to SpawnServerPath, a class with no android.* import, so test_android_lifecycle.py compiles and runs it with plain javac - the parse had no host-side coverage at all, and its on-device failure mode is "the knob had no effect". Only spawn gets a path, deliberately: it is the only transport that execs an image. Red-once, all four: a stub case whose result names the DirectGLES golden fails a --backend DirectVulkan run with "matched a golden another backend declares"; a synthetic PNG pair with a closed-form SSIM (identical = 1.0 exactly, constant black vs white = C1/(255^2+C1), inverted halves = (C2-2v)/(2v+C2)); the pre-fix resolveSpawnServerPath body fails eight of SpawnServerPathTest's checks; and the DirectVulkan TCP proof battery is the DirectGLES one under the other prefix, because an empty prefix selection raises nothing and reads as a clean pass. CI: test_run_tcp_matrix.py and test_compare_actuals.py join the R-16 step. The matrix driver's stub regressions had NO CI step - they were run by hand and recorded in a note - and they guard the tool that decides whether a device golden run passed. Counts in WSL: test_run_tcp_matrix 12 -> 18, tcp_lane_tools 4 -> 6, test_compare_actuals 12 new, test_android_lifecycle green with the new Java unit. --- .github/workflows/test.yml | 8 + .../plugin/trace/SpawnServerPath.java | 122 ++++ .../plugin/trace/TraceReplayActivity.java | 37 +- scripts/ci/tcp_lane_tools_test.py | 50 +- tools/trace_replay/README.md | 70 ++- tools/trace_replay/SpawnServerPathTest.java | 123 ++++ tools/trace_replay/compare_actuals.py | 550 ++++++++++++++++++ .../trace_replay/run_android_retrace_local.py | 187 +++++- tools/trace_replay/run_tcp_matrix.py | 71 ++- tools/trace_replay/test_android_lifecycle.py | 21 +- tools/trace_replay/test_compare_actuals.py | 288 +++++++++ tools/trace_replay/test_run_tcp_matrix.py | 153 ++++- tools/trace_replay/trace_cases.py | 130 +++++ 13 files changed, 1738 insertions(+), 72 deletions(-) create mode 100644 android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/SpawnServerPath.java create mode 100644 tools/trace_replay/SpawnServerPathTest.java create mode 100755 tools/trace_replay/compare_actuals.py create mode 100644 tools/trace_replay/test_compare_actuals.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c8afb67b..97dafc3c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3020,11 +3020,19 @@ jobs: # which is a question every branch can answer and none of which depends on the TEMPORARY # feat/disaggregated trigger at the top of this file. It costs a couple of seconds and needs # no build. + # P7 adds the last two. THE DEVICE MATRIX DRIVER HAD NO CI STEP AT ALL: its 12 stub + # regressions (now 18, with the DirectVulkan half) were run by hand in WSL and recorded in + # a note, which is the same "a gate nobody can make red" shape the paragraph above is + # about - and it guards the tool that decides whether a device golden run PASSED. + # compare_actuals is the new half of that evidence (actual-vs-actual SSIM between + # repeats); both are stub/synthetic-only, need no build, no GPU and no device. - name: The split and retrace negative controls still reject a red that is not theirs (R-16) run: | bash scripts/ci/control_smoke_test.sh python3 scripts/ci/test_trace_infrastructure.py python3 scripts/ci/tcp_lane_tools_test.py + python3 tools/trace_replay/test_run_tcp_matrix.py + python3 tools/trace_replay/test_compare_actuals.py # A GATE as of P3a (G5). "pool 与延迟释放原样搬" (ROADMAP.md:19) is meant literally: the # buffer pool, the deferred-release drain and the three persistently mapped rings move diff --git a/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/SpawnServerPath.java b/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/SpawnServerPath.java new file mode 100644 index 00000000..94494473 --- /dev/null +++ b/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/SpawnServerPath.java @@ -0,0 +1,122 @@ +package top.mobilegl.plugin.trace; + +import java.io.File; +import java.util.Locale; + +/** + * Resolves {@code MOBILEGL_IPC_SERVER_PATH} for the split arms, from the one process that can + * spell it. + * + *

P6: {@code MOBILEGL_TRANSPORT=spawn} NEEDS AN ABSOLUTE PATH TO AN EXEC-ABLE FILE, and on + * Android there is exactly one such place. An APK's {@code lib//} is the only directory an + * untrusted_app may exec from - W^X has forbidden the app's own data dirs since API 29 - and the + * packager only puts a file there if it is named {@code lib*.so}, which is why the server + * executable wears that name (P0 spike A proved the arrangement on two devices). + * + *

NOTHING OFF THE DEVICE CAN SPELL THAT PATH. {@code nativeLibraryDir} contains an + * install-time hash, so the host-side runner cannot pass {@code MOBILEGL_IPC_SERVER_PATH} the way + * it passes every other knob; the path has to be resolved HERE, by the process that is about to + * launch the server. Left unresolved, {@code LaunchServer} refuses BY NAME and the arm reds on + * "unresolvable image" - honest, but about the wrong thing. + * + *

AN EXPLICIT VALUE WINS. A caller that spoke about {@code MOBILEGL_IPC_SERVER_PATH} itself is + * pointing somewhere deliberately - or deliberately pointing NOWHERE - and silently replacing + * that would make the knob untestable. + * + *

This lives in a class of its own, with no {@code android.*} import, so that + * {@code tools/trace_replay/test_android_lifecycle.py} can compile and run it with plain javac. + * It is a hand-rolled parse of the same {@code K=V;K=V} string the native side parses, and the + * only lane that used to exercise it end to end was an on-device spawn retrace - which reports a + * parse bug as "the knob had no effect", the hardest shape to notice there is. + * + *

THE RULES ARE THE NATIVE SIDE'S, NOT A SECOND SET. They are the ones pinned by + * {@code tools/trace_replay/trace_env_overrides_test.cpp} and applied by + * {@code trace_replay_core.cpp:143} {@code ApplyEnvOverrides}: + *

+ */ +final class SpawnServerPath { + static final String TRANSPORT_KEY = "MOBILEGL_TRANSPORT"; + static final String SERVER_PATH_KEY = "MOBILEGL_IPC_SERVER_PATH"; + static final String SERVER_LIBRARY = "libMobileGLServer.so"; + + private SpawnServerPath() { + } + + /** + * Returns {@code envOverrides} with the resolved server path appended, or unchanged. + * + *

ONLY {@code spawn} GETS A PATH, and that is not an oversight: it is the only transport + * that execs an image. {@code monolith} runs no server role at all and {@code inproc} runs it + * on a thread of this process, so a server path there would be an unused variable that made + * the two arms differ in their environment for no reason. + */ + static String resolve(String envOverrides, String nativeLibraryDir) { + String entries = envOverrides == null ? "" : envOverrides; + if (nativeLibraryDir == null || nativeLibraryDir.isEmpty()) { + return entries; + } + if (!"spawn".equals(effectiveTransport(entries)) || mentions(entries, SERVER_PATH_KEY)) { + return entries; + } + String entry = + SERVER_PATH_KEY + "=" + new File(nativeLibraryDir, SERVER_LIBRARY).getAbsolutePath(); + return entries.isEmpty() ? entry : entries + ";" + entry; + } + + /** + * The transport the LIBRARY will see: the last assignment wins, lower-cased, and a trailing + * unset of the key means there is none. + * + *

Returns {@code null} when the list says nothing about the transport, which is how + * "ConfigLoader falls back to its monolith default" is spelled here. + */ + static String effectiveTransport(String envOverrides) { + String value = null; + for (String entry : split(envOverrides)) { + int separator = entry.indexOf('='); + if (separator < 0) { + if (entry.equals(TRANSPORT_KEY)) { + value = null; // The documented unset spelling. + } + continue; + } + if (separator == 0) { + continue; // An empty key never reaches setenv. + } + if (entry.substring(0, separator).equals(TRANSPORT_KEY)) { + value = entry.substring(separator + 1); + } + } + return value == null ? null : value.toLowerCase(Locale.ROOT); + } + + /** + * True when the caller named {@code key} at all - as an assignment OR as a bare unset. + * + *

Both spellings are the caller speaking. The unset one matters most: it is the only way + * to ask for "spawn with no image" and see {@code LaunchServer} refuse by name, and appending + * a resolved path after it would silently undo it, because the last entry wins. + */ + static boolean mentions(String envOverrides, String key) { + for (String entry : split(envOverrides)) { + int separator = entry.indexOf('='); + String name = separator < 0 ? entry : entry.substring(0, separator); + if (!name.isEmpty() && name.equals(key)) { + return true; + } + } + return false; + } + + private static String[] split(String envOverrides) { + return envOverrides == null ? new String[0] : envOverrides.split(";", -1); + } +} diff --git a/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/TraceReplayActivity.java b/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/TraceReplayActivity.java index 512f4004..72c14176 100644 --- a/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/TraceReplayActivity.java +++ b/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/TraceReplayActivity.java @@ -357,33 +357,20 @@ public final class TraceReplayActivity extends Activity { this.envOverrides = envOverrides; } - // P6: MOBILEGL_TRANSPORT=spawn NEEDS AN ABSOLUTE PATH TO AN EXEC-ABLE FILE, and on - // Android there is exactly one such place. An APK's lib// is the only directory an - // untrusted_app may exec from - W^X has forbidden the app's own data dirs since API 29 - - // and the packager only puts a file there if it is named lib*.so, which is why the server - // executable wears that name (P0 spike A proved the arrangement on two devices). + // P6: MOBILEGL_TRANSPORT=spawn NEEDS AN ABSOLUTE PATH TO AN EXEC-ABLE FILE, and this is + // the one process that can spell it. The reasoning, and the K=V;K=V parsing rules it has + // to share with ApplyEnvOverrides, live in SpawnServerPath - a class with no android.* + // import, so test_android_lifecycle.py compiles and runs it with plain javac. // - // NOTHING OFF THE DEVICE CAN SPELL THAT PATH. nativeLibraryDir contains an - // install-time hash, so the host-side runner cannot pass MOBILEGL_IPC_SERVER_PATH the way - // it passes every other knob; the path has to be resolved HERE, by the process that is - // about to launch the server. Left unresolved, LaunchServer refuses BY NAME and the arm - // reds on "unresolvable image" - honest, but about the wrong thing. - // - // AN EXPLICIT VALUE WINS. A caller that set MOBILEGL_IPC_SERVER_PATH itself is pointing - // somewhere deliberately, and silently replacing it would make that knob untestable. + // P7: THE TWO DECISIONS BELOW USED TO BE String.contains() ON THE WHOLE JOINED LIST, and + // neither of them was entry-aware. The one that bit: `--env MOBILEGL_IPC_SERVER_PATH` + // (no '='), the documented UNSET spelling and the only way to ask for "spawn with no + // image" and watch LaunchServer refuse by name, did not match "MOBILEGL_IPC_SERVER_PATH=" + // - so the resolved path was appended AFTER it and, because the last entry wins, the + // deliberate unset was silently undone. That is precisely the "silently replacing it + // would make that knob untestable" SpawnServerPath's own contract forbids. static String resolveSpawnServerPath(String envOverrides, String nativeLibraryDir) { - if (nativeLibraryDir == null || nativeLibraryDir.isEmpty()) { - return envOverrides; - } - if (!envOverrides.contains("MOBILEGL_TRANSPORT=spawn")) { - return envOverrides; - } - if (envOverrides.contains("MOBILEGL_IPC_SERVER_PATH=")) { - return envOverrides; - } - String serverPath = new File(nativeLibraryDir, "libMobileGLServer.so").getAbsolutePath(); - String entry = "MOBILEGL_IPC_SERVER_PATH=" + serverPath; - return envOverrides.isEmpty() ? entry : envOverrides + ";" + entry; + return SpawnServerPath.resolve(envOverrides, nativeLibraryDir); } static TraceReplayRequest from(Intent intent, File filesDir, String defaultBackend, diff --git a/scripts/ci/tcp_lane_tools_test.py b/scripts/ci/tcp_lane_tools_test.py index b84632fd..8ddbfbd9 100644 --- a/scripts/ci/tcp_lane_tools_test.py +++ b/scripts/ci/tcp_lane_tools_test.py @@ -23,33 +23,69 @@ def module(name): class Accounting(unittest.TestCase): - def test_each_tcp_case_needs_a_real_endpoint_and_pid(self): + def check_tcp_proof(self, backend): tally = module('junit_tally') with tempfile.TemporaryDirectory() as directory: root = Path(directory) - name = 'DirectGLES.Tcp.TriangleScenario.Draw' + prefix = backend + '.Tcp.' + name = prefix + 'TriangleScenario.Draw' junit, discovery, base = root / 'junit.xml', root / 'lane.json', root / 'case.log' junit.write_text(f'') discovery.write_text(json.dumps({'tests': [{'name': name, 'properties': [ {'name': 'ENVIRONMENT', 'value': ['MOBILEGL_LOG_FILE_PATH=' + str(base)]}]}]})) role = root / 'case.client.log' role.write_text('control=tcp data=stream server=127.0.0.1:40613 pid=42\n') - tally.require_tcp_proof(junit, 'DirectGLES.Tcp.', discovery) + tally.require_tcp_proof(junit, prefix, discovery) with self.assertRaises(ValueError): - tally.require_tcp_proof(junit, 'DirectGLES.Tcp.', discovery, require_run_ahead=True) + tally.require_tcp_proof(junit, prefix, discovery, require_run_ahead=True) armed = 'control=tcp data=stream server=127.0.0.1:40613 pid=42\nrun-ahead ARMED\n' role.write_text(armed) - tally.require_tcp_proof(junit, 'DirectGLES.Tcp.', discovery, require_run_ahead=True) + tally.require_tcp_proof(junit, prefix, discovery, require_run_ahead=True) for fallback in ('running lockstep', 'run-ahead DISARMED'): role.write_text(armed + fallback) with self.assertRaises(ValueError): - tally.require_tcp_proof(junit, 'DirectGLES.Tcp.', discovery, require_run_ahead=True) + tally.require_tcp_proof(junit, prefix, discovery, require_run_ahead=True) for bad in ('', 'control=tcp data=shm server=127.0.0.1:40613 pid=42', 'control=tcp data=stream server=127.0.0.1:40613 pid=0', 'spawn ARMED - the server role runs in pid 42'): role.write_text(bad) with self.assertRaises(ValueError): - tally.require_tcp_proof(junit, 'DirectGLES.Tcp.', discovery) + tally.require_tcp_proof(junit, prefix, discovery) + + def test_each_tcp_case_needs_a_real_endpoint_and_pid(self): + self.check_tcp_proof('DirectGLES') + + def test_the_directvulkan_tcp_lane_is_held_to_the_same_proof(self): + """P7 exit gate 3 runs this lane on Magma, and the proof is prefix-driven. + + `require_tcp_proof` selects by test-name prefix, so a DirectVulkan sweep asks it for + `DirectVulkan.Tcp.` - a prefix nothing in the tree had ever passed it. An empty + selection raises nothing at all, so a whole-backend typo would have read as a clean + pass; running the identical positive and negative battery under the other prefix is + what makes that impossible. + """ + self.check_tcp_proof('DirectVulkan') + + def test_a_prefix_that_matches_nothing_is_not_evidence(self): + """The shape the test above guards against, stated on its own. + + require_tcp_proof iterates the testcases whose name starts with the prefix and asserts + per case. With no match it iterates nothing and returns, which is indistinguishable from + "every case proved its arm" - so the CALLER has to count, and this records that the + function itself will not. + """ + tally = module('junit_tally') + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + junit, discovery = root / 'junit.xml', root / 'lane.json' + junit.write_text('') + discovery.write_text(json.dumps({'tests': []})) + tally.require_tcp_proof(junit, 'DirectVulkan.Tcp.', discovery) + self.assertEqual(tally.tally(junit, 'DirectVulkan.Tcp.'), (0, 0, 0)) + # And the same junit under the prefix that DOES match has no private log path, so + # the proof refuses rather than passing on the strength of the name alone. + with self.assertRaises(ValueError): + tally.require_tcp_proof(junit, 'DirectGLES.Tcp.', discovery) def test_equal_counts_do_not_hide_a_missing_case(self): parity = module('spawn_lane_parity') diff --git a/tools/trace_replay/README.md b/tools/trace_replay/README.md index 45d36ca0..62781136 100644 --- a/tools/trace_replay/README.md +++ b/tools/trace_replay/README.md @@ -286,6 +286,41 @@ unflushed persistent maps, e.g. the Create fixtures), pass sundial-lite fixture), pass `--ez avoid_angle_llvmpipe_explicit_lod_bias true` so the replay runs with `MOBILEGL_ESPRYT_AVOID_EXPLICIT_LOD_BIAS=1`. +### Repeats, the CI split subset, and keeping every repeat's evidence + +`run_android_retrace_local.py` drives the above through `trace-replay-ci.sh`. Three +options exist for the P7-7 device diagnostics, where the question is not "did this +case pass" but "which cases diverge, and is the divergence reproducible": + +| option | what it does | +| --- | --- | +| `--matrix` | run the **CI split subset** (the set `retrace-split` runs; 39 cases today) instead of the whole manifest. `--all` additionally includes the non-CI workloads such as rd12, which ID-P7-4 excludes from the exit-gate-3 denominator. Each case runs only the backends its own `ci_backends` names - and a case NAMED with `--case` that does not run the requested backend is an error, not a silent skip. | +| `--repeat N` | replay each case/backend N times. Repeats after the first pass `--reuse-fixture`, so they measure the replay and not the push. | +| `--archive-dir DIR` | copy each repeat's `result.json`, actual PNG, both role logs, `transport-proof.json` and `logcat.txt` to `DIR/-/repeat-NN/` **before the next run overwrites them**, plus `DIR/run.json` recording the APK SHA-256, the arm and the environment. | + +The archive exists because `.trace-work/android-retrace-result/-/` is +keyed by case and backend and by nothing else: without `--archive-dir` the only +artefact that survives a three-repeat run is the last one, and "were the three +repeats the same picture?" becomes unanswerable after the fact. + +```sh +python tools/trace_replay/run_android_retrace_local.py \ + --matrix --backend DirectVulkan --use-pbuffer --transport inproc --repeat 3 \ + --archive-dir .trace-work/p7w1/E0-inproc +python3 tools/trace_replay/compare_actuals.py summary .trace-work/p7w1/E0-inproc \ + --json E0-inproc-summary.json +``` + +`compare_actuals.py` computes the pair `result.json` cannot: SSIM between two actual +PNGs. It is the retrace gate's OWN SSIM - a transcription of `ComputeChannelSsim` / +`ComputeRgbSsim` (`android-plugin/app/src/trace/cpp/trace_replay_core.cpp:630` and +`:671`), one global window per channel, C1 = (0.01*255)², C2 = (0.03*255)², alpha +ignored - and the crop comes from each repeat's own `result.json`, so the +`ssim_vs_first` column describes the same rectangle as the recorded `ssim_vs_golden`. +`summary` prints `case × repeat × ssim_vs_golden × ssim_vs_first × bit-identical`; +`compare A.png B.png` scores one pair; `--self-test` checks the transcription against +closed-form values. Unit tests: `python3 tools/trace_replay/test_compare_actuals.py`. + ## Benchmark mode (frame timing) Benchmark mode reuses the same fixtures as a performance harness instead of a @@ -438,6 +473,38 @@ fixture directory and can be overridden with `--fixtures`. Repeat `--case` or minecraft-1.21.4-rd12-odinlite-in-world` permits that non-CI workload separately; an explicit `split:false` always remains excluded. +### Both backends, one driver + +`--backend DirectVulkan` runs P7 exit gate 3's device matrix with the same +run-ahead proof, resume and watchdog the DirectGLES matrix uses. Three things are +per backend and the rest is deliberately shared: + +- **selection** - a case is planned for a backend only if its own `ci_backends` + names it, so `minecraft-1.21.4-fabric-iris-iterationrp-in-world` appears in a + DirectVulkan sweep (39 cases) and not in a DirectGLES one (38). A `--case` named + explicitly that does not run a requested backend is a refusal that names the + case, not one plan fewer and no message; +- **golden and threshold** - from `backend_overrides.` in + `trace_cases.json` when the case declares one. A per-backend `golden` REPLACES + the shared `golden`/`alternate_golden` pair for that backend rather than adding + to it, and a run that matched an image another backend declares is refused by + name. (`alternate_golden` alone cannot express this: it is an OR across + backends, so a DirectVulkan run matching the DirectGLES picture passes.) The + block may restate only `golden`, `alternate_golden` and `ssim_threshold` - + restating `target_call` or a crop would stop the two backends retracing the same + frame through the same window, which is what makes the two numbers comparable; +- **result naming and checkpoint key** - `/credit-/`, plus the + golden, alternate golden and threshold recorded in each `results.json` row. + +A case with no `backend_overrides` is resolved to itself, so DirectGLES plans, +identities and checkpoint keys are unchanged by this option's existence - and the +key is dropped from the fingerprint even for the backend that overrides nothing, +so giving DirectVulkan its own golden does not invalidate the DirectGLES +checkpoints. **The CTest catalog emitter and the APK matrix refuse a case that +declares `backend_overrides`**: both carry one golden path for both backends, so +emitting such a case through them would register one backend's arm against the +other's image. Teach them the key in the same commit that adds an override. + The watchdog defaults to **300 seconds without log progress** and a separate **7200-second absolute ceiling** (`--idle-seconds`, `--max-seconds`). It observes runner output and trace/client/server log changes. It deliberately ignores the @@ -463,7 +530,8 @@ KEYCODE_WAKEUP` every 15 seconds during a case. It does not install anything or change global power/idle settings. Its own adb log is excluded from the progress watchdog, so wakeup messages cannot conceal a stalled trace. -Tool regression tests use short process stubs, never a GPU/device/trace: +Tool regression tests use short process stubs, never a GPU/device/trace. They run +in CI beside the other executable negative controls (`test.yml`, the R-16 step): ```bash python3 tools/trace_replay/test_run_tcp_matrix.py diff --git a/tools/trace_replay/SpawnServerPathTest.java b/tools/trace_replay/SpawnServerPathTest.java new file mode 100644 index 00000000..4a748b14 --- /dev/null +++ b/tools/trace_replay/SpawnServerPathTest.java @@ -0,0 +1,123 @@ +package top.mobilegl.plugin.trace; + +import java.io.File; + +/** + * Host-side checks for the split arms' MOBILEGL_IPC_SERVER_PATH resolution. + * + *

The only lane that used to run this parse end to end was an on-device spawn retrace, which + * is slow, indirect, and reports a parse bug as "the knob had no effect" - so a defect in it + * survived from P6 to P7. Run by tools/trace_replay/test_android_lifecycle.py with plain javac, + * beside the TraceReplaySession lifecycle checks. + * + *

Deliberately assert-free, for the same reason trace_env_overrides_test.cpp is: -ea is not + * on by default and a check compiled away is a green run that checked nothing. + */ +public final class SpawnServerPathTest { + private static int failures = 0; + + private static final String DIR = "/data/app/~~hash==/top.mobilegl.plugin.trace-hash==/lib/arm64"; + private static final String RESOLVED = + "MOBILEGL_IPC_SERVER_PATH=" + new File(DIR, "libMobileGLServer.so").getAbsolutePath(); + + private static void expect(String what, String actual, String expected) { + if (expected.equals(actual)) { + return; + } + ++failures; + System.err.println(what + "\n got: [" + actual + "]\n expected: [" + expected + "]"); + } + + private static void expectTransport(String entries, String expected) { + String actual = SpawnServerPath.effectiveTransport(entries); + if (expected == null ? actual == null : expected.equals(actual)) { + return; + } + ++failures; + System.err.println("effectiveTransport(\"" + entries + "\") gave [" + actual + + "], expected [" + expected + "]"); + } + + public static void main(String[] args) { + // ---- the shapes the lanes really pass ------------------------------------------------- + // .github/workflows/apk.yml's spawn acceptance lane, verbatim. + String spawnLane = "MOBILEGL_TRANSPORT=spawn;MOBILEGL_IPC_ROLE_SPLIT_STATE=1;" + + "MOBILEGL_IPC_STRICT_ERRORS=1;MOBILEGL_IPC_RUN_AHEAD=1"; + expect("the spawn acceptance lane still gets its server path appended", + SpawnServerPath.resolve(spawnLane, DIR), spawnLane + ";" + RESOLVED); + // The inproc lane runs the server role on a thread HERE; a server path would be an unused + // variable that made the two arms' environments differ for no reason. + String inprocLane = "MOBILEGL_TRANSPORT=inproc;MOBILEGL_IPC_RUN_AHEAD=1"; + expect("inproc gets no server path", SpawnServerPath.resolve(inprocLane, DIR), inprocLane); + expect("monolith (an empty list) gets no server path", + SpawnServerPath.resolve("", DIR), ""); + expect("spawn alone still gets one", + SpawnServerPath.resolve("MOBILEGL_TRANSPORT=spawn", DIR), + "MOBILEGL_TRANSPORT=spawn;" + RESOLVED); + expect("no nativeLibraryDir means nothing to resolve", + SpawnServerPath.resolve(spawnLane, ""), spawnLane); + + // ---- an explicit value wins, in BOTH of its spellings ---------------------------------- + String explicit = "MOBILEGL_TRANSPORT=spawn;MOBILEGL_IPC_SERVER_PATH=/data/local/tmp/mine.so"; + expect("an explicit path is left alone", SpawnServerPath.resolve(explicit, DIR), explicit); + // THE DEFECT THIS FILE EXISTS FOR. `KEY` with no '=' is the documented UNSET + // (trace_env_overrides_test.cpp:92) and the only way to ask for "spawn with no image" and + // see LaunchServer refuse by name. contains("MOBILEGL_IPC_SERVER_PATH=") did not match + // it, so a path was appended after the unset and - last entry wins - undid it. + String unset = "MOBILEGL_TRANSPORT=spawn;MOBILEGL_IPC_SERVER_PATH"; + expect("the unset spelling is honoured, not overwritten", + SpawnServerPath.resolve(unset, DIR), unset); + String emptied = "MOBILEGL_TRANSPORT=spawn;MOBILEGL_IPC_SERVER_PATH="; + expect("an empty value is a value, and is left alone", + SpawnServerPath.resolve(emptied, DIR), emptied); + + // ---- entry boundaries ------------------------------------------------------------------ + // The key inside ANOTHER entry's value said "the caller set it" and suppressed the + // injection, so the spawn arm died on an unresolvable image for a log path's sake. + String insideValue = "MOBILEGL_TRANSPORT=spawn;" + + "MOBILEGL_LOG_FILE_PATH=/sdcard/MOBILEGL_IPC_SERVER_PATH=x.log"; + expect("a key inside another entry's value does not suppress the injection", + SpawnServerPath.resolve(insideValue, DIR), insideValue + ";" + RESOLVED); + // And the mirror image: nothing asked for spawn here. + String transportInsideValue = + "MOBILEGL_TRACE_NOTE=ran MOBILEGL_TRANSPORT=spawn yesterday"; + expect("a transport named inside another entry's value does not request spawn", + SpawnServerPath.resolve(transportInsideValue, DIR), transportInsideValue); + expect("a longer key that merely starts with the transport key is a different key", + SpawnServerPath.resolve("MOBILEGL_TRANSPORT_NOTE=spawn", DIR), + "MOBILEGL_TRANSPORT_NOTE=spawn"); + + // ---- the library's own parsing rules --------------------------------------------------- + expectTransport("", null); + expectTransport("MOBILEGL_TRANSPORT=spawn", "spawn"); + // ConfigLoader.cpp:322 lower-cases the value before matching it, so this IS a spawn run + // and it used to get no server path at all. + expectTransport("MOBILEGL_TRANSPORT=Spawn", "spawn"); + expect("a capitalised value is still a spawn run", + SpawnServerPath.resolve("MOBILEGL_TRANSPORT=Spawn", DIR), + "MOBILEGL_TRANSPORT=Spawn;" + RESOLVED); + // ApplyEnvOverrides applies entries in order with setenv(..., 1): the last one wins. + expectTransport("MOBILEGL_TRANSPORT=inproc;MOBILEGL_TRANSPORT=spawn", "spawn"); + expectTransport("MOBILEGL_TRANSPORT=spawn;MOBILEGL_TRANSPORT=inproc", "inproc"); + expect("a later inproc really is the arm, so no server path is added", + SpawnServerPath.resolve("MOBILEGL_TRANSPORT=spawn;MOBILEGL_TRANSPORT=inproc", DIR), + "MOBILEGL_TRANSPORT=spawn;MOBILEGL_TRANSPORT=inproc"); + // A bare key is an unset, so the library falls back to its monolith default. + expectTransport("MOBILEGL_TRANSPORT=spawn;MOBILEGL_TRANSPORT", null); + // Empty entries are dropped and an empty key never reaches setenv. + expectTransport(";;MOBILEGL_TRANSPORT=spawn;;", "spawn"); + expectTransport("=spawn", null); + // Only the FIRST '=' separates, so a value may contain one. + expectTransport("MOBILEGL_TRANSPORT=spawn=extra", "spawn=extra"); + + // LAST, so that running this file against the pre-fix implementation reports the parsing + // defects above rather than dying on this one line first. + expect("a null list is not an NPE", SpawnServerPath.resolve(null, DIR), ""); + + if (failures != 0) { + System.err.println("SpawnServerPath: " + failures + " check(s) failed"); + System.exit(1); + } + System.out.println("SpawnServerPath: all checks passed"); + } +} diff --git a/tools/trace_replay/compare_actuals.py b/tools/trace_replay/compare_actuals.py new file mode 100755 index 00000000..2955c2a1 --- /dev/null +++ b/tools/trace_replay/compare_actuals.py @@ -0,0 +1,550 @@ +#!/usr/bin/env python3 +"""Actual-vs-actual SSIM for archived retrace repeats, on the retrace gate's own SSIM. + +WHY THIS EXISTS. `result.json` carries ONE number per run: this attempt's actual against its +golden. P7-7 E0 asks a different question - "are three repeats of the same case on the same +device the same picture?" - and no artefact in the tree answers it, because every repeat +overwrites the previous one's `.trace-work/android-retrace-result/-/`. With the +repeats archived (`run_android_retrace_local.py --archive-dir`), this script computes the +missing pair: SSIM(repeat N, repeat 1) beside the recorded SSIM(repeat N, golden). + +THE SSIM IS NOT A NEW ONE. It is a transcription of the retrace gate's own comparator, +`android-plugin/app/src/trace/cpp/trace_replay_core.cpp:630` (`ComputeChannelSsim`) and `:671` +(`ComputeRgbSsim`) - a single global window per channel, C1 = (0.01*255)^2, C2 = (0.03*255)^2, +the three colour channels averaged and alpha ignored - and the crop comes from the repeat's own +`result.json`, so the window is the one the gate scored. A different SSIM (a gaussian-windowed +one, say) would produce numbers that could not be read beside the recorded ones at all, which is +the whole reason this file does not import a library implementation. + +Two modes: + + compare_actuals.py compare A.png B.png [--crop-x N --crop-y N --crop-width N --crop-height N] + compare_actuals.py summary [--json PATH] + compare_actuals.py --self-test +""" +from __future__ import annotations + +import argparse +import hashlib +import json +from operator import mul +from pathlib import Path +import re +import sys +import zlib + +# (0.01 * 255)^2 and (0.03 * 255)^2, verbatim from trace_replay_core.cpp:662-663. +C1 = 6.5025 +C2 = 58.5225 + +_SQUARES = [value * value for value in range(256)] + + +class ImageError(ValueError): + """A PNG this reader will not decode, or a pair it will not compare.""" + + +# --------------------------------------------------------------------------------------------- +# PNG decode. Deliberately stdlib-only (zlib): this script has to run in the device window on +# whatever interpreter is at hand, and an unavailable Pillow would turn "the repeats diverged" +# into "the tool did not run", which are not the same finding. The accepted subset is what +# libpng gives trace_replay_core after its own normalisation (ReadPngRgba, :278): 8- and 16-bit +# greyscale/RGB/greyscale+alpha/RGBA and 8-bit palette, non-interlaced. Anything else is +# REFUSED BY NAME rather than guessed at. +# --------------------------------------------------------------------------------------------- +_PNG_MAGIC = b"\x89PNG\r\n\x1a\n" +_CHANNELS = {0: 1, 2: 3, 3: 1, 4: 2, 6: 4} + + +def _chunks(data): + if not data.startswith(_PNG_MAGIC): + raise ImageError("not a PNG file") + offset = len(_PNG_MAGIC) + while offset + 8 <= len(data): + length = int.from_bytes(data[offset:offset + 4], "big") + kind = data[offset + 4:offset + 8] + body = data[offset + 8:offset + 8 + length] + if len(body) != length: + raise ImageError("truncated PNG chunk: " + kind.decode("ascii", "replace")) + yield kind, body + offset += 12 + length + + +def _unfilter(raw, width, height, bytes_per_pixel, row_bytes): + out = bytearray(height * row_bytes) + previous = bytearray(row_bytes) + position = 0 + for y in range(height): + filter_type = raw[position] + position += 1 + line = bytearray(raw[position:position + row_bytes]) + position += row_bytes + if len(line) != row_bytes: + raise ImageError("truncated PNG scanline") + if filter_type == 1: + for index in range(bytes_per_pixel, row_bytes): + line[index] = (line[index] + line[index - bytes_per_pixel]) & 0xFF + elif filter_type == 2: + for index in range(row_bytes): + line[index] = (line[index] + previous[index]) & 0xFF + elif filter_type == 3: + for index in range(row_bytes): + left = line[index - bytes_per_pixel] if index >= bytes_per_pixel else 0 + line[index] = (line[index] + ((left + previous[index]) >> 1)) & 0xFF + elif filter_type == 4: + for index in range(row_bytes): + left = line[index - bytes_per_pixel] if index >= bytes_per_pixel else 0 + up = previous[index] + upper_left = previous[index - bytes_per_pixel] if index >= bytes_per_pixel else 0 + estimate = left + up - upper_left + distance_left = abs(estimate - left) + distance_up = abs(estimate - up) + distance_upper_left = abs(estimate - upper_left) + if distance_left <= distance_up and distance_left <= distance_upper_left: + predictor = left + elif distance_up <= distance_upper_left: + predictor = up + else: + predictor = upper_left + line[index] = (line[index] + predictor) & 0xFF + elif filter_type != 0: + raise ImageError("unknown PNG filter type %d" % filter_type) + out[y * row_bytes:(y + 1) * row_bytes] = line + previous = line + return out + + +class Image: + """An 8-bit RGBA raster, the shape trace_replay_core compares in.""" + + __slots__ = ("width", "height", "pixels") + + def __init__(self, width, height, pixels): + self.width = width + self.height = height + self.pixels = pixels + + def row(self, y): + stride = self.width * 4 + return self.pixels[y * stride:(y + 1) * stride] + + +def read_png_rgba(path): + data = Path(path).read_bytes() + header = None + palette = b"" + transparency = b"" + compressed = bytearray() + for kind, body in _chunks(data): + if kind == b"IHDR": + header = body + elif kind == b"PLTE": + palette = body + elif kind == b"tRNS": + transparency = body + elif kind == b"IDAT": + compressed += body + elif kind == b"IEND": + break + if header is None or len(header) < 13: + raise ImageError("PNG has no IHDR: " + str(path)) + width = int.from_bytes(header[0:4], "big") + height = int.from_bytes(header[4:8], "big") + depth = header[8] + color_type = header[9] + interlace = header[12] + if width <= 0 or height <= 0: + raise ImageError("PNG has a zero dimension: " + str(path)) + if interlace != 0: + raise ImageError("interlaced PNG is not supported: " + str(path)) + if color_type not in _CHANNELS: + raise ImageError("unsupported PNG colour type %d: %s" % (color_type, path)) + if depth not in (8, 16) or (color_type == 3 and depth != 8): + raise ImageError("unsupported PNG bit depth %d (colour type %d): %s" + % (depth, color_type, path)) + channels = _CHANNELS[color_type] + sample_bytes = depth // 8 + bytes_per_pixel = channels * sample_bytes + row_bytes = width * bytes_per_pixel + raw = _unfilter(zlib.decompress(bytes(compressed)), width, height, bytes_per_pixel, row_bytes) + + pixels = bytearray(width * height * 4) + for y in range(height): + source = raw[y * row_bytes:(y + 1) * row_bytes] + # png_set_strip_16: keep the high byte, exactly as the gate's reader does. + if sample_bytes == 2: + source = source[0::2] + base = y * width * 4 + if color_type == 6: + pixels[base:base + width * 4] = source + continue + for x in range(width): + out = base + x * 4 + if color_type == 0: + value = source[x] + pixels[out] = pixels[out + 1] = pixels[out + 2] = value + pixels[out + 3] = 255 + elif color_type == 2: + pixels[out:out + 3] = source[x * 3:x * 3 + 3] + pixels[out + 3] = 255 + elif color_type == 4: + value = source[x * 2] + pixels[out] = pixels[out + 1] = pixels[out + 2] = value + pixels[out + 3] = source[x * 2 + 1] + else: # palette + index = source[x] + if (index + 1) * 3 > len(palette): + raise ImageError("PNG palette index out of range: " + str(path)) + pixels[out:out + 3] = palette[index * 3:index * 3 + 3] + pixels[out + 3] = transparency[index] if index < len(transparency) else 255 + return Image(width, height, bytes(pixels)) + + +# --------------------------------------------------------------------------------------------- +# The gate's SSIM, transcribed. +# --------------------------------------------------------------------------------------------- +def channel_ssim(actual, golden, x0, y0, compare_width, compare_height, channel): + """trace_replay_core.cpp:630 ComputeChannelSsim, one global window over the crop.""" + count = float(compare_width) * float(compare_height) + sum_a = sum_g = sum_aa = sum_gg = sum_ag = 0 + stride = actual.width * 4 + golden_stride = golden.width * 4 + start_a = x0 * 4 + channel + stop_a = (x0 + compare_width) * 4 + channel + for row in range(compare_height): + base_a = (y0 + row) * stride + base_g = (y0 + row) * golden_stride + a = actual.pixels[base_a + start_a:base_a + stop_a:4] + g = golden.pixels[base_g + start_a:base_g + stop_a:4] + sum_a += sum(a) + sum_g += sum(g) + sum_aa += sum(map(_SQUARES.__getitem__, a)) + sum_gg += sum(map(_SQUARES.__getitem__, g)) + sum_ag += sum(map(mul, a, g)) + + mean_a = sum_a / count + mean_g = sum_g / count + variance_a = max(0.0, sum_aa / count - mean_a * mean_a) + variance_g = max(0.0, sum_gg / count - mean_g * mean_g) + covariance = sum_ag / count - mean_a * mean_g + luminance = (2.0 * mean_a * mean_g + C1) / (mean_a * mean_a + mean_g * mean_g + C1) + contrast_structure = (2.0 * covariance + C2) / (variance_a + variance_g + C2) + return luminance * contrast_structure + + +def rgb_ssim(actual, golden, x0, y0, compare_width, compare_height): + """trace_replay_core.cpp:671 ComputeRgbSsim - the three colour channels, alpha ignored.""" + total = 0.0 + for channel in range(3): + total += channel_ssim(actual, golden, x0, y0, compare_width, compare_height, channel) + return total / 3.0 + + +def resolve_crop(actual, golden, crop_x=0, crop_y=0, crop_width=0, crop_height=0): + """CompareAgainstOneGolden's bounds rules (trace_replay_core.cpp:683-717), verbatim. + + In particular: an uncropped pair of DIFFERENT sizes is a refusal, not a silent intersection. + Two repeats that rendered at different sizes are a finding, and averaging over whichever + rectangle happened to be common to both would bury it. + """ + if crop_width <= 0 and crop_height <= 0 and ( + actual.width != golden.width or actual.height != golden.height): + raise ImageError("image size %dx%d does not match %dx%d" + % (actual.width, actual.height, golden.width, golden.height)) + compare_width = crop_width if crop_width > 0 else actual.width + compare_height = crop_height if crop_height > 0 else actual.height + if (compare_width <= 0 or compare_height <= 0 or crop_x < 0 or crop_y < 0 + or crop_x + compare_width > actual.width or crop_y + compare_height > actual.height + or crop_x + compare_width > golden.width or crop_y + compare_height > golden.height): + raise ImageError("compare crop is outside image bounds") + return crop_x, crop_y, compare_width, compare_height + + +def mismatch_pixels(actual, golden, x0, y0, compare_width, compare_height): + """Exact RGB inequality count, the gate's `mismatchPixels` (trace_replay_core.cpp:719-736).""" + total = 0 + stride = actual.width * 4 + golden_stride = golden.width * 4 + for row in range(compare_height): + base_a = (y0 + row) * stride + x0 * 4 + base_g = (y0 + row) * golden_stride + x0 * 4 + a = actual.pixels[base_a:base_a + compare_width * 4] + g = golden.pixels[base_g:base_g + compare_width * 4] + if a == g: + continue + for index in range(compare_width): + if a[index * 4:index * 4 + 3] != g[index * 4:index * 4 + 3]: + total += 1 + return total + + +def compare_files(left, right, crop_x=0, crop_y=0, crop_width=0, crop_height=0): + a = read_png_rgba(left) + b = read_png_rgba(right) + x0, y0, width, height = resolve_crop(a, b, crop_x, crop_y, crop_width, crop_height) + return {"left": str(left), "right": str(right), "width": a.width, "height": a.height, + "crop": {"x": x0, "y": y0, "width": width, "height": height}, + "ssim": rgb_ssim(a, b, x0, y0, width, height), + "mismatch_pixels": mismatch_pixels(a, b, x0, y0, width, height)} + + +# --------------------------------------------------------------------------------------------- +# The archive summary. +# --------------------------------------------------------------------------------------------- +REPEAT_DIR = re.compile(r"^repeat-(\d+)$") + + +def digest(path): + h = hashlib.sha256() + with Path(path).open("rb") as stream: + for block in iter(lambda: stream.read(1024 * 1024), b""): + h.update(block) + return h.hexdigest() + + +def _actual_png(directory): + candidates = sorted(directory.glob("*-actual.png")) + return candidates[0] if candidates else None + + +def summarize(archive_dir): + """case x repeat x SSIM-vs-golden x SSIM-vs-first-repeat over an --archive-dir tree. + + SSIM-VS-GOLDEN IS READ, NOT RECOMPUTED. It is the number the device wrote into result.json, + against the golden the device matched; recomputing it here would silently answer a different + question whenever a run matched `alternate_golden`. SSIM-vs-first-repeat is the one this + script owns, and it uses the SAME crop result.json recorded, so the two columns describe the + same rectangle. + """ + archive = Path(archive_dir) + if not archive.is_dir(): + raise ImageError("not a directory: " + str(archive)) + rows = [] + for arm in sorted(p for p in archive.iterdir() if p.is_dir()): + repeats = [] + for child in arm.iterdir(): + match = REPEAT_DIR.match(child.name) if child.is_dir() else None + if match: + repeats.append((int(match.group(1)), child)) + repeats.sort(key=lambda pair: pair[0]) + first_actual = None + first_sha = None + for index, directory in repeats: + row = {"arm": arm.name, "repeat": index, "dir": str(directory)} + result_path = directory / "result.json" + result = None + if result_path.is_file(): + try: + result = json.loads(result_path.read_text(encoding="utf-8")) + except ValueError as error: + row["error"] = "unreadable result.json: " + str(error) + else: + row["error"] = "no result.json" + if result is not None: + row.update(backend=result.get("backend"), + passed=result.get("passed"), status_code=result.get("statusCode"), + ssim_vs_golden=result.get("ssim"), + ssim_threshold=result.get("ssimThreshold"), + matched_golden=Path(result.get("matchedGoldenPath") or "").name, + mismatch_pixels_vs_golden=result.get("mismatchPixels")) + actual = _actual_png(directory) + if actual is None: + row.setdefault("error", "no *-actual.png") + rows.append(row) + continue + row["actual"] = actual.name + row["actual_sha256"] = digest(actual) + if first_actual is None: + first_actual, first_sha = actual, row["actual_sha256"] + row["identical_to_first"] = row["actual_sha256"] == first_sha + crop = (int((result or {}).get("cropX", 0) or 0), int((result or {}).get("cropY", 0) or 0), + int((result or {}).get("cropWidth", 0) or 0), int((result or {}).get("cropHeight", 0) or 0)) + try: + comparison = compare_files(actual, first_actual, *crop) + row["ssim_vs_first"] = comparison["ssim"] + row["mismatch_pixels_vs_first"] = comparison["mismatch_pixels"] + row["crop"] = comparison["crop"] + except (ImageError, OSError) as error: + row["error"] = "actual-vs-first comparison failed: " + str(error) + rows.append(row) + + arms = {} + for row in rows: + arm = arms.setdefault(row["arm"], {"arm": row["arm"], "repeats": 0, "errors": 0, + "all_bitwise_identical": True, "passed_all": True, + "min_ssim_vs_golden": None, "min_ssim_vs_first": None}) + arm["repeats"] += 1 + if row.get("error"): + arm["errors"] += 1 + if not row.get("identical_to_first", False): + arm["all_bitwise_identical"] = False + if row.get("passed") is not True: + arm["passed_all"] = False + for key, source in (("min_ssim_vs_golden", "ssim_vs_golden"), ("min_ssim_vs_first", "ssim_vs_first")): + value = row.get(source) + if isinstance(value, (int, float)) and not isinstance(value, bool): + arm[key] = value if arm[key] is None else min(arm[key], value) + return {"archive": str(archive), "rows": rows, "arms": [arms[name] for name in sorted(arms)]} + + +def _cell(value, width, decimals=None): + if value is None: + text = "-" + elif isinstance(value, bool): + text = "yes" if value else "NO" + elif decimals is not None and isinstance(value, (int, float)): + text = ("%." + str(decimals) + "f") % value + else: + text = str(value) + return text.ljust(width) + + +def render_summary(summary, stream=sys.stdout): + # The arm column is sized from the data. A truncated case name in a divergence table is a + # name the reader has to guess at, and several of these differ only in their last word. + arm_width = max([len("arm")] + [len(row["arm"]) for row in summary["rows"]]) + columns = [("arm", arm_width), ("rep", 3), ("pass", 5), ("ssim_vs_golden", 16), + ("ssim_vs_first", 15), ("bit-identical", 13), ("px!=first", 9)] + stream.write(" ".join(name.ljust(width) for name, width in columns) + "\n") + stream.write(" ".join("-" * width for _, width in columns) + "\n") + for row in summary["rows"]: + stream.write(" ".join([ + _cell(row["arm"], arm_width), _cell(row["repeat"], 3), _cell(row.get("passed"), 5), + _cell(row.get("ssim_vs_golden"), 16, 9), _cell(row.get("ssim_vs_first"), 15, 9), + _cell(row.get("identical_to_first"), 13), + _cell(row.get("mismatch_pixels_vs_first"), 9)]).rstrip() + "\n") + if row.get("error"): + stream.write(" ! " + row["error"] + "\n") + stream.write("\n") + for arm in summary["arms"]: + stream.write("%s: %d repeat(s), %d error(s), all-bitwise-identical=%s, passed-all=%s, " + "min ssim vs golden=%s, min ssim vs first=%s\n" + % (arm["arm"], arm["repeats"], arm["errors"], + "yes" if arm["all_bitwise_identical"] else "NO", + "yes" if arm["passed_all"] else "NO", + "-" if arm["min_ssim_vs_golden"] is None else "%.9f" % arm["min_ssim_vs_golden"], + "-" if arm["min_ssim_vs_first"] is None else "%.9f" % arm["min_ssim_vs_first"])) + + +# --------------------------------------------------------------------------------------------- +# Self-test: synthetic pairs whose SSIM is a closed form, so the transcription above cannot +# drift without something going red here. +# --------------------------------------------------------------------------------------------- +def _png_bytes(width, height, rgba): + def chunk(kind, body): + return (len(body).to_bytes(4, "big") + kind + body + + zlib.crc32(kind + body).to_bytes(4, "big")) + raw = bytearray() + for y in range(height): + raw.append(0) + raw += rgba[y * width * 4:(y + 1) * width * 4] + return (_PNG_MAGIC + + chunk(b"IHDR", width.to_bytes(4, "big") + height.to_bytes(4, "big") + + bytes([8, 6, 0, 0, 0])) + + chunk(b"IDAT", zlib.compress(bytes(raw))) + + chunk(b"IEND", b"")) + + +def _solid(width, height, value): + return bytes([value, value, value, 255] * (width * height)) + + +def self_test(): + import tempfile + + failures = [] + + def expect(name, actual, expected, tolerance=1e-12): + if abs(actual - expected) > tolerance: + failures.append("%s: got %.17g, expected %.17g" % (name, actual, expected)) + + with tempfile.TemporaryDirectory(prefix="compare-actuals-selftest-") as directory: + root = Path(directory) + width, height = 8, 4 + black = root / "black.png" + white = root / "white.png" + black.write_bytes(_png_bytes(width, height, _solid(width, height, 0))) + white.write_bytes(_png_bytes(width, height, _solid(width, height, 255))) + # Identical constant images: luminance and contrast are both exactly 1. + expect("identical", compare_files(black, black)["ssim"], 1.0, 0.0) + # Constant 0 vs constant 255: contrast is 1 (no variance anywhere), luminance is + # C1 / (255^2 + C1). + expect("black vs white", compare_files(black, white)["ssim"], 9.999000099990002e-05) + halves = bytearray() + inverse = bytearray() + for y in range(height): + for x in range(width): + left = 0 if x < width // 2 else 255 + halves += bytes([left, left, left, 255]) + right = 255 - left + inverse += bytes([right, right, right, 255]) + a = root / "halves.png" + b = root / "inverse.png" + a.write_bytes(_png_bytes(width, height, bytes(halves))) + b.write_bytes(_png_bytes(width, height, bytes(inverse))) + # Equal means and variances, covariance = -variance: luminance 1, contrast negative. + expect("inverted halves", compare_files(a, b)["ssim"], -0.9964064683569576) + if compare_files(a, b)["mismatch_pixels"] != width * height: + failures.append("inverted halves: every pixel should differ") + # The crop is honoured: the left half of `halves` is black and the left half of + # `inverse` is white, so cropping to it reproduces the black/white number. + cropped = compare_files(a, b, 0, 0, width // 2, height)["ssim"] + expect("cropped half", cropped, 9.999000099990002e-05) + # A size mismatch with no crop is a refusal, not an intersection. + small = root / "small.png" + small.write_bytes(_png_bytes(4, 4, _solid(4, 4, 0))) + try: + compare_files(black, small) + except ImageError: + pass + else: + failures.append("a size mismatch with no crop was silently compared") + + if failures: + for line in failures: + print("compare_actuals self-test: " + line, file=sys.stderr) + return 1 + print("compare_actuals self-test: all checks passed") + return 0 + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--self-test", action="store_true") + sub = parser.add_subparsers(dest="mode") + compare = sub.add_parser("compare", help="SSIM between two actual PNGs") + compare.add_argument("left", type=Path) + compare.add_argument("right", type=Path) + compare.add_argument("--crop-x", type=int, default=0) + compare.add_argument("--crop-y", type=int, default=0) + compare.add_argument("--crop-width", type=int, default=0) + compare.add_argument("--crop-height", type=int, default=0) + summary = sub.add_parser("summary", help="case x repeat table over an --archive-dir tree") + summary.add_argument("archive", type=Path) + summary.add_argument("--json", type=Path, help="also write the table as JSON") + args = parser.parse_args(argv) + if args.self_test: + return self_test() + try: + if args.mode == "compare": + print(json.dumps(compare_files(args.left, args.right, args.crop_x, args.crop_y, + args.crop_width, args.crop_height), + indent=2, allow_nan=False)) + return 0 + if args.mode == "summary": + report = summarize(args.archive) + render_summary(report) + if args.json: + args.json.write_text(json.dumps(report, indent=2, allow_nan=False) + "\n", + encoding="utf-8") + return 0 if all(not row.get("error") for row in report["rows"]) else 1 + except (ImageError, OSError) as error: + print("compare_actuals: " + str(error), file=sys.stderr) + return 2 + parser.print_help() + return 2 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/trace_replay/run_android_retrace_local.py b/tools/trace_replay/run_android_retrace_local.py index 6297cea0..0938ca6a 100644 --- a/tools/trace_replay/run_android_retrace_local.py +++ b/tools/trace_replay/run_android_retrace_local.py @@ -1,5 +1,7 @@ #!/usr/bin/env python3 import argparse +import datetime +import hashlib import json import math import shutil @@ -7,7 +9,8 @@ import subprocess import sys from pathlib import Path -from trace_cases import case_with_defaults, load_trace_cases +from trace_cases import (case_for_backend, case_with_defaults, ci_backends, ci_trace_cases, + load_trace_cases, split_trace_cases) ROOT = Path(__file__).resolve().parents[2] @@ -246,6 +249,86 @@ def run_case(case, backend, extra_args=None, timeout_seconds=None, env_overrides return result.returncode +# ------------------------------------------------------------------------------------------- +# P7-7 repeat archiving. +# +# WHY IT IS NOT OPTIONAL FOR E0. `.trace-work/android-retrace-result/-/` is keyed +# by case and backend and by nothing else, so repeat 2 overwrites repeat 1 and the only artefact +# that survives a three-repeat run is the last one. That makes "is this case's divergence +# REPRODUCIBLE, and are the three repeats the same picture?" - the first question P7-7 E0 asks - +# unanswerable after the fact, and the device window is the one place where re-running to find +# out is expensive. --archive-dir copies each repeat out before the next one starts. +# +# WHAT IS ARCHIVED, and why each piece: result.json (the SSIM the gate scored and which golden +# it matched), the actual PNG (the only thing an actual-vs-actual comparison can be computed +# from), both role logs and transport-proof.json (which arm actually ran - a divergence measured +# on an arm that silently fell back to monolith is not a finding), and logcat.txt (a crash that +# still wrote a result). The golden is copied ONCE per arm, not per repeat. +# ------------------------------------------------------------------------------------------- +ARCHIVED_PER_REPEAT = ("result.json", "mobilegl.log", "mobilegl.client.log", "mobilegl.server.log", + "transport-proof.json", "logcat.txt", "benchmark.json") + + +def file_sha256(path): + h = hashlib.sha256() + with Path(path).open("rb") as stream: + for block in iter(lambda: stream.read(1024 * 1024), b""): + h.update(block) + return h.hexdigest() + + +def archive_repeat(archive_dir, case, backend, repeat_index): + """Copy one repeat's evidence out of the overwritten result root. Returns the destination.""" + arm = f"{safe_case(case['name'])}-{backend}" + source = RESULT_ROOT / arm + destination = Path(archive_dir) / arm / ("repeat-%02d" % repeat_index) + destination.mkdir(parents=True, exist_ok=True) + for name in ARCHIVED_PER_REPEAT: + candidate = source / name + if candidate.is_file(): + shutil.copyfile(candidate, destination / name) + for actual in sorted(source.glob("*-actual.png")): + shutil.copyfile(actual, destination / actual.name) + for diff in sorted(source.glob("*-diff.png")): + shutil.copyfile(diff, destination / diff.name) + # The goldens sit beside the repeats, once, so the archive is self-contained without + # carrying the same megabyte three times. + for golden in sorted(source.glob("*-golden.png")) + sorted(source.glob("*-alternate-golden.png")): + target = Path(archive_dir) / arm / golden.name + if not target.exists(): + shutil.copyfile(golden, target) + return destination + + +def write_archive_manifest(archive_dir, args, backends, cases): + """What this archive IS, recorded beside it rather than in the operator's shell history. + + The APK hash in particular: `find_trace_apk` takes the newest of two output directories, so + "which binary produced these pictures" is a question the archive has to answer itself. + """ + apk = find_trace_apk() + manifest = { + "tool": "run_android_retrace_local.py", + "written_at": datetime.datetime.now(datetime.timezone.utc).isoformat(timespec="seconds"), + "argv": sys.argv[1:], + "backends": list(backends), + "transport": args.transport, + "use_pbuffer": bool(args.use_pbuffer), + "repeat": args.repeat, + "env": list(args.env), + "transport_env": transport_env(args), + "transport_proof_args": transport_proof_args(args), + "package": BACKENDS["DirectGLES"]["package"], + "cases": [case["name"] for case in cases], + "apk": str(apk) if apk else None, + "apk_sha256": file_sha256(apk) if apk else None, + } + path = Path(archive_dir) / "run.json" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(manifest, indent=2, sort_keys=True) + "\n", encoding="utf-8") + return manifest + + def read_benchmark(case, backend, run_index): """Reads the benchmark.json the run just pulled and files it under the run number.""" result_dir = RESULT_ROOT / f"{safe_case(case['name'])}-{backend}" @@ -404,6 +487,33 @@ def parse_args(): parser.add_argument("--case", action="append", dest="cases", help="Case name to run; may be repeated.") parser.add_argument("--backend", action="append", choices=sorted(BACKENDS), help="Backend to run; may be repeated.") parser.add_argument("--all", action="store_true", help="Run every case in the APK workflow matrix.") + parser.add_argument( + "--matrix", + action="store_true", + help="Run the CI SPLIT SUBSET - the cases the retrace-split lane runs - rather than " + "every case in the manifest. This is the set P7 exit gate 3 is scored on: --all " + "additionally includes the non-CI workloads (rd12), which ID-P7-4 excludes from " + "that denominator. Each case runs only the backends its own ci_backends names.", + ) + parser.add_argument( + "--repeat", + type=int, + default=1, + metavar="N", + help="Replay each case/backend N times. With --archive-dir each repeat's result.json " + "and actual PNG are kept, so compare_actuals.py can compute actual-vs-actual SSIM " + "between repeats; without it the repeats overwrite one another and only the last " + "one survives.", + ) + parser.add_argument( + "--archive-dir", + type=Path, + metavar="DIR", + help="Copy each repeat's evidence to DIR/-/repeat-NN/ before the next " + "run overwrites the result root, and write DIR/run.json recording the APK hash, " + "the arm and the environment. Read it with " + "`python3 tools/trace_replay/compare_actuals.py summary DIR`.", + ) parser.add_argument("--keep-results", action="store_true", help="Do not clear the previous result root.") parser.add_argument( "--env", @@ -506,35 +616,92 @@ def main(): args = parse_args() selected_backends = args.backend or list(BACKENDS) selected_names = set(args.cases or []) - selected_cases = [case_with_defaults(case) for case in CASES if args.all or case["name"] in selected_names] + if args.matrix: + # The CI split subset, resolved by the manifest's own loader so that a case added later + # is in it unless it says otherwise (trace_cases.py explains why the key is an opt-out). + pool = split_trace_cases(ci_trace_cases(CASES)) + else: + pool = CASES + selected_cases = [case_with_defaults(case) for case in pool + if args.all or args.matrix or case["name"] in selected_names] if not selected_cases: - print("No cases selected. Use --all or --case NAME.", file=sys.stderr) + print("No cases selected. Use --matrix, --all or --case NAME.", file=sys.stderr) return 2 if args.benchmark and args.benchmark_repeats < 1: print("--benchmark-repeats must be at least 1.", file=sys.stderr) return 2 + if args.repeat < 1: + print("--repeat must be at least 1.", file=sys.stderr) + return 2 + if args.benchmark and args.repeat != 1: + print("--repeat is the correctness lane's repeat count; use --benchmark-repeats.", + file=sys.stderr) + return 2 + if args.benchmark and args.transport != "monolith": + # REFUSED RATHER THAN RUN UNDER A NAME THAT LIES. run_benchmark_case passes args.env + # alone, so --benchmark --transport spawn has always produced a MONOLITH run labelled + # spawn (tools/device_bench/p6/README.md records the same pitfall). The knob the + # benchmark path does honour is --env, so say that instead of silently ignoring this. + print("--benchmark ignores --transport; pass the arm explicitly, e.g. " + f"--env MOBILEGL_TRANSPORT={args.transport} " + "--env MOBILEGL_IPC_ROLE_SPLIT_STATE=1 --env MOBILEGL_IPC_STRICT_ERRORS=1 " + "--env MOBILEGL_IPC_RUN_AHEAD=1", file=sys.stderr) + return 2 if not args.keep_results and RESULT_ROOT.exists(): shutil.rmtree(RESULT_ROOT) RESULT_ROOT.mkdir(parents=True, exist_ok=True) + if args.archive_dir: + write_archive_manifest(args.archive_dir, args, selected_backends, selected_cases) failures = 0 for case in selected_cases: for backend in selected_backends: + if backend not in ci_backends(case): + # A case restricted to one backend stays restricted, and the two ways of asking + # for it are different questions. A SWEEP skips it and says so - the + # DirectVulkan-only iris case is simply not part of a DirectGLES sweep. A case + # NAMED on the command line is a request, and silently doing nothing with a + # request is how an operator concludes an arm was measured when it was not. + message = (f"{case['name']} does not run {backend}: the case declares " + f"ci_backends = {', '.join(ci_backends(case))}") + if case["name"] in selected_names: + print(message, file=sys.stderr) + return 2 + print("--- skipping " + message, flush=True) + continue + # The golden and threshold this backend is scored against (trace_cases.py's + # backend_overrides); the case unchanged when it declares none. + resolved = case_for_backend(case, backend) if args.benchmark: print(f"=== Android benchmark: {case['name']} / {backend} ===", flush=True) # No SSIM verdicts to render here; the summary page is for the correctness lane. - failures += run_benchmark_case(case, backend, args) + failures += run_benchmark_case(resolved, backend, args) continue - print(f"=== Android retrace: {case['name']} / {backend} ===", flush=True) - rc = run_case(case, backend, extra_args=transport_proof_args(args), - env_overrides=transport_env(args) + list(args.env), - use_pbuffer=args.use_pbuffer) + for repeat in range(1, args.repeat + 1): + print(f"=== Android retrace: {case['name']} / {backend} " + f"(repeat {repeat}/{args.repeat}) ===", flush=True) + extra = list(transport_proof_args(args)) + if repeat > 1: + # The repeats measure the REPLAY, not the push: re-extracting and + # re-installing between them would put an installer inside the window and + # would not change a byte of the input. + extra.append("--reuse-fixture") + rc = run_case(resolved, backend, extra_args=extra, + env_overrides=transport_env(args) + list(args.env), + use_pbuffer=args.use_pbuffer) + if args.archive_dir: + try: + kept = archive_repeat(args.archive_dir, resolved, backend, repeat) + print(f"archived repeat {repeat} to {kept}", flush=True) + except OSError as error: + print(f"failed to archive repeat {repeat}: {error}", file=sys.stderr) + failures += 1 + if rc not in (0, 2): + failures += 1 try: render_summary() except Exception as error: print(f"failed to render summary: {error}", file=sys.stderr) failures += 1 - if rc not in (0, 2): - failures += 1 return 1 if failures else 0 diff --git a/tools/trace_replay/run_tcp_matrix.py b/tools/trace_replay/run_tcp_matrix.py index 21bf4efe..a07a8ea6 100644 --- a/tools/trace_replay/run_tcp_matrix.py +++ b/tools/trace_replay/run_tcp_matrix.py @@ -3,6 +3,22 @@ Requires Linux/WSL process groups. The old local CTest/manifest TIMEOUT is not a TCP deadline: log inactivity and an explicit absolute ceiling are separate. + +BOTH BACKENDS, ONE DRIVER (P7). `--backend` selects DirectGLES (the default, and the P6.5 +residual 39-case golden matrix) or DirectVulkan (P7 exit gate 3's device matrix). The three +things that have to be per backend are, and the rest is deliberately shared: + + * SELECTION - a case is planned for a backend only if its own `ci_backends` names it, so the + DirectVulkan-only iris case appears in a DirectVulkan sweep and not in a DirectGLES one, + and a `--case` named explicitly that does not run a requested backend is an error rather + than a silent one-plan-fewer; + * GOLDEN AND THRESHOLD - from `backend_overrides.` in trace_cases.json when the case + declares one, and a run that matched ANOTHER backend's declared golden is refused by name + (the shared `alternate_golden` is an OR across backends and cannot express this); + * RESULT NAMING AND CHECKPOINT KEY - `/credit-/`, which already carried the + backend, plus the golden/threshold recorded in each results.json row. + +`--require-run-ahead` is orthogonal to the backend and applies to both. """ from __future__ import annotations import argparse @@ -22,7 +38,8 @@ import sys import time import uuid -from trace_cases import load_trace_case_manifest, ci_backends +from trace_cases import (backend_override_goldens, case_for_backend, ci_backends, + load_trace_case_manifest) ROOT = Path(__file__).resolve().parents[2] OWNED_ENV = {"MOBILEGL_TRANSPORT", "MOBILEGL_IPC_CONTROL", "MOBILEGL_IPC_DATA", @@ -61,7 +78,7 @@ def atomic_json(path, value): temporary.unlink(missing_ok=True) -def select_cases(manifest_path, requested): +def select_cases(manifest_path, requested, backends=None): manifest = load_trace_case_manifest(manifest_path) raw = read_json(manifest_path) explicit = {case["name"]: case.get("split", raw.get("defaults", {}).get("split")) @@ -79,6 +96,15 @@ def select_cases(manifest_path, requested): # An explicit split:false remains a refusal, including in this explicit selection. if explicit[name] is False or (not case["split"] and case.get("ci", True)): raise ValueError("case opts out of split: " + name) + # A NAMED CASE THAT DOES NOT RUN THE REQUESTED BACKEND IS AN ERROR, NOT A QUIET DROP. + # The default selection is a subset and filtering it per backend is correct - the + # DirectVulkan-only iris case is simply absent from a DirectGLES sweep. But + # `--case X --backend DirectVulkan` where X is DirectGLES-only used to produce one + # fewer plan and no message at all, and with a single --case it produced the generic + # "no selected case supports the requested backend" instead of naming X. + if requested and backends and not [b for b in backends if b in ci_backends(case)]: + raise ValueError(f"{name} does not run any requested backend: " + f"ci_backends = {', '.join(ci_backends(case))}") selected.append(case) if not selected: raise ValueError("the manifest selected no split cases") @@ -122,6 +148,11 @@ def definitions(command): def plan_case(case, backend, catalog, args, hashes): + # THE GOLDEN AND THE THRESHOLD ARE PER BACKEND FROM HERE ON. case_for_backend returns the + # case unchanged when it declares no `backend_overrides`, so every DirectGLES plan, its + # identity fingerprint and its checkpoint key are bit-for-bit what they were before. + resolved = case_for_backend(case, backend) + foreign_goldens = backend_override_goldens(case, backend) name = f"MobileGLTraceReplay.{case['name']}.{backend}" test = next((catalog[key] for key in (name, name + ".TCP", name + ".SPAWN", name + ".SPLIT") if key in catalog), None) @@ -146,18 +177,19 @@ def plan_case(case, backend, catalog, args, hashes): "TRACE_CROP_Y": "crop_y", "TRACE_CROP_WIDTH": "crop_width", "TRACE_CROP_HEIGHT": "crop_height", "TRACE_COHERENT_AS_FLUSH": "coherent_as_flush"} for key, field in keys.items(): - value = case.get(field, False if field == "coherent_as_flush" else 0.99 if field == "ssim_threshold" else 0) + value = resolved.get(field, False if field == "coherent_as_flush" else 0.99 if field == "ssim_threshold" else 0) values[key] = ("ON" if value else "OFF") if isinstance(value, bool) else str(value) inputs = [] for key, field in (("TRACE_ARCHIVE", "trace_archive"), ("TRACE_GOLDEN", "golden"), ("TRACE_ALTERNATE_GOLDEN", "alternate_golden")): - path = (fixtures / case[field]).resolve() if case.get(field) else None + path = (fixtures / resolved[field]).resolve() if resolved.get(field) else None values[key] = str(path) if path else "" if path: stat = path.stat() inputs.append({"path": str(path), "size": stat.st_size, "mtime_ns": stat.st_mtime_ns, **({"sha256": digest(path)} if key != "TRACE_ARCHIVE" else {})}) - trace_file = Path(case["trace_file"]) + foreign = {str((fixtures / relative).resolve()) for relative in foreign_goldens} + trace_file = Path(resolved["trace_file"]) if trace_file.is_absolute() or ".." in trace_file.parts: raise ValueError("trace_file must remain inside the extracted input directory") for path in (runner, args.library, script): @@ -170,13 +202,14 @@ def plan_case(case, backend, catalog, args, hashes): relevant = {key: value for key, value in env.items() if key in case_keys or key.startswith(("MOBILEGL_", "MGITEST_", "EGL_", "LIBGL_", "MESA_", "VK_", "__GL", "__EGL", "LD_")) or key in ("DISPLAY", "WAYLAND_DISPLAY", "XDG_RUNTIME_DIR", "DRI_PRIME", "GALLIUM_DRIVER")} - identity = fingerprint({"case": case, "backend": backend, "credit": args.credit, + identity = fingerprint({"case": resolved, "backend": backend, "credit": args.credit, "endpoint": args.endpoint, "token": args.token, "require_run_ahead": args.require_run_ahead, "artifacts": {str(p): hashes[str(p)] for p in (runner, args.library, script)}, "inputs": inputs, "environment": relevant, "command": command}) - return {"case": case, "backend": backend, "test": test, "command": command, "values": values, + return {"case": resolved, "backend": backend, "test": test, "command": command, "values": values, "script": script, "runner": runner, "identity": identity, + "foreign_goldens": foreign, "key": f"{backend}/credit-{args.credit}/{case['name']}"} @@ -321,7 +354,15 @@ def validate_result(plan, work, endpoint, require_run_ahead=False): if Path(result.get("actualPath", "")).resolve() != actual.resolve() or not actual.is_file(): raise ValueError("result has no current-attempt actual image") goldens = {str(Path(plan["values"][key]).resolve()) for key in ("TRACE_GOLDEN", "TRACE_ALTERNATE_GOLDEN") if plan["values"][key]} - if str(Path(result.get("matchedGoldenPath", "")).resolve()) not in goldens: + matched = str(Path(result.get("matchedGoldenPath", "")).resolve()) + if matched not in goldens: + # THE WRONG-BACKEND GOLDEN IS NAMED AS SUCH. Matching an image another backend declared + # for this case is not "a different golden" in the ordinary sense - it is the arm going + # green against the other backend's picture, which is the one failure a two-backend + # golden matrix exists to catch, so it says so. + if matched in plan.get("foreign_goldens", set()): + raise ValueError("result matched a golden another backend declares for this case, " + "not " + plan["backend"] + "'s: " + matched) raise ValueError("result matched a different golden") score = result.get("ssim") if isinstance(score, bool) or not isinstance(score, (float, int)) or not math.isfinite(score) or score < float(plan["values"]["TRACE_SSIM_THRESHOLD"]): @@ -399,7 +440,11 @@ def main(argv=None): parser.add_argument("--token", default=os.environ.get("MOBILEGL_IPC_TOKEN", "")) parser.add_argument("--out", type=Path, required=True) parser.add_argument("--case", action="append") - parser.add_argument("--backend", action="append", choices=("DirectGLES", "DirectVulkan")) + parser.add_argument("--backend", action="append", choices=("DirectGLES", "DirectVulkan"), + help="repeatable; default DirectGLES. A case is planned for a requested " + "backend only when its own ci_backends carries it, and its golden " + "and SSIM threshold come from that backend's backend_overrides " + "block when the manifest declares one.") parser.add_argument("--credit", type=int, choices=(1, 2, 3), default=2) parser.add_argument("--resume", action="store_true") parser.add_argument("--require-run-ahead", action="store_true", @@ -421,7 +466,7 @@ def main(argv=None): if len(backends) != len(set(backends)): parser.error("duplicate --backend") try: - cases = select_cases(args.manifest, args.case) + cases = select_cases(args.manifest, args.case, backends) document = read_json(args.catalog) if args.catalog else json.loads(subprocess.check_output( ["ctest", "--test-dir", str(args.build_dir), "--show-only=json-v1"], text=True)) catalog = {test["name"]: test for test in document["tests"]} @@ -462,6 +507,12 @@ def main(argv=None): row = {"key": plan["key"], "case": plan["case"]["name"], "backend": plan["backend"], "credit": args.credit, "ci": plan["case"].get("ci", True), "identity": plan["identity"], "require_run_ahead": args.require_run_ahead, + # Which image and which threshold THIS backend was scored against. With + # a per-backend golden in play, a results.json that named neither could + # not be read months later without the manifest of the day beside it. + "golden": plan["values"]["TRACE_GOLDEN"], + "alternate_golden": plan["values"]["TRACE_ALTERNATE_GOLDEN"], + "ssim_threshold": plan["values"]["TRACE_SSIM_THRESHOLD"], "work": str(attempt / "work"), "status": "running", "started_at_ns": time.time_ns()} checkpoint["runs"].append(row) atomic_json(checkpoint_path, checkpoint) diff --git a/tools/trace_replay/test_android_lifecycle.py b/tools/trace_replay/test_android_lifecycle.py index ba205c95..542dd0ec 100644 --- a/tools/trace_replay/test_android_lifecycle.py +++ b/tools/trace_replay/test_android_lifecycle.py @@ -25,13 +25,22 @@ def main(): raise RuntimeError("TraceReplayActivity must handle CONFIG_ASSETS_PATHS") if changes & 0xF80 != 0xF80: raise RuntimeError("TraceReplayActivity must retain the fixed-size render surface on display changes") + # Every trace-flavour Java class that has NO android.* import belongs here, because javac + # plus `java` is the only way any of them is exercised outside an APK on a device. P7 added + # SpawnServerPath: a hand-rolled K=V;K=V parse whose failure mode on device is "the knob had + # no effect", which is why a defect in it survived from P6. + java_units = [ + ("TraceReplaySession", "TraceReplaySessionTest"), + ("SpawnServerPath", "SpawnServerPathTest"), + ] with tempfile.TemporaryDirectory(prefix="mobilegl-replay-lifecycle-") as output: - subprocess.run([ - "javac", "--release", "11", "-d", output, - str(trace / "java/top/mobilegl/plugin/trace/TraceReplaySession.java"), - str(root / "tools/trace_replay/TraceReplaySessionTest.java"), - ], check=True) - subprocess.run(["java", "-cp", output, "top.mobilegl.plugin.trace.TraceReplaySessionTest"], check=True) + sources = [] + for unit, test in java_units: + sources.append(str(trace / f"java/top/mobilegl/plugin/trace/{unit}.java")) + sources.append(str(root / f"tools/trace_replay/{test}.java")) + subprocess.run(["javac", "--release", "11", "-d", output, *sources], check=True) + for _, test in java_units: + subprocess.run(["java", "-cp", output, f"top.mobilegl.plugin.trace.{test}"], check=True) print("Android replay manifest and lifecycle checks passed") diff --git a/tools/trace_replay/test_compare_actuals.py b/tools/trace_replay/test_compare_actuals.py new file mode 100644 index 00000000..996538e9 --- /dev/null +++ b/tools/trace_replay/test_compare_actuals.py @@ -0,0 +1,288 @@ +#!/usr/bin/env python3 +"""Synthetic-only regressions for the actual-vs-actual SSIM and the archive summary. + +No device, no PNG the retrace produced and no golden: every raster here is written by the test +so that each expected SSIM is a closed form (see compare_actuals.self_test for the derivations). +The summary half runs on a fabricated --archive-dir tree, because the failure it has to catch - +"three repeats were called identical while one of them had no actual image at all" - is a shape +of the DIRECTORY, not of the images. +""" +import json +from pathlib import Path +import sys +import tempfile +import unittest + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import compare_actuals as ca +import run_android_retrace_local as retrace + + +# The two closed forms the self-test derives. Written out here as literals rather than +# recomputed from C1/C2, so a typo in the constants cannot agree with itself. +BLACK_VS_WHITE = 9.999000099990002e-05 +INVERTED_HALVES = -0.9964064683569576 + + +def solid(width, height, value, alpha=255): + return bytes([value, value, value, alpha] * (width * height)) + + +def write_png(path, width, height, rgba): + Path(path).write_bytes(ca._png_bytes(width, height, rgba)) + + +class Ssim(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="compare-actuals-") + self.root = Path(self.temp.name) + + def tearDown(self): + self.temp.cleanup() + + def test_known_pairs(self): + black, white = self.root / "a.png", self.root / "b.png" + write_png(black, 8, 4, solid(8, 4, 0)) + write_png(white, 8, 4, solid(8, 4, 255)) + self.assertEqual(ca.compare_files(black, black)["ssim"], 1.0) + self.assertAlmostEqual(ca.compare_files(black, white)["ssim"], BLACK_VS_WHITE, places=15) + self.assertEqual(ca.compare_files(black, white)["mismatch_pixels"], 32) + + def test_alpha_is_ignored_and_rgb_is_averaged(self): + """Alpha must not enter the score: the gate averages three channels, not four.""" + opaque, transparent = self.root / "o.png", self.root / "t.png" + write_png(opaque, 4, 4, solid(4, 4, 200, alpha=255)) + write_png(transparent, 4, 4, solid(4, 4, 200, alpha=0)) + self.assertEqual(ca.compare_files(opaque, transparent)["ssim"], 1.0) + self.assertEqual(ca.compare_files(opaque, transparent)["mismatch_pixels"], 0) + + def test_crop_selects_the_window_and_a_size_mismatch_is_refused(self): + halves, inverse = self.root / "h.png", self.root / "i.png" + left, right = bytearray(), bytearray() + for _ in range(4): + for x in range(8): + value = 0 if x < 4 else 255 + left += bytes([value, value, value, 255]) + right += bytes([255 - value] * 3 + [255]) + write_png(halves, 8, 4, bytes(left)) + write_png(inverse, 8, 4, bytes(right)) + self.assertAlmostEqual(ca.compare_files(halves, inverse)["ssim"], INVERTED_HALVES, places=15) + self.assertAlmostEqual(ca.compare_files(halves, inverse, 0, 0, 4, 4)["ssim"], + BLACK_VS_WHITE, places=15) + small = self.root / "s.png" + write_png(small, 4, 4, solid(4, 4, 0)) + with self.assertRaises(ca.ImageError): + ca.compare_files(halves, small) + + def test_eight_and_sixteen_bit_and_rgb_decode_to_the_same_raster(self): + """The three encodings the fixtures and the device actually produce agree.""" + def chunk(kind, body): + import zlib + return (len(body).to_bytes(4, "big") + kind + body + + zlib.crc32(kind + body).to_bytes(4, "big")) + + def encode(path, width, height, depth, color_type, samples): + import zlib + raw = bytearray() + per_row = len(samples) // height + for y in range(height): + raw.append(0) + raw += samples[y * per_row:(y + 1) * per_row] + Path(path).write_bytes( + ca._PNG_MAGIC + + chunk(b"IHDR", width.to_bytes(4, "big") + height.to_bytes(4, "big") + + bytes([depth, color_type, 0, 0, 0])) + + chunk(b"IDAT", zlib.compress(bytes(raw))) + + chunk(b"IEND", b"")) + + rgba = self.root / "rgba.png" + rgb = self.root / "rgb.png" + rgba16 = self.root / "rgba16.png" + write_png(rgba, 2, 2, solid(2, 2, 96)) + encode(rgb, 2, 2, 8, 2, bytes([96, 96, 96] * 4)) + # 16-bit big-endian samples; png_set_strip_16 keeps the high byte, so RGB reads 96 + # and alpha reads 255 - the same raster the 8-bit files above carry. + encode(rgba16, 2, 2, 16, 6, bytes([96, 0, 96, 0, 96, 0, 255, 0] * 4)) + first = ca.read_png_rgba(rgba) + for other in (rgb, rgba16): + decoded = ca.read_png_rgba(other) + self.assertEqual((decoded.width, decoded.height), (first.width, first.height)) + self.assertEqual(decoded.pixels, first.pixels, other.name) + + +class Summary(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="compare-actuals-summary-") + self.root = Path(self.temp.name) + + def tearDown(self): + self.temp.cleanup() + + def repeat(self, arm, index, rgba=None, ssim=1.0, passed=True, crop=(0, 0, 0, 0), + write_result=True, write_actual=True): + directory = self.root / arm / ("repeat-%02d" % index) + directory.mkdir(parents=True) + if write_actual: + write_png(directory / (arm + "-actual.png"), 8, 4, + rgba if rgba is not None else solid(8, 4, 64)) + if write_result: + (directory / "result.json").write_text(json.dumps({ + "passed": passed, "statusCode": 0 if passed else 1, "backend": "DirectVulkan", + "ssim": ssim, "ssimThreshold": 0.99, "matchedGoldenPath": "/x/y-golden.png", + "mismatchPixels": 0, "cropX": crop[0], "cropY": crop[1], + "cropWidth": crop[2], "cropHeight": crop[3]}), encoding="utf-8") + return directory + + def test_identical_repeats_are_reported_bitwise_identical(self): + arm = "Example-DirectVulkan" + for index in (1, 2, 3): + self.repeat(arm, index) + report = ca.summarize(self.root) + self.assertEqual([row["repeat"] for row in report["rows"]], [1, 2, 3]) + self.assertTrue(all(row["identical_to_first"] for row in report["rows"])) + self.assertTrue(all(row["ssim_vs_first"] == 1.0 for row in report["rows"])) + self.assertEqual(report["arms"][0]["all_bitwise_identical"], True) + self.assertEqual(report["arms"][0]["min_ssim_vs_golden"], 1.0) + + def test_a_diverging_repeat_is_not_hidden_by_an_equal_ssim_against_the_golden(self): + """The whole point of the column: same score, different picture. + + Both repeats can score the same SSIM against the golden while differing from each + other, and result.json alone can never say so. + """ + arm = "Example-DirectVulkan" + self.repeat(arm, 1, rgba=solid(8, 4, 0), ssim=0.98) + self.repeat(arm, 2, rgba=solid(8, 4, 255), ssim=0.98) + report = ca.summarize(self.root) + second = report["rows"][1] + self.assertFalse(second["identical_to_first"]) + self.assertAlmostEqual(second["ssim_vs_first"], BLACK_VS_WHITE, places=15) + self.assertEqual(second["mismatch_pixels_vs_first"], 32) + self.assertFalse(report["arms"][0]["all_bitwise_identical"]) + self.assertAlmostEqual(report["arms"][0]["min_ssim_vs_first"], BLACK_VS_WHITE, places=15) + + def test_the_result_json_crop_is_the_window_used(self): + arm = "Example-DirectVulkan" + left, right = bytearray(), bytearray() + for _ in range(4): + for x in range(8): + value = 0 if x < 4 else 255 + left += bytes([value, value, value, 255]) + right += bytes([255 - value] * 3 + [255]) + self.repeat(arm, 1, rgba=bytes(left), crop=(0, 0, 4, 4)) + self.repeat(arm, 2, rgba=bytes(right), crop=(0, 0, 4, 4)) + report = ca.summarize(self.root) + self.assertEqual(report["rows"][1]["crop"], {"x": 0, "y": 0, "width": 4, "height": 4}) + self.assertAlmostEqual(report["rows"][1]["ssim_vs_first"], BLACK_VS_WHITE, places=15) + + def test_a_missing_actual_or_result_is_an_error_row_not_a_silent_pass(self): + arm = "Example-DirectVulkan" + self.repeat(arm, 1) + self.repeat(arm, 2, write_actual=False) + self.repeat(arm, 3, write_result=False) + report = ca.summarize(self.root) + self.assertEqual(report["rows"][1]["error"], "no *-actual.png") + self.assertEqual(report["rows"][2]["error"], "no result.json") + self.assertIsNone(report["rows"][2].get("ssim_vs_golden")) + self.assertEqual(report["arms"][0]["errors"], 2) + self.assertFalse(report["arms"][0]["all_bitwise_identical"]) + # The renderer must survive a row with no numbers at all. + import io + buffer = io.StringIO() + ca.render_summary(report, buffer) + self.assertIn("no *-actual.png", buffer.getvalue()) + + def test_a_failed_repeat_is_carried_into_the_rollup(self): + arm = "Example-DirectVulkan" + self.repeat(arm, 1, ssim=1.0) + self.repeat(arm, 2, ssim=0.97, passed=False) + report = ca.summarize(self.root) + self.assertFalse(report["arms"][0]["passed_all"]) + self.assertEqual(report["arms"][0]["min_ssim_vs_golden"], 0.97) + + def test_self_test_of_the_transcribed_ssim_passes(self): + self.assertEqual(ca.self_test(), 0) + + +class Archive(unittest.TestCase): + """The other half: run_android_retrace_local writes the tree summarize() reads. + + Tested together because the failure this pair exists to prevent spans both - the result + root is keyed by case and backend and by NOTHING else, so a second repeat overwrites the + first, and the archive is the only thing standing between "three repeats" and "one + surviving picture". A test of either half alone would not see a key mismatch between them. + """ + + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="retrace-archive-") + self.root = Path(self.temp.name) + self.previous_result_root = retrace.RESULT_ROOT + retrace.RESULT_ROOT = self.root / "result-root" + self.case = {"name": "Example/case:1", "backend_overrides": None} + self.arm = retrace.safe_case(self.case["name"]) + "-DirectVulkan" + self.result_dir = retrace.RESULT_ROOT / self.arm + self.result_dir.mkdir(parents=True) + + def tearDown(self): + retrace.RESULT_ROOT = self.previous_result_root + self.temp.cleanup() + + def write_run(self, value, ssim): + """Overwrite the result root the way a real repeat does.""" + write_png(self.result_dir / (self.arm + "-actual.png"), 8, 4, solid(8, 4, value)) + (self.result_dir / "result.json").write_text(json.dumps( + {"passed": True, "statusCode": 0, "backend": "DirectVulkan", "ssim": ssim, + "ssimThreshold": 0.99, "matchedGoldenPath": "/x/g.png", "mismatchPixels": 0, + "cropX": 0, "cropY": 0, "cropWidth": 0, "cropHeight": 0}), encoding="utf-8") + (self.result_dir / "mobilegl.client.log").write_text("run-ahead ARMED\n", encoding="utf-8") + (self.result_dir / "transport-proof.json").write_text('{"passed": true}', encoding="utf-8") + write_png(self.result_dir / (self.arm + "-golden.png"), 8, 4, solid(8, 4, 1)) + + def test_each_repeat_survives_the_next_one_and_summarizes(self): + archive = self.root / "archive" + for index, (value, ssim) in enumerate(((10, 1.0), (10, 1.0), (200, 0.97)), start=1): + self.write_run(value, ssim) + kept = retrace.archive_repeat(archive, self.case, "DirectVulkan", index) + self.assertTrue((kept / "result.json").is_file()) + self.assertTrue((kept / (self.arm + "-actual.png")).is_file()) + self.assertTrue((kept / "mobilegl.client.log").is_file()) + self.assertTrue((kept / "transport-proof.json").is_file()) + # The golden is copied ONCE per arm, beside the repeats rather than inside each. + self.assertTrue((archive / self.arm / (self.arm + "-golden.png")).is_file()) + self.assertFalse((archive / self.arm / "repeat-01" / (self.arm + "-golden.png")).is_file()) + report = ca.summarize(archive) + self.assertEqual([row["repeat"] for row in report["rows"]], [1, 2, 3]) + self.assertTrue(report["rows"][1]["identical_to_first"]) + self.assertFalse(report["rows"][2]["identical_to_first"]) + self.assertEqual(report["rows"][2]["ssim_vs_golden"], 0.97) + self.assertLess(report["rows"][2]["ssim_vs_first"], 1.0) + # The directory name the archive chose is the one the summary reports. + self.assertEqual({row["arm"] for row in report["rows"]}, {self.arm}) + + def test_a_repeat_that_produced_nothing_leaves_an_empty_directory_not_the_previous_one(self): + """A failed repeat must not inherit the previous repeat's artefacts. + + This is the concrete shape tools/device_bench/p6/README.md records for benchmark.json: + the result root is not cleared between runs, so a run that produced nothing leaves the + LAST one's files in place. Archiving copies whatever is there, so the guarantee this + test pins is the weaker, honest one - the archive reports what the result root held - + and the summary must still be able to say the two repeats are not the same evidence. + """ + archive = self.root / "archive" + self.write_run(10, 1.0) + retrace.archive_repeat(archive, self.case, "DirectVulkan", 1) + (self.result_dir / "result.json").unlink() + (self.result_dir / (self.arm + "-actual.png")).unlink() + kept = retrace.archive_repeat(archive, self.case, "DirectVulkan", 2) + self.assertFalse((kept / "result.json").exists()) + report = ca.summarize(archive) + # Both artefacts are gone; the first missing one is the one named. + self.assertEqual(report["rows"][1]["error"], "no result.json") + self.assertNotIn("ssim_vs_first", report["rows"][1]) + self.assertNotIn("actual_sha256", report["rows"][1]) + self.assertEqual(report["arms"][0]["errors"], 1) + self.assertFalse(report["arms"][0]["all_bitwise_identical"]) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/tools/trace_replay/test_run_tcp_matrix.py b/tools/trace_replay/test_run_tcp_matrix.py index 3bbd4710..c1a15e42 100644 --- a/tools/trace_replay/test_run_tcp_matrix.py +++ b/tools/trace_replay/test_run_tcp_matrix.py @@ -34,7 +34,9 @@ if behavior!='no-result': if arm=='server-only':(work/'output/mobilegl.server.log').write_text('run-ahead ARMED\n') result={'passed':True,'statusCode':0,'backend':values['TRACE_BACKEND'], 'targetCall':int(values['TRACE_TARGET_CALL']),'tracePath':str(work/'input'/values['TRACE_FILE']), - 'actualPath':str(actual),'matchedGoldenPath':values['TRACE_GOLDEN'],'ssim':1.0} + 'actualPath':str(actual), + 'matchedGoldenPath':os.environ.get('CASE_MATCH_GOLDEN') or values['TRACE_GOLDEN'], + 'ssim':float(os.environ.get('CASE_SSIM','1.0'))} (work/'output/result.json').write_text(json.dumps(result)) if behavior=='timeout':time.sleep(60) raise SystemExit(7 if behavior=='failed' else 0) @@ -63,19 +65,25 @@ class MatrixTests(unittest.TestCase): def tearDown(self): self.temp.cleanup() - def write_inputs(self, behavior='success', extra_cases=None): - cases=[{'name':'Example','trace_archive':'fixture.tgz','trace_file':'trace.trace', - 'golden':'golden.png','target_call':42,'width':16,'height':16,'timeout_seconds':.05}] + def write_inputs(self, behavior='success', extra_cases=None, case_overrides=None, + backends=('DirectGLES',)): + case={'name':'Example','trace_archive':'fixture.tgz','trace_file':'trace.trace', + 'golden':'golden.png','target_call':42,'width':16,'height':16,'timeout_seconds':.05} + case.update(case_overrides or {}) + cases=[case] if extra_cases:cases.extend(extra_cases) self.manifest.write_text(json.dumps({'defaults':{'ssim_threshold':.99},'cases':cases})) - command=[sys.executable,str(self.stub),'-DTRACE_REPLAY_EXE='+sys.executable, - '-DMOBILEGL_LIBRARY=/old/library','-DTRACE_CASE_NAME=Example','-DTRACE_BACKEND=DirectGLES', - '-DTRACE_ARCHIVE='+str(self.fixtures/'fixture.tgz'),'-DTRACE_OUTPUT_DIR=/old/output', - '-P',str(self.tool/'run_trace_case.cmake')] - self.catalog.write_text(json.dumps({'kind':'ctestInfo','tests':[{ - 'name':'MobileGLTraceReplay.Example.DirectGLES','command':command, - 'properties':[{'name':'ENVIRONMENT','value':['CASE_BEHAVIOR='+behavior]}, - {'name':'TIMEOUT','value':.05},{'name':'WORKING_DIRECTORY','value':str(self.root)}]}]})) + tests=[] + for backend in backends: + command=[sys.executable,str(self.stub),'-DTRACE_REPLAY_EXE='+sys.executable, + '-DMOBILEGL_LIBRARY=/old/library','-DTRACE_CASE_NAME=Example','-DTRACE_BACKEND='+backend, + '-DTRACE_ARCHIVE='+str(self.fixtures/'fixture.tgz'),'-DTRACE_OUTPUT_DIR=/old/output', + '-P',str(self.tool/'run_trace_case.cmake')] + tests.append({'name':'MobileGLTraceReplay.Example.'+backend,'command':command, + 'properties':[{'name':'ENVIRONMENT','value':['CASE_BEHAVIOR='+behavior]}, + {'name':'TIMEOUT','value':.05}, + {'name':'WORKING_DIRECTORY','value':str(self.root)}]}) + self.catalog.write_text(json.dumps({'kind':'ctestInfo','tests':tests})) def run_main(self, *extra): args=['--catalog',str(self.catalog),'--source',str(self.source),'--library',str(self.library), @@ -89,7 +97,14 @@ class MatrixTests(unittest.TestCase): def set_arm(self, arm, extra=None): catalog=matrix.read_json(self.catalog) - catalog['tests'][0]['properties'][0]['value'] += ['CASE_ARM='+arm,*(extra or [])] + for test in catalog['tests']: + test['properties'][0]['value'] += ['CASE_ARM='+arm,*(extra or [])] + self.catalog.write_text(json.dumps(catalog)) + + def set_case_env(self, entries): + catalog=matrix.read_json(self.catalog) + for test in catalog['tests']: + test['properties'][0]['value'] += list(entries) self.catalog.write_text(json.dumps(catalog)) def test_required_arm_forces_both_environment_knobs_and_records_actual_arm(self): @@ -206,6 +221,118 @@ class MatrixTests(unittest.TestCase): self.assertEqual(matrix.select_cases(self.manifest,['Extra'])[0]['name'],'Extra') with self.assertRaises(ValueError):matrix.select_cases(self.manifest,['OptOut']) + # ------------------------------------------------------------------------------------ + # P7: the same driver runs the DirectVulkan device matrix. + # ------------------------------------------------------------------------------------ + VULKAN_ONLY={'trace_archive':'fixture.tgz','trace_file':'trace.trace','golden':'golden.png', + 'target_call':42,'width':16,'height':16,'ci_backends':['DirectVulkan']} + GLES_ONLY=dict(VULKAN_ONLY,ci_backends=['DirectGLES']) + + def plan_args(self, **extra): + args=types.SimpleNamespace(source=self.source,library=self.library,fixtures=self.fixtures, + runner=Path(sys.executable),endpoint='tcp://127.0.0.1:9', + token='x',credit=2,require_run_ahead=False,base_env={'BASE':'same'}) + for key,value in extra.items():setattr(args,key,value) + return args + + def plan_for(self, backend, case_overrides=None): + self.write_inputs(case_overrides=case_overrides,backends=('DirectGLES','DirectVulkan')) + catalog={test['name']:test for test in matrix.read_json(self.catalog)['tests']} + case=matrix.select_cases(self.manifest,None,[backend])[0] + return matrix.plan_case(case,backend,catalog,self.plan_args(),{}) + + def test_selection_honours_per_case_backends_and_names_an_impossible_pairing(self): + self.write_inputs(extra_cases=[dict(self.VULKAN_ONLY,name='VulkanOnly'), + dict(self.GLES_ONLY,name='GlesOnly')]) + # The default subset is every split case; the per-backend filter happens in main(), + # so both restricted cases survive selection itself. + self.assertEqual([c['name'] for c in matrix.select_cases(self.manifest,None,['DirectVulkan'])], + ['Example','VulkanOnly','GlesOnly']) + # A case NAMED on the command line that cannot run the requested backend is an error + # that says which case and what it does run - not one plan fewer and no message. + with self.assertRaises(ValueError) as caught: + matrix.select_cases(self.manifest,['GlesOnly'],['DirectVulkan']) + self.assertIn('GlesOnly',str(caught.exception)) + self.assertIn('DirectGLES',str(caught.exception)) + self.assertEqual(matrix.select_cases(self.manifest,['GlesOnly'],['DirectGLES'])[0]['name'],'GlesOnly') + self.assertEqual(matrix.select_cases(self.manifest,['VulkanOnly'],['DirectVulkan'])[0]['name'],'VulkanOnly') + + def test_directvulkan_plans_its_own_golden_and_threshold(self): + overrides={'backend_overrides':{'DirectVulkan':{'golden':'vulkan-golden.png', + 'ssim_threshold':0.95}}} + (self.fixtures/'vulkan-golden.png').write_bytes(b'vulkan golden image') + vulkan=self.plan_for('DirectVulkan',overrides) + self.assertEqual(Path(vulkan['values']['TRACE_GOLDEN']).name,'vulkan-golden.png') + self.assertEqual(vulkan['values']['TRACE_SSIM_THRESHOLD'],'0.95') + self.assertEqual(vulkan['values']['TRACE_ALTERNATE_GOLDEN'],'') + self.assertTrue(vulkan['key'].startswith('DirectVulkan/')) + gles=self.plan_for('DirectGLES',overrides) + self.assertEqual(Path(gles['values']['TRACE_GOLDEN']).name,'golden.png') + self.assertEqual(gles['values']['TRACE_SSIM_THRESHOLD'],'0.99') + self.assertTrue(gles['key'].startswith('DirectGLES/')) + self.assertNotEqual(gles['identity'],vulkan['identity']) + # Each backend knows which goldens belong to the other one - including the SHARED + # golden, which stops being neutral the moment one backend replaces it. + self.assertEqual({Path(p).name for p in gles['foreign_goldens']},{'vulkan-golden.png'}) + self.assertEqual({Path(p).name for p in vulkan['foreign_goldens']},{'golden.png'}) + + def test_directgles_planning_is_untouched_by_another_backends_override(self): + """The 'keep DirectGLES byte-identical' claim, asserted rather than asserted-in-prose.""" + (self.fixtures/'vulkan-golden.png').write_bytes(b'vulkan golden image') + before=self.plan_for('DirectGLES') + after=self.plan_for('DirectGLES',{'backend_overrides':{'DirectVulkan':{ + 'golden':'vulkan-golden.png','ssim_threshold':0.95}}}) + self.assertEqual(before['values'],after['values']) + self.assertEqual(before['key'],after['key']) + # The identity fingerprint is what --resume compares, so it must not move either. + self.assertEqual(before['identity'],after['identity']) + + def test_a_directgles_golden_never_satisfies_a_directvulkan_run(self): + """RED-ONCE: the wrong-backend golden is refused, by name.""" + (self.fixtures/'vulkan-golden.png').write_bytes(b'vulkan golden image') + self.write_inputs(case_overrides={'backend_overrides':{'DirectVulkan':{ + 'golden':'vulkan-golden.png'}}},backends=('DirectGLES','DirectVulkan')) + self.set_case_env(['CASE_MATCH_GOLDEN='+str(self.fixtures/'golden.png')]) + self.assertEqual(self.run_main('--backend','DirectVulkan'),1) + row=self.checkpoint()[-1] + self.assertEqual(row['status'],'failed') + self.assertIn('another backend declares',row['error']) + self.assertIn('golden.png',row['error']) + self.assertEqual(row['returncode'],1) + # The control: the same stub reporting the DirectVulkan golden passes. + self.write_inputs(case_overrides={'backend_overrides':{'DirectVulkan':{ + 'golden':'vulkan-golden.png'}}},backends=('DirectGLES','DirectVulkan')) + self.set_case_env(['CASE_MATCH_GOLDEN='+str(self.fixtures/'vulkan-golden.png')]) + self.assertEqual(self.run_main('--backend','DirectVulkan'),0) + row=self.checkpoint()[-1] + self.assertEqual(row['status'],'passed') + self.assertEqual(row['backend'],'DirectVulkan') + self.assertEqual(Path(row['golden']).name,'vulkan-golden.png') + + def test_directvulkan_run_ahead_proof_and_checkpoint_keys_are_per_backend(self): + self.write_inputs(backends=('DirectGLES','DirectVulkan')) + self.set_arm('respect-env',['MOBILEGL_IPC_RUN_AHEAD=0','MOBILEGL_IPC_VERB_BARRIER=0']) + self.assertEqual(self.run_main('--backend','DirectGLES','--backend','DirectVulkan', + '--require-run-ahead'),0) + rows=self.checkpoint() + self.assertEqual([row['backend'] for row in rows],['DirectGLES','DirectVulkan']) + self.assertEqual({row['key'] for row in rows}, + {'DirectGLES/credit-2/Example','DirectVulkan/credit-2/Example'}) + self.assertTrue(all(row['actual_arm']=={'armed':True,'lockstep':False,'disarmed':False} + for row in rows)) + # Each backend keeps its own checkpoint entry: resuming one must not consume the other. + self.assertEqual(self.run_main('--backend','DirectVulkan','--require-run-ahead','--resume'),0) + self.assertEqual(len(self.checkpoint()),2) + + def test_a_directvulkan_lockstep_log_is_refused_exactly_as_a_directgles_one_is(self): + self.write_inputs(backends=('DirectGLES','DirectVulkan')) + self.set_arm('lockstep') + self.assertEqual(self.run_main('--backend','DirectVulkan','--require-run-ahead'),1) + row=self.checkpoint()[-1] + self.assertEqual(row['backend'],'DirectVulkan') + self.assertEqual(row['status'],'failed') + self.assertIn('required run-ahead',row['error']) + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/tools/trace_replay/trace_cases.py b/tools/trace_replay/trace_cases.py index ffa80339..37817bae 100644 --- a/tools/trace_replay/trace_cases.py +++ b/tools/trace_replay/trace_cases.py @@ -39,8 +39,15 @@ KNOWN_CASE_KEYS = frozenset({ "verify", "split", "avoid_angle_llvmpipe_explicit_lod_bias", + "backend_overrides", }) +# The only fields a per-backend block may restate. Deliberately three: they are the three the +# comparison is made of. Letting a backend restate `target_call` or `crop_*` would mean the two +# backends stopped retracing the same frame of the same trace through the same window, and the +# rates would no longer be comparable - which is the one thing the two-backend matrix is for. +BACKEND_OVERRIDE_KEYS = frozenset({"golden", "alternate_golden", "ssim_threshold"}) + def load_trace_case_manifest(path=TRACE_CASES_JSON): with Path(path).open("r", encoding="utf-8") as file: @@ -108,10 +115,131 @@ def load_trace_case_manifest(path=TRACE_CASES_JSON): f"{name} is marked split but excluded from CI, so the split matrix would drop it" ) merged["split"] = split + validate_backend_overrides(merged) cases.append(merged) return {"defaults": defaults, "cases": cases} +def validate_backend_overrides(case): + """`backend_overrides` names the golden/threshold ONE backend uses instead of the shared one. + + P7 needs it because the device DirectVulkan matrix cannot be scored against the DirectGLES + picture. Until now the two backends shared `golden` plus an optional `alternate_golden`, and + `alternate_golden` is an OR, not a per-backend key: a DirectVulkan run that matched the + DirectGLES golden passed, and a DirectGLES run that matched a Magma-only image passed too. + That is exactly the "went green on the wrong arm" shape, so a per-backend golden REPLACES the + shared pair for that backend rather than being added to it (see case_for_backend). + + Everything here is a load-time refusal rather than a runtime surprise, for the reason the + KNOWN_CASE_KEYS comment above gives: the failure mode of a manifest key is a lane that + quietly measures something else. + """ + overrides = case.get("backend_overrides") + if overrides is None: + return + name = case["name"] + if not isinstance(overrides, dict) or not overrides: + raise ValueError(f"backend_overrides must be a non-empty object for {name}") + allowed = ci_backends(case) + for backend, entry in overrides.items(): + if backend not in CI_BACKENDS: + raise ValueError(f"unknown backend in backend_overrides for {name}: {backend}") + if backend not in allowed: + # An override for a backend the case never runs can only ever be dead text, and a + # dead override reads exactly like a live one to whoever audits the golden next. + raise ValueError( + f"backend_overrides names {backend} but {name} does not run it " + f"(ci_backends = {', '.join(allowed)})") + if not isinstance(entry, dict) or not entry: + raise ValueError(f"backend_overrides[{backend}] must be a non-empty object for {name}") + unknown = sorted(set(entry) - BACKEND_OVERRIDE_KEYS) + if unknown: + raise ValueError( + f"backend_overrides[{backend}] for {name} may only restate " + f"{', '.join(sorted(BACKEND_OVERRIDE_KEYS))}; got {', '.join(unknown)}") + for key in ("golden", "alternate_golden"): + if key in entry and not (isinstance(entry[key], str) and entry[key]): + raise ValueError(f"backend_overrides[{backend}].{key} must be a non-empty " + f"path for {name}") + threshold = entry.get("ssim_threshold") + if "ssim_threshold" in entry and (isinstance(threshold, bool) + or not isinstance(threshold, (int, float))): + raise ValueError(f"backend_overrides[{backend}].ssim_threshold must be a number " + f"for {name}") + if "alternate_golden" in entry and "golden" not in entry: + # Otherwise the block would add a second acceptable image to the SHARED golden, + # which is the OR this key exists to remove. + raise ValueError(f"backend_overrides[{backend}] for {name} sets alternate_golden " + f"without golden; state the backend's own golden explicitly") + + +def case_for_backend(case, backend): + """The case as ONE backend sees it: its own golden pair and threshold, or the shared ones. + + Returns the case UNCHANGED - the same object - when it declares no `backend_overrides` at + all, so a manifest without the key produces byte-identical selections, environments and run + fingerprints to the ones it produced before the key existed. + + THE KEY ITSELF IS DROPPED from the result even for a backend that overrides nothing, and + that is deliberate: `run_tcp_matrix` fingerprints the resolved case, so leaving the block in + would make "give DirectVulkan its own golden" invalidate every DirectGLES checkpoint in the + same run - a full re-sweep of an arm whose golden, threshold and trace did not move. + """ + if "backend_overrides" not in case: + return case + merged = {key: value for key, value in case.items() if key != "backend_overrides"} + overrides = (case.get("backend_overrides") or {}).get(backend) + if overrides: + if "golden" in overrides: + # REPLACE, do not extend: an override's golden is this backend's whole answer. + merged["alternate_golden"] = "" + merged.update(overrides) + return merged + + +def backend_override_goldens(case, backend): + """The goldens this case declares for OTHER backends - what `backend` must never match. + + Two sources, and the second is the one that is easy to miss: when THIS backend replaced the + shared golden, the shared `golden`/`alternate_golden` stop being neutral and become the + other backends' images. A DirectVulkan run that matched the shared (DirectGLES) picture is + precisely the accident the per-backend key exists to make impossible, so it is named here + rather than reported as "a different golden". + """ + overrides = case.get("backend_overrides") or {} + foreign = set() + if "golden" in (overrides.get(backend) or {}): + for key in ("golden", "alternate_golden"): + if case.get(key): + foreign.add(case[key]) + for other, entry in overrides.items(): + if other == backend: + continue + for key in ("golden", "alternate_golden"): + if entry.get(key): + foreign.add(entry[key]) + resolved = case_for_backend(case, backend) + for key in ("golden", "alternate_golden"): + foreign.discard(resolved.get(key)) + return foreign + + +def refuse_backend_overrides(cases, consumer): + """Consumers that cannot express a per-backend golden must say so, not drop it silently. + + `add_trace_replay_test_for_backends` and the APK matrix both carry ONE golden path for both + backends. Emitting a case that declares a per-backend golden through either of them would + register the DirectVulkan arm against the DirectGLES image - the precise failure this key was + added to close - so they refuse until they learn the key. + """ + named = [case["name"] for case in cases if case.get("backend_overrides")] + if named: + raise ValueError( + f"{consumer} cannot express backend_overrides yet, and the following case(s) declare " + f"it: {', '.join(named)}. Teach {consumer} the key in the same commit that adds the " + f"override, or the arm it emits runs against another backend's golden.") + + def load_trace_cases(path=TRACE_CASES_JSON): return load_trace_case_manifest(path)["cases"] @@ -143,6 +271,7 @@ def fixture_files(case, fixture_root): def github_apk_case(case): + refuse_backend_overrides([case], "the APK retrace matrix (github_apk_case)") result = dict(case) for key in ("trace_archive", "golden", "alternate_golden"): if result.get(key): @@ -332,6 +461,7 @@ def cmake_quote(value): def emit_cmake(cases, fixture_root): + refuse_backend_overrides(cases, "the CTest catalog emitter (emit_cmake)") root = fixture_root.rstrip("/\\") lines = ["# Generated from tools/trace_replay/trace_cases.json.", ""] keys = [