Skip to content

Latest commit

 

History

History
400 lines (326 loc) · 22.5 KB

File metadata and controls

400 lines (326 loc) · 22.5 KB

Mistakes log: things we tried that were bad

Append-only record of changes that landed (or were attempted) and turned out wrong, harmful or misjudged. Each entry exists so a future round does not re-litigate settled ground on incomplete information. Newest first.

Where a mistake produced a standing decision, the mechanism and the evidence live in the ADR named at the end of the entry and are not repeated here.

Naming note (2026-05-09). Older entries and older CSV rows use g3, g4, g4mini, lion. Those are the same hardware as yosemite, quicksilver, mini-g4, mini-intel. sawtooth (G4 AGP tower) joined after the rename.


2026-09-25: batching multiple download chunks per server tick wedged the download on real hardware (#64)

Measured (self-host rig, mini-intel server + mini-g4 client, negligible LAN RTT) that in-protocol download throughput was bounded by sys_ticrate (one svcdp_downloaddata chunk queued per server tick, ~50ms default) and not by ack round-trip time. The obvious fix looked like: queue several chunks per tick instead of one, flushed together. First cut moved the queuing off the per-ack trigger onto the tick itself (to avoid a related burst-of-acks-in-one-SV_ReadClientMessage-pass bug that the naive per-ack version hit first). That version got much further -- then died on real hardware the moment one flush's payload passed a few KB: UDP_Write, sendto: Message too long. Datagram_SendMessage's own fragmentation (net_dgrm.c, chopping at MAX_DATAGRAM) only fires above 64000 bytes, so anything smaller than that goes out as one raw sendto() -- and real UDP sockets reject a send bigger than the path can carry without fragmentation long before 64000. The codebase already knew this: SV_SendClientMessages's signon-buffer flush batches multiple buffers per send only for a local (same-process) client, if (!local) break; after one for everybody else. A batched-download-chunks fix would have broken that same constraint for every real network client, having never been exercised above roughly a normal per-frame message's size before.

Shipped instead: a bigger single chunk (1024 -> 1400 bytes, one raw Ethernet-MTU-safe UDP datagram, still one chunk per tick). ~1.3x, not the ~8x batching would have been if it had worked, but it doesn't wedge. gl_fullbright_zbias-style toggleability wasn't needed here since there's no per-machine tradeoff, just a hardware-real ceiling.

Lesson: this codebase already draws the "one flush, one send, no batching for a real network client" line once (the signon-buffer local-vs- network split) -- check for that line before assuming a bigger single reliable message is free just because the in-memory buffer (MAX_MSGLEN 64000) has room for it. The buffer size and the safe wire size are not the same number, and only real hardware exposes the gap.


2026-08-31: imac-2019's DMG-installed launch hung on a live Desktop-folder TCC prompt, not a crash

Cutting the v1.15.7 release DMG, deploy-dmg.sh + smoke-dmg.sh (the real install-and-launch path) hung on imac-2019 with no fps line and no crash report. ps aux showed the process genuinely alive (S state, near-zero CPU) for 2+ minutes -- not dead, blocked. The user was physically at the machine and confirmed it directly: a live "Quakespasm.app would like to access files in your Desktop folder" TCC dialog was sitting on screen, needing a human click. Allowing it unblocked the launch immediately (215.8 fps, clean).

Root cause: every target, imac-2019 included, installs to ~/Desktop/quake/, and Desktop is TCC-protected on modern macOS. This repo's own README already documents this exact failure and its fix ("Put the game folder in /Applications, not on the Desktop") -- just for a human's manual install, not the fleet's own deploy path, which still uses ~/Desktop/quake/ everywhere including imac-2019. A version bump changes the ad-hoc-signed binary's CDHash (Info.plist is inside the sealed resources), so macOS treats each new build as unrecognized and re-prompts -- this will recur on every future release, and no agent can dismiss it remotely; it needs a human at the screen every time. smoke-dmg.sh's 45s timeout reports this as FAIL (crash-or-hang) on imac-2019, which is misleading -- it's neither.

Lesson: a hung process that is genuinely alive (not zombied, not exited, low CPU, ps shows it running) on a machine with a real display is a candidate for a blocked GUI dialog, not a crash — check for one, or ask whoever's physically there, before writing it up as a crash/hang bug in the engine itself. Not fixed here (deploy path still targets Desktop); v1.15.7 shipped with the user manually unblocking the one dialog.

2026-08-28: a modern tool's default silently assumes a modern Mac -- check every one against the actual old-Mac target, never infer from "it compiled"

Same shape hit three times in one session, none landed broken (caught before shipping each time), but the pattern is worth naming so it isn't rediscovered the hard way:

  1. G5 slice on Panther/Tiger: build.sh's g5 case is deliberately -mmacosx-version-min=10.5 (Leopard-only, matches this repo's stated floor). dyld picks that slice on ANY G5 CPU regardless of the booted OS (fat-binary selection is CPU-subtype based, not OS-aware), so it loads on g5-panther/g5-tiger/quad-tiger and crashes: dyld: Symbol not found: ___stderrp/_kTISPropertyUnicodeKeyLayoutData, missing on the older libSystem/Carbon. Not a bug -- confirmed 3 for 3, always the same cause.
  2. create-dmg's DMG layout (issue #38): tested before wiring into make-dmg.sh, not after. create-dmg's own verbose output showed a GPT (GUID Partition Table) image -- our existing DMGs are Apple Partition Map, which is what PowerPC Mac OS X needs; GPT support in Apple's own tooling is Intel-era. Adopting it as-is would have made every release DMG unmountable on the fleet's oldest, most floor-critical machines. Not wired in until this is resolved.
  3. imac-2019 as an Intel build host (issue #37): moving the i386 slice there looked like a free win ("no cross-compiler needed"), but its default clang SDK is Sequoia's, a dozen OS releases newer than the 10.4 deployment target -- nothing like the Lion minis, where "no isysroot" naturally resolves to a compatible OS because the compiler runs ON a Lion-class machine. Fixed by pinning an explicit -isysroot at an old SDK already staged there, verified via otool -l/lipo before trusting it, not assumed. The sibling lion/x86_64 slice has no such SDK available to pin at all and was deliberately left on the Lion minis rather than guessed at.

The lesson: a modern host running a modern tool with an old -mmacosx-version-min/deployment-target flag is not the same as building on (or for) the actual old OS. Weak-linking and deployment-target flags paper over API surface, not partition schemes, ABI-era startup objects, or which SDK's headers actually got compiled against. "It compiled" and "the flag looks right" are not verification -- check the actual output (lipo -detailed_info, otool -l for LC_VERSION_MIN_MACOSX, or the packaging tool's own verbose output) every time a build or packaging step moves to newer hardware, before trusting it near a release.


