diff --git a/.gitignore b/.gitignore index 763ad28..f7756c8 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ build-profile-timex/ build-bench/ __pycache__/ build-bench-std/ +.claude/worktrees/ diff --git a/AGENTS.md b/AGENTS.md index 0b734b0..51b63fa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,6 +55,12 @@ with `pkill -f 'zesarux --configfile'` first): (Timex: `--machine TC2068 --enabletimexvideo` and the timex TAP.) +On a headless host add `--vo null --ao null --audiovolume 0`. Without +`--vo null` the Cocoa build blocks before it opens the ZRCP port, so the +emulator sits at 0 % CPU with nothing listening and every tool times out +waiting for it. The audio flags matter whenever the host has a speaker: the +game starts its AY effects as soon as it runs. + ### Autopilot bench builds Unattended measurement uses the AUTOPILOT define: invulnerable plane, diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bb52c9..1ac5a20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,24 @@ both TAPs to every tagged release. GitHub Pages, embedded in the vendored JSSpeccy 3 emulator (`site/`, `.github/workflows/pages.yml`), linked from the README. +### Fixed + +- Shots no longer erase the river banks. A two-pixel projectile was drawn by + storing its mask over the whole screen byte and cleaned up by writing plain + water, both assuming the byte was entirely river. Because a bullet keeps the + X it was fired at while the river narrows above it, and because its collision + test only asks whether the two lit pixels meet land, a byte holding both the + shot and bank pixels lost its land. On a straight bank the notch was + permanent. Projectiles and the splash now compose against live terrain and + their cleanup restores real world bytes. +- The tank splash lands on its target instead of eight pixels to the right of + it. Its aim point is a centre but was consumed as a left edge, which put the + sprite's second byte on the right bank edge column. +- Ships and the helicopter no longer spawn partly on land. All three asked for + a safe position using a scanline above the playfield, because raising their + active flag overwrote the row they meant to sample; ten percent of ship + spawns were affected. + ## [0.3.0] - 2026-07-26 ### Added diff --git a/docs/TODO.md b/docs/TODO.md index 4b619db..b196f7c 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -1,191 +1,284 @@ # TODO Distilled from the nine performance/correctness reviews written on 2026-07-25 -(`*review*.md`, removed after verification). Every item below was re-verified -against the current sources on 2026-07-26; everything the reviews recommended -that had already landed (mostly via commits 767e27b, f8aef7d, d54a02e, -a9a1f39, 424f82c) was dropped. +(`*review*.md`, removed after verification), re-verified against the sources on +2026-07-26. Items the reviews recommended that had already landed were dropped. +The single correctness item and the three cleanups were completed on 2026-07-26 +and are recorded under "Settled" below. -Context for priorities: the autopilot bench currently holds 50 Hz with -~0–0.5 % frame overruns and the worst frames are full-HALT frames. All -performance items are therefore opportunistic — run -`tools/zrcp_tail_profiler.py` on a bench build before and after any of them. +Context for priorities: the autopilot bench does NOT hold a steady 50 Hz, and +the earlier figure of "~0-0.5 % frame overruns" does not reproduce. Measured +with `tools/zrcp_tail_profiler.py` over five windows per build at different +course positions, unmodified code ranges from 0.0 % to 15.4 % overrun frames +depending on what is happening in the sampled window. Read the metric +correctly: a window reporting a high overrun share together with a high idle +share (one measured 43 % overrun with 56 % idle) is the game running at half +rate - once work exceeds a frame, the `halt` is reached after the interrupt has +already fired and waits for the next one, so heavy frames alternate with long +halts. That is a real overrun, not an artifact. + +Measurement caveats, learned the hard way: + +- **A scene-matched comparison between two builds is not achievable with this + harness.** Enemy spawns come from the LFSR, a shift register whose value is + scrambled by a single extra draw, so once two runs differ by one frame their + actor populations diverge chaotically and never reconverge. +- **Do not anchor on emulated time.** An overrunning frame delays the next + `halt`, so a busier build advances fewer logical frames per emulated second: + a t-state anchor is correlated with the effect being measured. Anchoring on + the game's own progress (counting wraps of `course_block_head`, four logical + frames per block under the fixed-speed autopilot) aligns the course position + to within a frame or two, which is the best available. +- Always run a control build through the identical protocol, pool several + windows, and report the actor population per window so like-for-like pairs + can be identified afterwards. +- `enter-cpu-step` fails under `--vo null`, so the history ring stays at its + default 1M entries (~1.8 s), cannot be enlarged on a headless host, and + breakpoint-based timing is unavailable. A longer `PLAY_SECONDS` only + overwrites the ring; 5 s per cycle is enough. +- The ROM tape loader needs about three real minutes. Use the ZRCP `smartload` + command instead: the game reaches user code in roughly 30 s. ## Correctness -1. **Projectile draw/cleanup assumes the whole byte is water — can nick - bank/island edges permanently.** `write_water_projectile_2xn` - (`src/sprite_renderer.asm:2996-3042`) stores the 2-px mask byte raw, the - splash draws via opaque `write_water_sprite_2xn` (:2651, call sites - :852-888), and cleanup restores plain water (`transition_background=0` in - `transition_bullet_direct` :1246-1247 and `transition_shell_direct` - :1931-1932, direct zero writes in `restore_flying_shell_row` :1993-2000). - A shell clamped near a bank (`src/entities.asm:1121-1135`) can overlap - terrain pixels outside the two collision-checked mask bits; the dirty pass - replays only per-block deltas, so a damaged byte on a straight section is - never repaired. Fix: compose draw bytes as `world_background OR mask` - (`get_world_background_byte` / `load_world_background_triplet`, - `src/sprite_renderer.asm:2194` / :2340) and switch cleanup to - `transition_background=1` so `fill_world_background_rect` (:2490) restores - true world bytes. Manual check: shoot along a straight bank edge and look - for lasting nicks. - -## Performance — larger items +1. **Resident sprites eat terrain along island and bank edges. Confirmed, + partly fixed, and the remaining half is the largest known correctness bug.** + All five resident actors damage the world, on both the draw path (the opaque + `write_water_sprite_2xn` / `_1xn` / `_shifted_2xn` / `_shifted_4xn`) and the + cleanup path (`fill_uniform_sprite_rect` with `E=0`, and + `transition_background = 0` in the exception branches). Measured over 200 s of + autopilot: 18 513 damaging writes, half of them full `0xFF` bytes. FUEL is the + worst offender, then ship1, ship0, the helicopter, the balloon. Present on + `main`, not introduced by the projectile work. + + **Discard the obvious theory first.** The damage is NOT the river meandering + into a latched X. A world-anchored sprite advances `y += speed_pixels` on + exactly the frames the course advances by the same amount, and + `get_block_index_for_y` is `head - ((y+7-phase)>>3)`, so both terms move + together and the index cancels: a resident sprite sits over the same course + blocks for its whole life and the terrain under it never changes (128 120 + samples, no violation). The bend budget of four pixels per block is irrelevant. + + **The actual cause is vertical extent.** The safe X comes from ONE scanline + sample, but these sprites are 8 to 32 scanlines tall and therefore span two to + five course blocks, whose banks differ by up to four pixels and whose island + edge jumps a whole byte column between adjacent blocks + (`fork_left_offsets`/`fork_widths`). The rows below the sampled one land on + terrain nobody checked. The patrol clamps have the same defect: + `patrol_helicopter` (`src/entities.asm:584`) and `patrol_ship1` (`:197`) call + `get_pixel_lane_bounds` with the actor's top row only. + + A second cause, the three spawns that clobbered A and so sampled row 1, is + fixed. It was amplifying this one: ten percent of ship spawns landed partly + on land. + + Two fix families. Composing the resident writers against + `load_world_background_triplet` and restoring real world bytes mirrors the + projectile fix but costs time on the most-drawn sprites. Making the placement + correct instead - intersect `get_pixel_lane_bounds` over every block the + sprite's height covers, at spawn and in the patrol clamps, and defer the spawn + when the intersection is too narrow - costs nothing at runtime and should be + sufficient, because a sprite that provably fits the water for its full height + can keep its opaque writer. Prefer the second; it is also the smaller change. + + Two things to know before working on it. `render_dirty_rows` replays only the + per-block delta columns, so damage to a column that two adjacent blocks share + is never repaired - confirmed by watching one damaged column travel the whole + playfield untouched. And the damage is not cosmetic: + `check_player_background_pixels` (`src/entities.asm:2308`) tests the player + against the model, not the framebuffer, so eroded land stays lethal while + looking like water. Timex is unverified but shares the call-site logic. + + The FUEL depot is NOT the immune contrast case, as previously recorded here: + `load_world_background_triplet` only makes OTHER sprites compose correctly + over the depot; the depot's own writer and its own water fill still assume + water. + +## Performance 2. **Stage bridge destruction across two frames.** `destroy_bridge_restore` - (`src/entities.asm:1959-1979`) still rebuilds all 16 world rows in the - same frame as the explosion, score and attribute repaint, called - mid-`update_bullet` (:1571) — the most expensive single frame left. Add a - `bridge_destroying` counter next to `bridge_restore_y`/`bridge_restore_rows` + (`src/entities.asm:1959-1979`) rebuilds all 16 world rows in the same frame as + the explosion, score and attribute repaint, called mid-`update_bullet` + (:1571) - the most expensive single frame left. Add a `bridge_destroying` + counter next to `bridge_restore_y`/`bridge_restore_rows` (`src/state.asm:127-128`) and let `update_bridge` finish 8 rows per frame. Optional follow-up if the destroy frame is still near budget: a "zero-interior-first" variant of `render_v3_row_indexed` - (`src/course_renderer.asm:974`) so the destroy path can drop + (`src/course_renderer.asm:935`) so the destroy path can drop `render_full_world_row` (`src/entities.asm:1966-1972`). -3. **Hoist DI/EI + SP save out of the blitters to frame level.** Nine - SP-driven blitters each pay their own `di` / `ld (sprite_saved_sp),sp` … - restore / `ei` bracket (`src/sprite_renderer.asm` :2103/2129, :2168/2188, - :2635/2648, :2672/2689, :2714/2733, :2776/2816, :2858/2914, :2960/2993, - :3229/3248). Do `di` once after the `halt` in `main_loop` - (`src/main.asm:64`) and `ei` before every path's next `halt` (including - pause/crash/game-over paths), save SP once, delete the per-blit brackets. - Every remaining call site of these blitters (board setup/init paths - included) must be audited to run inside the DI window — while SP points - into `screen_line_table` a stray interrupt corrupts it, so land as its own - commit and verify with `make profile`. A few hundred T/frame. - -4. **Specialize `transition_bullet_direct` like the flying shell.** The - player bullet (4 rows × ≤2 bytes, moves 6 px/frame so ΔY > height) always - falls through generic `cleanup_resident_sprite_delta` → - `fill_water_rect_preserve_bridge` (`src/sprite_renderer.asm:1215-1249`). - Mirror the shell's direct-restore fast path (`restore_flying_shell_row` - model, :1967-2001), keeping the bridge-band skip — the bullet does fly - over intact bridges. Coordinate with item 1, which changes what "restore" - writes. - -5. **Vertical delta masks for scrolling resident sprites (FUEL first) — - profile first.** Fixed-X sprites still repair the exiting strip and then - fully redraw every row each scroll frame (`transition_fuel_direct` - `src/sprite_renderer.asm:1493-1517` → 32-row `write_water_sprite_1xn`; - same shape for balloon :1442-1455, ships :1261-1280/:1324-1342, - helicopter :1563-1581). Boot-generated `old[row+speed] XOR new[row]` - tables (generation in `tools/build.py`, source rows in - `src/sprite_data.asm:141`) would update only changed rows. Cheaper - fallback if this is skipped: cut the ~55 T/row `pop/add/djnz` overhead in - the `write_water_sprite_1xn` row loop (`src/sprite_renderer.asm:2638-2646`), - ~400 T/frame while FUEL is active. - -## Performance — smaller items - -6. **Incremental row addressing in projectile writers.** - `write_water_projectile_row` still calls `calc_screen_line_addr` every row - (`src/sprite_renderer.asm:3019-3042`, heights 2-4); step `L += 32`, on - carry `H += 8` like `render_dirty_rows` does - (`src/course_renderer.asm:876-885`). Same trick applies to the two per-row - calls in `restore_flying_shell_row` (:1989). May be subsumed by item 1's - rewrite of these paths. - -7. **`fill_water_rect_preserve_bridge`: test `bridge_active` once.** The - per-row loop re-tests the bridge and calls `fill_uniform_sprite_rect` once - per row with B=1, paying the DI/SP preamble each time - (`src/sprite_renderer.asm:628-679`, preamble :2168-2188). With no bridge - (or a rect that provably misses the band), issue one call for the whole - rect. Item 3 removes part of the per-call cost; this removes the rest. - -8. **`snapshot_resident_sprite_state`: LDIR or per-actor gating.** Still ~40 + Two notes before starting. Playtesting puts the stutter at the explosion + itself rather than spread over the second that follows, which points here and + not at anything the destroyed band does later - the one attempt at the latter + is in the rejected list below. And the model does NOT have to be split by + row: flip `bridge_active`/`destroyed_road_active` atomically as the code + already does and stage only the bitmap rebuild. For a frame or two the screen + then shows road the model calls water, which corrupts nothing, because + nothing else repaints those rows and the explosion covers them. + + The planned shape is a visual one, agreed with the author: the span crumbles + outward from the column the shot hit, over roughly six to eight frames, which + is how long the band stays on screen at fast scroll. Purely cosmetic - the + whole span stops being lethal the moment it is hit, exactly as now - so no + per-column collision or world-model state is needed. + +3. **Hoist DI/EI + SP save out of the blitters to frame level.** Nine SP-driven + blitters each pay their own `di` / `ld (sprite_saved_sp),sp` ... restore / `ei` + bracket. Do `di` once after the `halt` in `main_loop` (`src/main.asm:64`) and + `ei` before every path's next `halt` (including pause/crash/game-over paths), + save SP once, delete the per-blit brackets. Every remaining call site of these + blitters (board setup/init paths included) must be audited to run inside the + DI window - while SP points into `screen_line_table` a stray interrupt + corrupts it, so land as its own commit and verify with the profiler. A few + hundred T/frame, and the highest-risk item on this list for the smallest + measured gain: deprioritized accordingly. + +4. **Specialize `transition_bullet_direct` like the flying shell.** The player + bullet (4 rows x <=2 bytes, moves 6 px/frame so DeltaY > height) always falls + through generic `cleanup_resident_sprite_delta` -> the transition fill. Mirror + the shell's direct-restore fast path (`restore_flying_shell_row` model). Note + the restore now writes composed world bytes, so the fast path must too. + +5. **Vertical delta masks for scrolling resident sprites (FUEL first) - profile + first.** Fixed-X sprites repair the exiting strip and then fully redraw every + row each scroll frame (`transition_fuel_direct` -> 32-row + `write_water_sprite_1xn`; same shape for balloon, ships, helicopter). + Boot-generated `old[row+speed] XOR new[row]` tables (generation in + `tools/build.py`, source rows in `src/sprite_data.asm:141`) would update only + changed rows. Cheaper fallback: cut the ~55 T/row `pop/add/djnz` overhead in + the `write_water_sprite_1xn` row loop. Coordinate with correctness item 1, + which may move these writers to the world compositor anyway. + +6. **`fill_water_rect_preserve_bridge`: test `bridge_active` once.** The per-row + loop re-tests the bridge and calls `fill_uniform_sprite_rect` once per row + with B=1, paying the DI/SP preamble each time. With no bridge (or a rect that + provably misses the band), issue one call for the whole rect. + +7. **`snapshot_resident_sprite_state`: LDIR or per-actor gating.** Still ~40 discrete `ld a,(nn)` / `ld (nn),a` pairs every frame (`src/sprite_renderer.asm:144-223`). Either reorder the live fields in `src/state.asm:46-119` so the snapshotted bytes form contiguous blocks - matching the existing destination blocks (`src/state.asm:146-165`, - :217-234) and copy with LDIR (~200 T/frame; verify field order against the - Timex consumers in `src/render_timex.asm` first), or skip inactive actors' - blocks. Low priority. - -9. **Standard-build attribute repaints: delta restore + register loop.** - (a) `restore_standard_saved_*` (`src/main.asm:234-334`) restores the whole - old attribute rect on every trigger; for scroll-only movement (same X, - Y moved <8 px) restore only the rows the new rect no longer covers, - mirroring the Timex delta cleanup (`src/sprite_renderer.asm:226-237`). - (b) `paint_object_attribute_row` (`src/main.asm:1193-1212`) and the - F/U/E/L painter loop (:815-847) still round-trip width/value/row counters - through RAM every row; hold them in registers. Low priority — repaints are - already dirty-gated by `@STANDARD_ATTR_CHANGED`. - -10. **`generate_block` register cleanups (~200-250 T/block).** Keep - `gen_center_x`/`gen_half_x` in a register pair from the motion step - through clamp and edge conversion (`src/course_renderer.asm:100-282`; - RAM reads at :151-175, :187-270), keep `course_block_head` in a register - (seven reads in the conversion section), drop both `push af`/`pop af` - pairs (:202/206, :244/248). The flat-banks override - (`course_flat_banks`, :195-201, :237-243) must still win over the - register-held values. - -11. **`rebuild_block_delta` residual RAM traffic.** The compare loop keeps + matching the existing destination blocks and copy with LDIR (~200 T/frame; + verify field order against the Timex consumers in `src/render_timex.asm` + first), or skip inactive actors' blocks. Low priority. + +8. **Standard-build attribute repaints: delta restore + register loop.** + (a) `restore_standard_saved_*` (`src/main.asm:234-334`) restores the whole + old attribute rect on every trigger; for scroll-only movement (same X, Y + moved <8 px) restore only the rows the new rect no longer covers, mirroring + the Timex delta cleanup. (b) `paint_object_attribute_row` and the F/U/E/L + painter loop still round-trip width/value/row counters through RAM every row; + hold them in registers. Low priority - repaints are already dirty-gated by + `@STANDARD_ATTR_CHANGED`. + +9. **`generate_block` register cleanups (~200-250 T/block).** Keep + `gen_center_x`/`gen_half_x` in a register pair from the motion step through + clamp and edge conversion (`src/course_renderer.asm:100-282`), keep + `course_block_head` in a register (seven reads in the conversion section), + drop both `push af`/`pop af` pairs. The flat-banks override + (`course_flat_banks`) must still win over the register-held values. + +10. **`rebuild_block_delta` residual RAM traffic.** The compare loop keeps `block_delta_build_col`/`block_delta_build_count` in RAM (`src/course_renderer.asm:399-413`); move them to registers. Only if the profiler still shows `generate_block` hot afterwards: replace the - byte-compare with a geometric delta derived from old/new edge cols + - island intervals (mirror `dirty_island_changed`, :1067-1147). - -12. **IM2 minimal handler — measure first.** The game still runs the ROM IM1 - ISR (with KEY-SCAN) every frame (`src/main.asm:33`, comment :58-59). - Profile the ISR share with `tools/zrcp_tail_profiler.py`; if worth the - ~1,000-2,000 T/frame, install IM2 with a bare `reti` handler — this - requires adding `im`, `ld i,a` and `reti` encodings to `tools/build.py` - in the same change (repo convention). - -13. **Micro (bundle with other work only).** - - Add `cpl` (0x2F) to `tools/build.py` and replace the five `xor 255` in - the land/bridge-tank blitters (`src/sprite_renderer.asm:2723`, :2728, - :2969, :2974, :2982). - - Shift-0 two-byte row variant for `xor_sprite_shifted_2xn` - (:2060-2130); runs twice per frame while a hit explosion is live - (`src/main.asm:130-141`). + byte-compare with a geometric delta derived from old/new edge cols + island + intervals. + +11. **IM2 minimal handler - measure first.** The game still runs the ROM IM1 ISR + (with KEY-SCAN) every frame (`src/main.asm:33`). Profile the ISR share; if + worth the ~1,000-2,000 T/frame, install IM2 with a bare `reti` handler - this + requires adding `im`, `ld i,a` and `reti` encodings to `tools/build.py` in the + same change (repo convention). + +12. **Micro (bundle with other work only).** + - Add `cpl` (0x2F) to `tools/build.py` and replace the five `xor 255` in the + land/bridge-tank blitters. + - Shift-0 two-byte row variant for `xor_sprite_shifted_2xn`; runs twice per + frame while a hit explosion is live. - Per-row `sprite_write_spill` test still present in - `write_intact_bridge_tank_shifted_2xn` (:2978-2980) and - `write_world_sprite_shifted_2xn` (:3153-3155) — the other shifted - writers already select spill/no-spill variants before the loop. - - `inc de` → `inc e` in blit row loops, only after adding alignment (or + `write_intact_bridge_tank_shifted_2xn` and `write_world_sprite_shifted_2xn` + - the other shifted writers already select spill/no-spill variants before + the loop. + - `inc de` -> `inc e` in blit row loops, only after adding alignment (or page-fit assertions) for the cached rows in `src/sprite_cache.asm`. -## Cleanups - -14. **Dead code.** No callers anywhere: `get_course_background_byte_indexed` - (`src/course_renderer.asm:438`, orphaned by the run-fill rewrite), - `calc_river_center_col` (`src/course_renderer.asm:1252`), - `timex_next_attribute_row` (`src/render_timex.asm:168-189`, byte-for-byte - duplicate of `timex_advance_object_row_fast` :145-166). Delete after a - final grep; reword the comment at `src/course_renderer.asm:287` if it - references a removed routine. - -15. **Cold island/fallback path: document or remove.** The `count=255` - overflow fallback (`src/course_renderer.asm:397`, :417-419, :887-947) - and the ~187-line island case logic in `render_v3_row_indexed` - (:1030-1216) are reachable only from that practically-unreachable - fallback and from bridge repair (`src/entities.asm:1305`), where islands - are impossible by construction (bridge zones generate `island=255`, - forks end ~32 blocks before a bridge). Either document it as a safety - net, or remove it and assert at build time that a block delta never - exceeds 15 pairs. - -16. **`prepare_transition_old_projectile_x` masks `A and 7` twice** - (`src/sprite_renderer.asm:774-795`, masks at :777 and :782-783). Compute - once and branch on the 0/7/other cases; readability only, mirror - `prepare_transition_new_projectile_x` (:797-811). +## Settled on 2026-07-26 + +- **Projectiles erasing bank pixels.** Reproduced on the autopilot bench by + comparing the display file against the game's own `block_bitmap_rows` cache: + 71 damage events in 160 s, erased runs of 1-5 px at bank-edge bytes, + persisting up to 4.08 s and permanently on a straight section. Fixed by + composing draw bytes as terrain XOR mask and restoring real world bytes; + re-measured at 0 events against a pre-fix control and a no-shots noise floor. + The primary culprit was the player bullet, not the splash: the review had + guessed the splash, and static reading wrongly cleared the bullet. +- **Splash centre/edge confusion**, a second defect none of the reviews saw. +- **Dead code**: `get_course_background_byte_indexed`, `calc_river_center_col` + and `timex_next_attribute_row` deleted. +- **The block-delta overflow fallback** is documented as a safety net rather + than removed. Worth knowing: `render_v3_row_indexed` has only one live caller + (bridge repair), and its island half never executes at all, because bridge + zones are generated without islands. The `dirty_` prefix on those labels + invites the opposite conclusion. +- **`prepare_transition_old_projectile_x`** masked twice and branched on a case + that could never differ; the branch was dead, not merely ugly. +- **The spawn row for ships and the helicopter.** All three raised their active + flag through A and then called `calc_safe_river_x[_wide]`, which takes Y in A, + so they placed themselves using a scanline above the playfield. Spawn-on-land + fell from 4.6 % to 0.8 % of spawns; what remains is the vertical-extent cause + in correctness item 1. ## Considered and rejected (do not revisit without new evidence) -- **Trimming `PLAYFIELD_BOTTOM` 168→160.** Proposed by three reviews as a - ~500 T/frame saving; after the dirty-row rewrite an unchanged row is - nearly free, so the saving collapsed. Now purely a design tradeoff - (8 fewer visible scanlines), not a performance fix. +- **Taking the destroyed road off the renderer's slow path.** Written, measured, + reverted. `destroyed_road_active` does keep all sixteen band rows on the + per-byte query engine until the band scrolls off, but routing the fourteen + rows that carry no edge line to the fast copy moved the motivating window from + 43.2 % to 42.0 % overrun frames - nothing. The band-slow family is at most + 1.16 % of executed instructions in any window measured, and + `fill_world_background_byte_loop` alone is 0.30 %; a three-per-mille cost + cannot produce a 43 % overrun rate. The error was treating those symbols as + the cause because they appeared only in the bad window, without checking their + magnitude - check the magnitude. Structurally there is nothing to win either: + `fill_world_background_rect` runs only from sprite cleanup, the bridge + corridor bans most spawns, and `finish_player_transition_direct` already + switches the player to a full opaque redraw when it overlaps the band, so a + destroyed road usually sees no cleanup on it at all. The attempt also broke + the FUEL depot: the fast path copies terrain without the depot overlay that + `get_world_background_byte` applies, costing whole bytes on band row 0 in 41 + of 200 frames against 0 of 200 with the hunk reverted. + +- **Trimming `PLAYFIELD_BOTTOM` 168->160.** Proposed by three reviews as a + ~500 T/frame saving; after the dirty-row rewrite an unchanged row is nearly + free, so the saving collapsed. Now purely a design tradeoff (8 fewer visible + scanlines), not a performance fix. - **Speed-cap refinements** (dropping individual actors from the heavy-scene list, 1-1-2 cadence). The whole limiter was deleted; fast scroll is a flat - 2 px/frame everywhere (`src/input.asm:109-116`) and 50 Hz holds. + 2 px/frame everywhere (`src/input.asm:109-116`). - **Replacing `bridge_fill_full_bitmap_row` with `fill_uniform_sprite_rect`.** - The former is now an unrolled ~11 T/byte fill; the proposed replacement - would be slower. + The former is now an unrolled ~11 T/byte fill; the proposed replacement would + be slower. - **Merging the delta count byte into the ops record.** The separate - 8×-mirrored count page is load-bearing (free circular predecessor step, + 8x-mirrored count page is load-bearing (free circular predecessor step, `src/course_renderer.asm:420-435`). +- **Composing projectile bytes with OR** rather than XOR, as the review + suggested. OR makes a shot invisible over land (a set pixel on a set + background); XOR punches a water-coloured hole and matches how the player and + the other crossing actors already render over mixed terrain. +- **Caching the terrain triplet across a projectile's rows**, to undo the cost + the correctness fix added. The premise was that all eight scanlines of a + course block share one terrain row, so a four-row bullet should need one fetch + instead of four. But `resolve_course_block_index` + (`src/sprite_renderer.asm:2370-2391`) already caches the block index and the + rows left in it precisely for writers that walk Y sequentially, which the + projectile writer does - measured, most calls take that cheap path + (`resolve_block_recompute` 8190 instructions against 27294 for the resolver). + What remains per row is the bridge test, `block_bitmap_address`, three byte + reads and the FUEL-column check: a ceiling near 1 % of a frame, of which a + cache would recover about half. Against that, the cache would have to be + conditional on there being no bridge band (rows 1 and 14 of the band differ + from the rest) and no FUEL column overlap (the depot sprite is indexed by + `y - fuel_y`), in code whose correctness was established empirically. The + measured like-for-like cost of the whole fix was about 2 percentage points of + overrun frames, so this is not where the time is - item 2 is. diff --git a/docs/renderer.md b/docs/renderer.md index 40e5db9..478049d 100644 --- a/docs/renderer.md +++ b/docs/renderer.md @@ -53,10 +53,20 @@ scanlines, so the renderer updates: - 19 scanlines at 1 px per frame, - 38 scanlines at 2 px per frame. -On every affected scanline it normally replays only the precomputed bytes whose -terrain value changed; complex fork transitions fall back to the bounded edge -renderer. The full 6144-byte bitmap is rendered only during startup and after -`R`. +On every affected scanline it replays only the precomputed bytes whose terrain +value changed. The full 6144-byte bitmap is rendered only during startup and +after `R`. + +The delta list holds at most 16 byte pairs, and a block that would need more +is marked `count=255` so the scanline falls back to the complete edge renderer +(`render_v3_row_indexed`). That fallback is a safety net rather than a working +case: a bank edge moves at most four pixels per block and an island opens one +byte wide and grows or tapers one byte per side per step, so consecutive blocks +never differ in enough bytes to overflow. `render_v3_row_indexed` therefore has +only one live caller - bridge repair - and because every block of a bridge zone +is generated without an island, its island-handling half never runs either. +Both are kept so that a future wider terrain feature degrades correctly instead +of drawing a truncated delta; neither is worth optimizing. ## Colour, bridge, and sprites diff --git a/src/course_renderer.asm b/src/course_renderer.asm index db29b4d..42ec15f 100644 --- a/src/course_renderer.asm +++ b/src/course_renderer.asm @@ -284,8 +284,8 @@ course_right_x_ready: rebuild_block_bitmap_row: ; Materialize the complete 32-byte world row for the newly generated ; course block. Its geometry is five contiguous runs, so emit those runs - ; directly instead of asking get_course_background_byte_indexed thirty-two - ; times. Runtime mixed-terrain sprite composition still sees the identical + ; directly instead of classifying each of the thirty-two columns on its + ; own. Runtime mixed-terrain sprite composition still sees the identical ; materialized row. ld a,(course_block_head) ld (block_bitmap_build_index),a @@ -415,7 +415,14 @@ rebuild_block_delta_next: jr nz,rebuild_block_delta_byte jr store_block_delta_count rebuild_block_delta_overflow: - ld a,255 ; rare complex block uses old renderer + ; Safety net, not a working case. The current generator cannot fill 16 + ; pairs: each bank edge moves at most four pixels per block, so it dirties + ; a couple of bytes, and an island opens at one byte wide and grows or + ; tapers by one byte per side per step (fork_left_offsets / fork_widths in + ; main.asm). A block therefore differs from its predecessor in a handful of + ; bytes. Keep the marker so a future wider feature degrades into the + ; complete edge renderer instead of rendering a truncated delta. + ld a,255 ld (block_delta_build_count),a store_block_delta_count: ; The count page holds 32 live entries mirrored eight times, so the dirty @@ -435,45 +442,6 @@ store_block_delta_mirror: djnz store_block_delta_mirror ret -get_course_background_byte_indexed: - ; Input A=byte column, L=course block index. Output A=world byte. - ld b,a - ld h,HIGH(block_left_col) - cp (hl) - jr c,indexed_background_land - jr nz,indexed_background_check_right - ld h,HIGH(block_left_mask) - ld a,(hl) - ret -indexed_background_check_right: - ld a,b - ld h,HIGH(block_right_col) - cp (hl) - jr c,indexed_background_check_island - jr nz,indexed_background_land - ld h,HIGH(block_right_mask) - ld a,(hl) - ret -indexed_background_check_island: - ld h,HIGH(block_island_left) - ld a,(hl) - cp 255 - jr z,indexed_background_water - ld c,a - ld a,b - cp c - jr c,indexed_background_water - ld h,HIGH(block_island_right) - cp (hl) - jr c,indexed_background_land - jr z,indexed_background_land -indexed_background_water: - xor a - ret -indexed_background_land: - ld a,255 - ret - block_bitmap_address: ; Input A=block index 0..31, C=column 0..31. The 1KB cache is four ; consecutive aligned pages, eight complete course rows per page. @@ -922,8 +890,10 @@ dirty_delta_replay: jr dirty_row_advance dirty_row_fallback: - ; Reconstruct the row Y and block index which the fast loop keeps - ; implicit, then run the full edge renderer with the registers saved. + ; Cold path: only a count=255 block reaches it, which the generator never + ; produces (see rebuild_block_delta_overflow). Reconstruct the row Y and + ; block index which the fast loop keeps implicit, then run the full edge + ; renderer with the registers saved. ld a,(dirty_rows_remaining) ld b,a ld a,19 @@ -975,6 +945,12 @@ render_v3_row_indexed: ; Dirty rows advance by exactly eight scanlines, so their circular block ; index is maintained by the caller. Bridge repair still enters above and ; calculates the first index normally. + ; + ; This routine has exactly two callers, and neither is the scroll pass: + ; dirty_row_fallback, which the generator never triggers, and bridge repair + ; via render_v3_row. Scrolling replays precomputed deltas in + ; dirty_delta_replay instead. So despite the dirty_ label prefix below, + ; nothing here is on the hot path - correctness matters, speed does not. ld a,(dirty_y) cp 16 ret c @@ -1030,6 +1006,11 @@ render_v3_row_indexed: ; Compare the old and new island intervals. Unchanged plateaus cost no ; writes; changing tapers touch only bytes exposed at either edge instead ; of clearing and repainting the whole overlapping island. + ; + ; In practice this comparison always finds no island at all: the only live + ; caller is bridge repair, and a bridge zone is generated with island=255 + ; on every block. It is kept because the routine is also the declared + ; fallback for a block delta that overflows. ld a,(row_block_index) dec a and 31 @@ -1249,13 +1230,6 @@ get_bounds_for_y: ld e,(hl) ret -calc_river_center_col: - call get_bounds_for_y - ld a,d - add a,e - srl a - ret - get_pixel_lane_bounds: ; Input A=Y, C=current pixel X. Output D=min X and E=max X for the ; 16-pixel object's current water lane. During a fork the island changes diff --git a/src/entities.asm b/src/entities.asm index 68a2252..023f7c7 100644 --- a/src/entities.asm +++ b/src/entities.asm @@ -121,6 +121,7 @@ spawn_ship0: ld (ship0_y),a ld a,1 ld (ship0_active),a + ld a,(ship0_y) ; the active flag above clobbered the Y call calc_safe_river_x_wide ld (ship0_x),a ret @@ -169,6 +170,7 @@ spawn_ship1: ld (ship1_y),a ld a,1 ld (ship1_active),a + ld a,(ship1_y) ; the active flag above clobbered the Y call calc_safe_river_x_wide ld (ship1_x),a ld a,1 @@ -558,6 +560,7 @@ spawn_helicopter: ld (helicopter_y),a ld a,1 ld (helicopter_active),a + ld a,(helicopter_y) ; the active flag above clobbered the Y call calc_safe_river_x ld (helicopter_x),a ld a,(helicopter_move) @@ -1181,7 +1184,13 @@ move_tank_shell_left: ret land_tank_shell: + ; tank_shell_target_x is a centre: it is clamped against the lane exactly + ; like the jet's own centre. tank_shell_x is a left edge for every consumer, + ; the splash draw and its cleanup rect included, so convert once here. + ; Without this the 16-pixel splash sat eight pixels right of its landing + ; point and its second byte covered the right bank edge. ld a,(tank_shell_target_x) + sub 8 ld (tank_shell_x),a call start_ay_splash ld a,10 diff --git a/src/render_timex.asm b/src/render_timex.asm index 1305ddd..d8e1dcf 100644 --- a/src/render_timex.asm +++ b/src/render_timex.asm @@ -165,29 +165,6 @@ timex_advance_object_third_fast: inc h ret -timex_next_attribute_row: - ; Input/output HL=the same byte column on adjacent Timex attribute lines. - ; Spectrum display memory advances H inside an 8-line character band; - ; crossing its last line advances L by 32 and folds H to the next band. - ld a,h - and 7 - cp 7 - jr z,timex_next_attribute_band - inc h - ret -timex_next_attribute_band: - ld a,l - add a,32 - ld l,a - jr c,timex_next_attribute_third - ld a,h - sub 7 - ld h,a - ret -timex_next_attribute_third: - inc h - ret - prepare_timex_fuel_geometry: ld a,1 ld (object_attr_width),a diff --git a/src/sprite_renderer.asm b/src/sprite_renderer.asm index 3296785..9df7be4 100644 --- a/src/sprite_renderer.asm +++ b/src/sprite_renderer.asm @@ -772,15 +772,11 @@ prepare_transition_new_wide_ready: ret prepare_transition_old_projectile_x: - ; A is the actual two-pixel left edge. + ; A is the actual two-pixel left edge. The pair spills into the following + ; byte only at pixel offset 7, so this is the same test as + ; prepare_transition_new_projectile_x below, writing the old fields. ld c,a and 7 - ld a,1 - jr nz,prepare_transition_old_projectile_check - jr prepare_transition_old_projectile_ready -prepare_transition_old_projectile_check: - ld a,c - and 7 cp 7 ld a,1 jr nz,prepare_transition_old_projectile_ready @@ -885,7 +881,7 @@ draw_current_splash_table_ready: ld c,a ld a,(tank_shell_y) ld b,6 - jp write_water_sprite_2xn + jp write_world_sprite_2xn ; the splash may straddle a bank byte draw_current_balloon_direct: ld a,(balloon_active) @@ -1274,7 +1270,7 @@ transition_bullet_old_ready: transition_bullet_new_ready: ld a,4 ld (transition_height),a - xor a + ld a,1 ; restore real terrain, not blanket water ld (transition_background),a call cleanup_resident_sprite_delta jp draw_current_bullet_direct @@ -1959,7 +1955,7 @@ transition_shell_direct: ld (transition_old_y),a ld a,(tank_shell_y) ld (transition_new_y),a - xor a + ld a,1 ; restore real terrain, not blanket water ld (transition_background),a ; Projectile -> splash changes both height and representation. Restore the @@ -2017,17 +2013,26 @@ restore_flying_shell_clip: ld a,b cp PLAYFIELD_BOTTOM ret nc + ; Restore true world bytes, not plain water: the shell's byte can hold bank + ; pixels even though its two lit pixels never do. + ld a,(transition_old_col) + ld c,a + ld a,b + push bc ; the terrain query clobbers B and C + call load_world_background_triplet + pop bc + ld a,b call calc_screen_line_addr ld a,(transition_old_col) add a,l ld l,a - xor a + ld a,(world_background_byte_0) ld (hl),a ld a,(transition_old_width) cp 2 ret c inc l - xor a + ld a,(world_background_byte_1) ld (hl),a ret @@ -3104,8 +3109,12 @@ bridge_tank_direct_next_row: ret write_water_projectile_2xn: - ; Input A=Y, C=pixel X, B=height. Store the two-pixel mask directly over - ; guaranteed water; unlike XOR this cannot remove an already visible shot. + ; Input A=Y, C=pixel X, B=height. A two-pixel shot is not guaranteed to sit + ; over a whole water byte: bullet_x is latched when the shot is fired and + ; the river keeps narrowing above it, and the collision test only asks + ; whether the two lit pixels meet land. Storing the bare mask therefore used + ; to erase whatever bank pixels shared the byte, permanently on a straight + ; section, so each byte is composed against fresh world geometry instead. ld (world_write_y),a ld a,b ld (world_write_rows),a @@ -3130,15 +3139,26 @@ write_water_projectile_row: ld a,(world_write_y) cp PLAYFIELD_BOTTOM ret nc + ld a,(world_write_col) + ld c,a + ld a,(world_write_y) + call load_world_background_triplet + ld a,(world_write_y) call calc_screen_line_addr ld a,(world_write_col) add a,l ld l,a + ld a,(world_background_byte_0) + ld b,a ld a,(world_write_byte_0) + xor b ; a shot over land reads as a water hole ld (hl),a ld a,(world_write_byte_1) or a jr z,write_water_projectile_skip_spill + ld b,a + ld a,(world_background_byte_1) + xor b inc l ld (hl),a write_water_projectile_skip_spill: