sky.c: D245 headless probe + opt-in fixed-shift A/B candidate

Adds a GE_D245V env-gated probe dumping the water fan's raw S/T span each
call, and a GE_D245_FIXEDSHIFT env var (skyPortForceFanShift) that forces a
constant tc-shift k instead of the per-frame adaptive pick. Both are
diagnostic/opt-in only -- default behavior is unchanged (verify.sh
frigate/"surface 2" PASS with the env vars unset).

Headless GE_INPUTSCRIPT rotation on Frigate found k genuinely flips (kT
4->3) during ordinary view turning, which the original single-frame D227V
trace never caught -- new evidence for why the M-116 allowShift=TRUE fix
didn't hold up live. The fixed-k candidate is NOT the default; it needs a
live A/B (unset vs 3/4/5) to know if it actually looks better, not another
blind ship.

Claude-Session: https://claude.ai/code/session_01KwZViAFaXua3PKgHHjNgvi

docs/dev/findings-index.csv

@@ -140,7 +140,7 @@ D240,OPEN,"Player gunshot SFX timing/cadence reportedly does not match the N64 o
 D241,OPEN,Player gunshot SFX goes silent under some conditions —,"OPEN — not newly root-caused; high confidence this is D207 reproduced, not an independent bug."
 D242,?,Player-death sequence UX regression vs N64 original:,NOT A PORT-LOGIC BUG as far as static analysis can tell.
 D243,OPEN,"Dam end-of-level cutscene (the abseil/bungee sequence, D148/D160's subject):",OPEN — M-109 re-analysis of the same session's raw logs corrects the timing/pattern claims above; the D134/pendingGfx race hypothesis is likely wrong.
-D245,OPEN,IsWater water: two competing patterns with a visible seam that moves as the player looks around (mouse/aim) —,"OPEN — REOPENED (M-116 live playtest, same session)…"
+D245,OPEN,IsWater water: two competing patterns with a visible seam that moves as the player looks around (mouse/aim) —,"OPEN — REOPENED, M-142 headless probe found the tc-shift k measurably flips during ordinary view rotation (new root-cause evidence); an opt-in GE_D245_FIXEDSHIFT candidate is build-verified but needs…"
 D246,OPEN,"Tiny pixel strips at the left and right edges of the screen, ""like overscan"" —","OPEN — one candidate ruled out (M-116 follow-up), root cause not found."
 D247,RESOLVED,Black bars top and bottom of screen —,"NOT A BUG — confirmed faithful, unmodified N64 decomp behaviour, no code change made."
 D248,FIXED,"Port runs a stable 30 FPS instead of 60 FPS game-wide (M-117, user asked to verify FPS after the D209 timing-keystone fixes).","FIXED, root-caused to a genuine pointer-width ABI bug in src/sched.c (compiled for the PC build, not excluded)."

docs/dev/findings.md

