Merge pull request #45 from jkdansereau/probe/d217-texload-invariance

probe(D217): decode is proven correct — parked defect ② fix does NOT fix the visible symptom

docs/dev/GE-ENV-PROBES.md

@@ -70,6 +70,7 @@ Two classes:
 | `GE_D69STAN` | `src/game/stan.c:266,276` | D69 stan-tile trace | dead |
 | `GE_D85DUMP` | `src/game/bg.c:2510` | D85 `bgWidenRoomGdl` 8→16B dump | dead (D85 closed) |
 | `GE_D85TEX` | `src/game/image.c:2479`, `src/game/tex.c:859,1050` | D85 texpool-full events (Depot analysis) | dead (D85 closed) — was useful for §3 |
+| `GE_D217TEX` | `src/game/image.c:234` (`texInflateZlib`) | D217: logs `texnum`/`format`/`numcolours`/`palette[0..1]` straight off the decode bitstream (before any arena/pointer math) — the decode-correctness check that proved the affected texture (grip, `texnum=1608`) decodes byte-identically across levels, ruling out a decode/ROM-data bug. | **live** (D217 open — investigation instrument) |
 | `GE_D86` | `src/game/model.c:6295`, `src/game/objecthandler_2.c:143` | D86 model rwdata trace | dead (D86 closed) |
 | `GE_D87` | `src/game/ramromreplay.c:296,363,381` | D87 `ramromfilestructure` endianness | dead (D87 closed) |
 | `GE_D88` | `src/game/prop.c:1367`, `src/game/stan.c:3090` | D88 `Usetup*Z` propDef stream | dead (D88 family closed; D88.4 resolved) |

docs/dev/findings.md

@@ -8649,3 +8649,82 @@ applying it — do not repeat this fix:**
   that is the one load-bearing assumption ("same weapon, same ROM bytes")
   this whole investigation has taken for granted but never directly
   checked with a probe.
