Flush battle VWF CHR per dirty region instead of the full buffer - #33
Merged
Merged
Conversation
Inventory slot N's CHR runs $70:3C00 + N*160, so slot 4 spans $70:3E80..$70:3F1F and overwrote the gate bytes with glyph pixels.
The battle VWF tile + tilemap + inventory-CHR DMA pass can run ~40 scanlines and starts late in the NMI (after OAM DMA and the flying-HDMA chain), so its tail spilled past the ~37-line vblank into active scan and tore VRAM. Only the restore half of the forced-blank handshake survived (vanilla NMI tail $02:837F: lda $6CC1 / sta $2100); the set half was missing, so the screen was never blanked during the overrun. Set $2100=$80 at the head of dma_transfer whenever any DMA work is queued (pending_transfer_mask / tilemap_pending_mask / dma_dirty_slots), letting the DMA tail blank the first active lines; $02:837F restores brightness. Idle frames skip the blank so there's no visible black strip.
The NMI VWF tile DMA blasted the whole 0x1000 CHR buffer every dirty frame (~24 scanlines), which on stacked frames overran vblank. The abandoned SMART path tried per-region transfers but was dead code (SMART undefined) and its count was wrong (0x400 vs the real 0x300). Turn pending_transfer_mask into a dirty-region bitmask: bit 0 = transfer queued (set by deinit), bits 1-4 = which of the four VWF regions (messages/monsters/names/commands) was re-rendered. Each init path sets its region bit; dma_transfer flushes only the flagged regions, each a 0x300-byte slice at buffer_ptr + N*0x300 -> VRAM $B000 + N*0x300. Worst case (all four) is 0xC00 (~18 lines), comfortably inside vblank. Inventory CHR already rides the separate dma_dirty_slots path, so init_inventory_region no longer writes pending_transfer_mask (its old base store would corrupt the region bits). Two more sites in the slot pre-render also stored the tile_id base into that byte -- leftover from before it became a bitmask -- which clobbered CHR_PENDING and every region bit on every inventory slot render; dropped both, they only needed A for the allocator thunk. The per-region bit sets grew the gated init bodies past 8-bit branch range, so the long intra-block branches move to brl / a near trampoline. With the typical 1-region frame now ~4.5 lines, the forced-blank set stays only as a backstop for pathological stacked frames.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Battle command-window input during a stacked-DMA frame could overrun
vblank when the VWF NMI hook blasted the whole 0x1000 CHR buffer on
every dirty frame. Turns the flush into a per-region bitmask (messages/
monsters/names/commands, 0x300 bytes each) so a typical 1-region frame
DMAs ~4.5 lines instead of ~24, and moves the gate state that tracks it
out of the CHR buffer entirely.
That relocation matters on its own: the gate bytes used to live at
$70:3F00, inside VWF_CHR_BUFFER, and inventory slot 4's CHR slice
($70:3E80..$70:3F1F) overwrote them with glyph pixels -- the battle
monster-name window never rendered on current master. Repro'd and
confirmed fixed against a live battle savestate (before/after
screenshots, WRAM/VRAM byte comparison).
A second, unrelated clobber in the same area: two sites in the
inventory-slot pre-render stored the slot's tile_id base into
pending_transfer_mask -- leftover from before that byte became a
dirty-region bitmask. Every inventory slot render wiped CHR_PENDING and
all four region bits, so the battle names window (which only renders
once per battle) lost its one flush and stayed black for the rest of
the fight. Both stray stores only needed A for the allocator thunk that
follows; dropped.
Forced-blank on queued NMI DMA work stays in as a backstop for
pathological stacked frames -- the common case no longer needs it.