@@ -547,7 +547,7 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121).
 | D241 | **Player gunshot SFX goes silent under some conditions — reported on Bunker with the alarm sounding, "or some other factor" — but only for PP7 and AK47, Klobb…** — full `## D241` entry at file tail | **OPEN — not newly root-caused; high confidence this is D207 reproduced, not an independent bug.** Recommend folding this repro into D207 (or closing D241 as a duplicate once D207 is reopened) rather than tracking separately.… |
 | D242 | **Player-death sequence UX regression vs N64 original: PC requires an LMB click to proceed to the mission-failed/file-select screen; user recalls N64 auto-plays…** — full `## D242` entry at file tail | **NOT A PORT-LOGIC BUG as far as static analysis can tell.** Confidence HIGH that the LMB-click skip path is faithful original behavior (Z_TRIG-mapped, exists unmodified in `bondview2.c`), not a defect to remove.… |
 | D243 | **Dam end-of-level cutscene (the abseil/bungee sequence, D148/D160's subject): the camera violently shakes/"spazzes" during each of the scripted camera-angle…** — full `## D243` entry at file tail | **OPEN — M-109 re-analysis of the same session's raw logs corrects the timing/pattern claims above; the D134/pendingGfx race hypothesis is likely wrong.** Confidence MEDIUM on the correction, LOW-MEDIUM on the new lead.… |
-| D245 | **IsWater water: two competing patterns with a visible seam that moves as the player looks around (mouse/aim) — "looks almost exactly like skybox" (M-116, user…** — full `## D245` entry at file tail | **OPEN — REOPENED (M-116 live playtest, same session)… |
+| D245 | **IsWater water: two competing patterns with a visible seam that moves as the player looks around (mouse/aim) — "looks almost exactly like skybox" (M-116, user…** — full `## D245` entry at file tail | OPEN — REOPENED, M-142 headless probe found the tc-shift `k` measurably flips during ordinary view rotation (new root-cause evidence); an opt-in `GE_D245_FIXEDSHIFT` candidate is build-verified but needs a live A/B, not shipped as default.… |
 | D246 | **Tiny pixel strips at the left and right edges of the screen, "like overscan" — look like raw pixels, not 3D graphics (M-116, user QA report).** — full `## D246` entry at file tail | **OPEN — one candidate ruled out (M-116 follow-up), root cause not found.** Step (1) from the original plan is done… |
 | D247 | **Black bars top and bottom of screen — possible aspect-ratio defect (M-116, user QA report).** — full `## D247` entry at file tail | **NOT A BUG — confirmed faithful, unmodified N64 decomp behaviour, no code change made.** `GE_PCDUMP` captures (both against the pre-existing save and a fresh-EEPROM run, to rule out a stale `Screen` option from earlier test sessions) show the exact same inset on Frigate… |
 | D248 | **Port runs a stable 30 FPS instead of 60 FPS game-wide (M-117, user asked to verify FPS after the D209 timing-keystone fixes).** — full `## D248` entry at file tail | **FIXED, root-caused to a genuine pointer-width ABI bug in `src/sched.c` (compiled for the PC build, not excluded).** `osScAddClient()` (`src/sched.c:187-199`) stashes a per-client "present every retrace vs.… |
@@ -10718,6 +10718,10 @@ crash class C2 (Facility `-level_34`, Runway `-level_35`): `import_texture_i8` A
 
 **Status (from §F table):** **OPEN — REOPENED (M-116 live playtest, same session): the `allowShift=TRUE` mechanistic fix below did NOT resolve the symptom.** User report on Frigate post-fix: water still "glitches, kinda goes back and forth between two graphics too quickly," and still "changes depending on your view unlike the sky" (i.e. the sky, using the identical `skyPortBeginFan`/`skyPortEmitTileShift` mechanism post-D227, no longer has this problem, but water still does after getting the same treatment). This means the `allowShift=FALSE→TRUE` change either isn't the actual fix, or there's a second mechanism specific to water's two-texunit cross-fade that the tc-shift alone doesn't address — most likely candidate: the shift only rescales the *shared* vertex tc uniformly, but the tile-0-vs-tile-1 sample-point difference is set by each tile's own `uls`/`ult` (RDP tile-space, not vertex-tc-space) — if per-frame `k` changes granularity of the *quantized* shared coordinate before each tile's independent `uls`/`ult` subtraction, the two tiles' effective sampled texel could drift relative to each other frame-to-frame in a way the static `GE_D227V` trace (single frame, single camera angle) never exercised. Needs a fresh live investigation, not a repeat of the same static-trace approach — a scripted or live multi-frame capture while rotating view, watching both tiles' *decoded* texel positions (not just vertex tc) frame over frame. Do not re-apply the same `allowShift=TRUE` reasoning without new evidence; the superseded mechanism write-up below is unconfirmed-by-playtest and may be wrong. The code change (`allowShift=TRUE` on the water quad) is left in place — it's not shown to be actively harmful, and reverting it isn't obviously better than leaving it, pending the fresh investigation. **Superseded analysis (unconfirmed by playtest, kept for reference):** ~~FIXED (port-layer, `src/game/sky.c`, `#ifdef PORT`) — confirmed the `allowShift=FALSE` tolerance was the cause, and the "flattens the cross-fade" objection was wrong.~~ Changed the water quad's call from `skyPortBeginFan(sp274, s1, FALSE)` to `..., TRUE)`, so it now gets D227's tc-shift safety valve like every other sky fan. Why the old objection doesn't hold: `skyPortEmitTileShift` replays **both** tiles' captured `SETTILE` commands with the identical `k`, so both texunits' decoded S/T are rescaled by the same `2^k` *before* each tile's own (unshifted, tile-native) `uls`/`ult` offset is applied by the RDP — the offset between tile 0 and tile 1's sample points (what drives the cross-fade) is preserved, not flattened, because the shift and the offset act at different stages. **Verified mechanistically:** `GE_D227V=1` on Frigate (`-level_26`) shows a water fan vertex spanning S=-166..299317, T=-6451..288410 in S10.5 units — tens of thousands of texels, the same order of magnitude as D227's pre-fix sky overflow — now getting `kS=4 kT=4` (previously forced to 0), decoding to small in-range tc values (100s, not wrapped). **Verified no regression:** `verify.sh frigate` and `verify.sh "surface 2"` (the two `IsWater` levels) both PASS crash-free post-fix; non-water sky levels (`statue`/`-level_22`) unaffected since only the water call site changed. Screenshot comparison at a fixed scripted-intro camera angle (frame 500, both builds) showed no regression, but that specific angle didn't happen to frame the overflowing part of the fan either before or after — the sinf()-driven cross-fade also makes frame-for-frame screenshot diffs across separate runs noisy (each run's shimmer phase differs), so a same-frame pixel diff wasn't a clean confirmation tool here; the `GE_D227V` trace is the real evidence — **but it turned out not to be evidence the fix actually worked in live play.** Cross-ref D227 (mechanism), D229 (made this visible).
 
+**M-142 update: the fresh live-ish investigation the reopening asked for, done headlessly (scripted camera turn, no human/Deck needed) — real new evidence, one untested candidate, still OPEN pending live A/B.** Added a `GE_D245V` probe (env-gated, `src/game/sky.c`'s water block) dumping the water fan's raw S/T span every call; ran it on Frigate (`-level_26`) headless with a `GE_INPUTSCRIPT` script pulsing `CRIGHT` every 15 frames for ~800 calls (turns the player in place, no controller/Deck needed). Two concrete findings: **(1) the water quad's span (~290,000–390,000 raw S10.5 units in this capture) is roughly 10x bigger than sky's typical span** — meaning water needs `k=4` just to fit, not `k=0` like the "normally inactive" comment above `skyPortPickShift` assumes (that comment is accurate for sky, not water). **(2) `k` is NOT stable during ordinary view rotation:** `spanT` was tracked crossing `skyPortPickShift`'s k=3/k=4 breakpoint (~260,088 units) mid-turn (call 409→410 in the capture), i.e. `kT` measurably flips between two values from nothing more than the player turning in place. This is a real, previously-undocumented mechanism the original single-frame `GE_D227V` trace (one camera angle) could never have caught, and it directly supports the reopening's own suspicion that a per-frame `k` change re-quantizes the shared vertex tc and could produce exactly the "glitches... changes depending on your view" symptom — even though the tile-0-vs-tile-1 *relative* offset stays correctly preserved across a k change (that part of the original D227/D245 mechanism reasoning was right; the newly-found problem is the *absolute* quantization popping when k itself changes, not the cross-fade flattening the old theory worried about).
+
+**Untested candidate, explicitly NOT shipped as the default** (per this row's own "don't repeat the same mistake" lesson from the superseded `allowShift=TRUE` fix): `skyPortForceFanShift()` + `GE_D245_FIXEDSHIFT=<k>` env var (`sky.c`, `#ifdef PORT`) lets a live session force a single constant `k` for the whole water draw instead of the per-frame adaptive pick, eliminating the transition this session found — but at the cost of coarser tc quantization *always* (a fixed `k=5` collapses to whole-texel granularity, which could itself look worse on the shimmer cross-fade; that trade-off can only be judged by eyes on the actual water, not another headless capture). Build-verified crash-free with `GE_D245_FIXEDSHIFT=5` on Frigate (300+ frames); `verify.sh frigate`/`verify.sh "surface 2"` both PASS with the env var unset (default path byte-identical to the current reopened behaviour — nothing changes for players until someone opts in). **Next step for whoever has live Frigate/Surface-2 access:** play with `GE_D245_FIXEDSHIFT` unset (current behaviour, confirm the glitch still reproduces) vs. set to 3, 4, and 5 in turn, and report which (if any) looks better — that's the missing data point, not more static tracing.
+
 ## D246 — Tiny pixel strips at the left and right edges of the screen, "like overscan" — look like raw pixels, not 3D graphics (M-116, user QA report).
 
 **Tiny pixel strips at the left and right edges of the screen, "like overscan" — look like raw pixels, not 3D graphics (M-116, user QA report).** Uninvestigated. The port renders at native 640×480 (`GE_NATIVE_W/H`; `gfx_current_native_aspect` set in `port/src/video.c:253,570`) and scales to the window on present, so the leading candidate is a scaling/blit rounding artifact: non-integer scale factors leaving 1–2 columns of uncleared backbuffer content at the edges (or an off-by-one in the blit rect). Secondary candidates: an RDP fill-rect/scissor off-by-one in a game DL or the F10 overlay hook (`gfx_run()` seam, D184), or GL edge filtering on the scaled texture.

src/game/sky.c

@@ -26,8 +26,16 @@
  * See the comment above skyPortBeginFan()'s definition. */
 static void skyPortBeginFan(SkyRelated38 *v, s32 n, bool allowShift);
 static void skyPortEndFan(void);
+/* D245 (M-142): same reason -- the water call site (further down) needs to
+ * override the adaptive shift picked by skyPortBeginFan() before the
+ * definition itself. */
+static void skyPortForceFanShift(s32 kS, s32 kT);
 /* D227 (M-100): same reason -- the texSelect call sites precede the definition. */
 static void skyPortCaptureTile(Gfx *start, Gfx *end);
+/* D227 (M-100): relocated up here (was just above skyPortPickShift, further
+ * down) so the D245 water call site's GE_D245_FIXEDSHIFT clamp can use it --
+ * same forward-declare reason as the functions above. */
+#define SKY_TC_MAX_SHIFT 5
 #endif
 
 // bss
@@ -930,6 +938,43 @@ Gfx *skyRender(Gfx *gdl)
              * comment above skyPortBeginFan() for why this doesn't flatten
              * the cross-fade. */
             skyPortBeginFan(sp274, s1, TRUE); /* water: see skyPortBeginFan */
+
+            /* D245 (M-142): opt-in A/B candidate -- see skyPortForceFanShift's
+             * comment. Off by default (unset env => identical to the current
+             * allowShift=TRUE behaviour this row is still reopened against). */
+            {
+                const char *fs = getenv("GE_D245_FIXEDSHIFT");
+                if (fs) {
+                    s32 kFixed = atoi(fs);
+                    if (kFixed < 0) kFixed = 0;
+                    if (kFixed > SKY_TC_MAX_SHIFT) kFixed = SKY_TC_MAX_SHIFT;
+                    skyPortForceFanShift(kFixed, kFixed);
+                }
+            }
+
+            /* D245 (M-142, reopened): dumps the water quad's raw S/T span
+             * every call, so a scripted multi-frame capture can show whether
+             * the span crosses skyPortPickShift's ~30719-unit threshold (and
+             * therefore the tc-shift k) as the camera rotates -- the
+             * still-open question after allowShift=TRUE didn't fix the live
+             * symptom. Recomputed locally rather than reading skyPortBeginFan's
+             * statics (defined later in this TU, not forward-declared). */
+            if (getenv("GE_D245V")) {
+                static int callN = 0;
+                f32 minS = sp274[0].unk20, maxS = sp274[0].unk20;
+                f32 minT = sp274[0].unk24, maxT = sp274[0].unk24;
+                s32 vi;
+                for (vi = 1; vi < s1; vi++) {
+                    if (sp274[vi].unk20 < minS) minS = sp274[vi].unk20;
+                    if (sp274[vi].unk20 > maxS) maxS = sp274[vi].unk20;
+                    if (sp274[vi].unk24 < minT) minT = sp274[vi].unk24;
+                    if (sp274[vi].unk24 > maxT) maxT = sp274[vi].unk24;
+                }
+                fprintf(stderr, "D245V call=%d s1=%d minS=%.1f maxS=%.1f minT=%.1f maxT=%.1f "
+                        "spanS=%.1f spanT=%.1f\n", callN++, s1,
+                        (double) minS, (double) maxS, (double) minT, (double) maxT,
+                        (double) (maxS - minS), (double) (maxT - minT));
+            }
 #endif
             if (s1 == 4)
             {
@@ -1626,7 +1671,6 @@ static bool s_skyFanFoldSet = FALSE;
  * Defect-2 tessellation; skyPortPickShift clamps rather than overflowing, so
  * the failure mode stays graceful.
  */
-#define SKY_TC_MAX_SHIFT 5
 
 /* One tile period expressed in unk20/unk24's units: 64 texels * 32 = 2048.
  * The fold must be a whole number of these or the rebase shifts the texture. */
@@ -1795,6 +1839,29 @@ static void skyPortEndFan(void)
     s_skyFanShiftT = 0;
 }
 
+/* D245 (M-142): overrides the shift skyPortBeginFan() just picked. A GE_D245V
+ * capture (scripted camera turn, Frigate, headless) proved the water quad's
+ * raw S/T span sits in the ~290k-390k unit range -- roughly 10x sky's typical
+ * span -- and, critically, that skyPortPickShift's adaptive k genuinely
+ * CHANGES during ordinary view rotation (observed kT flipping 4->3 as spanT
+ * crossed skyPortPickShift's per-k breakpoint mid-turn). That contradicts the
+ * "normally k==0, this is a rare safety valve" assumption the mechanism was
+ * verified against (a single static frame, one camera angle) -- every k
+ * transition re-quantizes the shared vertex tc at a different granularity,
+ * which is a plausible mechanism for "glitches, goes back and forth... changes
+ * depending on your view" even with both texunits' relative offset preserved
+ * (D245's already-verified allowShift=TRUE reasoning). Untested candidate,
+ * NOT the default: forcing a single fixed k for the whole water draw removes
+ * the transition, at the cost of coarser (whole-k5-step) tc quantization
+ * always, which could itself look worse -- needs a live A/B, not another
+ * blind ship (see the finding's own "don't repeat the same mistake" note).
+ * GE_D245_FIXEDSHIFT=<k> (0..SKY_TC_MAX_SHIFT) opts in; unset changes nothing. */
+static void skyPortForceFanShift(s32 kS, s32 kT)
+{
+    s_skyFanShiftS = kS;
+    s_skyFanShiftT = kT;
+}
+
 /*
  * D176(a) Path B (M-46) -- black-sky fix. Full rationale in
  * docs/dev/findings.md section F, "D176(a)".