diff --git a/docs/dev/findings-index.csv b/docs/dev/findings-index.csv index b51fc124..01439e82 100644 --- a/docs/dev/findings-index.csv +++ b/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)." diff --git a/docs/dev/findings.md b/docs/dev/findings.md index 3d0c2eca..df88b0a8 100644 --- a/docs/dev/findings.md +++ b/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=` 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. diff --git a/src/game/sky.c b/src/game/sky.c index e3daa934..6c41f0f1 100644 --- a/src/game/sky.c +++ b/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= (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)".