fix(vk): with per-queue fences, a CPU read-back waits for the GPU work that writes it again

gpu/vk/vk_replay.cpp

@@ -2293,9 +2293,16 @@ void invalidate(uint64_t gpu, uint64_t size) {
             // the work not recorded yet is stopped at an event wait. Catching up forced it past that wait, ahead of the work it
             // waits for (OoT's presentation work before the scene's: the fog of the frame before, for a frame, at 1 in 20 of the
             // exposure readbacks). The write-back is waited for on the recordings' timeline instead, which a stopped queue
-            // does not hold back
+            // does not hold back. The stopped work whose waits are met goes on first, never forced (sync_settle): it may be what
+            // writes this range, and its write-back is known only once it is recorded. Without it the CPU read the range
+            // before that work, with nothing to wait for (once d6c5ddcc's catch-up no longer forced anything, it was left
+            // out here too). RX_VK_PER_QUEUE_FENCES=0 waits for every queue's work instead: the game's readbacks 19-87 ms
+            // each on an RX 5500M, against ~1 ms here
             const bool unforced = !async_on() && sync_waits_on() && !per_queue_fences_off();
-            if (behind0 && !no_catchup && !unforced) wait_queue_empty(), w = true;   // (work handed over may write it back too)
+            if (behind0 && !no_catchup) {   // (work handed over may write it back too)
+                unforced ? sync_settle() : wait_queue_empty();
+                w = true;
+            }
             const auto w1 = std::chrono::steady_clock::now();
             const uint64_t v = memory_writeback_value(host->gpu_guest(gpu), size, nullptr);
             if (v > completed()) {

tests/test_readback_settle.py

@@ -0,0 +1,39 @@
+"""A CPU readback of GPU results lets the met stopped work on first, with per-queue fences too (gpu/vk/vk_replay.cpp's invalidate).
+
+A range's write-back value is known only once the work that writes it is recorded. With per-queue fences (the default) the
+readback skipped the catch-up, written when a catch-up forced stopped work past its waits; d6c5ddcc made it let on only
+the met waits, but the readback kept skipping it. Work stopped at a wait that was met by then (a CPU event, signalled,
+not applied yet) was not recorded, its write-back unknown, and the CPU read the range before it, waiting for nothing.
+Reads the source: the readback settles (never forcing) before it looks the value up.
+python -I -B tests/test_readback_settle.py
+"""
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[1]
+
+
+def body(src, signature):
+    """the text of the function that starts with signature, up to its closing brace at column 0"""
+    start = src.index(signature)
+    return src[start:src.index("\n}\n", start)]
+
+
+def main():
+    replay = (ROOT / "gpu" / "vk" / "vk_replay.cpp").read_text(encoding="utf-8")
+    inv = body(replay, "void invalidate(uint64_t gpu, uint64_t size) {")
+    lookup = "const uint64_t v = memory_writeback_value(host->gpu_guest(gpu), size, nullptr);"
+    if lookup not in inv:
+        raise SystemExit("invalidate no longer looks up the range's write-back value: update this test")
+    before = inv[:inv.index(lookup)]
+    if "!no_catchup && !unforced" in before:
+        raise SystemExit("the readback skips its catch-up with per-queue fences: stopped work that writes the range is "
+                         "read before it is recorded")
+    settle = before.rfind("unforced ? sync_settle() : wait_queue_empty();")
+    if settle < 0 or "if (behind0 && !no_catchup) {" not in before[:settle]:
+        raise SystemExit("the readback's catch-up before the write-back lookup is gone, or no longer settles with "
+                         "per-queue fences (sync_settle: the met waits only, never forced)")
+    print("PASS: a readback settles the met stopped work before it looks up the write-back")
+
+
+if __name__ == "__main__":
+    main()