diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d03e843..4a0e39b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -19,3 +19,12 @@ repos: files: \.(s|i)$ require_serial: true pass_filenames: true + - id: pytest + name: pytest (full suite, pre-push only) + description: Run kintsuki + golden tests before push to catch + patch regressions that build + format hooks can't see. + entry: pytest -q + language: system + pass_filenames: false + always_run: true + stages: [pre-push] diff --git a/src/battle/message.s b/src/battle/message.s index 93a240a..3eb6003 100644 --- a/src/battle/message.s +++ b/src/battle/message.s @@ -24,6 +24,8 @@ init: Base tile ID for ring buffer area - byte A: the base tile id """ + + ; tile_ring_base_tile should be set to your VWF tile area start ; With 0x128 dynamic + 0x128 immortal = 0x250 (592) tiles total ; But tile IDs are 1 byte (0-255), so max usable is 0xFF @@ -38,6 +40,8 @@ allocate_tiles: Allocate next 8-tile slot Returns: A = starting tile_id (byte), X = allocation ID (word) """ + + ; Calculate tile_id: base_tile + (head * TILES_PER_ENTRY) lda.w tile_ring_head ; Multiply by 8 (shift left 3 times) @@ -55,6 +59,8 @@ commit_allocation: Commit the allocation (call after rendering to tiles) X = allocation ID """ + + { ; Advance head pointer lda.w tile_ring_head @@ -83,6 +89,8 @@ Get tile_id of a specific allocation by ID A = allocation ID (word) Returns: A = starting tile_id (byte), Carry = 0 if found, 1 if expired """ + + { ; Check if ID is still valid (within current range) sec @@ -266,6 +274,8 @@ clear_buffer: Wipe the 16×region_size 4bpp tile buffer for the currently-allocated VWF tile id (sets each plane row to $FF/$00). """ + + pha phx phy @@ -336,6 +346,8 @@ display_char: Render `A` (char) at tilemap offset `Y` into the message VWF buffer ; preserves A/X/Y, returns Y = `tilemap_offset`. """ + + pha phx phy @@ -358,80 +370,80 @@ _display_char: jsr.w _adjust_bits_left_for_kerning pla } + +; Space ($FF) renders blank ; the 8-row loop only OR-stores zeros (no +; visible effect) and reads 8 glyph bytes plus 8 inx ops we can skip. +; Bump X past the glyph rows so `brk_bits_left` reads the width byte +; from the right offset, then jump straight to the width-update tail. + lda.b current_char + cmp #0xff + bne _not_space rep #0x20 - lda.w #0x0008 - sta.b counter + txa + clc + adc.w #0x0008 + tax sep #0x20 + jmp.w brk_bits_left +_not_space: -char_line_loop: +; Tier-2 hoist: classify bits_left_on_tile once per glyph, dispatch to +; one of 8 specialized 8-iteration loops (aligned + shift_0..shift_7). +; Each loop bakes the asl count into its body so the per-row +; cmp/jmp-table/mul-ladder dance disappears. Loops jump to brk_bits_left +; on completion. +; +; X holds the font pointer at entry. The shift dispatch clobbers X for +; the indirect jump, so the font pointer is stashed in `temp` first and +; each shift loop restores X from there on entry. The aligned path +; keeps X untouched so it bypasses the save / restore. rep #0x20 - lda.w #0x0000 + lda.w #0x0008 + sta.b counter sep #0x20 lda.b bits_left_on_tile - cmp #0x08 - bne _shift - -_read_8x8_char: - lda.l assets_menu_font_dat, x - xba - lda.b #0x00 - xba - inx - xba - bra _store + beq _aligned_loop -_shift: -; PPU multiplication is being used by the NMI which wrecks char lines once in a while phx - lda.l assets_menu_font_dat, x - xba - lda #0x00 - xba + rep #0x20 + pla + sta.b temp + sep #0x20 -; make jump_table_pointer - pha lda.b bits_left_on_tile asl - tax + pha + lda #0x00 + xba pla + tax + jmp.w (_shift_dispatch, x) + +_shift_dispatch: + .dw _shift_loop_0 + .dw _shift_loop_1 + .dw _shift_loop_2 + .dw _shift_loop_3 + .dw _shift_loop_4 + .dw _shift_loop_5 + .dw _shift_loop_6 + .dw _shift_loop_7 + .macro vwf_row_body(n) { rep #0x20 - jmp.w (_mul_table, x) -_mul_table: - .dw _mul_0 - .dw _mul_1 - .dw _mul_2 - .dw _mul_3 - .dw _mul_4 - .dw _mul_5 - .dw _mul_6 - .dw _mul_7 - .dw _mul_8 - -_mul_8: -_mul_7: - asl ; 1 -_mul_6: - asl ; 2 -_mul_5: - asl ; 3 -_mul_4: - asl ; 4 -_mul_3: - asl ; 5 -_mul_2: - asl ; 6 -_mul_1: - asl ; 7 -_mul_0: + lda.w #0x0000 sep #0x20 - plx + lda.l assets_menu_font_dat, x inx - -_store: - + .if n > 0 { + rep #0x20 + .for k := 0, n { + asl + } + sep #0x20 + } xba phx tyx @@ -444,15 +456,79 @@ _store: plx iny iny + dec.b counter + } -_next_line: +_aligned_loop: + lda.l assets_menu_font_dat, x + inx + phx + tyx + ora.l buffer_ptr + 1, x + sta.l buffer_ptr + 1, x + txy + plx + iny + iny dec.b counter - bne char_line_loop + bne _aligned_loop + jmp.w brk_bits_left - rep #0x20 - stz.b temp - lda.w #0x0000 - sep #0x20 +_shift_loop_0: + ldx.b temp +_shift_loop_0_body: + vwf_row_body(0) + bne _shift_loop_0_body + jmp.w brk_bits_left + +_shift_loop_1: + ldx.b temp +_shift_loop_1_body: + vwf_row_body(1) + bne _shift_loop_1_body + jmp.w brk_bits_left + +_shift_loop_2: + ldx.b temp +_shift_loop_2_body: + vwf_row_body(2) + bne _shift_loop_2_body + jmp.w brk_bits_left + +_shift_loop_3: + ldx.b temp +_shift_loop_3_body: + vwf_row_body(3) + bne _shift_loop_3_body + jmp.w brk_bits_left + +_shift_loop_4: + ldx.b temp +_shift_loop_4_body: + vwf_row_body(4) + bne _shift_loop_4_body + jmp.w brk_bits_left + +_shift_loop_5: + ldx.b temp +_shift_loop_5_body: + vwf_row_body(5) + bne _shift_loop_5_body + jmp.w brk_bits_left + +_shift_loop_6: + ldx.b temp +_shift_loop_6_body: + vwf_row_body(6) + bne _shift_loop_6_body + jmp.w brk_bits_left + +_shift_loop_7: + ldx.b temp +_shift_loop_7_body: + vwf_row_body(7) + bne _shift_loop_7_body + jmp.w brk_bits_left brk_bits_left: @@ -529,6 +605,18 @@ _finalize: _get_kerning_adjustment_binary_search: { +; Space ($FF) never appears in any font's kerning pair table; bail out +; before the bank push so external callers (battle_msg_kerning_binary_ext +; and Python tooling) skip the search too. Caller is in 16-bit M. + sep #0x20 + lda.b prev_char + cmp #0xff + beq _space_skip + lda.b current_char + cmp #0xff + beq _space_skip + rep #0x20 + phb pea.w font_table >> 16 plb @@ -627,6 +715,14 @@ not_found: plb rts +_space_skip: +; Space-pair early-out: 8-bit M from the entry check, no bank push or +; search bounds were pushed yet. Just signal "not found" and return. + rep #0x20 + lda.w #0x0000 + sec + rts + found_pair: ; This label kept for compatibility but shouldn't be reached ; in binary search version @@ -693,6 +789,8 @@ put char write to the tilemap if needed maintain counters """ + + cmp #0x42 bcc put_fixed_char_dakuten put_fixed_char_no_dakuten: @@ -714,6 +812,8 @@ init: inits the renderer for the messages window flips the flag for enabling the messages renderer. """ + + jsr.l battle_flags.set_vwf_render jsr.w battle_render.init rtl @@ -738,6 +838,8 @@ set. Writes `$FF` to `render_skipped` so the gated trampoline can short-circuit DrawText and the matching `deinit_gated` skips the DMA signal. """ + + jsr.l battle_flags.set_vwf_render jsr.w battle_render.init_monsters_gated rtl @@ -756,6 +858,8 @@ deinit: the renderer disables messages renderer falling back to fixed mode. """ + + jsr.l battle_flags.clear_vwf_render ; vram transfer was moved to a trampoline in the battle nmi. lda.l battle_render.pending_transfer_mask @@ -768,6 +872,8 @@ Companion to `init_*_gated`: always flips the flag back, only signals DMA (sets bit 0 of pending_transfer_mask) if the matching init actually rendered. Reads `render_skipped` to decide. """ + + jsr.l battle_flags.clear_vwf_render lda.l battle_render.render_skipped bne _deinit_gated_done @@ -796,6 +902,8 @@ INIDISP write at `$02:837F` after the NMI DMA chain. On idle frames (nothing queued) forced-blank is NOT set ; vblank stays normal length, no visible black strip. """ + + pha phx phy @@ -917,6 +1025,8 @@ new_line_escape_code_handler: Handler for the ` ` text-stream escape: allocate a fresh tile via `render_allocator.increment`, reset bits_left_on_tile to 8, and advance the tilemap offset by one row (16 tiles). """ + + ; we might have something of interest in Y we might know where we are in the previous iteration ? pha ;jsr.w battle_render.tilemap_write diff --git a/src/kerning.s b/src/kerning.s index 9c9e536..64f2646 100644 --- a/src/kerning.s +++ b/src/kerning.s @@ -25,6 +25,18 @@ A=0+sec on miss. Trashes A/Y/flags, preserves X. _dialog_get_kerning_adjustment_binary_search: { +; Space ($FF) never appears in any font's kerning pair table; bail +; before reading the count when either side of the pair is a space. +; Caller is in 16-bit M. + lda.b CURRENT_C + and.w #0x00ff + cmp.w #0x00ff + beq _return_zero + lda.b CURRENT_C + and.w #0xff00 + cmp.w #0xff00 + beq _return_zero + ldy.w #0x1100 lda.b [font_addr], y ; NumKerningPairs (16-bit) beq _return_zero diff --git a/src/small_vwf/render.s b/src/small_vwf/render.s index 1e6d0a7..73448f8 100644 --- a/src/small_vwf/render.s +++ b/src/small_vwf/render.s @@ -455,6 +455,17 @@ _overflow: _get_kerning_adjustment_binary_search: { +; Space ($FF) never appears in any font's kerning pair table; bail +; before the bank push so callers skip the binary search entirely. + sep #0x20 + lda.b prev_char + cmp #0xff + beq _space_skip + lda.b current_char + cmp #0xff + beq _space_skip + rep #0x20 + phb pea.w font_table >> 16 plb @@ -523,6 +534,12 @@ not_found: plb rts +_space_skip: + rep #0x20 + lda.w #0x0000 + sec + rts + _found: iny iny @@ -570,6 +587,8 @@ tilemap_write: inc.b tilemap_offset inc.b tilemap_offset} ) + + pla rts } diff --git a/tests/_profile/profile_battle_vwf.py b/tests/_profile/profile_battle_vwf.py new file mode 100644 index 0000000..b403fc4 --- /dev/null +++ b/tests/_profile/profile_battle_vwf.py @@ -0,0 +1,92 @@ +"""Profile the battle VWF blit path with all redraw gates forced dirty. + +Loads ff4-battle-ext.kss at the Battle_ext seed point, pokes every +redraw-gate dirty bit so DrawCharNames + DrawMonsterNames + DrawCmd +fire on every profiled frame, then runs `profile_start` scoped to the +display_char neighbourhood in bank $20 and reports per-function call +counts + inclusive / exclusive cycles. + +Use as the A/B baseline before/after VWF optimisations: + + python3 tests/_profile/profile_battle_vwf.py > /tmp/vwf.before + # ... apply change, rebuild ... + python3 tests/_profile/profile_battle_vwf.py > /tmp/vwf.after + diff /tmp/vwf.before /tmp/vwf.after +""" +import sys + +sys.path.insert(0, "tests") +from _ff4kintsuki import kss_path, load_emu_from_kss + +PROFILE_FRAMES = 60 +WARMUP_FRAMES = 30 + +# Pull display_char neighbourhood from build/ff4.sym so the bench +# follows allocator address shifts. +SYM = {} +for line in open("build/ff4.sym"): + parts = line.strip().split(" ", 1) + if len(parts) != 2: + continue + bank, _, off = parts[0].partition(":") + try: + SYM[parts[1]] = (int(bank, 16) << 16) | int(off, 16) + except ValueError: + pass + + +def label(pc: int) -> str: + near = max((p for p in SYM.values() if p <= pc), default=None) + if near is None: + return f"0x{pc:06x}" + name = next(n for n, p in SYM.items() if p == near) + return name if pc == near else f"{name}+0x{pc - near:x}" + + +display_char = SYM["battle_render.display_char"] + +# Scope: 4 KB around display_char captures the whole blit + kerning +# tail without dragging in unrelated bank-20 callers. +lo = display_char & ~0xFFF +hi = lo + 0x1000 + + +def force_redraw(emu): + emu.write(0x7EEF9A, 0xFF) # battle_menu_dirty: chars + cmd + status + emu.write(0x7EEF9B, 0xFF) # battle_monster_dirty: all monster slots + emu.write(0x703C01, 0xFF) # region_dirty_bits: slice-2 queue + + +def run(): + emu = load_emu_from_kss(kss_path("ff4-battle-ext.kss"), settle_frames=0) + force_redraw(emu) + emu.run_frames(WARMUP_FRAMES) + force_redraw(emu) + emu.profile_start(lo=lo, hi=hi) + for _ in range(PROFILE_FRAMES): + emu.run_frames(1) + force_redraw(emu) + stats = emu.profile_stop() + emu.close() + return stats + + +def main(): + stats = run() + incl_total = sum(s.incl_cycles for s in stats) + excl_total = sum(s.excl_cycles for s in stats) + print(f"Scope: 0x{lo:06x}..0x{hi:06x}") + print(f"Frames profiled: {PROFILE_FRAMES} (warmup: {WARMUP_FRAMES})") + print(f"Functions captured: {len(stats)}") + print(f"Total inclusive cycles: {incl_total:,}") + print(f"Total exclusive cycles: {excl_total:,}") + print() + print(f"{'function':<48} {'calls':>8} {'incl':>12} {'excl':>12} {'avg/call':>10}") + print("-" * 96) + for s in sorted(stats, key=lambda x: -x.excl_cycles)[:20]: + avg = s.excl_cycles // max(1, s.calls) + print(f"{label(s.pc):<48} {s.calls:>8} {s.incl_cycles:>12} {s.excl_cycles:>12} {avg:>10}") + + +if __name__ == "__main__": + main() diff --git a/tests/goldens/battle_init/battle_init.png b/tests/goldens/battle_init/battle_init.png new file mode 100644 index 0000000..c8f0955 Binary files /dev/null and b/tests/goldens/battle_init/battle_init.png differ diff --git a/tests/savestates/ff4-battle-ext.kss b/tests/savestates/ff4-battle-ext.kss new file mode 100644 index 0000000..28f8fae Binary files /dev/null and b/tests/savestates/ff4-battle-ext.kss differ diff --git a/tests/test_battle_init.py b/tests/test_battle_init.py new file mode 100644 index 0000000..a8765e4 --- /dev/null +++ b/tests/test_battle_init.py @@ -0,0 +1,64 @@ +"""Battle-init smoke test. + +Regression guard for the bank-20 relocation work. The +`.alloc bank20_modules` wrap (overflow Q#13 / Q#14 era) shifted bank-20 +symbol addresses in a way that caused the engine to BRK / STP during +battle redraw on the first menu interaction. The build still succeeded +and the IPS coverage looked normal, so a pure build-side check would +have missed it. This test boots the patched ROM into a battle-running +savestate, lets a few frames settle, asserts the CPU never halted, and +records a PNG golden of the framebuffer. + +Recorded once via UPDATE_GOLDENS=1; replays as a pixel-match thereafter. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from _ff4kintsuki import ( + assert_screenshot_matches_golden, + kss_path, + load_emu_from_kss, +) + +GOLDENS = Path(__file__).parent / "goldens" / "battle_init" +KSS = kss_path("ff4-battle-ext.kss") + + +@pytest.fixture +def battle_emu(): + emu = load_emu_from_kss(KSS, settle_frames=0) + # Force every redraw gate dirty before settling so the smoke test + # exercises DrawCharNames / DrawMonsterNames / DrawCmdWindow on the + # first frame. Without this the slice-2 gates stay clean (the gate + # state only re-arms on writer-side state changes like ATB rotation + # or HP delta) and the golden captures an empty header strip. + emu.write(0x7EEF9A, 0xFF) # battle_menu_dirty: all chars + cmd + status + emu.write(0x7EEF9B, 0xFF) # battle_monster_dirty: all monster slots + emu.write(0x703C01, 0xFF) # region_dirty_bits: slice-2 queue + emu.run_frames(600) + yield emu + emu.close() + + +def test_battle_init_no_stp(battle_emu): + """If the CPU executed STP during the settle frames, the patch is + broken: the BRK trap (or any rogue opcode) halted the emulator. + Catches the bank20_modules wrap regression that fired + `brk_handler -> STP` from $02:A455 during DrawText.""" + s = battle_emu.get_state() + assert s.stp == 0, ( + f"CPU halted via STP during battle init: PC=${s.pc:04X} " + f"PB=${s.b:02X} A=${s.a:04X}. The bank-20 reloc patch likely " + "corrupted bank-02 code; check `git log -- ff4.s` for recent " + "allocator or *= changes." + ) + + +def test_battle_init_screen_golden(battle_emu): + """Pixel-match the framebuffer 600 frames into the battle state. + Sensitive to any rendering regression that survives the no-STP + check (garbled tiles, wrong palette, broken text).""" + assert_screenshot_matches_golden(battle_emu, GOLDENS / "battle_init.png")