Same-pass feedback: a read-only image load of a colour attachment takes the snapshot too (#1009)

dm/games/010074F013262000/README.md

@@ -179,11 +179,45 @@ closes with A; stand near an enemy to auto-attack and use the left stick to walk
 During a movie, Plus then a held X skips it. The diagnostic watchdog is off when
 `DM_WATCH` is absent; setting it to `0` still enables its default stall threshold.
 
+## Rendering
+
+**Cutscene zone flicker (GitHub issue #1009, resolved).** Reported on an M4 Max
+in Events Theater scenes 1–3. The lighting-zone shader `15816d836842bfb5`
+(`shd_lgt.wishp`, bound as `15816d836842bfb5_b`) reads its own R32Uint colour
+attachment as image slot 0 between barriers, to skip zones it has already
+marked. The translated slot is `texture2d<uint>` with no access qualifier. An
+open Metal pass returned the attachment from the end of the previous frame, so
+each zone bit inverted on alternate frames. `fb_prepare` now snapshots a
+read-only image of a colour attachment the same way it snapshots a texture
+sample ([A read-only image load of a colour attachment](../../../docs/reference/rendering.md#a-read-only-image-load-of-a-colour-attachment)).
+
+Measured on an Apple M2 (8 GB) with two hosts that differ only by this change,
+built at `5474c6f` (shader chain `d1043b780a74`, the chain of the libraries
+built for this game), 48 consecutive distinct presents per burst. On chapter 1's
+second scene, the battle with Colony Sigma (the thumbnail to the right of the
+Prologue), the host before the change reverses every frame: one stretch
+alternates about 38.3 and 40.6 (sign-flip fraction 1.00, even/odd gap 2.24,
+within-group spread 0.52), another about 64 and 58 after a cut; 8 of 10 bursts
+have a flip fraction of 0.83–1.00. The host with the change holds one level:
+its highest flip fraction across 11 bursts is 0.28, and the stretch near 40
+steps by 0.12 on average. A milestone in its log names `15816d836842bfb5_b`
+slot 0 on the snapshot path. The chapter 1 prologue does not show the
+two-level step on either host.
+
+| run | frames, bursts left out | frame p50 | GPU p50 | fps |
+|---|---|---|---|---|
+| before | 13040–15307 | 53.2 ms | 50.4 ms | 18.3 |
+| after | 5521–7913 | 55.2 ms | 53.0 ms | 17.2 |
+
+After CONTINUE the host with the change walks the first save's field (frame
+p50 51.7 ms, GPU p50 49.9 ms, 19.1 fps).
+
 ## Found on this game
 
 - [Monolith ARH1, XBC1 and HCPS/SLCT shader containers](../../../docs/reference/shaders.md#monolith-arh1-xbc1-and-hcpsslct-shader-containers).
 - [Subgroup operations rewritten for a one-lane subgroup](../../../docs/reference/shaders.md#subgroup-operations-rewritten-for-a-one-lane-subgroup).
 - [IMAGE usage does not split texture storage](../../../docs/reference/rendering.md#image-usage-does-not-split-texture-storage).
+- [A read-only image load of a colour attachment](../../../docs/reference/rendering.md#a-read-only-image-load-of-a-colour-attachment).
 - [Rasterizer discard preserves vertex storage writes](../../../docs/reference/rendering.md#rasterizer-discard-preserves-vertex-storage-writes).
 - [Primitive barriers order vertex storage writes](../../../docs/reference/rendering.md#primitive-barriers-order-vertex-storage-writes).
 - [Transform feedback captured by a vertex entry point](../../../docs/reference/rendering.md#transform-feedback-captured-by-a-vertex-entry-point): interleaved outputs and geometry capture.

dm/gfx/nvn/nvn_mtl.m

@@ -7488,6 +7488,9 @@ static int encode_args_(id<MTLCommandEncoder> en, NvShader *sh, int nvn_stage, i
             int tid = reg < 32 ? (int)(S.img[nvn_stage][reg] & 0xfffff) : 0;
             uint64_t trid; int real;
             id<MTLTexture> t = img_binding(tid, b, &trid, &real);
+            int snapped = 0;   // same-pass feedback through a read-only image: the snapshot fb_prepare registered, matched on the texture img_binding just returned (a level/layer view stays that view)
+            if (real && !b->rw && !b->atom && g_fb_n && g_fb_seq == g_draw_seq && nvn_stage == 1)
+                for (int q = 0; q < g_fb_n; q++) if (t == g_fb_src[q]) { t = g_fb_use[q]; trid = g_fb_rid[q]; snapped = 1; break; }
             if (b->atom && !img_atomic_ok(t, b)) {   // see dummy_img_atomic; an unbound slot's write sink is not atomic-capable either
                 if (real) { id<MTLTexture> base = t.parentTexture ? t.parentTexture : t;
                     RED_ONCE(93, "image atomics on a texture Metal cannot run them on (shader %s stage %d slot %d texture ID %d: Metal format %lu of base format %lu, type %lu for %u, %s): they go to a write sink",
@@ -7499,7 +7502,7 @@ static int encode_args_(id<MTLCommandEncoder> en, NvShader *sh, int nvn_stage, i
             if (passdump()) PD("W %llu st%d reg%d tid=%d %s ser=%llu fmt=%lu %lux%lu view_sl=%lu array=%lu target=%lu shader wants target %u", g_stat_pass, nvn_stage, reg, tid, real ? "real" : "dummy", real && tid < g_ntreg ? g_treg[tid].t.serial : 0ull,
                                   (unsigned long)t.pixelFormat, (unsigned long)t.width, (unsigned long)t.height, (unsigned long)(t.parentTexture ? t.parentRelativeSlice : 0), (unsigned long)t.arrayLength, (unsigned long)t.textureType, (unsigned)b->dtt);
             slots[aid] = trid;
-            USE(t, MTLResourceUsageRead | MTLResourceUsageWrite);
+            USE(t, snapped ? MTLResourceUsageRead : MTLResourceUsageRead | MTLResourceUsageWrite);   // the snapshot is shader-read only (fb_snap): a read-only image declares no write access (below)
         } else if (b->kind == NVN_RB_TBUF) {   // Typed buffer: the buffer texture (target BUFFER) in texture slot r
             int tid = (int)(tex_slot_h(S.tex[nvn_stage], S.img[nvn_stage], reg) & 0xfffff);
             void *tv = NULL; uint64_t trid = tid ? tex_fast_rid(tid, &tv) : 0; __unsafe_unretained id<MTLTexture> t = (__bridge id<MTLTexture>)tv;
@@ -9260,7 +9263,14 @@ void ms_after(void) {
 // visible to texture reads only through nvnCommandBufferBarrier, so a snapshot taken since the game's last barrier (and in this submission) holds everything the game may expect to read; later such
 // draws reuse it without ending the pass, unless the pass also read that attachment directly. Breath of the Wild's G-buffer decals write and sample their own targets, about 460 draws a frame outdoors:
 // a fresh snapshot for each took the Mac's GPU time from about 4 to 28 ms a frame, reuse until a barrier needs about ten a frame (GPU time within the noise of none).
-// DM_NVN_FB_SNAP=0 none, all a fresh snapshot for every draw sampling an attachment of its pass (diagnostic)
+// A read-only image load is the same rule. Xenoblade Chronicles 3's lighting zone mask (15816d836842bfb5, shd_lgt.wishp) reads its own R32Uint attachment as image slot 0 between barriers, to skip zones
+// it has already marked. The translated slot is texture2d<uint> with no access qualifier (MSL's default access::sample; the reflection's rw stays 0, since that type contains neither access::write nor
+// access::read_write), so the snapshot fb_snap makes with shader-read usage is a legal binding and is declared read-only for residency. Read straight from the attachment, an open pass shows the image
+// the end of the previous frame instead of this pass's clear and draws, and each zone bit turns off and on every other frame: the flicker in rendered cutscenes. The texture compared with the attachment,
+// and the one encode_args compares with g_fb_src, is the object img_binding would bind: a RegisterImage view already names one level and layer, and a swizzle-only binding is img_fit's identity view of
+// that same range. A declared pixel class that needs a reinterpreting view is left as is (that view is a different object). An image read of an attachment nothing in the pass has written sets g_fb_rmask,
+// so a later writer starts a new pass, as a texture read does.
+// DM_NVN_FB_SNAP=0 none, all a fresh snapshot for every draw sampling or image-loading an attachment of its pass (diagnostic)
 static int g_fb_mode = -1;   // 0 none, 1 writers with reuse until a barrier, 2 every sampling draw, fresh
 static uint64_t g_fb_bar, g_fb_reuse, g_fb_split;   // game barriers so far (OP_BARRIER with bits); draws that reused a snapshot; passes ended before a write to an attachment an earlier draw read
 static uint64_t g_fb_pass = UINT64_MAX; static uint32_t g_fb_wmask, g_fb_rmask, g_fb_dread;   // the current Metal render pass: color attachments written so far, sampled directly; a draw has read its depth attachment (or could have: bindless)
@@ -9299,14 +9309,29 @@ static void fb_prepare(NvProg *pr) {
         || S.ds.fail_b != NVN_STENCIL_OP_KEEP || S.ds.zfail_b != NVN_STENCIL_OP_KEEP || S.ds.zpass_b != NVN_STENCIL_OP_KEEP)));
     if (g_enc && dsw && g_fb_dread) { g_fb_split++; rp_end(); fb_pass_sync(); }   // the same for its depth and stencil (see rtc_on)
     if (R.rt.depth && (fs->rf.bindless || fs->rf.dyn)) g_fb_dread = 1;
-    for (int i = 0; i < fs->rf.nb && g_fb_n < 4; i++) { const nvn_rbind *b = &fs->rf.b[i]; if (b->kind != NVN_RB_TEX || b->reg >= NTEX) continue;
-        int tid = (int)(tex_slot_h(S.tex[1], S.img[1], b->reg) & 0xfffff); if (!tid) continue;
+    for (int i = 0; i < fs->rf.nb && g_fb_n < 4; i++) { const nvn_rbind *b = &fs->rf.b[i];
+        // Read-only image slots take the same snapshot as texture slots (see the note above g_fb_mode). reg < 32 is encode_args' own bound for S.img
+        int is_img = b->kind == NVN_RB_IMG && !b->rw && !b->atom && b->reg < 32;
+        if (!is_img && (b->kind != NVN_RB_TEX || b->reg >= NTEX)) continue;
+        int tid = is_img ? (int)(S.img[1][b->reg] & 0xfffff) : (int)(tex_slot_h(S.tex[1], S.img[1], b->reg) & 0xfffff); if (!tid) continue;
         os_unfair_lock_lock(&g_rmu); uint64_t ser = tid < g_ntreg && g_treg[tid].ok ? g_treg[tid].t.serial : 0; os_unfair_lock_unlock(&g_rmu);
         if (!ser) continue;
         if (R.rt.depth && R.rt.depth->serial == ser) g_fb_dread = 1;
+        // The image's texture is resolved once, the way encode_args will bind it, so a view of one level or layer is what the checks and g_fb_src use
+        id<MTLTexture> img_t = nil;
+        if (is_img) {
+            os_unfair_lock_lock(&g_rmu);
+            void *rawv = NULL; tex_fast_rid(tid, &rawv); id<MTLTexture> raw = (__bridge id<MTLTexture>)rawv;
+            int reinterpret = raw && b->dpf != NVN_DPF_FLOAT && mtl_pf_dpf(raw.pixelFormat) != b->dpf;   // img_binding makes a class view here (img_fit); that object is not the attachment
+            if (!reinterpret) { uint64_t trid; int real; img_t = img_binding(tid, b, &trid, &real); if (!real) img_t = nil; }
+            os_unfair_lock_unlock(&g_rmu);
+            if (!img_t) continue;
+        }
         for (int k = 0; k < R.rt.n && k < 8 && g_fb_n < 4; k++) { if (!R.rt.color[k] || R.rt.color[k]->serial != ser) continue;
             uint64_t aser; int lv, sl; rt_sub(&R.rt, k, &aser, &lv, &sl);
-            os_unfair_lock_lock(&g_rmu); void *tv = NULL; tex_fast_rid(tid, &tv); id<MTLTexture> t = (__bridge id<MTLTexture>)tv; os_unfair_lock_unlock(&g_rmu);
+            id<MTLTexture> t;
+            if (is_img) t = img_t;
+            else { os_unfair_lock_lock(&g_rmu); void *tv = NULL; tex_fast_rid(tid, &tv); t = (__bridge id<MTLTexture>)tv; os_unfair_lock_unlock(&g_rmu); }
             if (!t) continue;
             int dup = 0; for (int q = 0; q < g_fb_n; q++) dup |= g_fb_src[q] == t; if (dup) continue;
             id<MTLTexture> base = t.parentTexture ? t.parentTexture : t; NSUInteger tl = t.parentTexture ? t.parentRelativeLevel : 0, ts = t.parentTexture ? t.parentRelativeSlice : 0;

dm/gfx/tests/Makefile

@@ -253,6 +253,8 @@ t_geometry_primitive_id: t_geometry_primitive_id.m t_geometry.m $(MTL_T_DEP)
 .PHONY: check_t_feedback
 check_t_feedback: t_feedback
 	MTL_DEBUG_LAYER=1 ./t_feedback
+	DM_NVN_FB_SNAP=all MTL_DEBUG_LAYER=1 ./t_feedback --image
+	DM_NVN_FB_SNAP=0 MTL_DEBUG_LAYER=1 ./t_feedback --image
 t_mip_extent: t_mip_extent.m ../nvn/hle_nvn.c ../../host/crash.c ../../host/hle_bigstack.c t_host_stubs.c $(GFX) $(GFX_H)
 	clang $(CFLAGS) -fobjc-arc -o $@ t_mip_extent.m ../../host/crash.c ../../host/hle_bigstack.c t_host_stubs.c $(filter-out ../nvn/nvn_mtl.m,$(GFX)) $(FW)
 .PHONY: check_t_mip_extent