From caf5b700f59f7ba3e8f5b42f2fb7bf2843ac5bb1 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Tue, 23 Jun 2026 15:22:29 +0100 Subject: [PATCH 01/10] Fix low-res DDFSTOP fetch precision Model low-res DDFSTOP with 4-CCK precision while keeping DDFSTRT anchored to the low-res fetch block grid. This preserves the expected fetch count for partial-stop windows and keeps bitplane modulo math aligned with hardware. Update DDF regression coverage and the chipset internals note for the 15-, 10-, and 23-word low-res fetch windows. --- docs/internals/chipset.md | 11 ++++++----- src/bus.rs | 21 +++++++++++---------- src/chipset/agnus.rs | 15 ++++++++------- src/video/bitplane.rs | 18 +++++++++++------- 4 files changed, 36 insertions(+), 29 deletions(-) diff --git a/docs/internals/chipset.md b/docs/internals/chipset.md index 24983f0f..a93a8a0f 100644 --- a/docs/internals/chipset.md +++ b/docs/internals/chipset.md @@ -23,11 +23,12 @@ granularity than the unit in hi-res/super-hi-res and wide-FMODE modes, so a DDFSTOP landing mid-unit extends the fetch through the unit starting at-or-after it (`agnus::bitplane_fetch_blocks`; the CDTV trademark screen's hi-res $64/$A8 window fetches 20 words per row, not the truncated -18). Lo-res OCS DDFSTRT and DDFSTOP are both rounded to the 8-CCK -fetch-block grid before the whole-unit completion rule is applied: bit 2 of -DDFSTOP does not select an extra low-res block, so a $4A/$B6 window fetches -14 words per row and a $64/$A5 window fetches 9. Wide-FMODE units (16/32 CCK) -use the same rule rather than moving DDFSTRT down to an absolute grid. In +18). Lo-res OCS DDFSTRT is anchored on the 8-CCK fetch-block grid, but +DDFSTOP keeps its 4-CCK precision before the whole-unit completion rule is +applied: bit 2 of DDFSTOP can select the next low-res block, so a $4A/$B6 +window fetches 15 words per row, a $64/$A5 window fetches 10, and a +$28/$D4 window fetches 23. Wide-FMODE units (16/32 CCK) use the same rule +rather than moving DDFSTRT down to an absolute grid. In lo-res, the plane-order slots for a wide unit are packed into the unit's first eight CCKs; the remaining CCKs are free for other bus users. If a bitplane fetch block occupies sprite 7's late DMA slot at $30, sprite 7 DMA diff --git a/src/bus.rs b/src/bus.rs index 2cbca949..169431bd 100644 --- a/src/bus.rs +++ b/src/bus.rs @@ -8392,11 +8392,8 @@ fn effective_ddf_start_hpos_raw(bplcon0: u16, raw: u16) -> u16 { } fn effective_ddf_stop_hpos(bplcon0: u16, raw: u16) -> u16 { - if bitplane_hires_like_ddf(bplcon0) { - raw & 0x00FC - } else { - raw & 0x00F8 - } + let _ = bplcon0; + raw & 0x00FC } fn effective_ddf_start_hpos(bplcon0: u16, raw: u16) -> u16 { @@ -11187,16 +11184,20 @@ mod tests { bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x003C, 0x0040, false), bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0038, 0x0040, false) ); - // Low-res DDF comparators resolve to the 8-CCK fetch block grid; bit 2 - // is ignored for both start and stop. The sequencer still completes the - // final block selected by that effective stop. + // Low-res DDFSTRT resolves to the 8-CCK fetch block grid, but DDFSTOP + // keeps 4-CCK precision. A stop in the second half of a low-res block + // selects the next block, and the sequencer still completes it. assert_eq!( bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x004A, 0x00B6, false), - 14 + 15 ); assert_eq!( bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0064, 0x00A5, false), - 9 + 10 + ); + assert_eq!( + bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0028, 0x00D4, false), + 23 ); // Hires DDF has 4-cck granularity: the start's low bits shift the // window by half a fetch unit. The sequencer still runs whole 8-cck diff --git a/src/chipset/agnus.rs b/src/chipset/agnus.rs index 87d31494..d7d1bbe0 100644 --- a/src/chipset/agnus.rs +++ b/src/chipset/agnus.rs @@ -465,11 +465,8 @@ pub fn effective_bitplane_ddf_start_hpos(bplcon0: u16, raw: u16) -> u16 { } pub fn effective_bitplane_ddf_stop_hpos(bplcon0: u16, raw: u16) -> u16 { - if bitplane_hires_like_ddf(bplcon0) { - raw & 0x00FC - } else { - raw & 0x00F8 - } + let _ = bplcon0; + raw & 0x00FC } pub fn effective_bitplane_ddf_window( @@ -1737,11 +1734,15 @@ mod tests { fn lores_ddfstop_uses_fetch_block_granularity() { assert_eq!( bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x004A, 0x00B6, false), - 14 + 15 ); assert_eq!( bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0064, 0x00A5, false), - 9 + 10 + ); + assert_eq!( + bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0028, 0x00D4, false), + 23 ); } diff --git a/src/video/bitplane.rs b/src/video/bitplane.rs index 5d112d48..fa316cf9 100644 --- a/src/video/bitplane.rs +++ b/src/video/bitplane.rs @@ -6215,11 +6215,8 @@ fn effective_ddf_start_hpos_raw(hires: bool, raw: u16) -> u16 { } fn effective_ddf_stop_hpos(hires: bool, raw: u16) -> u16 { - if hires { - raw & 0x00FC - } else { - raw & 0x00F8 - } + let _ = hires; + raw & 0x00FC } fn effective_ddf_start_hpos(hires: bool, raw: u16) -> u16 { @@ -7841,14 +7838,21 @@ mod tests { ddfstop: 0x00B6, ..blank_state() }; - assert_eq!(lores_partial_stop.words_per_row(false, 320), 14); + assert_eq!(lores_partial_stop.words_per_row(false, 320), 15); let lores_odd_stop = RenderState { ddfstrt: 0x0064, ddfstop: 0x00A5, ..blank_state() }; - assert_eq!(lores_odd_stop.words_per_row(false, 320), 9); + assert_eq!(lores_odd_stop.words_per_row(false, 320), 10); + + let lores_second_half_stop = RenderState { + ddfstrt: 0x0028, + ddfstop: 0x00D4, + ..blank_state() + }; + assert_eq!(lores_second_half_stop.words_per_row(false, 320), 23); let ecs_equal = RenderState { agnus_revision: AgnusRevision::Ecs8372Rev4, From 01d77d7480f2839e94d4734fd5aa4205cbd44bf4 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Tue, 23 Jun 2026 15:45:26 +0100 Subject: [PATCH 02/10] Fix COPCON byte writes setting CDANG Model COPCON byte writes as a mirrored 68000 byte value for the one-bit Agnus control latch. This lets byte bit operations such as bset #1,COPCON enable CDANG before Copper-driven blitter register moves. Add regression coverage for the byte-write path and document the COPCON/CDANG behavior in the timing internals. --- docs/internals/timing.md | 2 ++ src/bus.rs | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/docs/internals/timing.md b/docs/internals/timing.md index 0057cbc0..753b142e 100644 --- a/docs/internals/timing.md +++ b/docs/internals/timing.md @@ -200,6 +200,8 @@ wait for the blitter to go idle. `TODO.md` (`t=165s`/`t=180s` frame dumps). Copper writes to "dangerous" registers are gated by COPCON's CDANG bit. +COPCON is a one-bit Agnus control latch; byte writes use the mirrored 68000 +byte value, so a byte bit operation such as `bset #1,COPCON` sets CDANG. References: HRM [Coprocessor Hardware](https://www.theflatnet.de/pub/cbm/amiga/AmigaDevDocs/hard_2.html). diff --git a/src/bus.rs b/src/bus.rs index 169431bd..fd64d9d8 100644 --- a/src/bus.rs +++ b/src/bus.rs @@ -3584,7 +3584,9 @@ impl Bus { 1 => { let b = (val & 0xFF) as u16; let word_off = off & 0xFFE; - let word = if let Some(cur) = self.custom_byte_write_latch(word_off) { + let word = if custom_byte_write_mirrors_to_word(word_off) { + (b << 8) | b + } else if let Some(cur) = self.custom_byte_write_latch(word_off) { if off & 1 == 0 { (cur & 0x00FF) | (b << 8) } else { @@ -8198,6 +8200,10 @@ impl Bus { } } +fn custom_byte_write_mirrors_to_word(off: u16) -> bool { + matches!(off & 0xFFE, 0x02E) +} + fn bus_slots_for_cpu_access(size: usize) -> u32 { (size.max(1) as u32).div_ceil(2) } @@ -11126,6 +11132,13 @@ mod tests { let _ = bus.custom_write(0xDFF075, 1, 0x01); assert_eq!(bus.blitter.bltadat, 0x8001); + // COPCON's one-bit Agnus control latch sees the mirrored byte value, + // so a 68000 byte bit operation such as `bset #1,COPCON` enables CDANG. + let _ = bus.custom_write(0xDFF02E, 1, 0x02); + assert_eq!(bus.agnus.copcon, 0x0002); + let _ = bus.custom_write(0xDFF02E, 1, 0x00); + assert_eq!(bus.agnus.copcon, 0x0000); + // Audio registers do NOT use the addressed-lane latch: a real // 68000 drives a byte write onto both data-bus halves and Paula // latches the full word, so `move.b #v,AUDxPER/VOL` mirrors the From 54dd1abdcc2a1b3fdcfcbaa372e96b8a966f1087 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Tue, 23 Jun 2026 22:08:17 +0100 Subject: [PATCH 03/10] Mask DDF registers by Agnus revision, not fetch grid The DDF comparator precision is a property of the Agnus revision, not the display resolution. OCS keeps 4-CCK precision (mask 0xFC) on both DDFSTRT and DDFSTOP; ECS/AGA keep 2-CCK precision (mask 0xFE). Replace the prior model, which rounded lo-res DDFSTRT to the 8-CCK fetch-block grid while giving DDFSTOP 4-CCK precision, with a single revision-based register mask applied to both registers before the whole-fetch-unit completion rule runs. The grid-rounding error overcounted the row by one fetch unit on some lo-res OCS windows: a $34/$D4 window with a 44-byte pitch now fetches 21 words (was 23), so a blitter-drawn bottom border lands on the correct display rows instead of bleeding across the wrong ones. Thread the Agnus revision through the live collision and DDF placement helpers (LiveCollisionControl, live_fetch_origin_native_shift, the capture-side ddf helpers) so the masked comparator value is used consistently on both the DMA and shifter-origin sides. Update the chipset/video internals docs to describe the revision-based masking. --- docs/internals/chipset.md | 18 ++++---- docs/internals/video.md | 7 +-- src/bus.rs | 94 +++++++++++++++++++++++++++------------ src/chipset/agnus.rs | 31 +++++++------ src/video/bitplane.rs | 72 ++++++++++++++++++++---------- 5 files changed, 143 insertions(+), 79 deletions(-) diff --git a/docs/internals/chipset.md b/docs/internals/chipset.md index a93a8a0f..91155e3b 100644 --- a/docs/internals/chipset.md +++ b/docs/internals/chipset.md @@ -18,16 +18,14 @@ the DDFSTRT comparator, then each fetch block/unit uses the BPLCON0 value visible at that block's first cycle. A mid-row BPLCON0 plane-count change therefore cannot retroactively fetch earlier words, but it can add or remove planes for later blocks in the same row. The sequence completes -whole fetch units: DDF registers have finer -granularity than the unit in hi-res/super-hi-res and wide-FMODE modes, so -a DDFSTOP landing mid-unit extends the fetch through the unit starting -at-or-after it (`agnus::bitplane_fetch_blocks`; the CDTV trademark -screen's hi-res $64/$A8 window fetches 20 words per row, not the truncated -18). Lo-res OCS DDFSTRT is anchored on the 8-CCK fetch-block grid, but -DDFSTOP keeps its 4-CCK precision before the whole-unit completion rule is -applied: bit 2 of DDFSTOP can select the next low-res block, so a $4A/$B6 -window fetches 15 words per row, a $64/$A5 window fetches 10, and a -$28/$D4 window fetches 23. Wide-FMODE units (16/32 CCK) use the same rule +whole fetch units: the DDF register value is first masked to the Agnus +revision's comparator precision (OCS keeps 4-CCK precision; ECS/AGA keep +2-CCK precision), then a DDFSTOP landing mid-unit extends the fetch through +the unit starting at-or-after it (`agnus::bitplane_fetch_blocks`; the CDTV +trademark screen's hi-res $64/$A8 window fetches 20 words per row, not the +truncated 18). In lo-res OCS, bit 2 of DDFSTRT and DDFSTOP remains visible +to the 8-CCK fetch-unit count: $34/$D4 fetches 21 words, $28/$D4 fetches +23, and $4A/$B6 fetches 15. Wide-FMODE units (16/32 CCK) use the same rule rather than moving DDFSTRT down to an absolute grid. In lo-res, the plane-order slots for a wide unit are packed into the unit's first eight CCKs; the remaining CCKs are free for other bus users. If a diff --git a/docs/internals/video.md b/docs/internals/video.md index 8188ebcc..c12416cd 100644 --- a/docs/internals/video.md +++ b/docs/internals/video.md @@ -91,9 +91,10 @@ The mapping from beam coordinates to framebuffer x is anchored by constants that encode the hardware's fetch-to-display pipeline delays -- register writes, palette writes, and bitplane data each land at their own documented offset, and the bitplane fetch reference differs between lo-res -and hi-res. Wide-FMODE DMA fetches start from raw DDFSTRT and complete -whole units, but the displayed shifter origin is still quantized by the -FMODE fetch gulp; the renderer keeps those two effects separate. These +and hi-res. Wide-FMODE DMA fetches start from the revision-masked DDFSTRT +comparator value and complete whole units, but the displayed shifter origin +is still quantized by the FMODE fetch gulp; the renderer keeps those two +effects separate. These anchors were calibrated against real-hardware captures and other emulators; `COPPERLINE_HCENTER=0` and `COPPERLINE_OVERSCAN=full` help when re-checking them. diff --git a/src/bus.rs b/src/bus.rs index fd64d9d8..c72362a2 100644 --- a/src/bus.rs +++ b/src/bus.rs @@ -5077,6 +5077,7 @@ impl Bus { VIDEO_COLLISION_TIMING_SAMPLE_RATE, ); let current_control = LiveCollisionControl::from_current( + self.agnus.revision(), self.denise.bplcon0, self.denise.bplcon1, self.denise.bplcon3, @@ -5169,6 +5170,7 @@ impl Bus { VIDEO_COLLISION_TIMING_SAMPLE_RATE, ); let current_control = LiveCollisionControl::from_current( + self.agnus.revision(), self.denise.bplcon0, self.denise.bplcon1, self.denise.bplcon3, @@ -5273,6 +5275,7 @@ impl Bus { VIDEO_COLLISION_TIMING_SAMPLE_RATE, ); let current_control = LiveCollisionControl::from_current( + self.agnus.revision(), self.denise.bplcon0, self.denise.bplcon1, self.denise.bplcon3, @@ -5362,6 +5365,7 @@ impl Bus { self.ensure_current_collision_sprite_index(); let row = self.current_frame_bitplane_rows[fb_y].as_ref(); let current_control = LiveCollisionControl::from_current( + self.agnus.revision(), self.denise.bplcon0, self.denise.bplcon1, self.denise.bplcon3, @@ -5858,7 +5862,7 @@ impl Bus { fn record_ddfstrt_write_match_miss(&mut self, ddfstrt: u16) { let bplcon0 = self.effective_bitplane_bplcon0(); - let ddfstart = u32::from(effective_ddf_hpos(bplcon0, ddfstrt)); + let ddfstart = u32::from(effective_ddf_hpos(self.agnus.revision(), bplcon0, ddfstrt)); if ddfstart != 0 && ddfstart == self.agnus.hpos { self.bitplane_ddfstart_miss = Some(BitplaneDdfStartMiss { vpos: self.agnus.vpos, @@ -8070,6 +8074,7 @@ impl Bus { }; let native_samples_per_pixel = if bitplane_shres(bplcon0) { 2 } else { 1 }; let fetch_start_native_x = live_fetch_start_native_x( + self.agnus.revision(), bplcon0, self.denise.diwstrt, self.effective_diwhigh(), @@ -8307,10 +8312,6 @@ fn bitplane_hires(bplcon0: u16) -> bool { bplcon0 & 0x8000 != 0 && !bitplane_shres(bplcon0) } -fn bitplane_hires_like_ddf(bplcon0: u16) -> bool { - bitplane_hires(bplcon0) || bitplane_shres(bplcon0) -} - // Capture-side twins of the agnus fetch-mode helpers (see agnus.rs). fn bitplane_fetch_quantum(fmode: u16) -> u32 { match fmode & 0x0003 { @@ -8385,25 +8386,30 @@ fn next_chip_bus_quantum_at(hpos: u32, line_cck: u32) -> u32 { CHIP_BUS_SLOT_CCK.min(line_cck.saturating_sub(hpos).max(1)) } -fn effective_ddf_hpos(bplcon0: u16, raw: u16) -> u16 { - effective_ddf_start_hpos_raw(bplcon0, raw) +fn effective_ddf_hpos(revision: AgnusRevision, bplcon0: u16, raw: u16) -> u16 { + effective_ddf_start_hpos_raw(revision, bplcon0, raw) } -fn effective_ddf_start_hpos_raw(bplcon0: u16, raw: u16) -> u16 { - if bitplane_hires_like_ddf(bplcon0) { - raw & 0x00FC +fn ddf_register_mask(revision: AgnusRevision) -> u16 { + if matches!(revision, AgnusRevision::Ocs) { + 0x00FC } else { - raw & 0x00F8 + 0x00FE } } -fn effective_ddf_stop_hpos(bplcon0: u16, raw: u16) -> u16 { +fn effective_ddf_start_hpos_raw(revision: AgnusRevision, bplcon0: u16, raw: u16) -> u16 { let _ = bplcon0; - raw & 0x00FC + raw & ddf_register_mask(revision) } -fn effective_ddf_start_hpos(bplcon0: u16, raw: u16) -> u16 { - let start = effective_ddf_start_hpos_raw(bplcon0, raw); +fn effective_ddf_stop_hpos(revision: AgnusRevision, bplcon0: u16, raw: u16) -> u16 { + let _ = bplcon0; + raw & ddf_register_mask(revision) +} + +fn effective_ddf_start_hpos(revision: AgnusRevision, bplcon0: u16, raw: u16) -> u16 { + let start = effective_ddf_start_hpos_raw(revision, bplcon0, raw); if start == 0 { 0 } else { @@ -8419,8 +8425,8 @@ fn effective_ddf_window( harddis: bool, ) -> Option<(u16, u16)> { let (hard_start, hard_stop) = ddf_hard_bounds(harddis); - let start = effective_ddf_start_hpos_raw(bplcon0, ddfstrt); - let mut stop = effective_ddf_stop_hpos(bplcon0, ddfstop); + let start = effective_ddf_start_hpos_raw(revision, bplcon0, ddfstrt); + let mut stop = effective_ddf_stop_hpos(revision, bplcon0, ddfstop); if start == 0 || start > hard_stop { return None; } @@ -8816,6 +8822,7 @@ fn live_manual_sprite_playfield_collision_bits_in_range( let control = playfield_control.control_for_x(x); let Some(collision) = live_bitplane_collision_pixel_at( row, + control.agnus_revision, control.bplcon0, control.bplcon1, control.clxcon, @@ -9439,6 +9446,7 @@ fn live_bitplane_collision_bits_in_range( } let Some(collision) = live_bitplane_collision_pixel_at( row, + control.agnus_revision, control.bplcon0, control.bplcon1, control.clxcon, @@ -9468,6 +9476,7 @@ struct LivePlayfieldCollisionPixel { #[derive(Clone, Copy)] struct LiveCollisionControl { + agnus_revision: AgnusRevision, bplcon0: u16, bplcon1: u16, bplcon3: u16, @@ -9481,6 +9490,7 @@ struct LiveCollisionControl { impl LiveCollisionControl { fn from_current( + agnus_revision: AgnusRevision, bplcon0: u16, bplcon1: u16, bplcon3: u16, @@ -9492,6 +9502,7 @@ impl LiveCollisionControl { bpldat: [u16; 8], ) -> Self { Self { + agnus_revision, bplcon0, bplcon1, bplcon3, @@ -9506,6 +9517,7 @@ impl LiveCollisionControl { fn from_snapshot(snapshot: RenderRegisterSnapshot) -> Self { Self { + agnus_revision: snapshot.agnus_revision, bplcon0: snapshot.bplcon0, bplcon1: snapshot.bplcon1, bplcon3: snapshot.bplcon3, @@ -9723,6 +9735,7 @@ fn live_sprite_playfield_collision_bits_in_range( } let Some(collision) = live_bitplane_collision_pixel_at( row, + control.agnus_revision, control.bplcon0, control.bplcon1, control.clxcon, @@ -9986,6 +9999,7 @@ fn live_manual_bpl_word_collision_bits( fn live_bitplane_collision_pixel_at( row: &CapturedBitplaneRow, + agnus_revision: AgnusRevision, bplcon0: u16, bplcon1: u16, clxcon: u16, @@ -10011,10 +10025,11 @@ fn live_bitplane_collision_pixel_at( let native_samples_per_pixel = if shres { 2 } else { 1 }; let output_native_x = ((framebuffer_x - window_x_start) as usize / pixel_repeat) * native_samples_per_pixel; - let fetch_start_native_x = live_fetch_start_native_x(bplcon0, diwstrt, diwhigh, ddfstrt); + let fetch_start_native_x = + live_fetch_start_native_x(agnus_revision, bplcon0, diwstrt, diwhigh, ddfstrt); let relative_native_x = output_native_x.checked_sub(fetch_start_native_x)?; - let native_x = - relative_native_x + live_fetch_origin_native_offset(bplcon0, diwstrt, diwhigh, ddfstrt); + let native_x = relative_native_x + + live_fetch_origin_native_offset(agnus_revision, bplcon0, diwstrt, diwhigh, ddfstrt); let fetched_pixels = row.words_per_row * 16; // Collision sampling stays on the pre-AGA 6-plane decode until CLXCON2 / // 8-plane collisions land (plan 3.4). @@ -10115,20 +10130,30 @@ fn live_scroll_for_plane(bplcon1: u16, plane: usize) -> usize { } } -fn live_fetch_start_native_x(bplcon0: u16, diwstrt: u16, diwhigh: DiwHigh, ddfstrt: u16) -> usize { - (-live_fetch_origin_native_shift(bplcon0, diwstrt, diwhigh, ddfstrt)).max(0) as usize +fn live_fetch_start_native_x( + agnus_revision: AgnusRevision, + bplcon0: u16, + diwstrt: u16, + diwhigh: DiwHigh, + ddfstrt: u16, +) -> usize { + (-live_fetch_origin_native_shift(agnus_revision, bplcon0, diwstrt, diwhigh, ddfstrt)).max(0) + as usize } fn live_fetch_origin_native_offset( + agnus_revision: AgnusRevision, bplcon0: u16, diwstrt: u16, diwhigh: DiwHigh, ddfstrt: u16, ) -> usize { - live_fetch_origin_native_shift(bplcon0, diwstrt, diwhigh, ddfstrt).max(0) as usize + live_fetch_origin_native_shift(agnus_revision, bplcon0, diwstrt, diwhigh, ddfstrt).max(0) + as usize } fn live_fetch_origin_native_shift( + agnus_revision: AgnusRevision, bplcon0: u16, diwstrt: u16, diwhigh: DiwHigh, @@ -10154,8 +10179,9 @@ fn live_fetch_origin_native_shift( } else { 2 }; - let ddf_native_shift = - (effective_ddf_start_hpos(bplcon0, ddfstrt) as i32 - standard_ddf) * ddf_native_scale; + let ddf_native_shift = (effective_ddf_start_hpos(agnus_revision, bplcon0, ddfstrt) as i32 + - standard_ddf) + * ddf_native_scale; display_native_shift - ddf_native_shift } @@ -11197,16 +11223,20 @@ mod tests { bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x003C, 0x0040, false), bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0038, 0x0040, false) ); - // Low-res DDFSTRT resolves to the 8-CCK fetch block grid, but DDFSTOP - // keeps 4-CCK precision. A stop in the second half of a low-res block - // selects the next block, and the sequencer still completes it. + // OCS DDF registers keep 4-CCK precision. The low-res fetch + // sequencer still completes whole 8-CCK units, so DDFSTRT bit 2 can + // change the row length when DDFSTOP is aligned to the same unit. assert_eq!( bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x004A, 0x00B6, false), 15 ); assert_eq!( bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0064, 0x00A5, false), - 10 + 9 + ); + assert_eq!( + bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0034, 0x00D4, false), + 21 ); assert_eq!( bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0028, 0x00D4, false), @@ -16287,6 +16317,7 @@ mod tests { ], }; let control = LiveCollisionControl::from_current( + AgnusRevision::Ocs, 0x2400, 0, 0, @@ -16338,6 +16369,7 @@ mod tests { ], }; let control = LiveCollisionControl::from_current( + AgnusRevision::Ocs, 0x1000, 0, 0, @@ -16393,6 +16425,7 @@ mod tests { #[test] fn sprite_sprite_clxdat_waits_for_bpl1dat_display_enable() { let control = LiveCollisionControl::from_current( + AgnusRevision::Ocs, 0x1000, 0, 0, @@ -16465,6 +16498,7 @@ mod tests { #[test] fn live_sprite_sprite_clxdat_skips_already_latched_bits() { let control = LiveCollisionControl::from_current( + AgnusRevision::Ocs, 0x1000, 0, 0, @@ -16536,6 +16570,7 @@ mod tests { ], }; let control = LiveCollisionControl::from_current( + AgnusRevision::Ocs, 0x1000, 0, 0, @@ -16604,6 +16639,7 @@ mod tests { #[test] fn brdsprt_bypasses_bpl1dat_display_enable_for_live_sprite_clxdat() { let control = LiveCollisionControl::from_current( + AgnusRevision::Ocs, BPLCON0_ECSENA | 0x1000, 0, BPLCON3_BRDSPRT, diff --git a/src/chipset/agnus.rs b/src/chipset/agnus.rs index d7d1bbe0..134fb453 100644 --- a/src/chipset/agnus.rs +++ b/src/chipset/agnus.rs @@ -420,10 +420,6 @@ fn bitplane_hires(bplcon0: u16) -> bool { bplcon0 & 0x8000 != 0 && !bitplane_shres(bplcon0) } -fn bitplane_hires_like_ddf(bplcon0: u16) -> bool { - bitplane_hires(bplcon0) || bitplane_shres(bplcon0) -} - fn bitplane_fetch_cck_per_word(bplcon0: u16) -> u32 { if bitplane_shres(bplcon0) { 2 @@ -456,17 +452,22 @@ fn bitplane_fetch_unit(bplcon0: u16, fmode: u16) -> u32 { bitplane_fetch_period(bplcon0, fmode).max(8) } -pub fn effective_bitplane_ddf_start_hpos(bplcon0: u16, raw: u16) -> u16 { - if bitplane_hires_like_ddf(bplcon0) { - raw & 0x00FC +fn ddf_register_mask(revision: AgnusRevision) -> u16 { + if matches!(revision, AgnusRevision::Ocs) { + 0x00FC } else { - raw & 0x00F8 + 0x00FE } } -pub fn effective_bitplane_ddf_stop_hpos(bplcon0: u16, raw: u16) -> u16 { +pub fn effective_bitplane_ddf_start_hpos(revision: AgnusRevision, bplcon0: u16, raw: u16) -> u16 { let _ = bplcon0; - raw & 0x00FC + raw & ddf_register_mask(revision) +} + +pub fn effective_bitplane_ddf_stop_hpos(revision: AgnusRevision, bplcon0: u16, raw: u16) -> u16 { + let _ = bplcon0; + raw & ddf_register_mask(revision) } pub fn effective_bitplane_ddf_window( @@ -477,8 +478,8 @@ pub fn effective_bitplane_ddf_window( harddis: bool, ) -> Option<(u16, u16)> { let (hard_start, hard_stop) = ddf_hard_bounds(harddis); - let start = effective_bitplane_ddf_start_hpos(bplcon0, ddfstrt); - let mut stop = effective_bitplane_ddf_stop_hpos(bplcon0, ddfstop); + let start = effective_bitplane_ddf_start_hpos(revision, bplcon0, ddfstrt); + let mut stop = effective_bitplane_ddf_stop_hpos(revision, bplcon0, ddfstop); if start == 0 || start > hard_stop { return None; } @@ -1738,7 +1739,11 @@ mod tests { ); assert_eq!( bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0064, 0x00A5, false), - 10 + 9 + ); + assert_eq!( + bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0034, 0x00D4, false), + 21 ); assert_eq!( bitplane_words_per_row(AgnusRevision::Ocs, 0x0000, 0, 0x0028, 0x00D4, false), diff --git a/src/video/bitplane.rs b/src/video/bitplane.rs index fa316cf9..f14ac9c6 100644 --- a/src/video/bitplane.rs +++ b/src/video/bitplane.rs @@ -735,8 +735,8 @@ impl ControlState { fn has_valid_ddf_window(&self) -> bool { let hires_like = self.hires() || self.shres(); - let start = effective_ddf_start_hpos_raw(hires_like, self.ddfstrt); - let stop = effective_ddf_stop_hpos(hires_like, self.ddfstop); + let start = effective_ddf_start_hpos_raw(self.agnus_revision, hires_like, self.ddfstrt); + let stop = effective_ddf_stop_hpos(self.agnus_revision, hires_like, self.ddfstop); (start == 0 && stop == 0) || effective_ddf_window( self.agnus_revision, @@ -776,14 +776,16 @@ impl ControlState { } // The displayed shifter origin moves in whole fetch gulps, matching // the renderer's placement (see `fetch_origin_native_shift`). This is - // separate from the DMA slot positions, which start at raw DDFSTRT. - // Each colour clock of DDF shift moves the picture two lo-res H units. + // separate from the DMA slot positions, which start at the + // revision-masked DDFSTRT comparator value. Each colour clock of DDF + // shift moves the picture two lo-res H units. let gulp = self.fetch_period() as i32; let align = |hpos: i32| -> i32 { (hpos.div_euclid(gulp) * gulp).max(BITPLANE_DDF_HARD_START as i32) }; let standard_ddf = if hires_like { 0x003C } else { 0x0038 }; - let aligned_start = align(effective_ddf_start_hpos(hires_like, self.ddfstrt) as i32); + let aligned_start = + align(effective_ddf_start_hpos(self.agnus_revision, hires_like, self.ddfstrt) as i32); let start_h = STANDARD_DIW_HSTART + (aligned_start - align(standard_ddf)) * 2; // Fetched H width: one word spans 16 lo-res, 8 hi-res, or 4 super-hi-res // H units, so the standard 20/40/80-word row is 320 H units wide. @@ -823,9 +825,10 @@ impl ControlState { }; // The displayed picture position is quantized to the fetch-period // grid (one FMODE gulp per plane). The DMA sequencer itself starts at - // raw DDFSTRT, but the shifter consumes data in whole 1/2/4-word - // gulps, so a DDFSTRT moved within one gulp changes how much tail data - // is fetched without necessarily moving the visible picture. With + // the revision-masked DDFSTRT comparator value, but the shifter + // consumes data in whole 1/2/4-word gulps, so a DDFSTRT moved within + // one gulp changes how much tail data is fetched without necessarily + // moving the visible picture. With // FMODE=0 the gulp equals the DDF granularity and nothing changes // (boot-screen insert-disk art is drawn for the continuous placement: // its negative modulos overlap rows so the hand/disk's right edge @@ -843,10 +846,13 @@ impl ControlState { // fetch position is not visible. (hpos.div_euclid(gulp) * gulp).max(BITPLANE_DDF_HARD_START as i32) }; - let ddf_native_shift = - (align(effective_ddf_start_hpos(self.hires() || self.shres(), self.ddfstrt) as i32) - - align(standard_ddf)) - * ddf_native_scale; + let ddf_native_shift = (align(effective_ddf_start_hpos( + self.agnus_revision, + self.hires() || self.shres(), + self.ddfstrt, + ) as i32) + - align(standard_ddf)) + * ddf_native_scale; // The render loop measures output pixels from the CLAMPED display // window start: the framebuffer cannot show anything left of // DIW_HSTART_FB0, so a window programmed further left (extreme @@ -859,7 +865,11 @@ impl ControlState { let clamped_window_native = ((DIW_HSTART_FB0 - diw_h_start as i32).max(0) * 2) / pixel_repeat as i32; let mut origin_shift = display_native_shift - ddf_native_shift + clamped_window_native; - let ddf_start = effective_ddf_start_hpos(self.hires() || self.shres(), self.ddfstrt); + let ddf_start = effective_ddf_start_hpos( + self.agnus_revision, + self.hires() || self.shres(), + self.ddfstrt, + ); if !self.hires() && !self.shres() && self.fetch_quantum() == 1 @@ -2402,6 +2412,7 @@ fn native_frame_width_for_control(control: ControlState) -> usize { fn bitplane_fetch_hpos_for_plane(control: ControlState, word_idx: usize, plane: usize) -> u32 { let start = u32::from(effective_ddf_start_hpos( + control.agnus_revision, control.hires() || control.shres(), control.ddfstrt, )); @@ -2478,6 +2489,7 @@ fn line_fetch_plan_for_word( let mut plan = LineFetchPlan::empty(); if control_segments.is_empty() { let start = u32::from(effective_ddf_start_hpos( + base_control.agnus_revision, base_control.hires() || base_control.shres(), base_control.ddfstrt, )); @@ -6206,21 +6218,26 @@ fn read_chip_word_wrapping(ram: &[u8], addr: u32) -> u16 { u16::from_be_bytes([ram[a], ram[(a + 1) & mask]]) } -fn effective_ddf_start_hpos_raw(hires: bool, raw: u16) -> u16 { - if hires { - raw & 0x00FC +fn ddf_register_mask(revision: AgnusRevision) -> u16 { + if matches!(revision, AgnusRevision::Ocs) { + 0x00FC } else { - raw & 0x00F8 + 0x00FE } } -fn effective_ddf_stop_hpos(hires: bool, raw: u16) -> u16 { +fn effective_ddf_start_hpos_raw(revision: AgnusRevision, hires: bool, raw: u16) -> u16 { let _ = hires; - raw & 0x00FC + raw & ddf_register_mask(revision) } -fn effective_ddf_start_hpos(hires: bool, raw: u16) -> u16 { - let start = effective_ddf_start_hpos_raw(hires, raw); +fn effective_ddf_stop_hpos(revision: AgnusRevision, hires: bool, raw: u16) -> u16 { + let _ = hires; + raw & ddf_register_mask(revision) +} + +fn effective_ddf_start_hpos(revision: AgnusRevision, hires: bool, raw: u16) -> u16 { + let start = effective_ddf_start_hpos_raw(revision, hires, raw); if start == 0 { 0 } else { @@ -6236,8 +6253,8 @@ fn effective_ddf_window( harddis: bool, ) -> Option<(u16, u16)> { let (hard_start, hard_stop) = ddf_hard_bounds(harddis); - let start = effective_ddf_start_hpos_raw(hires, ddfstrt); - let mut stop = effective_ddf_stop_hpos(hires, ddfstop); + let start = effective_ddf_start_hpos_raw(revision, hires, ddfstrt); + let mut stop = effective_ddf_stop_hpos(revision, hires, ddfstop); if start == 0 || start > hard_stop { return None; } @@ -7845,7 +7862,14 @@ mod tests { ddfstop: 0x00A5, ..blank_state() }; - assert_eq!(lores_odd_stop.words_per_row(false, 320), 10); + assert_eq!(lores_odd_stop.words_per_row(false, 320), 9); + + let lores_four_cck_start = RenderState { + ddfstrt: 0x0034, + ddfstop: 0x00D4, + ..blank_state() + }; + assert_eq!(lores_four_cck_start.words_per_row(false, 320), 21); let lores_second_half_stop = RenderState { ddfstrt: 0x0028, From f1db1727fb708aa978acf17b4b68ec4f2b50fe0f Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 24 Jun 2026 07:33:16 +0100 Subject: [PATCH 04/10] Fix copper palette and manual sprite timing Adjust the modeled COLORxx output phase so same-line copper palette writes line up with Denise's final colour output timing. This removes the stray right-edge colour cell and restores the expected low-res copper colour-cell placement. Treat direct SPRxDATA writes as manually armed sprite data independent of the DMA descriptor vertical window, while preserving the empty-window behavior for latched DMA sprite state. This models copper-fed sprite data used after bitplane DMA has overwritten the sprite descriptor stream. Update timing documentation and regression tests for the new palette phase and direct sprite-data behavior. --- docs/internals/timing.md | 4 +- src/bus.rs | 8 ++-- src/video/bitplane.rs | 93 +++++++++++++++++++++++++++++----------- 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/docs/internals/timing.md b/docs/internals/timing.md index 753b142e..1e1ee406 100644 --- a/docs/internals/timing.md +++ b/docs/internals/timing.md @@ -141,12 +141,12 @@ shared by the live bus path and the blitter-deadline predictor's cloned simulation, so prediction and execution cannot drift apart. For the OCS low-res renderer, a same-line `COLORxx` write at beam `hpos` -starts affecting pixels at `(hpos - $34) * 4` (`COLOR_WRITE_HPOS_FB0` in +starts affecting pixels at `(hpos - $38) * 4` (`COLOR_WRITE_HPOS_FB0` in `src/video/bitplane.rs`); beam-timed placement is anchored at `COPPER_WAIT_HPOS_FB0` ($28), and bitplane-control writes add the fetch-to-display pipeline offset (`BITPLANE_CONTROL_PIPELINE_FB`). AGA BPLCON4's sprite palette-base byte uses Lisa's earlier sprite colour-lookup -path at `(hpos - $36) * 4` (`SPRITE_PALETTE_CONTROL_HPOS_FB0`). Because +path at `(hpos - $3A) * 4` (`SPRITE_PALETTE_CONTROL_HPOS_FB0`). Because colour MOVEs are spaced four colour clocks apart, a gradient changes colour every 8 lores pixels, matching hardware rather than the previous (too-fine) 4-pixel spacing. Tests: diff --git a/src/bus.rs b/src/bus.rs index c72362a2..42467688 100644 --- a/src/bus.rs +++ b/src/bus.rs @@ -10330,7 +10330,7 @@ mod tests { const STANDARD_DIW_HSTART: i32 = 0x81; const STANDARD_VISIBLE_X0: usize = ((STANDARD_DIW_HSTART - RENDER_DIW_HSTART_FB0) * 2) as usize; - const RENDER_COLOR_WRITE_HPOS_FB0: u32 = 0x34; + const RENDER_COLOR_WRITE_HPOS_FB0: u32 = 0x38; static BUS_TEMP_COUNTER: AtomicU64 = AtomicU64::new(0); #[test] @@ -12261,14 +12261,14 @@ mod tests { &mut bus, 0x0100, RENDER_VISIBLE_START_VPOS, - RENDER_COPPER_WAIT_HPOS_FB0 + 30, + RENDER_COPPER_WAIT_HPOS_FB0 + 34, &[(0x0182, 0x00F0)], ); let event_hpos = bus.current_render_events()[0].hpos; // MOVE write lands on its second-word fetch, two color clocks into the - // 4-color-clock cadence from the start hpos (+30). - assert_eq!(event_hpos, RENDER_COPPER_WAIT_HPOS_FB0 + 32); + // 4-color-clock cadence from the start hpos (+34). + assert_eq!(event_hpos, RENDER_COPPER_WAIT_HPOS_FB0 + 36); let words_per_row = bitplane_words_per_row( bus.agnus.revision(), bus.denise.bplcon0, diff --git a/src/video/bitplane.rs b/src/video/bitplane.rs index f14ac9c6..dabdda06 100644 --- a/src/video/bitplane.rs +++ b/src/video/bitplane.rs @@ -58,14 +58,15 @@ const DIW_HSTART_FETCH_REFERENCE_HIRES: i32 = 0x83; const COPPER_WAIT_HPOS_FB0: i32 = 0x28; /// COLORxx writes feed Denise's final colour-selection/output path, not the /// upstream bitplane shifter. Treat the left output edge as the first visible -/// colour-register position rather than the earlier bitplane-control domain. -/// Moved left by 8 colour clocks alongside the other origin anchors. -const COLOR_WRITE_HPOS_FB0: i32 = 0x34; +/// colour-register position rather than the earlier bitplane-control domain; +/// in low-res copper gradients, writes four colour clocks apart advance by one +/// 16-pixel colour cell. +const COLOR_WRITE_HPOS_FB0: i32 = 0x38; /// AGA BPLCON4's low sprite-palette byte follows Lisa's sprite colour lookup /// path, which reaches sprite output earlier than ordinary COLORxx palette /// writes. Keep it separate from COLOR replay so copper palette gradients stay /// in the Denise palette-output phase on OCS/ECS. -const SPRITE_PALETTE_CONTROL_HPOS_FB0: i32 = 0x36; +const SPRITE_PALETTE_CONTROL_HPOS_FB0: i32 = 0x3A; /// SPRxPOS writes update the Denise horizontal comparator seven CCK ahead of /// the normal register/output beam domain. Manual sprite replays use this /// earlier domain so adjacent position writes can abut at their programmed @@ -1508,6 +1509,7 @@ struct BeamSpriteState { sprdata: [u16; 8], sprdatb: [u16; 8], spr_armed: [bool; 8], + direct_data_armed: [bool; 8], /// Lisa only: FMODE SPR32/SPAGEM widen manual sprites too. A CPU/Copper /// SPRxDATA/SPRxDATB write loads the same 16-bit value into every word /// of the wide holding register, so a manual wide sprite repeats its @@ -1550,6 +1552,7 @@ impl BeamSpriteState { sprdata: state.sprdata, sprdatb: state.sprdatb, spr_armed, + direct_data_armed: [false; 8], aga: matches!(state.agnus_revision, AgnusRevision::AgaAlice), fmode: state.fmode, held: *held, @@ -1572,10 +1575,12 @@ impl BeamSpriteState { 0x2 => { self.sprctl[idx] = val; self.spr_armed[idx] = false; + self.direct_data_armed[idx] = false; } 0x4 => { self.sprdata[idx] = val; self.spr_armed[idx] = true; + self.direct_data_armed[idx] = true; } 0x6 => self.sprdatb[idx] = val, _ => {} @@ -1615,19 +1620,21 @@ impl BeamSpriteState { x_stop, }); } - let vstart = sprite_vstart(pos, ctl); - let vstop = sprite_vstop(ctl); - // Normal pair: [vstart, vstop). Equal start/stop is an empty window; - // only a strictly inverted pair wraps through the frame boundary. - let in_window = if vstop == vstart { - false - } else if vstop > vstart { - beam_y >= vstart && beam_y < vstop - } else { - beam_y >= vstart || beam_y < vstop - }; - if !in_window { - return None; + if !self.direct_data_armed[sprite] { + let vstart = sprite_vstart(pos, ctl); + let vstop = sprite_vstop(ctl); + // Normal pair: [vstart, vstop). Equal start/stop is an empty window; + // only a strictly inverted pair wraps through the frame boundary. + let in_window = if vstop == vstart { + false + } else if vstop > vstart { + beam_y >= vstart && beam_y < vstop + } else { + beam_y >= vstart || beam_y < vstop + }; + if !in_window { + return None; + } } let width_words = if self.aga { sprite_width_words_from_fmode(self.fmode) @@ -7337,7 +7344,7 @@ mod tests { ); assert_eq!( beam_to_framebuffer_x_unclamped(COLOR_WRITE_HPOS_FB0 as u32), - 48 + 64 ); assert_eq!( sprite_palette_control_framebuffer_x(SPRITE_PALETTE_CONTROL_HPOS_FB0 as u32), @@ -8616,24 +8623,58 @@ mod tests { } #[test] - fn manual_sprite_vstart_equal_vstop_is_empty() { + fn latched_sprite_vstart_equal_vstop_is_empty() { let mut initial_state = blank_state(); let beam_y = PAL_VISIBLE_LINE0; let (pos, ctl) = sprite_control_words(beam_y as u16, beam_y as u16, DIW_HSTART_FB0 as u16); initial_state.sprpos[0] = pos; initial_state.sprctl[0] = ctl; + initial_state.sprdata[0] = 0xFFFF; + initial_state.spr_armed[0] = true; - let events = [cpu_event( - beam_y as u32, - COPPER_WAIT_HPOS_FB0 as u32, - 0x144, - 0xFFFF, - )]; - let manual_sprite_lines = manual_sprite_lines_from_events(&initial_state, &events); + let manual_sprite_lines = manual_sprite_lines_from_events_with_visible_line0( + &initial_state, + &[], + &[None; 8], + PAL_VISIBLE_LINE0, + FB_HEIGHT, + true, + ); assert!(manual_sprite_lines[0].is_empty()); } + #[test] + fn direct_sprite_data_write_ignores_dma_vertical_window() { + let mut initial_state = blank_state(); + let beam_y = PAL_VISIBLE_LINE0 + 12; + let (pos, ctl) = sprite_control_words( + PAL_VISIBLE_LINE0 as u16, + PAL_VISIBLE_LINE0 as u16, + DIW_HSTART_FB0 as u16, + ); + initial_state.sprpos[0] = pos; + initial_state.sprctl[0] = ctl; + + let manual_sprite_lines = manual_sprite_lines_from_events_with_visible_line0( + &initial_state, + &[cpu_event( + beam_y as u32, + COPPER_WAIT_HPOS_FB0 as u32, + 0x144, + 0xFFFF, + )], + &[None; 8], + PAL_VISIBLE_LINE0, + FB_HEIGHT, + false, + ); + + assert!(manual_sprite_lines[0] + .iter() + .any(|line| line.beam_y == beam_y)); + } + #[test] fn manual_sprite_data_write_replicates_words_across_fmode_wide_register() { let mut initial_state = blank_state(); From cf442ee1b875d7c0086b1dd62a9e810a5c3cd615 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 24 Jun 2026 08:14:16 +0100 Subject: [PATCH 05/10] Fix same-line DIW start display origin Compute playfield line bounds from the control spans that are actually active across the scanline instead of always unioning the stale base DIW window. This keeps an already-reached DIW start as the shifter origin for same-line clipping, while allowing a DIWSTRT write at x=0 to replace the previous line origin before the comparator fires. Add a regression for line-start DIWSTRT rewrites so narrowed display windows no longer render a stale horizontal slice from the previous start position. --- src/video/bitplane.rs | 95 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 79 insertions(+), 16 deletions(-) diff --git a/src/video/bitplane.rs b/src/video/bitplane.rs index dabdda06..3a8220b8 100644 --- a/src/video/bitplane.rs +++ b/src/video/bitplane.rs @@ -2330,26 +2330,67 @@ fn line_display_window_bounds( visible_line0: i32, ) -> Option<(usize, usize)> { let mut bounds = None; - if base_control.display_window_contains_line(line, visible_line0) { - bounds = Some(base_control.display_window_x()); - } + let mut control = base_control; + let mut run_start = 0usize; for segment in control_segments { - if !segment - .control - .display_window_contains_line(line, visible_line0) - { - continue; - } - let (segment_x_start, segment_x_stop) = segment.control.display_window_x(); - match &mut bounds { - Some((x_start, x_stop)) => { - *x_start = (*x_start).min(segment_x_start); - *x_stop = (*x_stop).max(segment_x_stop); + let run_stop = segment.x.min(FB_WIDTH); + merge_display_window_run_bounds( + &mut bounds, + control, + line, + visible_line0, + run_start, + run_stop, + ); + control = segment.control; + run_start = run_stop; + } + merge_display_window_run_bounds( + &mut bounds, + control, + line, + visible_line0, + run_start, + FB_WIDTH, + ); + bounds.filter(|(x_start, x_stop)| x_start < x_stop) +} + +fn merge_display_window_run_bounds( + bounds: &mut Option<(usize, usize)>, + control: ControlState, + line: usize, + visible_line0: i32, + run_start: usize, + run_stop: usize, +) { + if run_start >= run_stop || !control.display_window_contains_line(line, visible_line0) { + return; + } + let (window_x_start, window_x_stop) = control.display_window_x(); + if window_x_start >= run_start && window_x_start <= run_stop { + // The horizontal DIW start comparator has fired while this control was + // active, so it establishes the shifter origin even if a same-line + // write clips all visible pixels before the next control run. + match bounds { + Some((bounds_start, _)) => { + *bounds_start = (*bounds_start).min(window_x_start); } - None => bounds = Some((segment_x_start, segment_x_stop)), + None => *bounds = Some((window_x_start, window_x_start)), } } - bounds.filter(|(x_start, x_stop)| x_start < x_stop) + let x_start = window_x_start.max(run_start); + let x_stop = window_x_stop.min(run_stop); + if x_start >= x_stop { + return; + } + match bounds { + Some((bounds_start, bounds_stop)) => { + *bounds_start = (*bounds_start).min(x_start); + *bounds_stop = (*bounds_stop).max(x_stop); + } + None => *bounds = Some((x_start, x_stop)), + } } fn line_max_display_planes( @@ -7306,6 +7347,28 @@ mod tests { assert_eq!(state.diw_v_stop(), 0x12D); } + #[test] + fn line_start_diw_write_replaces_previous_horizontal_display_bounds() { + let base = ControlState { + diwstrt: ((PAL_VISIBLE_LINE0 as u16) << 8) | STANDARD_DIW_HSTART as u16, + diwstop: (((PAL_VISIBLE_LINE0 + 1) as u16) << 8) | 0x00C1, + ..ControlState::default() + }; + let narrowed = ControlState { + diwstrt: ((PAL_VISIBLE_LINE0 as u16) << 8) | 0x00A0, + ..base + }; + let segments = [ControlSegment { + x: 0, + control: narrowed, + }]; + + assert_eq!( + line_display_window_bounds(base, &segments, 0, PAL_VISIBLE_LINE0), + Some(narrowed.display_window_x()) + ); + } + #[test] fn beam_position_converts_to_line_and_segment_x() { let line = PAL_VISIBLE_LINE0 + 25; From af694253e9334c88587ca600574aafb5e43681d4 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 24 Jun 2026 09:27:06 +0100 Subject: [PATCH 06/10] Let BPL1DAT enable sprites outside vertical DIW Treat the per-scanline BPL1DAT display-enable latch as sufficient to make OCS/ECS sprites visible vertically, while still applying horizontal DIW clipping and the first-BPL1DAT x gate. Apply the same rule to live sprite collision replay so visual output and CLXDAT agree when software manually feeds BPL1DAT on lines without bitplane DMA. Add renderer and live-collision regressions for manual BPL1DAT enabling sprites on a vertically closed DIW line. --- src/bus.rs | 83 +++++++++++++++++++++++++++++------- src/video/bitplane.rs | 98 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 162 insertions(+), 19 deletions(-) diff --git a/src/bus.rs b/src/bus.rs index 42467688..0b5e8c27 100644 --- a/src/bus.rs +++ b/src/bus.rs @@ -9351,16 +9351,12 @@ fn live_sprite_visible_x_range_for_control( return Some((x_start, x_stop)); } let display_enable_x = display_enable_x?; - if beam_y < 0 - || !display_window_contains_vpos( - control.diwstrt, - control.diwstop, - control.diwhigh, - beam_y as u32, - ) - { + if beam_y < 0 { return None; } + // A manual BPL1DAT write records `display_enable_x` for this scanline; + // that is enough to make OCS/ECS sprites active vertically, while DIW + // still clips the horizontal span. let (window_x_start, window_x_stop) = live_display_window_x(control.diwstrt, control.diwstop, control.diwhigh); let x_start = x_start.max(display_enable_x).max(window_x_start); @@ -9387,12 +9383,10 @@ fn live_sprite_pixel_inside_display_window( if display_enable_x.is_none_or(|enable_x| framebuffer_x < enable_x) { return false; } - display_window_contains_vpos( - control.diwstrt, - control.diwstop, - control.diwhigh, - beam_y as u32, - ) && live_display_window_contains_x( + // See `live_sprite_visible_x_range_for_control`: display_enable_x carries + // the per-line BPL1DAT/DMA latch, so do not apply a separate DIW vertical + // test here. + live_display_window_contains_x( control.diwstrt, control.diwstop, control.diwhigh, @@ -16685,6 +16679,67 @@ mod tests { ); } + #[test] + fn manual_bpl1dat_display_enable_allows_live_sprite_clxdat_on_vertically_closed_diw_line() { + let control = LiveCollisionControl::from_current( + AgnusRevision::Ocs, + 0x1000, + 0, + 0, + 0, + ((RENDER_VISIBLE_START_VPOS as u16 + 10) << 8) | RENDER_DIW_HSTART_FB0 as u16, + ((RENDER_VISIBLE_START_VPOS as u16 + 20) << 8) | 0x00C1, + DiwHigh::ocs_implicit(), + 0x0038, + [0; 8], + ); + let replay = LiveCollisionLineReplay { + line_start: control, + segments: Vec::new(), + }; + let sources = [ + LiveSpriteCollisionSource { + group: 0, + hstart: RENDER_DIW_HSTART_FB0, + hsub_70ns: false, + words: [0x8000, 0, 0, 0], + requires_odd_enable: false, + }, + LiveSpriteCollisionSource { + group: 1, + hstart: RENDER_DIW_HSTART_FB0, + hsub_70ns: false, + words: [0x8000, 0, 0, 0], + requires_odd_enable: false, + }, + ]; + + assert_eq!( + live_sprite_sprite_collision_bits( + &sources, + &replay, + RENDER_VISIBLE_START_VPOS as i32, + 0, + 2, + None, + 0, + ) & (1 << 9), + 0 + ); + assert_eq!( + live_sprite_sprite_collision_bits( + &sources, + &replay, + RENDER_VISIBLE_START_VPOS as i32, + 0, + 2, + Some(0), + 0, + ) & (1 << 9), + 1 << 9 + ); + } + #[test] fn bpldat_writes_update_latched_planes_for_live_playfield_clxdat() { let clxdat_after_row_capture = |bpldat_hpos: Option| { diff --git a/src/video/bitplane.rs b/src/video/bitplane.rs index 3a8220b8..f3420961 100644 --- a/src/video/bitplane.rs +++ b/src/video/bitplane.rs @@ -6118,20 +6118,25 @@ fn sprite_display_enable_x_for_y( fn sprite_pixel_inside_display_window( control: ControlState, - y: usize, + _y: usize, x: usize, - visible_line0: i32, + _visible_line0: i32, display_enable_x: Option, ) -> bool { if control.border_sprite_enabled() { return true; } - if display_enable_x.is_none_or(|enable_x| x < enable_x) { + let Some(enable_x) = display_enable_x else { return false; - } - if !control.display_window_contains_line(y, visible_line0) { + }; + if x < enable_x { return false; } + // OCS/ECS Denise starts sprite display for this scanline after the first + // BPL1DAT load, whether that load came from bitplane DMA or a manual + // copper/CPU write. DIW still clips horizontally, but a manual BPL1DAT + // write can make sprites visible on lines where the vertical bitplane + // window is closed. let (x_start, x_stop) = control.display_window_x(); x >= x_start && x < x_stop } @@ -9699,6 +9704,89 @@ mod tests { assert_eq!(fb[4], rgb12_to_rgba8(0x0F00)); } + #[test] + fn manual_bpl1dat_display_enable_allows_sprites_on_vertically_closed_diw_line() { + let mut state = RenderState { + dmacon: DMACON_DMAEN | DMACON_SPREN, + diwstrt: (((PAL_VISIBLE_LINE0 + 10) as u16) << 8) | DIW_HSTART_FB0 as u16, + diwstop: (((PAL_VISIBLE_LINE0 + 20) as u16) << 8) | 0x00C1, + ..blank_state() + }; + state.palette.write_ocs(17, 0x0F00); + let ram = vec![0; 64]; + let base_palettes = [state.palette; FB_HEIGHT]; + let palette_segments = vec![Vec::new(); FB_HEIGHT]; + let base_controls = [ControlState::from_render_state(&state); FB_HEIGHT]; + let control_segments = vec![Vec::new(); FB_HEIGHT]; + let playfield_mask = vec![0u8; FB_PIXELS]; + let mut collision_pixels = vec![CollisionPixel::default(); FB_PIXELS]; + let captured = [CapturedSpriteLine { + sprite: 0, + hstart: DIW_HSTART_FB0, + hsub_70ns: false, + beam_y: PAL_VISIBLE_LINE0, + data: 0x8000, + datb: 0, + attached: false, + data_ext: [0; 3], + datb_ext: [0; 3], + width_words: 1, + }]; + + let mut fb = vec![rgb12_to_rgba8(0); FB_PIXELS]; + let display_disabled = [None; FB_HEIGHT]; + render_sprites_with_manual_lines_and_writes( + &state, + &ram, + &mut fb, + SpriteClip { + x_start: 0, + x_stop: FB_WIDTH, + y_start: 0, + y_stop: FB_HEIGHT, + }, + &base_palettes, + &palette_segments, + &base_controls, + &control_segments, + &display_disabled, + &playfield_mask, + &mut collision_pixels, + &captured, + true, + None, + PAL_VISIBLE_LINE0, + ); + assert_eq!(fb[0], rgb12_to_rgba8(0)); + + let mut fb = vec![rgb12_to_rgba8(0); FB_PIXELS]; + let mut display_enabled = [None; FB_HEIGHT]; + display_enabled[0] = Some(0); + render_sprites_with_manual_lines_and_writes( + &state, + &ram, + &mut fb, + SpriteClip { + x_start: 0, + x_stop: FB_WIDTH, + y_start: 0, + y_stop: FB_HEIGHT, + }, + &base_palettes, + &palette_segments, + &base_controls, + &control_segments, + &display_enabled, + &playfield_mask, + &mut collision_pixels, + &captured, + true, + None, + PAL_VISIBLE_LINE0, + ); + assert_eq!(fb[0], rgb12_to_rgba8(0x0F00)); + } + #[test] fn brdsprt_bypasses_first_bpl1dat_display_enable_gate() { let mut state = RenderState { From 85c0126af61622c6d384f70967badec5f8eca1aa Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 24 Jun 2026 10:11:24 +0100 Subject: [PATCH 07/10] Beam-time visible CPU palette writes Treat any CPU COLOR register write after the visible frame start as a beam-timed Denise palette change instead of requiring a large palette block before replaying by position. This keeps late-frame palette preparation from recolouring already-displayed bitplane data, and adds regressions for single and 30-entry CPU palette updates while preserving pre-visible writes as frame-start state. --- src/video/bitplane.rs | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/video/bitplane.rs b/src/video/bitplane.rs index f3420961..dd2839c3 100644 --- a/src/video/bitplane.rs +++ b/src/video/bitplane.rs @@ -2146,19 +2146,11 @@ fn apply_render_events_and_collect_display_plan_events_with_visible_line0( } fn cpu_palette_writes_are_beam_timed(events: &[BeamRegisterWrite], visible_line0: i32) -> bool { - let mut visible_cpu_palette_writes = 0usize; - for event in events { - if matches!(event.source, BeamWriteSource::Cpu) + events.iter().any(|event| { + matches!(event.source, BeamWriteSource::Cpu) && matches!(event.offset & 0x01FE, 0x180..=0x1BE) && (event.vpos as i32) > visible_line0 - { - visible_cpu_palette_writes += 1; - if visible_cpu_palette_writes >= 64 { - return true; - } - } - } - false + }) } #[cfg_attr(not(test), allow(dead_code))] @@ -7781,7 +7773,7 @@ mod tests { } #[test] - fn ordinary_visible_cpu_palette_writes_seed_frame_base_without_beam_segments() { + fn visible_cpu_palette_write_replays_by_beam_position() { let mut state = blank_state(); let mut base_palettes = [state.palette; FB_HEIGHT]; let mut palette_segments = vec![Vec::new(); FB_HEIGHT]; @@ -7801,14 +7793,18 @@ mod tests { ); let line = (0x45 - 0x2C) as usize; - assert_eq!(base_palettes[0][0], 0x0FFF); - assert_eq!(base_palettes[line][0], 0x0FFF); - assert!(palette_segments[line].is_empty()); + assert_eq!(base_palettes[0][0], 0x0103); + assert_eq!(base_palettes[line][0], 0x0103); + assert_eq!(base_palettes[line + 1][0], 0x0FFF); + assert_eq!(palette_segments[line].len(), 1); + assert_eq!(palette_segments[line][0].x, 0); + assert_eq!(palette_segments[line][0].entry, 0); + assert_eq!(palette_segments[line][0].value, 0x0FFF); assert_eq!(state.palette[0], 0x0FFF); } #[test] - fn dense_visible_cpu_palette_writes_replay_by_beam_position() { + fn small_visible_cpu_palette_batch_replays_by_beam_position() { let mut state = blank_state(); let mut base_palettes = [state.palette; FB_HEIGHT]; let mut palette_segments = vec![Vec::new(); FB_HEIGHT]; @@ -7816,7 +7812,7 @@ mod tests { let mut control_segments = vec![Vec::new(); FB_HEIGHT]; let mut manual_bpl_segments = Vec::new(); let mut events = Vec::new(); - for idx in 0..64 { + for idx in 0..30 { events.push(cpu_event( 0x45 + idx as u32, COPPER_WAIT_HPOS_FB0 as u32, From 44b5749f7398bc99d3a37f91589a20a97b5f5a8a Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 24 Jun 2026 11:07:50 +0100 Subject: [PATCH 08/10] Fix normal blitter shift start carry Reset normal-mode A/B shift carry at the first word of each new BLTSIZE while keeping carry across words and rows within the same blit. This matches the hardware rule that the first shifted source word zero-fills, and prevents stale A-channel latch bits from being ORed into newly drawn text rows. Update the focused blitter regression and internals timing note to describe the per-blit zero-fill boundary. --- docs/internals/timing.md | 18 +++++++++--------- src/chipset/blitter.rs | 35 +++++++++++++++++++---------------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/docs/internals/timing.md b/docs/internals/timing.md index 1e1ee406..114f4c8d 100644 --- a/docs/internals/timing.md +++ b/docs/internals/timing.md @@ -227,18 +227,18 @@ state exists. D output is delayed through the hold register: after source fetches, the first D phase is the HRM "-" bubble and does not claim the chip bus because no destination word is queued yet. The first destination word is written on the next D slot and the final word in the F flush slot. -Normal-mode A/B barrel-shifter carry is not cleared by the -BLTSIZE row counter; it carries from the last source word of one row into -the first source word of the next, while masks, modulos, and fill carry -still observe row boundaries. Line blits use L1-L4 phases (L2 latches the -C source word, L3 propagates, L4 stores); line-mode B data loads pass -through the current B shifter at write time, and at completion the -hardware-visible ASH, BSH, SIGN, and low-word BLTAPT accumulator state is -written back. Tests: +Normal-mode A/B barrel-shifter carry is cleared at the first word of a new +BLTSIZE, then carries from the last source word of one row into the first +source word of the next inside that blit; masks, modulos, and fill carry +still observe row boundaries. Line blits use L1-L4 phases (L2 latches the C +source word, L3 propagates, L4 stores); line-mode B data loads pass through +the current B shifter at write time, and at completion the hardware-visible +ASH, BSH, SIGN, and low-word BLTAPT accumulator state is written back. Tests: `scheduled_normal_mode_bbusy_start_delay_precedes_first_source_slot`, `blit_pipeline_identifies_idle_cycles_per_hrm_diagrams`, `scheduled_line_mode_latches_c_source_before_store_phase`, -`scheduled_shift_carry_crosses_normal_mode_row_boundary`. +`scheduled_shift_carry_crosses_normal_mode_row_boundary`, +`scheduled_a_shift_zero_fills_first_word_of_new_blit`. ### Mid-operation register writes diff --git a/src/chipset/blitter.rs b/src/chipset/blitter.rs index db1e8b8c..3d015565 100644 --- a/src/chipset/blitter.rs +++ b/src/chipset/blitter.rs @@ -164,8 +164,6 @@ pub struct Blitter { pending: Option, dma_addr_mask: u32, - #[serde(skip, default)] - bltaold: u16, } #[derive(Debug, serde::Serialize, serde::Deserialize)] @@ -318,7 +316,6 @@ impl Blitter { bltbdat: 0, bltcdat: 0, bltsizv: 0, - bltaold: 0, bltbold: 0, bltbold_init: true, line_bdat: 0, @@ -621,8 +618,12 @@ impl Blitter { let mut cpt = self.bltcpt; let mut dpt = self.bltdpt; - let mut a_prev: u16 = self.bltaold; - let mut b_prev: u16 = self.bltbold; + let mut a_prev: u16 = 0; + let mut b_prev: u16 = if use_b || self.bltbold_init { + 0 + } else { + self.bltbold + }; for _row in 0..h { let mut fill_state: u16 = fci; // Buffer this row's D words so fill mode can process them @@ -715,8 +716,7 @@ impl Blitter { self.bltbpt = bpt & ptr_mask; self.bltcpt = cpt & ptr_mask; self.bltdpt = dpt & ptr_mask; - self.bltaold = a_prev; - if use_b { + if !use_b { self.bltbold = b_prev; } } @@ -1089,8 +1089,12 @@ impl NormalBlitState { bpt: blitter.bltbpt, cpt: blitter.bltcpt, dpt: blitter.bltdpt, - a_prev: blitter.bltaold, - b_prev: blitter.bltbold, + a_prev: 0, + b_prev: if use_b || blitter.bltbold_init { + 0 + } else { + blitter.bltbold + }, cur_a: 0, cur_b: 0, cur_c: 0, @@ -1479,8 +1483,7 @@ impl NormalBlitState { blitter.bltbpt = self.bpt & ptr_mask; blitter.bltcpt = self.cpt & ptr_mask; blitter.bltdpt = self.dpt & ptr_mask; - blitter.bltaold = self.a_prev; - if self.use_b { + if !self.use_b { blitter.bltbold = self.b_prev; } } @@ -1795,11 +1798,11 @@ mod tests { ); } - /// The A-channel barrel shifter's old-word latch is not cleared by - /// BLTSIZE. Split shifted blits can carry a boundary bit through BLTAOLD - /// when software starts the next blit without writing BLTADAT. + /// A new BLTSIZE starts the A-channel barrel shifter with zero fill. + /// Carry still crosses row boundaries inside one blit, but it must not + /// leak from the previous blit into the first word of the next one. #[test] - fn scheduled_a_shift_carry_survives_bltsize_restart() { + fn scheduled_a_shift_zero_fills_first_word_of_new_blit() { let mut ram = vec![0u8; 256]; write_word(&mut ram, 0x10, 0x0001); write_word(&mut ram, 0x12, 0x8000); @@ -1820,7 +1823,7 @@ mod tests { b.start_scheduled((1u16 << 6) | 1, &ram); while !b.tick_scheduled_slot(&mut ram) {} - assert_eq!(read_word(&ram, 0x22), 0xC000); + assert_eq!(read_word(&ram, 0x22), 0x4000); } /// Verify BZERO surfaces correctly when all output bits are zero. From 5179950073927d659aa33fea4a59d51b9dd280a1 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 24 Jun 2026 13:58:15 +0100 Subject: [PATCH 09/10] Keep copper colour writes phase-aligned with bitplanes COLOR_WRITE_HPOS_FB0 was moved 0x34 -> 0x38 in "Fix copper palette and manual sprite timing" to square up the OCS pure-copper colour grid, but that desynced copper/CPU COLORxx writes from the bitplane pixels they recolour and scrambled lo-res ECS playfields (the Sanity terrain) into misaligned vertical colour bands. OCS Denise (8362) and ECS Denise (8373) have identical colour-change timing -- the only OCS/ECS colour-path difference is the OCS 12-bit value mask -- and Denise emits a copper colour change in the same beam slot as the bitplane pixels it affects (MiniMig adds a one-lores-pixel bitplane delay expressly "for alignment of bitplane data and copper colour change"; WinUAE and vAmiga agree). Restore the phase-aligned 0x34 anchor and move SPRITE_PALETTE_CONTROL_HPOS_FB0 back to 0x36 in lockstep. Update the timing doc and the affected render tests. --- docs/internals/timing.md | 21 +++++++++++++-------- src/bus.rs | 8 ++++---- src/video/bitplane.rs | 24 ++++++++++++++++-------- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/docs/internals/timing.md b/docs/internals/timing.md index 114f4c8d..ee11a7d6 100644 --- a/docs/internals/timing.md +++ b/docs/internals/timing.md @@ -140,16 +140,21 @@ check is applied by `Copper::step_eligible_slot`, the single primitive shared by the live bus path and the blitter-deadline predictor's cloned simulation, so prediction and execution cannot drift apart. -For the OCS low-res renderer, a same-line `COLORxx` write at beam `hpos` -starts affecting pixels at `(hpos - $38) * 4` (`COLOR_WRITE_HPOS_FB0` in +For the low-res renderer, a same-line `COLORxx` write at beam `hpos` +starts affecting pixels at `(hpos - $34) * 4` (`COLOR_WRITE_HPOS_FB0` in `src/video/bitplane.rs`); beam-timed placement is anchored at `COPPER_WAIT_HPOS_FB0` ($28), and bitplane-control writes add the -fetch-to-display pipeline offset (`BITPLANE_CONTROL_PIPELINE_FB`). AGA -BPLCON4's sprite palette-base byte uses Lisa's earlier sprite colour-lookup -path at `(hpos - $3A) * 4` (`SPRITE_PALETTE_CONTROL_HPOS_FB0`). Because -colour MOVEs are spaced four colour clocks apart, a gradient changes -colour every 8 lores pixels, matching hardware rather than the previous -(too-fine) 4-pixel spacing. Tests: +fetch-to-display pipeline offset (`BITPLANE_CONTROL_PIPELINE_FB`). This +anchor keeps a copper colour change aligned with the bitplane pixels it +recolours: Denise (and MiniMig, which adds a one-lores-pixel bitplane delay +"for alignment of bitplane data and copper colour change") emits the new +colour in the same beam slot as those pixels. OCS Denise (8362) and ECS +Denise (8373) share this timing; the only OCS/ECS colour-path difference is +the OCS 12-bit value mask. (AGA Lisa delays colour changes by one hires +pixel relative to OCS/ECS per WinUAE; that sub-colour-clock offset is not +yet modelled.) AGA BPLCON4's sprite palette-base byte uses Lisa's earlier +sprite colour-lookup path at `(hpos - $36) * 4` +(`SPRITE_PALETTE_CONTROL_HPOS_FB0`). Tests: `copper_move_writes_visible_registers_on_second_dma_slot`, `copper_move_spends_four_color_clocks_leaving_alternate_cycles_free`. diff --git a/src/bus.rs b/src/bus.rs index 0b5e8c27..0e5584d4 100644 --- a/src/bus.rs +++ b/src/bus.rs @@ -10324,7 +10324,7 @@ mod tests { const STANDARD_DIW_HSTART: i32 = 0x81; const STANDARD_VISIBLE_X0: usize = ((STANDARD_DIW_HSTART - RENDER_DIW_HSTART_FB0) * 2) as usize; - const RENDER_COLOR_WRITE_HPOS_FB0: u32 = 0x38; + const RENDER_COLOR_WRITE_HPOS_FB0: u32 = 0x34; static BUS_TEMP_COUNTER: AtomicU64 = AtomicU64::new(0); #[test] @@ -12255,14 +12255,14 @@ mod tests { &mut bus, 0x0100, RENDER_VISIBLE_START_VPOS, - RENDER_COPPER_WAIT_HPOS_FB0 + 34, + RENDER_COPPER_WAIT_HPOS_FB0 + 30, &[(0x0182, 0x00F0)], ); let event_hpos = bus.current_render_events()[0].hpos; // MOVE write lands on its second-word fetch, two color clocks into the - // 4-color-clock cadence from the start hpos (+34). - assert_eq!(event_hpos, RENDER_COPPER_WAIT_HPOS_FB0 + 36); + // 4-color-clock cadence from the start hpos (+30). + assert_eq!(event_hpos, RENDER_COPPER_WAIT_HPOS_FB0 + 32); let words_per_row = bitplane_words_per_row( bus.agnus.revision(), bus.denise.bplcon0, diff --git a/src/video/bitplane.rs b/src/video/bitplane.rs index dd2839c3..d7895c26 100644 --- a/src/video/bitplane.rs +++ b/src/video/bitplane.rs @@ -56,17 +56,25 @@ const DIW_HSTART_FETCH_REFERENCE_HIRES: i32 = 0x83; // clocks in lockstep with DIW_HSTART_FB0 (16 lo-res pixels) so register writes // and bitplane pixels still register against each other after widening. const COPPER_WAIT_HPOS_FB0: i32 = 0x28; -/// COLORxx writes feed Denise's final colour-selection/output path, not the -/// upstream bitplane shifter. Treat the left output edge as the first visible -/// colour-register position rather than the earlier bitplane-control domain; -/// in low-res copper gradients, writes four colour clocks apart advance by one -/// 16-pixel colour cell. -const COLOR_WRITE_HPOS_FB0: i32 = 0x38; +/// COLORxx writes feed Denise's final colour-selection/output path. Denise +/// keeps copper/CPU colour-register changes phase-aligned with the bitplane +/// serializer output: a colour write and the bitplane pixels it recolours reach +/// the DAC together. (MiniMig models this explicitly with a one-lores-pixel +/// bitplane delay added "for alignment of bitplane data and copper colour +/// change"; WinUAE applies colour changes in the same beam slot as the +/// bitplane pixels.) OCS Denise (8362) and ECS Denise (8373) share this timing +/// exactly -- the only OCS/ECS colour-path difference is the OCS 12-bit value +/// mask -- so this anchor is revision-independent across OCS/ECS. +/// +/// TODO: AGA Lisa delays colour changes by one hires pixel relative to +/// OCS/ECS (WinUAE: "AGA color changes are 1 hires pixel delayed"). That +/// sub-colour-clock offset is not yet modelled here. +const COLOR_WRITE_HPOS_FB0: i32 = 0x34; /// AGA BPLCON4's low sprite-palette byte follows Lisa's sprite colour lookup /// path, which reaches sprite output earlier than ordinary COLORxx palette /// writes. Keep it separate from COLOR replay so copper palette gradients stay /// in the Denise palette-output phase on OCS/ECS. -const SPRITE_PALETTE_CONTROL_HPOS_FB0: i32 = 0x3A; +const SPRITE_PALETTE_CONTROL_HPOS_FB0: i32 = 0x36; /// SPRxPOS writes update the Denise horizontal comparator seven CCK ahead of /// the normal register/output beam domain. Manual sprite replays use this /// earlier domain so adjacent position writes can abut at their programmed @@ -7404,7 +7412,7 @@ mod tests { ); assert_eq!( beam_to_framebuffer_x_unclamped(COLOR_WRITE_HPOS_FB0 as u32), - 64 + 48 ); assert_eq!( sprite_palette_control_framebuffer_x(SPRITE_PALETTE_CONTROL_HPOS_FB0 as u32), From fad03a540f67707b22b9f043ffef3a83cbd74ad2 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 24 Jun 2026 17:26:55 +0100 Subject: [PATCH 10/10] Fix lo-res bitplane fetch origin for late DDFSTRT in a standard DIW A single-word lo-res bitplane fetch whose DDFSTRT sits after the standard $38 slot (e.g. a copper-fed chunky mosaic at DDFSTRT $48) in a standard DIWSTRT $81 window had its first cell column clipped and a partial column orphaned at the right edge: the standard one-sample lo-res phase bias (DIWSTRT $81 vs the $80 fetch reference) lands the clipped-sample count one off the fetch-gulp grid. Mirror the existing early-DDF (< $38) correction for the late case, gated on the standard +1 phase bias (display_native_shift == 1) so non-standard windows, which carry their own phase, are left alone. Also add STOP comments at COLOR_WRITE_HPOS_FB0 / RENDER_COLOR_WRITE_HPOS_FB0 warning against retuning the Denise colour-output anchor to fix a horizontally shifted picture: it has been moved and reverted several times, and the real cause is bitplane fetch/DDF alignment or sprite arming. --- src/bus.rs | 10 ++++++++++ src/video/bitplane.rs | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/bus.rs b/src/bus.rs index 0e5584d4..f58b3995 100644 --- a/src/bus.rs +++ b/src/bus.rs @@ -10324,6 +10324,16 @@ mod tests { const STANDARD_DIW_HSTART: i32 = 0x81; const STANDARD_VISIBLE_X0: usize = ((STANDARD_DIW_HSTART - RENDER_DIW_HSTART_FB0) * 2) as usize; + // STOP. If you are here because some scene's colours or copper-driven + // picture look horizontally shifted and you are tempted to nudge this + // value (or `COLOR_WRITE_HPOS_FB0` in src/video/bitplane.rs, which this + // mirrors): you are almost certainly wrong. This anchor is the Denise + // COLORxx output phase, calibrated so copper/CPU colour writes line up + // with the bitplane pixels they recolour (OCS == ECS; verified against + // MiniMig, MiSTer Minimig-AGA, WinUAE and vAmiga). We have moved it "to + // fix" a scene several times and every time had to move it back, because + // the real bug was elsewhere (bitplane fetch/DDF alignment, sprite arming, + // etc.). Find the actual cause; do not retune this number. const RENDER_COLOR_WRITE_HPOS_FB0: u32 = 0x34; static BUS_TEMP_COUNTER: AtomicU64 = AtomicU64::new(0); diff --git a/src/video/bitplane.rs b/src/video/bitplane.rs index d7895c26..67e06bfb 100644 --- a/src/video/bitplane.rs +++ b/src/video/bitplane.rs @@ -69,6 +69,13 @@ const COPPER_WAIT_HPOS_FB0: i32 = 0x28; /// TODO: AGA Lisa delays colour changes by one hires pixel relative to /// OCS/ECS (WinUAE: "AGA color changes are 1 hires pixel delayed"). That /// sub-colour-clock offset is not yet modelled here. +/// +/// STOP before retuning this. If a scene's colours or copper-driven picture +/// look horizontally shifted, the cause is almost never this anchor -- it has +/// been moved "to fix" demos several times and always had to be moved back +/// (the real bug was bitplane fetch/DDF alignment, sprite arming, etc.). This +/// value keeps copper colour writes aligned with the bitplane pixels they +/// recolour and is the same on OCS and ECS; leave it at 0x34. const COLOR_WRITE_HPOS_FB0: i32 = 0x34; /// AGA BPLCON4's low sprite-palette byte follows Lisa's sprite colour lookup /// path, which reaches sprite output earlier than ordinary COLORxx palette @@ -879,17 +886,27 @@ impl ControlState { self.hires() || self.shres(), self.ddfstrt, ); - if !self.hires() - && !self.shres() - && self.fetch_quantum() == 1 - && i32::from(ddf_start) < standard_ddf - && origin_shift > 0 - { - // Single-word lo-res fetches that start before the standard $38 - // slot expose whole 16-pixel groups. The standard one-sample - // lo-res phase bias must not push a standard-width DIW one sample - // past that completed early-DDF row at the right edge. - origin_shift -= 1; + if !self.hires() && !self.shres() && self.fetch_quantum() == 1 { + let ddf = i32::from(ddf_start); + if ddf < standard_ddf && origin_shift > 0 { + // Single-word lo-res fetches that start before the standard $38 + // slot expose whole 16-pixel groups. The standard one-sample + // lo-res phase bias must not push a standard-width DIW one sample + // past that completed early-DDF row at the right edge. + origin_shift -= 1; + } else if ddf > standard_ddf && origin_shift < 0 && display_native_shift == 1 { + // Mirror of the above for a standard-width DIW whose DDFSTRT is + // *late*: the picture is positioned in whole fetch gulps, but the + // standard one-sample lo-res phase bias (DIWSTRT $81 vs the $80 + // fetch reference, i.e. display_native_shift == 1) lands the + // clipped-sample count one off the gulp grid -- clipping the + // first cell column and orphaning a partial column at the right + // edge (e.g. a copper-fed bitplane mosaic fetched with DDFSTRT + // $48 in a standard DIW). Non-standard windows carry their own + // phase and are positioned by display_native_shift directly, so + // gate this on the standard +1 bias only. + origin_shift -= 1; + } } origin_shift }