diff --git a/dm/games/01004AB00A260000/README.md b/dm/games/01004AB00A260000/README.md index 30e15751ac..458e05c684 100644 --- a/dm/games/01004AB00A260000/README.md +++ b/dm/games/01004AB00A260000/README.md @@ -28,6 +28,8 @@ No `render_cond`: `SetRenderEnableConditional` is drawn unconditionally, as in B FromSoftware's engine draws forward into a 1920x1080 RGBA16F target, with depth of field, light shafts and bloom at quarter size. +- **Current: image-only ambient occlusion loses its attachment layer ([#187](https://github.com/seedds/nxnative/issues/187#issuecomment-6519569185)).** Jacob-Tate traced near-black Asylum areas to the zone-dependent ambient-occlusion pass `c1d0b7f1a8a50799`, whose image writes target layers of its own color attachment. The [attachment-store fix](../../../docs/reference/rendering.md#image-stores-into-an-attachment-of-their-own-pass) is reproduced and verified using synthetic GPU shaders on an M2 Max; the game's route and the reported iPhone torch patches have not been tested locally. The report also identified a separate PrimitiveID pipeline fault, already handled by the existing general pipeline path. The artifacts in [#308](https://github.com/seedds/nxnative/issues/308) are not established to have this cause. + - **Resolved: distant walls and floors smeared sideways.** The map materials (`FRPG_Phn_*Lit*`) read their combined texture / sampler handles from a uniform buffer (`fp_t_cb13_*`, resolved at replay by `ubo_handle`), and the game keeps 12 sampler pools of 2,304 entries, each with its own samplers under the same IDs (ID 261 is clamp-to-edge with LOD 0..0 in one pool, repeat with mips in others), one per command buffer that records the scene. The replay resolved uniform-buffer handles in the first pool, so the walls' tiling base textures (u ≈ −3.45) took a clamp-to-edge sampler and repeated their edge column across the wall. The replay now keeps the texture and sampler pools bound (`OP_SET_TEX_POOL` / `OP_SET_SAMP_POOL`) and adds their bases to handles read from uniform buffers (`ubo_tex_id` / `ubo_samp_id` in `nvn_mtl.m`). Found by highlighting the six most common fragment shaders of the scene pass (`DM_NVN_HILITE`), then the next six, and reading the wall draw's binding lines (`DM_NVN_PASSDUMP_ALL`). - **Resolved: 6,734 red lines in one run.** The list of command buffers that bound a non-first pool had 64 entries and was never cleared, and this game binds a non-first pool in more than 64. The bases of bound handles now live in each command buffer (`cmd_t` `tex_base` / `samp_base`). @@ -66,6 +68,8 @@ SDK 4.5.0 boots, one each: General host changes first needed here; each applies to every game: +- **Image stores preserved across an unchanged color attachment’s tile store (synthetic fix; game validation open):** [Image stores into an attachment of their own pass](../../../docs/reference/rendering.md#image-stores-into-an-attachment-of-their-own-pass). + - **SDK 4's GLSLC outputs read through their section headers** (`glslc_section`) and **uniform-buffer handles resolved against the bound pools** (`OP_SET_TEX_POOL` / `OP_SET_SAMP_POOL`, `ubo_tex_id` / `ubo_samp_id`, `cmd_t` `tex_base` / `samp_base`): [GLSLC outputs read through their section headers](../../../docs/reference/shaders.md#glslc-outputs-read-through-their-section-headers), [Handles from uniform buffers take the bound pools' bases](../../../docs/reference/shaders.md#handles-from-uniform-buffers-take-the-bound-pools-bases), [Bases of bound handles kept in the command buffer](../../../docs/reference/shaders.md#bases-of-bound-handles-kept-in-the-command-buffer). - **The dvdbnd / DCX / BND3 reader** (`bdt_shaders`): [GLSLC outputs in FromSoftware's BND3 and dvdbnd archives](../../../docs/reference/shaders.md#glslc-outputs-in-fromsoftwares-bnd3-and-dvdbnd-archives). - **SDK 4's movie decoder constructor and its buffer without a sync object** (`hle_movie.c`), **`friends::GetBlockedUserList` without a `Uid`**, **the UTF-16 profanity masking by name** (`hle_misc.c`), and **`Poll` / `Select` with nothing to wait on sleeping out their timeout** (`hle_net.c`): [SDK 4's `movie::Decoder` constructor without a priority](../../../docs/reference/movies.md#sdk-4s-moviedecoder-constructor-without-a-priority), [SDK 4's `movie::Buffer` has no sync object](../../../docs/reference/movies.md#sdk-4s-moviebuffer-has-no-sync-object), [SDK 4's `friends::GetBlockedUserList` without a `Uid`](../../../docs/reference/services.md#sdk-4s-friendsgetblockeduserlist-without-a-uid), [`nn::ngc::ProfanityFilter` vtable and by-name overloads](../../../docs/reference/services.md#nnngcprofanityfilter-vtable-and-by-name-overloads), [Socket `Poll` and `Select` with nothing to wait on](../../../docs/reference/services.md#socket-poll-and-select-with-nothing-to-wait-on). diff --git a/dm/gfx/nvn/nvn_geometry.inc b/dm/gfx/nvn/nvn_geometry.inc index 3300ee46df..cd0c1f445a 100644 --- a/dm/gfx/nvn/nvn_geometry.inc +++ b/dm/gfx/nvn/nvn_geometry.inc @@ -452,7 +452,7 @@ static int geo_mesh_draw(NvProg *pr, const p_draw *dw, const p_draw_idx *di) { g_geo_omit_colors = 0; g_geo_program = nil; g_geo_mesh.on = 0; args_dirty_all(); if (g_geo_mesh.miss) { g_geo_mesh_wait++; return 0; } - swrite_note(pr); udc_note_draw(pr); g_wr_epoch++; g_mdx_epoch++; // the kernel's storage writes + swrite_note(pr); udc_note_draw(pr, 0); g_wr_epoch++; g_mdx_epoch++; // the kernel's storage writes if (!g_geo_mesh_n++) MLOG("geometry mesh: first mesh draw, vertex %s kernel %s, %u triangles", pr->sh[0]->key, g_geo_mesh.lean->key, g_geo_mesh.a.count / 3); return 1; } @@ -660,7 +660,7 @@ static void geo_gs_one(id ce, const geo_job *j) { enc_c_args(ce, j->lean_job, 0); enc_c_pso(ce, g_geo_lean_k[j->direct_count ? 2 : 0]); enc_c_dispatch(ce, MTLSizeMake((j->groups + 63) / 64, 1, 1), MTLSizeMake(64, 1, 1)); } - swrite_note(j->pr); udc_note_draw(j->pr); g_wr_epoch++; g_mdx_epoch++; + swrite_note(j->pr); udc_note_draw(j->pr, 0); g_wr_epoch++; g_mdx_epoch++; } static void geo_raster(const geo_job *j, int last_in_run) { g_geo_bind[1] = j->output; diff --git a/dm/gfx/nvn/nvn_mtl.m b/dm/gfx/nvn/nvn_mtl.m index 1140f62fde..952a9d9ffc 100644 --- a/dm/gfx/nvn/nvn_mtl.m +++ b/dm/gfx/nvn/nvn_mtl.m @@ -3016,7 +3016,7 @@ static void pg_commit(id cb) { // Hand this submission's pas // ---------- Store lookahead: at pass end, decide from the later commands of the same submission whether each attachment's write-back is redundant ---------- // On TBDR every attachment is written back to memory in full when the pass ends; Switch (Maxwell immediate-mode rendering) has no such cost. If an attachment is cleared in full before being read, the write-back is redundant (DontCare). // For the decision see mtl_sub_index / sdx_decide; only what can be proven within the same submission is accepted, and anything still undecided at the end of the submission is always written back. -typedef struct { uint64_t ser, bytes; uint32_t lv, sl; int idx; uint8_t depth, state, ridx, drawn, why, load, wrote, wwhy; const char *wkey; int wslot; } sd_att; // depth: 0 color 1 depth 2 stencil (the two parts of a combined depth-stencil texture are decided separately) // state 0 undecided 1 needed 2 redundant; idx: color attachment index, -1 for depth +typedef struct { uint64_t ser, bytes; uint32_t lv, sl; int idx; uint8_t depth, state, ridx, drawn, why, load, wrote, wwhy, img, rast; const char *wkey; int wslot; } sd_att; // depth: 0 color 1 depth 2 stencil (the two parts of a combined depth-stencil texture are decided separately) // state 0 undecided 1 needed 2 redundant; idx: color attachment index, -1 for depth // Depth and stencil have independent store decisions, even in one combined texture. // Eight color targets therefore need ten records (XC2 uses the full set). enum { SD_ATT_MAX = 8 + 2 }; @@ -3024,13 +3024,13 @@ static void pg_commit(id cb) { // Hand this submission's pas static int g_udc_ab = -1, g_udc_ab_on; // Attachments this pass did not write (depth / stencil test only, color write mask 0, etc.) get their write-back changed to DontCare: the content is already in memory, saving one full write-back. // On by default: on Mac, same-process on / off screenshots follow the same distribution as the null control; on A19 Pro, private Depth32F_S8 / RGBA8 read back after DontCare differ in 0 places. -// DM_NVN_UNWRITTEN_DC=0 turns it off (for equivalence comparison) +// DM_NVN_UNWRITTEN_DC=0 turns optional savings off (for equivalence comparison); the image-only color store rule in sd_finish remains mandatory. static int udc_on(void) { if (g_udc_ab < 0) { const char *e = getenv("DM_NVN_UNWRITTEN_DC"); g_udc_ab = !(e && !strcmp(e, "0")); const char *a = getenv("DM_NVN_UDC_AB"); if (a && !strcmp(a, "1")) { g_udc_ab = 0; g_udc_ab_on = 0; } } return g_udc_ab || g_udc_ab_on; } -// DM_NVN_TBDR=0 disables lookahead and pass merging. The independent unwritten-attachment proof remains active; DM_NVN_UNWRITTEN_DC=0 disables that too. +// DM_NVN_TBDR=0 disables lookahead and pass merging. DM_NVN_UNWRITTEN_DC=0 also disables optional unwritten-attachment savings; neither disables image-write preservation. // Only for equivalence comparison (the same replay run on / off, comparing the picture byte by byte), not a product configuration // Mario: off by default (only DM_NVN_TBDR=1 turns it on). These optimizations infer read / write relations from "which textures are bound in texture slots", while 38% of this game's shaders use bindless textures (handles in uniform buffers, NVN_RB_TEXU), // so the inference would miss readers - e.g. deciding an attachment is never read afterwards and not storing it. Turn them back on once they recognize bindless readers. @@ -3516,7 +3516,7 @@ static void attach(MTLRenderPassAttachmentDescriptor *a, const tex_t *t, const v g_nsd = 0; { for (int i = 0; i < R.rt.n && i < 8; i++) { MTLRenderPassColorAttachmentDescriptor *c = pd.colorAttachments[i]; if (!c.texture || bcrt_is(c.texture)) continue; - g_sd[g_nsd] = (sd_att){0}; g_sd[g_nsd].ser = R.rt.color[i]->serial; g_sd[g_nsd].bytes = att_bytes(c.texture, c.level); g_sd[g_nsd].lv = (uint32_t)c.level; g_sd[g_nsd].sl = (uint32_t)(c.texture.textureType == MTLTextureType3D ? c.depthPlane : c.slice); g_sd[g_nsd].idx = i; g_sd[g_nsd].load = c.loadAction == MTLLoadActionLoad; g_sd[g_nsd].wrote = R.clear_c[i] != 0; g_nsd++; + g_sd[g_nsd] = (sd_att){0}; g_sd[g_nsd].ser = R.rt.color[i]->serial; g_sd[g_nsd].bytes = att_bytes(c.texture, c.level); g_sd[g_nsd].lv = (uint32_t)c.level; g_sd[g_nsd].sl = (uint32_t)(c.texture.textureType == MTLTextureType3D ? c.depthPlane : c.slice); g_sd[g_nsd].idx = i; g_sd[g_nsd].load = c.loadAction == MTLLoadActionLoad; g_sd[g_nsd].wrote = g_sd[g_nsd].rast = R.clear_c[i] != 0; g_nsd++; c.storeAction = MTLStoreActionUnknown; } id dt = pd.depthAttachment.texture, stt = pd.stencilAttachment.texture; uint64_t db = att_bytes(dt, pd.depthAttachment.level), sb = 0; @@ -4881,21 +4881,14 @@ static void swrite_note(NvProg *pr) { if (g_pass_swrite) { g_vbuf_write_epoch++; return; } // further shader writes also invalidate aligned vertex snapshots if (prog_swrites(pr)) { g_pass_swrite = 1; g_vbuf_write_epoch++; } } -static void udc_note_draw(NvProg *pr) { +static void udc_note_draw(NvProg *pr, uint32_t raster_mask) { int td = tbdr_on(); - if (!td && !udc_on()) { - uint32_t cm = pr && pr->sh[1] ? pr->sh[1]->rf.color_mask : 0; - if (R.has_rt && cm >> 1) for (int i = 1; i < R.rt.n && i < 8; i++) - if ((cm >> i & 1) && ((S.chmask.mask >> (4 * i)) & 0xf) && rt_dup_slot(&R.rt, i)) - RED_ONCE(79, "color slot %d holds the same texture as an earlier slot (ser=%llu) and this draw writes it (mask %#x): these writes are lost", i, (unsigned long long)R.rt.color[i]->serial, (S.chmask.mask >> (4 * i)) & 0xf); - return; - } if (td && R.has_rt) for (int i = 0; i < R.rt.n && i < 8; i++) if (R.rt.color[i]) { zskip_del(R.rt.color[i]->serial); g_zskip_cand_ok[i] = 0; } // Zero-source set: once really drawn it is no longer all zeros if (td && pr) for (int st = 0; st < 6; st++) { __unsafe_unretained NvShader *sh = pr->sh[st]; if (!sh) continue; for (int i = 0; i < sh->rf.nb; i++) if (sh->rf.b[i].kind == NVN_RB_IMG) { zskip_clear(); break; } } // Image slots are writable: conservatively clear int image_write = 0; - uint32_t shader_color_mask = 0; + NvShader *fs = pr ? pr->sh[1] : nil; + uint32_t shader_color_mask = fs ? (fs->rf.color_known ? fs->rf.color_mask : UINT32_MAX) : 0; if (pr) for (int st = 0; st < 6; st++) { __unsafe_unretained NvShader *sh = pr->sh[st]; if (!sh) continue; - if (st == 1) shader_color_mask = sh->rf.color_known ? sh->rf.color_mask : UINT32_MAX; // A writable image, atomic image pointer or storage pointer may alias an attachment. Do not infer its identity from bound texture slots (bindless shaders need not use them). for (int i = 0; i < sh->rf.nb; i++) { const nvn_rbind *b = &sh->rf.b[i]; if (b->rw && (b->kind == NVN_RB_IMG || b->kind == NVN_RB_SBUFP || b->kind == NVN_RB_SBUFU)) { image_write = 1; break; } } @@ -4906,6 +4899,8 @@ static void udc_note_draw(NvProg *pr) { RED_ONCE(79, "color slot %d holds the same texture as an earlier slot (ser=%llu) and this draw writes it (mask %#x): these writes are lost", i, (unsigned long long)R.rt.color[i]->serial, (S.chmask.mask >> (4 * i)) & 0xf); for (int i = 0; i < g_nsd; i++) { sd_att *t = &g_sd[i]; + if (image_write) t->img = 1; + if (t->depth == 0 && t->idx >= 0 && (raster_mask >> t->idx & 1)) t->rast = 1; if (image_write) { t->wrote = 1; t->wwhy = 1; } else if (t->depth == 0 && t->idx >= 0 && (((S.chmask.mask >> (4 * t->idx)) & 0xf) || (shader_color_mask >> t->idx & 1))) { t->wrote = 1; t->wwhy = (S.chmask.mask >> (4 * t->idx)) & 0xf ? 2 : 3; } else if (t->depth == 1 && S.ds.dw) { t->wrote = 1; t->wwhy = 4; } @@ -7473,7 +7468,10 @@ static void sd_finish(void) { && !xv_disabled(xtag = xv_tag_pass(R.rt.color[t->idx], t->lv, t->idx, &R.rt)); int xvdead = xvcand && xv_trial_get(xtag, 0) >= XV_TRIAL; // Only after the trial period is the write-back really skipped if (xvcand && !xvdead) xv_set(t->ser, t->lv, t->sl, XV_PROBE, xtag); // Trial period: write back as usual, only track the next touch - int dead = t->state == 2 || udc_dead || xvdead; MTLStoreAction sa = dead ? MTLStoreActionDontCare : MTLStoreActionStore; + // A loaded color tile with no raster output or clear holds only the old memory contents. Storing it can overwrite image writes to the same texture + // (Dark Souls' image-only AO pass). This is required for correctness even when store optimizations are disabled; mixed tile/image writers retain their store. + int img_dead = t->depth == 0 && t->load && t->img && !t->rast; + int dead = t->state == 2 || udc_dead || xvdead || img_dead; MTLStoreAction sa = dead ? MTLStoreActionDontCare : MTLStoreActionStore; if (xvdead) { xv_set(t->ser, t->lv, t->sl, XV_INVALID, xtag); g_xv_dc++; } if (t->depth == 1) [g_enc setDepthStoreAction:sa]; else if (t->depth == 2) [g_enc setStencilStoreAction:sa]; else [g_enc setColorStoreAction:sa atIndex:(NSUInteger)t->idx]; g_sd_n_all++; if (dead) { g_sd_n_dead++; g_sd_dead_bytes += t->bytes; g_bw_store -= t->bytes; if (udc_dead) g_udc_saved_bytes += t->bytes; } @@ -9477,6 +9475,18 @@ static void cs_hoist_try(NvProg *pr0, const p_draw *dw0, const p_draw_idx *di0) void mtl_geomesh_stats(uint64_t *n, uint64_t *wait) { *n = g_geo_mesh_n; *wait = g_geo_mesh_wait; } void mtl_geohoist_stats(uint64_t *moved, uint64_t *runs, uint64_t *stops) { *moved = g_geo_hoisted; *runs = g_geo_hoist_runs; memset(stops, 0, 6 * sizeof *stops); stops[3] = g_geo_dependency_splits; } void mtl_cshoist_stats(uint64_t *moved, uint64_t *depth, uint64_t *stops) { *moved = g_csh_moved; *depth = g_csh_depth; for (int i = 0; i < 8; i++) stops[i] = g_csh_stop[i]; } +// Physical color attachments the selected pipeline can write. A merged pass remaps logical outputs and channel masks; a specialized fragment's outputs are unknown here. +static uint32_t draw_color_writes(NvProg *pr) { + NvShader *fs = pr ? pr->sh[1] : nil; + if (!fs || S.rast_discard) return 0; + uint32_t outputs = g_pso_fsfn || ms_fs_override(fs) || !fs->rf.color_known ? UINT32_MAX : fs->rf.color_mask; + uint32_t mask = 0; + for (int i = 0; i < 8; i++) { + unsigned l = g_pso_lmap ? g_pso_lmap[i] : (unsigned)i; + if (l < 8 && ((S.chmask.mask >> (4 * l)) & 0xf) && (outputs >> l & 1)) mask |= 1u << i; + } + return mask; +} static void draw_(const p_draw *dw, const p_draw_idx *di); static void draw(const p_draw *dw, const p_draw_idx *di) { cp_stamp start = cp_begin(); draw_(dw, di); cp_end(5, start); } static void draw_(const p_draw *dw, const p_draw_idx *di) { @@ -9596,6 +9606,7 @@ static void draw_(const p_draw *dw, const p_draw_idx *di) { g_sm_nwait++; PD("M %llu slice-merged pass: no amplified pipeline yet for vertex %s fragment %s; split here", g_stat_pass, pr->sh[0] ? pr->sh[0]->key : "-", pr->sh[1] ? pr->sh[1]->key : "-"); rp_end(); en = rp_begin_ex(mode); if (!en) return; cur_ps = pso_get(pr, nil); cur_pull = g_pso_pull; cur_pr = pr; cur_ps_pass = g_pass.serial; g_pso_dirty = !cur_ps || cur_pull; } + uint32_t raster_mask = draw_color_writes(pr); // Capture the physical mapping before resetting the merged pipeline state. if (g_pso_lmap) { g_pso_lmap = NULL; g_pso_fsfn = nil; memset(&g_pso_fz, 0, sizeof g_pso_fz); g_pso_dirty = 1; } // The next draw fetches again according to its own situation __unsafe_unretained id ps = cur_ps; if (!ps) { PD("Z %llu draw skipped: no render pipeline (vertex %s fragment %s)", g_stat_pass, pr->sh[0] ? pr->sh[0]->key : "-", pr->sh[1] ? pr->sh[1]->key : "-"); return; } @@ -9735,7 +9746,7 @@ static void draw_(const p_draw *dw, const p_draw_idx *di) { #ifdef NVN_RT_RANGE_TEST rr_snap(dw,di); #endif - udc_note_draw(pr); + udc_note_draw(pr, raster_mask); swrite_note(pr); order_note(pr); if (g_geo_mesh.on) { geo_mesh_encode(en); return; } if (g_geo_program) { @@ -10600,7 +10611,7 @@ static void clear_tex_tri(id t, unsigned lv, unsigned l0, const p_cl static void clr_note(int slot, int depth, int stencil) { // as udc_note_draw for a draw: the attachment was written (zero-source skip, unwritten-attachment store) if (slot >= 0 && R.rt.color[slot]) { zskip_del(R.rt.color[slot]->serial); g_zskip_cand_ok[slot] = 0; } for (int i = 0; i < g_nsd; i++) { sd_att *t = &g_sd[i]; - if ((t->depth == 0 && slot >= 0 && t->idx == slot) || (t->depth == 1 && depth) || (t->depth == 2 && stencil)) { t->wrote = 1; t->wwhy = 2; } } + if ((t->depth == 0 && slot >= 0 && t->idx == slot) || (t->depth == 1 && depth) || (t->depth == 2 && stencil)) { t->wrote = t->rast = 1; t->wwhy = 2; } } } static uint64_t g_clr_tri[2]; // clears drawn as a triangle: colour, depth / stencil static void clear_color_tri(const p_clear_color *c) { diff --git a/dm/gfx/tests/Makefile b/dm/gfx/tests/Makefile index 8b9c607f1c..9e1960e089 100644 --- a/dm/gfx/tests/Makefile +++ b/dm/gfx/tests/Makefile @@ -363,6 +363,9 @@ check_t_attachment_capacity: t_attachment_capacity @for compiler in 0 1; do for td in 0 1; do for enc in 0 1; do \ DM_PSO_MTL4=$$compiler DM_NVN_TBDR=$$td DM_NVN_ENC_THREAD=$$enc MTL_DEBUG_LAYER=1 ./t_attachment_capacity || exit $$?; \ done; done; done +t_image_attachment_store: t_image_attachment_store.m $(MTL_T_DEP) + clang $(CFLAGS) -fobjc-arc -o $@ t_image_attachment_store.m $(MTL_T_SRC) $(FW) +check_t_image_attachment_store: RUN_ENV = MTL_DEBUG_LAYER=1 t_unwritten_store: t_unwritten_store.m $(MTL_T_DEP) clang $(CFLAGS) -fobjc-arc -o $@ t_unwritten_store.m $(MTL_T_SRC) $(FW) check_t_unwritten_store: RUN_ENV = MTL_DEBUG_LAYER=1 diff --git a/dm/gfx/tests/t_image_attachment_store.m b/dm/gfx/tests/t_image_attachment_store.m new file mode 100644 index 0000000000..255142274b --- /dev/null +++ b/dm/gfx/tests/t_image_attachment_store.m @@ -0,0 +1,121 @@ +// A loaded color attachment and a fragment image refer to the same array texture. +// The image-only draw must survive pass end; raster outputs and clears still need their tile store. +#include "../nvn/nvn_mtl.m" +void *front_metal_layer(void) { return NULL; } +module_t *module_of(const void *p) { (void)p; return NULL; } +module_t *g_mod; int g_nmod; +void svc_thread_dump(void) {} +uint64_t now_ns(void) { return bp_now(); } +static int checks, failed; +#define CHECK(c, label) do { checks++; if (!(c)) { failed++; fprintf(stderr,"FAIL: %s\n",label); } } while (0) +static tex_t color = {.magic=MAGIC('T','E','X',0),.serial=99801}; +static prog_t programs[3] = {{.serial=99802},{.serial=99803},{.serial=99804}}; +static void seed(void) { + for (unsigned layer=0;layer<4;layer++) { + MTL4RenderPassDescriptor *p=[MTL4RenderPassDescriptor new]; + p.colorAttachments[0].texture=tex_mtl(&color); p.colorAttachments[0].slice=layer; + p.colorAttachments[0].loadAction=MTLLoadActionClear; + p.colorAttachments[0].storeAction=MTLStoreActionStore; + p.colorAttachments[0].clearColor=MTLClearColorMake(0,0,0,1); + enc_r_end(enc_r_begin(p)); + } +} +static void run(int mode, int store_all) { + int raster=mode==1 || mode==4 || mode==6; + // Metal 4 encoders do not own pipeline lifetimes. Keep the boundary fixture's uncached pipeline alive through deferred replay and GPU completion. + __attribute__((objc_precise_lifetime)) id boundary_pipeline=nil; + static unsigned frame; + state_reset(); memset(&R,0,sizeof R); mtl_set_frame(++frame); mtl_sub_begin(); seed(); + g_udc_ab=!store_all; g_udc_ab_on=0; + S.img[1][0]=701; + struct { op_hdr rh; p_set_rt rt; op_hdr ph; p_bind_program pr; op_hdr dh; p_draw d; op_hdr end; } code={0}; + code.rh=(op_hdr){OP_SET_RT,sizeof(op_hdr)+sizeof(p_set_rt)}; code.rt.n=1; code.rt.color[0]=&color; + code.ph=(op_hdr){OP_BIND_PROGRAM,sizeof(op_hdr)+sizeof(p_bind_program)}; code.pr=(p_bind_program){&programs[mode==2||mode==3?2:raster],3}; + code.dh=(op_hdr){OP_DRAW,sizeof(op_hdr)+sizeof(p_draw)}; code.d=(p_draw){NVN_DRAW_PRIMITIVE_TRIANGLES,0,3}; + code.end=(op_hdr){OP_END,sizeof(op_hdr)}; + if(mode==5) S.chmask.mask=0; + NvProg *pr=prog_get(&programs[raster]); + if(mode==6) pr->sh[1]->rf.color_known=0; + if(mode==2) { R.has_rt=1; R.rt=code.rt; R.clear_c[0]=1; R.cc[0]=MTLClearColorMake(0,1,0,1); } + uint64_t discarded=g_sd_n_dead; + if(mode==7) { // Exercise the store-decision boundary with a real DontCare-load pass (image-only production passes normally load). + R.has_rt=1; R.rt=code.rt; + MTL4RenderPassDescriptor *pd=[MTL4RenderPassDescriptor new]; + pd.colorAttachments[0].texture=tex_mtl(&color); pd.colorAttachments[0].loadAction=MTLLoadActionDontCare; + pd.colorAttachments[0].storeAction=MTLStoreActionUnknown; g_enc=enc_r_begin(pd); + g_sd[0]=(sd_att){.ser=color.serial,.idx=0,.bytes=256,.load=0}; g_nsd=1; + NvProg *tail=prog_get(&programs[2]); NvShader *fs=tail->sh[1]; + MTLRenderPipelineDescriptor *d=[MTLRenderPipelineDescriptor new]; d.vertexFunction=tail->sh[0]->fn; d.fragmentFunction=fs->fn; + d.colorAttachments[0].pixelFormat=MTLPixelFormatRGBA8Unorm; d.colorAttachments[0].writeMask=MTLColorWriteMaskNone; + NSError *error=nil; id ps=[g_dev newRenderPipelineStateWithDescriptor:d error:&error]; + boundary_pipeline=ps; CHECK(ps!=nil,"DontCare-load image pipeline compiles"); + enc_r_pso(g_enc,ps); enc_r_viewport(g_enc,(MTLViewport){0,0,8,8,0,1}); + CHECK(encode_args((id)g_enc,fs,1,0),"DontCare-load image arguments encoded"); + udc_note_draw(tail,0); enc_r_draw(g_enc,MTLPrimitiveTypeTriangle,0,3); + } else mtl_exec((const uint8_t *)&code); + if(mode==4) { code.pr=(p_bind_program){&programs[2],3}; mtl_exec((const uint8_t *)&code.ph); } + if(mode==6) pr->sh[1]->rf.color_known=1; + if(mode==3) clear_color_tri(&(p_clear_color){.index=0,.mask=15,.rgba={0,1,0,1}}); + rp_end(); + if(mode==7) CHECK(g_sd_n_dead==discarded,"DontCare-load attachment keeps its store"); + id pixels=[g_dev newBufferWithLength:4096 options:MTLResourceStorageModeShared]; res_add(pixels); + for (unsigned layer=0;layer<4;layer++) + enc_b_copy_texture_buffer(blit(),tex_mtl(&color),layer,0,MTLOriginMake(0,0,0),MTLSizeMake(8,8,1),pixels,1024*layer,128,1024); + CHECK(mtl_wait(mtl_sub_commit(),5000000000ull),"GPU submission completes"); + CHECK(!mtl_last_commit_error(),"GPU submission has no error"); + for (unsigned layer=0;layer<4;layer++) { + if(mode==7&&!layer) continue; // DontCare leaves the attached layer unspecified. + uint8_t want[4]={32+layer*32,64,128,255}; if(raster&&!layer) {want[0]=255;want[1]=want[2]=0;} + if((mode==2||mode==3)&&!layer) {want[0]=want[2]=0;want[1]=255;} + int good=1; + for(unsigned y=0;y<8;y++) for(unsigned x=0;x<8;x++) + good&=!memcmp((const uint8_t *)pixels.contents+1024*layer+128*y+4*x,want,4); + char label[160]; snprintf(label,sizeof label,"%s layer %u survives pass end (unwritten-store optimization %s)",mode==7?"DontCare load and image":mode==4?"earlier raster and later image":mode==5?"zero-mask image":mode==6?"unknown reflection and image":mode==2?"load clear and image":mode==3?"explicit clear and image":raster?"raster and image":"image-only",layer,store_all?"off":"on"); + CHECK(good,label); + printf("%s: first pixel %u,%u,%u,%u\n",label,((uint8_t *)pixels.contents)[1024*layer],((uint8_t *)pixels.contents)[1024*layer+1],((uint8_t *)pixels.contents)[1024*layer+2],((uint8_t *)pixels.contents)[1024*layer+3]); + } +} +// The facts passed to store tracking must use the selected pipeline's physical target map, before draw_ resets it. +static void mapping_checks(NvProg *pr) { + state_reset(); + uint32_t original = pr->sh[1]->rf.color_mask; + pr->sh[1]->rf.color_mask = 4; + S.chmask.mask = 0xf00; + const uint8_t logical[8] = {2,255,255,255,255,255,255,255}; g_pso_lmap = logical; + CHECK(draw_color_writes(pr) == 1, "logical output 2 writes remapped physical attachment 0"); + S.chmask.mask = 0; + CHECK(draw_color_writes(pr) == 0, "a remapped zero channel mask still writes no color"); + g_pso_lmap = NULL; S.chmask.mask = 15; pr->sh[1]->rf.color_mask = 0; g_pso_fsfn = pr->sh[1]->fn; + CHECK(draw_color_writes(pr) == 1, "a specialized fragment does not inherit the original empty output map"); + g_pso_fsfn = nil; pr->sh[1]->rf.color_mask = original; + NvShader *fs=pr->sh[1]; pr->sh[1]=nil; + CHECK(draw_color_writes(pr) == 0, "no fragment shader writes no color attachment"); + pr->sh[1]=fs; +} +int main(void) { @autoreleasepool { + setenv("DM_NVN_TBDR","0",1); mtl_init(); + color.d=(nvn_texdesc){.target=NVN_TEXTURE_TARGET_2D_ARRAY,.format=NVN_FORMAT_RGBA8,.width=8,.height=8,.depth=4,.levels=1,.samples=1}; + MTLTextureDescriptor *td=tex_desc(&color,0); td.usage=MTLTextureUsageRenderTarget|MTLTextureUsageShaderRead|MTLTextureUsageShaderWrite|MTLTextureUsagePixelFormatView; + id texture=[g_dev newTextureWithDescriptor:td]; CHECK(texture!=nil,"array texture created"); res_add(texture); mtl_tex_init(&color); serial_put(color.serial,texture); mtl_reg_image(701,&color,NULL); // NOLINT(clang-analyzer-osx.cocoa.RetainCount): serial_put transfers a retained reference to g_by_serial; the analyzer does not model its C hash table. + NSError *error=nil; + id lib=[g_dev newLibraryWithSource:@"#include \nusing namespace metal;\n" + "vertex float4 v(uint n[[vertex_id]]){return float4(n==1?3.0:-1.0,n==2?3.0:-1.0,0,1);}\n" + "struct A{texture2d_array image[[id(0)]];};\n" + "void stores(float4 p,constant A&a,uint first){for(uint l=first;l<4;l++)a.image.write(float4(float(32+l*32)/255.0,64.0/255.0,128.0/255.0,1),uint2(p.xy),l);}\n" + "fragment void image_only(float4 p[[position]],constant A&a[[buffer(0)]]){stores(p,a,0);}\n" + "fragment void image_tail(float4 p[[position]],constant A&a[[buffer(0)]]){stores(p,a,1);}\n" + "fragment float4 both(float4 p[[position]],constant A&a[[buffer(0)]]){stores(p,a,1);return float4(1,0,0,1);}\n" + options:nil error:&error]; + CHECK(lib!=nil,"literal image-store shaders compile"); if(!lib){fprintf(stderr,"%s\n",error.localizedDescription.UTF8String);return 1;} + for(unsigned mode=0;mode<3;mode++) { // NOLINT(clang-analyzer-osx.cocoa.RetainCount): the preceding iteration's NvProg is retained by g_by_serial through serial_put. + NvProg *pr=[NvProg new]; pr->sh[0]=[NvShader new]; pr->sh[0]->fn=[lib newFunctionWithName:@"v"]; + NvShader *fs=[NvShader new]; pr->sh[1]=fs; fs->stage=1; fs->fn=[lib newFunctionWithName:mode==2?@"image_tail":mode?@"both":@"image_only"]; + fs->rf.color_known=1; fs->rf.color_mask=mode==1; fs->rf.nb=1; + fs->rf.b[0]=(nvn_rbind){.kind=NVN_RB_IMG,.aid=0,.reg=0,.rw=1,.dtt=MTLTextureType2DArray}; + fs->ae=[fs->fn newArgumentEncoderWithBufferIndex:0]; fs->arg_len=fs->ae.encodedLength; + serial_put(programs[mode].serial,pr); + } + mapping_checks(prog_get(&programs[0])); + for(int mode=0;mode<8;mode++) for(int off=0;off<2;off++) run(mode,off); // NOLINT(clang-analyzer-osx.cocoa.RetainCount): g_by_serial owns the fixture programs throughout GPU replay. + printf("IMAGE_ATTACHMENT_STORE %d checks, %d failed\n",checks,failed); return failed!=0; +} } diff --git a/dm/gfx/tests/t_unwritten_store.m b/dm/gfx/tests/t_unwritten_store.m index 67748382c8..1315663ea4 100644 --- a/dm/gfx/tests/t_unwritten_store.m +++ b/dm/gfx/tests/t_unwritten_store.m @@ -29,7 +29,7 @@ static result run(int enabled, int mode) { static uint64_t frame; uint64_t started=bp_now(); mtl_set_frame(++frame); g_udc_ab=enabled; g_udc_ab_on=0; state_reset(); memset(&R,0,sizeof R); mtl_sub_begin(); seed(); - S.ds=(ds_st){.dt=1,.dw=mode==1,.df=NVN_DEPTH_FUNC_LESS,.st=mode==2, + S.ds=(ds_st){.dt=1,.dw=mode==1||mode==7,.df=NVN_DEPTH_FUNC_LESS,.st=mode==2, .sf_f=NVN_STENCIL_FUNC_ALWAYS,.sf_b=NVN_STENCIL_FUNC_ALWAYS, .fail_f=NVN_STENCIL_OP_KEEP,.zfail_f=NVN_STENCIL_OP_KEEP,.zpass_f=NVN_STENCIL_OP_REPLACE, .fail_b=NVN_STENCIL_OP_KEEP,.zfail_b=NVN_STENCIL_OP_KEEP,.zpass_b=NVN_STENCIL_OP_REPLACE}; @@ -53,7 +53,7 @@ static result run(int enabled, int mode) { NvProg *alias=[NvProg new]; alias->sh[1]=[NvShader new]; alias->sh[1]->rf.color_mask=1; alias->sh[1]->rf.nb=1; int kinds[]={NVN_RB_IMG,NVN_RB_SBUFP,NVN_RB_SBUFU}; - alias->sh[1]->rf.b[0]=(nvn_rbind){.kind=kinds[mode-4],.rw=1}; udc_note_draw(alias); + alias->sh[1]->rf.b[0]=(nvn_rbind){.kind=kinds[(mode-4)%3],.rw=1}; udc_note_draw(alias, 1); } rp_end(); } @@ -79,7 +79,7 @@ int main(int argc,char **argv) { @autoreleasepool { NvProg *pr=[NvProg new]; pr->sh[0]=[NvShader new]; pr->sh[1]=[NvShader new]; pr->sh[0]->fn=[lib newFunctionWithName:@"v"]; pr->sh[1]->fn=[lib newFunctionWithName:@"f"]; pr->sh[1]->stage=1; pr->sh[1]->rf.color_mask=1; serial_put(program.serial,pr); - for (int case_id=0;case_id<(bench?12:7);case_id++) { + for (int case_id=0;case_id<(bench?12:8);case_id++) { int mode=bench?0:case_id; result off,on; if(case_id&1) { on=run(1,mode); off=run(0,mode); } @@ -87,7 +87,7 @@ int main(int argc,char **argv) { @autoreleasepool { CHECK(!memcmp(off.color,on.color,256)&&!memcmp(off.depth,on.depth,256)&&!memcmp(off.stencil,on.stencil,64),"store-all and elided planes match byte for byte"); int good=1; for(int i=0;i<64;i++) { uint8_t red[4]={255,0,0,255}; good &= !memcmp(on.color+4*i,red,4); - good &= on.depth[i]==(mode==1?.5f:mode==3?.625f:.75f); + good &= on.depth[i]==(mode==1||mode==7?.5f:mode==3?.625f:.75f); good &= on.stencil[i]==(mode==2?0x36:mode==3?0x63:0x2d); } CHECK(good,"all 64 pixels match hand-computed color/depth/stencil"); diff --git a/docs/agents/symptoms.md b/docs/agents/symptoms.md index 84313c92a7..c3952748b4 100644 --- a/docs/agents/symptoms.md +++ b/docs/agents/symptoms.md @@ -534,6 +534,7 @@ One line per symptom in the [subsystem references](../reference/): what you see - the NSP script took the game for an OpenGL one (`main imports EGL and names glShaderSource`), the GL shader capture found no shader, and the capture boot stopped with `[nvn-mtl] FATAL cannot read the shader map` from `h_nvnQueueSubmitCommands` called in an NRO (`nro8+0xc10d0`). → [NVN in NROs under an SDL main that names GL](../reference/rendering.md#nvn-in-nros-under-an-sdl-main-that-names-gl) - a jump to address 0 right after `glGetFloatv(0xb12)` (`lr=main+0x2d58c4`). → [Which GL functions a game calls](../reference/rendering.md#which-gl-functions-a-game-calls) - in a capture run every program links with no uniforms; the game's first `Shader:send` raises a Lua error and, on the Switch, its error handler quits (Any Lua error quits a LÖVE game). → [GL shaders put together at run time and captured](../reference/rendering.md#gl-shaders-put-together-at-run-time-and-captured) +- an image-only fragment draw's writes disappear from the subresource also bound as its color attachment when the pass ends; other image layers retain their writes. → [Image stores into an attachment of their own pass](../reference/rendering.md#image-stores-into-an-attachment-of-their-own-pass) - read-only depth/stencil attachments are written back after every pass, and the periodic unwritten-store savings counter remains zero with the default configuration. → [Unwritten attachment stores without lookahead](../reference/rendering.md#unwritten-attachment-stores-without-lookahead) ## Shaders diff --git a/docs/env_switches.tsv b/docs/env_switches.tsv index 87909a20cb..7f80af2121 100644 --- a/docs/env_switches.tsv +++ b/docs/env_switches.tsv @@ -170,7 +170,7 @@ DM_NVN_UB_LAZY switch `=`: the most deferred inline uniform updates kept acro DM_NVN_UB_N diagnostic `=`: the length of each phase of the same-process alternating measurement `DM_NVN_UB` (default 180, minimum 60) DM_NVN_UB_PARTIAL switch `0`: a uniform block the deferred updates cover only in part ends the pass, as before; by default its shadow starts from memory and takes the submission's queued updates (on by default) DM_NVN_UDC_AB diagnostic `=1` starts with the DontCare store for unwritten attachments off and turns it on after the second screenshot, for an on/off picture comparison in one run -DM_NVN_UNWRITTEN_DC switch `0` turns off storing attachments a render pass did not write as DontCare (on by default; part of the TBDR store lookahead), for equivalence comparisons +DM_NVN_UNWRITTEN_DC switch `0` disables optional stores elided for loaded, unchanged attachments (on by default, independent of TBDR lookahead), for equivalence comparisons; mandatory image-write preservation stays active DM_NVN_UPLOAD_REUSE switch 0 keeps the original one-block upload-slot policy; default retains bounded demand-sized primary and overflow buffers after completion. DM_NVN_VBUF_CACHE switch `0` turns off the cache of resolved vertex stream addresses and vertex argument table writes across draws (on by default), for A/B runs DM_NVN_VCONV_CHECK diagnostic `=1` scans the indices again at every reuse of a cached vertex conversion range (`vconv_range`) and reports differences diff --git a/docs/reference/rendering.md b/docs/reference/rendering.md index 7f0eebece1..c5adb71639 100644 --- a/docs/reference/rendering.md +++ b/docs/reference/rendering.md @@ -1115,10 +1115,18 @@ Digimon Story Cyber Sleuth: Complete Edition (Media.Vision's MVGL engine, SDK 6. - **Found on:** [Digimon Story Cyber Sleuth: Complete Edition](../../dm/games/010014E00DB56000/README.md): the USM movies' H.264 is decoded through the SDK's movie library (the host's decoder) into NV12 buffers (3,110,400 bytes a 1920x1080 frame); the game uploads the planes into a `GL_RED` and a `GL_RG` texture (two `glTexSubImage2D` a frame) and converts YUV in its own shader (in `main`, `SamplerY` / `SamplerUV`). +### Image stores into an attachment of their own pass + +- **Symptom:** an image-only fragment draw's writes disappear from the subresource also bound as its color attachment when the pass ends; other image layers retain their writes. +- **Cause:** a writable image or storage alias marked every attachment written, which retained the tile store. For a loaded color attachment with no raster output or clear, that store writes the old tile contents over the shader's image writes to the same texture. Disabling the unwritten-store optimization did not avoid this correctness fault. +- **Fix (resolved in synthetic GPU tests; game validation open):** track possible shader writes separately from color-tile writes. A loaded color attachment with possible image/storage writes and no tile writes uses `DontCare` at pass end, independently of the optimization switches. Record physical color writes before resetting a merged pipeline's logical-output map; specialized fragments have conservative unknown outputs. Pending and explicit clears count as tile writes. Depth/stencil and non-loaded attachments retain their existing decisions. Mixed raster/clear and image writes to the same subresource are outside this proof and still retain the tile store. +- **Check:** `make -C dm/gfx/tests GAME= check_t_image_attachment_store` executes literal shaders through production draw replay and reads all four image layers on the GPU, with the unwritten-store optimization on and off. It also checks raster outputs, an earlier raster draw followed by an image-only draw, pending and explicit clears, unknown output reflection, a zero channel mask, a native `DontCare`-load control, and physical/logical output mapping. `t_unwritten_store` additionally preserves depth writes with a possible image alias. +- **Found on:** [DARK SOULS: REMASTERED](../../dm/games/01004AB00A260000/README.md#rendering), reported and diagnosed by Jacob-Tate in [#187](https://github.com/seedds/nxnative/issues/187#issuecomment-6519569185), with the original store fix in [34c7453](https://github.com/Jacob-Tate/nxnative/commit/34c7453e081203627752edd877dde40452afe7b2). Independently reproduced with synthetic shaders on an Apple M2 Max, macOS 26.6; no local game or iOS validation. + ### Unwritten attachment stores without lookahead - **Symptom:** read-only depth/stencil attachments are written back after every pass, and the periodic unwritten-store savings counter remains zero with the default configuration. - **Cause:** `udc_note_draw` and `sd_finish` returned early when `DM_NVN_TBDR` was off. This also disabled the local unwritten-attachment proof, although it does not depend on future sampled-texture readers. The broader lookahead remains disabled because it does not understand bindless readers. -- **Fix:** collect per-pass write facts independently of the lookahead switch and omit only stores of attachments that were loaded and provably unchanged. Clears, depth writes, stencil writes and possible writable image/atomic/storage aliases retain their stores. Lookahead, pass merging and cross-submission invalidation remain disabled. `DM_NVN_UNWRITTEN_DC=0` restores store-all behavior. +- **Fix:** collect per-pass write facts independently of the lookahead switch and omit only stores of attachments that were loaded and provably unchanged. Clears, depth writes, stencil writes and possible writable image/atomic/storage aliases conservatively retain stores, except for the mandatory [image-only color-attachment rule](#image-stores-into-an-attachment-of-their-own-pass). Lookahead, pass merging and cross-submission invalidation remain disabled. `DM_NVN_UNWRITTEN_DC=0` disables optional unwritten-store savings; the image-write preservation rule stays active. - **Check:** `make -C dm/gfx/tests GAME= check_t_unwritten_store` performs real GPU readback against a store-all control and literal color/depth/stencil values, with lookahead disabled. It covers read-only depth/stencil, depth writes, stencil writes, pending clears, and writable images (including native image atomics) and both storage-buffer binding forms. `t_unwritten_store --bench` runs a synthetic 32-pass 1080p workload with balanced on/off order; it is not a game FPS measurement. - **Found on:** [NieR:Automata US 1.0.3](../../dm/games/010056B015FE8000/README.md#factory-combat-capture-and-9s-texture-follow-up), during iPhone factory-combat performance investigation. The change passes the GPU proof and rendering regressions on Mac; iPhone gameplay benefit remains to be measured.