+
+## D217 — M-89 cont.: decisive probe — decode is proven correct+identical across levels; the parked defect ② fix does NOT fix the visible symptom; new leading theory is a stale/dangling `tex->data` arena pointer, not a decode or cache-key bug
+
+Ran the actual empirical test the M-89 section above called for, rather than
+delegating it further. **Result answers the open question decisively and
+rules out both of this row's previous leads.**
+
+**Probe added** (`src/game/image.c`, `texInflateZlib`, `#ifdef PORT` +
+`getenv("GE_D217TEX")`, follows the `GE_TEXDUMP`/`GE_D85TEX` pattern):
+logs `texnum`/`format`/`numcolours`/`palette[0]`/`palette[1]` **straight off
+the decode bitstream**, before any arena/pointer math — the most direct
+possible check of "does this texture number decode to the same bytes on
+both levels."
+
+**Result:** ran BUNKER1 (`-level_09`) and FACILITY (`-level_34`) headless
+with `GE_D217TEX=1`. `texnum=1608` is the only `numcolours=1` texture common
+to both logs, and its `pal0` value (`0001`, black) matches the previously
+documented BUNKER1-correct value exactly — confirming this is the PP7 grip.
+**`texnum=1608` decodes to `pal0=0001` on BOTH levels, identically, every
+time it's called.** This directly disproves the M-89 "different ROM bytes
+per level" open question (lead a is dead — the decode is correct and
+level-invariant) and also rules out any remaining decode-math theory: the
+bug is not in `texInflateZlib`, `texGetDepthAndSize`, or `gfx_dp_load_tlut`
+(already shown correct M-89) — it is **strictly downstream of a provably
+correct decode**.
+
+**Then tested the parked defect ② fix (`fix/d217-model-texture-palette`,
+`f6fc0399` — the CI-texture-cache-key content-hash) directly against this
+exact FACILITY repro**, since "correct decode, wrong render" is precisely
+the signature that fix targets (a stale cache `HIT` serving a different
+material's already-decoded palette). **Built it, ran `-level_34`, captured
+frame 380: the grip is still solid green.** The already-written, previously
+untested-against-this-repro defect ② fix does **not** fix the visible
+symptom. This means defect ① and defect ② are **not the same underlying
+bug** — they were right to be tracked separately, and defect ②'s fix should
+still land (real, separate, still needs its 21-level sweep) but will not
+close this row on its own.
+
+**New leading theory (untested, most concrete lead yet):** since a fresh
+decode is proven correct, and the fast3d-side cache-key fix doesn't help,
+the render-time GBI command for this material must be reading a `tex->data`
+pointer whose *arena contents have changed* between when it was decoded and
+when the DL that references it is actually drawn — i.e. a **stale/dangling
+pointer into the `texpool` arena**, not a decode or cache-lookup bug. GE's
+`texpool` (`image.c` `texLoad`/`texInitPool`, two-ended `leftpos`/`rightpos`
+allocator) is reused across many textures per level; if it resets or the
+model-node/GDL that references texture 1608 caches a raw pointer/segment
+address resolved once (e.g. at weapon-pickup or level-load time) rather
+than re-resolving through `texFindInPool` every time it draws, then any
+*later* texture decode that reuses that same arena address (plausible on a
+texture-heavy level like FACILITY, less likely on the lighter BUNKER1) would
+silently overwrite the grip's palette bytes with whatever the next tenant's
+content is — explaining both "same weapon, different wrong color per level"
+(depends on what else that level's arena happened to decode nearby/after)
+and "clean on the lighter level" (less arena churn/reuse pressure).
+
+**Confidence:** HIGH the decode itself and the fast3d/cache-key layer are
+now both cleared (empirically, not just by inspection). MEDIUM-LOW on the
+stale-arena-pointer mechanism — plausible and consistent with all evidence
+so far, but not yet probed directly.
+
+**For whoever picks this up next:** probe whichever code path holds the
+`Gfx*`/model-node reference to this material's `SetTextureImage` address at
+*draw* time (`texWriteLoadToTmemAddr`'s caller chain from the weapon
+model-DL walk, not `texLoad` itself) and log the `tex->data` pointer value
++ a fresh read of the two bytes at the computed palette offset **at draw
+time**, alongside a second log of what `texInflateZlib` most recently wrote
+to that same address (extend the `GE_D217TEX` probe with an address field
+to correlate). If the drawn address's live content no longer matches what
+was last decoded there, that confirms the stale-arena-pointer theory
+directly and narrows the fix to wherever the model DL caches that pointer
+instead of re-resolving it. Probe left in tree (`GE_D217TEX`, env-gated,
+inert by default).
+
+**VERIFY performed this session:** `tools_pc/verify.sh bunker1` — PASS,
+no regression from the (inert-by-default) probe. `-level_09`/`-level_34`
+`GE_D217TEX=1` headless captures — decisive result above. Built and ran
+the parked defect ② fix directly against `-level_34` frame 380 — still
+green, screenshot evidence in this session's scratchpad.

src/game/image.c

@@ -224,6 +224,19 @@ s32 texInflateZlib(u8 *src, u8 *dst, s32 arg2, s32 forcenumimages, struct texpoo
         palette[i] = texReadBits(16);
     }
 
+#ifdef PORT
+    /* TEMP D217 (M-89 lead a): log the palette bytes as decoded straight off
+     * the bitstream, keyed by texture number, so a run on BUNKER1 and a run
+     * on FACILITY can be diffed to see whether the SAME texnum yields the
+     * SAME palette[0] -- if not, the divergence is upstream of texInflateZlib
+     * (ROM offset table / bitstream source), not the runtime TLUT-address
+     * math (already ruled out, see findings.md D217 M-89). */
+    if (getenv("GE_D217TEX"))
+        osSyncPrintf("D217TEX texnum=%d format=%d numcolours=%d pal0=%04x pal1=%04x\n",
+                     (int)arg4->rightpos->texturenum, (int)format, (int)numcolours,
+                     (unsigned)palette[0], (unsigned)(numcolours > 1 ? palette[1] : 0));
+#endif
+
     foundthething = FALSE;
 
     for (j = 0; j < numimages; j++)