2026-08-23: APPLE_client_storage on an async driver corrupted a GPU until power reset

Phase 2.2 enabled APPLE_client_storage for lightmap uploads: the driver keeps referencing lm->data instead of copying it. Safe on the synchronous PPC-era drivers it was measured on; on mini-sl (GeForce 9400, 10.6.8, asynchronous driver) it broke two ways. (1) R_BuildLightMap rewrites lm->data in place while queued draws still DMA-read it — under multi-threaded GL that was the issue #30 SIGSEGV inside the driver's worker thread. (2) GL_BuildLightmaps frees the previous map's lm->data right after Mod_ClearAll deletes the textures, before an async driver has actually dropped its DMA reference — that was a main-thread hang inside glDeleteTextures → gldReclaimTexture → mach_msg_trap, followed by a kernel-log flood of NVDA(OpenGL): Channel exception! Fifo: Parse Error (the GPU FIFO parsing freed memory), a corrupted display, and a machine that a graceful reboot could not recover: /sbin/reboot waits to kill processes, WindowServer was wedged unkillably inside the GPU kernel driver, and the shutdown hung mid-way for ~2.5 h until a power reset.

Fixed: client storage defaults OFF on non-PowerPC (-client-storage forces it for A/B), GL_BuildLightmaps drains the queue with glFinish before freeing where it is on, and qsreboot.sh --force (/sbin/reboot -q) exists for the next GPU-wedged machine.

Lesson: a zero-copy extension is a LIFETIME CONTRACT with the driver, and "the texture was deleted" is not "the driver is done with the memory" once the driver is asynchronous. An optimization measured safe on the hardware it was built for is not safe on a newer driver by default — gate pointer-handoff paths to the platforms they were proven on. And a graceful reboot can HANG, not just fail, when the wedged process sits in a kernel GPU fault — recovery tooling needs a forceful tier. Issue #30.

2026-08-23: benching an archived cvar silently re-configures the machine

An A/B on decals pinned +r_decals 0 on yosemite and mini-g4. Host_WriteConfiguration (host.c:386) writes id1/config.cfg on every clean exit, so the engine archived r_decals "0" on both. No per-machine bundle config sets r_decals (zero hits across scripts/bundle/*.cfg, against 13 files that set r_shadows), so nothing corrected it at launch. Decals were silently OFF on those two machines for hours, on a build that ships them ON, and only those two — the other four read "1". Repaired by hand.

-noarchautoexec stops the autoexec being READ. It does not stop config.cfg being WRITTEN, and there is no flag that does.

Lesson: a bench does not just measure a machine, it CONFIGURES it. Any CVAR_ARCHIVE cvar you pin persists on that box until someone notices, and it poisons the next A/B that relies on the default as well as ordinary play. After benching a CVAR_ARCHIVE cvar, check id1/config.cfg on that machine. Open as #28. Make an instrumentation-only cvar CVAR_NONE so it leaves no residue, as r_decal_stats does.

2026-08-23: a failed log fetch reported the PREVIOUS run's fps as a result

bench.sh did scp ... || true and then grepped a fixed path. When the copy failed the grep read whatever file was already there and recorded ITS number. Not a gap, and not an error: a complete, plausible row. It corrupted one G3 A/B before it was caught — a decals-OFF leg recorded 34.6 fps that was the decals-ON leg's run 3, proved by the stale file's mtime and its gated=0 counters. Both legs also shared one log filename, so the second leg overwrote the first leg's raw evidence.

Lesson: the contamination is DIRECTIONAL. The stale number comes from the other leg of the same A/B, so a fabricated result is biased toward "no difference" — the answer that stops anyone looking. A harness that fails toward "nothing to see here" is not self-correcting. Fixed in f3c8a490: delete first, treat a failed fetch as a failed run and record NA loudly, and put the cvars in the log name. It caught a real 3/3 G3 failure within the hour.

2026-08-20: "this port uses SDL2" was wrong; every shipped slice is SDL 1.2

Commit fd507839 asserted that QuakeSpasm "is on SDL2 and its bundled framework was already fat with arm64 in it", and used that to argue this port was structurally ahead of the Quake II and Quake III ports. Makefile.darwin:13 sets USE_SDL2=0 and otool -L build/quakespasm-fat shows every slice linking SDL at current version 12.4.0, i.e. SDL 1.2.15. The wrong idea came from MacOSX/SDL2.framework genuinely being vendored and fat, plus upstream having a live USE_SDL2 path, plus the arm64 probe only linking with USE_SDL2=1.

Lesson: check what the artifact links, not what the source tree contains. Same failure as the -faltivec entry below, in a different costume. ADR 0003.

2026-07-25: -faltivec silently un-stamps the ppc7400 cpusubtype

Adding -faltivec (required by the 10.3.9 SDK's Carbon headers) produced a slice stamped generic ppc (ALL) instead of ppc7400. Nothing in the build complained; lipo -info was the only signal. A generic ppc slice is a launch blocker on Tiger and Leopard, not a cosmetic flaw.

Lesson: do not trust the compiler to stamp the subtype. Assert it. A build that exits 0 and produces a runnable-looking binary can still be unlaunchable on a machine you did not test. ADR 0002.

2026-05-31: the DMG pipeline could ship a silently-corrupt binary

Caught on the Quake II sister port, whose make-dmg.sh was adapted from ours: a single flipped byte in the ppc7400 slice turned stw r31 into an illegal 64-bit-only opcode and crashed every G4 at init. Only the DMG copy was corrupt; hdiutil verify passed it.

Lessons: hdiutil verify is a container checksum, not a content check, verify the bytes inside the image against source, end to end, every build. Do not build release artifacts on the flakiest hardware in the fleet. Test the artifact the user actually runs, not the convenience path. ADR 0005.

2026-05-31: G3 Rage 128: a live in-game resolution SWITCH crashes the engine

A two-layer config where the overlay changed vid_width without a vid_restart left the engine running at the baseline's resolution; the user corrected it via the video menu, and that live fullscreen mode switch hard-crashed the driver.

Lessons: an overlay that sets a resolution cvar without a vid_restart is a silent no-op for the live mode and only shows up as a phantom in config.cfg, put the authoritative boot resolution and its one restart in ONE place. On a GPU that cannot survive a live mode switch, booting at the right resolution is not enough: the video menu and alt-enter are loaded guns. ADR 0006, ADR 0007.

2026-05-31: iMac G5: GLSL/VBO on the ATI Radeon 9600 hard-hangs the whole OS

Two red herrings before the real cause: fullscreen was blamed, then SDL was rebuilt for Leopard and it wedged anyway. The cause is the R300's Leopard GL 2.0 driver on the GLSL/VBO path.

Lessons: a GPU that advertises GL2 on an old Mac driver is a trap, the first GL-2.0-class GPU in a fleet exercises code paths nothing else did. Gate on the renderer string, not the CPU or the slice. GL 1.x here is not a compromise. ADR 0007.

2026-05-29: vid_bpp 32 hard-wedged mini-g4 (Radeon 9200)

A code-review "cheap win" (32bpp for a real stencil buffer, so r_shadows gets its self-intersection mask) wedged the whole OS during boot video-mode init on the 9200 and did not on the 9000. Intermittent on retest, which is worse to ship than a deterministic failure.

Lessons: an autoexec-driven boot vid_restart is not free on old GPU drivers. deploy.sh does not seed config.cfg, so a cvar the engine saved during a test run persists on the target and silently changes the next launch. A modern bench machine being offline means the headline visual win ships unverified, flag it loudly in the cfg and the commit message. ADR 0006, ADR 0007.

2026-05-10: rounds v9 and v10: a "code regression" that was a config change

A whole-fleet demo3 regression was blamed on the Ironwail flat-array efrags pattern, which was reverted; a v10 follow-up was then built to fix a regression that did not exist. The real cause was r_lavaalpha 0.6 in the autoexec.

Lessons: check a bench delta against the autoexec diff before blaming code, verify by re-running with id1/autoexec.cfg removed. "Same code, same machine, same bench script" is not enough; the cvar set in effect is part of the bench. Yosemite needs frequent reboots during long bench cycles. ADR 0009.

If revisited: the flat-array efrags pattern measured neutral on the G3 under the confound (15.10 legacy vs 15.00 flat) and its true impact is unknown. An r_lavaalpha_distance cvar mirroring r_dynamic_distance would let close-up lava blend while far-field stays opaque, recovering the effect on the G3 without the full-scene fillrate cost, about one round of work.

2026-05-09: round v6: stale-binary CSV pollution read as a 30% regression

Wrap rows were benched against a binary that had been superseded mid-day. Bisect showed the "72.20 baseline" was really 53.40, so the following round's "−30%" was −5.8%. Twelve rows were deleted from results.csv and the baseline re-run.

Lesson: after any mid-bench binary refresh, re-bench every affected machine before the wrap commit, not just the most suspect ones. "Stale binary" is invisible to the bench scripts. ADR 0009.

If revisited: the mini-g4 demo3 1024 +42% / 640 +46% wins from round v7 phase 1 (sky hoist) on the Radeon 9200's ATI driver are that round's headline. Confirm them in any future bisect.

2026-05-09: round v5 B5: the scalar dlight cast hoist. REVERT WAS WRONG

Reverted on a reading that sat inside a 6%-wide historical noise cluster. A proper same-session A/B showed +2.9% on mini-g4 demo3 1024, and B5 was re-applied.

Lesson, now load-bearing: never declare a regression without an apples-to-apples same-session A/B on the suspected target. ADR 0009. B5's genuine G4-vs-G4 split, and why it was not simply dropped, is ADR 0008.

2026-05-09: round v5 B3: Lion PGO and LTO, expected +5–12%, delivered nothing

Lion's clang is Apple clang 1.7 (LLVM 2.9): -fprofile-instr-generate is silently accepted and emits no instrumentation; -flto works and measures +0.2 fps inside a 0.8 fps spread.

Lesson: do not redo this on Lion's clang. Intel gains have to come from source-level changes, not compiler flags. ADR 0005.

2026-05-08: Pass A item 5: BGRA static-texture upload (reverted)

Tried. Extend Phase 2.1's GL_BGRA + GL_UNSIGNED_INT_8_8_8_8_REV upload format from per-frame lightmaps to all static textures (world brushes, alias skins, particle/sky/HUD pics) in TexMgr_LoadImage32, as an in-place RGBA→BGRA byte swap on the source buffer plus a format-constant change at the glTexImage2D call. Predicted as a load-time-only win with zero per-frame fps effect; risk tagged medium; toggleable behind -nobgra-static.

What went wrong. Reproducibly crashed all four bench targets during map load, G3 Panther / Rage 128, G4 Quicksilver / Radeon 9000 / Tiger, G4 mini / Radeon 9200 / Tiger, Lion x86_64 / GMA 950, with an identical signature:

EXC_BAD_ACCESS / SIGSEGV  in COM_FindFile + ~256
  ← Image_LoadImage (recursing through extensions)
  ← Mod_LoadTextures (replacement-image lookup)
  ← Mod_LoadBrushModel ← Mod_LoadModel ← CL_ParseServerInfo

The faulting register held an address in the high stack region (e.g. 0x3f9f08b4, 0xff3808b4, from r30/rsi pointing into trashed strings): the in-place swap corrupted hunk-allocated filename memory adjacent to the texture buffer, and the engine dereferenced garbage looking for the next replacement texture's filename.

Misdiagnosis on the way. The first fix (cea45842) gated the path on host_bigendian, assuming only PowerPC was affected because the engine disables EXT_packed_pixels on big-endian (gl_vidsdl.c:1424, upstream sezero/quakespasm#114). Lion crashed too. The endianness gate was a partial mask of a deeper bug.

Why the lightmap path works and this did not. R_BuildLightMap writes bytes directly in [B][G][R][A] order, the data is born in BGRA layout and the upload format consumes it correctly. The static path took an existing RGBA buffer, swapped in place, then handed it to the same upload format; somewhere between the swap, the optional resample / mipmap-down / alpha-edge-fix steps and the upload, it wrote past its allocation. The exact mechanism was never found, the byte-count math and buffer-size invariants all looked correct on inspection.

Fix. Reverted to GL_RGBA + GL_UNSIGNED_BYTE unconditionally. There is no per-frame hot loop attached, so the revert costs nothing measurable. The bgra_static_disabled flag and -nobgra-static cmdline stay in tree as inert plumbing.

Lessons.

  1. Smoke-bench every target before committing, even for a change tagged "load-time only, zero fps move". This shipped through a smoke that passed on all three machines then in the matrix (b186ae44, Lion 95.7 fps demo1 1024 with BGRA enabled). demo1 alone is not enough exposure, include a map transition.
  2. In-place buffer mutation is a footgun on shared hunk allocations. The replacement-image lookups happen before TexMgr_LoadImage32 returns, so corrupted state persists in any buffer still reachable. If revisited, allocate a fresh BGRA buffer and copy into it.
  3. "Apple's documented fast path" is not a green light. The combo is genuinely faster when the data layout matches, but "matches" needs to be proven, not inherited from a different data origin.
  4. Endian-only gating is suspicious as a fix. The same crash on Lion should have been a stronger signal that endianness was not the issue.
  5. Lion is a load-bearing third bench leg. Without Intel in the matrix the bug would have looked PowerPC-specific and the wrong fix could have shipped.

Cost. Three engine crashes on the G3 (one needing a hard reboot), one each on Quicksilver, mini-g4 and Lion; about 90 minutes before reverting. Commits: b186ae44 (original), cea45842 (partial endian gate), then the full revert.

Phase 1.1c: multitexture client-array conversion (reverted, c00a07a7 era)

Converting R_DrawTextureChains_Multitexture to client vertex arrays cost the G4 −3 to −4% on brush-heavy demos and was neutral on the G3, whose Rage 128 driver has no fast path for client arrays. Apple's Radeon 9000 driver has a glBegin fast path that beats glDrawArrays against client memory. It was restored later, gated on gl_apple_var_able so it only runs when the verts are already in driver-cached VAR memory (Phase 3.3, fdd1b09a): G4 640 141.75 → 151.00 (+6.5% vs 3.2), G4 1024 121.20 → 119.85 (−1.1%, still under the 2.3 peak of 123.35), 1024 is fillrate-bound, 640 is vert-submission-bound. CVA Lock stayed gated off on the R128 for in-game colour corruption (-r128-cva is the retest hatch).

Lesson: on these drivers, moving world geometry off immediate mode is neutral-to-negative unless the verts land in driver-owned memory. The same wall was hit again by gl_surfbatch (2026-05-31: −4.8% on G3 demo2 640, −4.2% on Lion demo1 640, neutral on G4 and G5, no machine winning) and by the Q2 sister port's gl_groupdraw (−3% on R128). Both are kept default-off and correct; full tables in docs/KNOBS.md. Do not re-chase this without a new mechanism.