fix(dsp1): run continuous-mode NEC DSP to its host-wait spin, not first RQM (v1.22.0) - #253
Conversation
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (25)
WalkthroughThe DSP-1 host interface now steps through the complete host-wait spin after DR accesses, preventing continuous-mode parameter misalignment and restoring changing Mode-7 raster results. A regression test, documentation, release notes, version metadata, and local tooling ignore rule were added or updated. ChangesDSP-1 continuous-mode raster correction
v1.22.0 release metadata
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RegressionTest
participant Upd77c25
participant DSPFirmware
RegressionTest->>Upd77c25: Send 0x02 parameters and 0x0a raster command
Upd77c25->>DSPFirmware: Step until RQM is set and PC stops changing
DSPFirmware-->>Upd77c25: Return parked host-wait state
RegressionTest->>Upd77c25: Read raster-cycle results
Upd77c25-->>RegressionTest: Return changing four-word raster values
🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 12-14: Move the DSP-1 user-visible fix entry from the 1.22.0
section into the existing [Unreleased] section in CHANGELOG.md, preserving the
entry text and leaving the 1.22.0 release notes unchanged.
In `@docs/STATUS.md`:
- Around line 28-34: Restrict the no-regression claims tied to dsp1_raster_ramp
to the DSP-1/Pilotwings path, removing or narrowing assertions covering DSP-2/4
and ST010/011 unless separate framebuffer-hash evidence is cited. Apply this
guidance in docs/STATUS.md lines 28-34, CHANGELOG.md lines 28-34, and
to-dos/VERSION-PLAN.md lines 1429-1435.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 55ae9401-d25e-4169-a390-578360065d07
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lockand included byCargo.lock
📒 Files selected for processing (25)
.gitignoreCHANGELOG.mdCargo.tomlREADME.mdcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-cart/src/coproc/upd77c25.rscrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-savestate/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlcrates/rustysnes-test-harness/tests/dsp1_raster_ramp.rsdocs/STATUS.mddocs/cart.mdto-dos/VERSION-PLAN.md
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: accuracysnes
- GitHub Check: lint
- GitHub Check: test-light
- GitHub Check: build
- GitHub Check: build demo + docs
🧰 Additional context used
📓 Path-based instructions (15)
**/Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Do not configure the
scriptingandscript-wasmbackends as simultaneously enabled features; they are mutually exclusive.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-savestate/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlCargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-core/Cargo.toml
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Do not commit or vendor the generatedsnesdev_wiki/mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits:<type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors as project releases.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-savestate/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlCargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlREADME.mdcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-core/Cargo.tomlCHANGELOG.mdcrates/rustysnes-test-harness/tests/dsp1_raster_ramp.rsdocs/cart.mddocs/STATUS.mdcrates/rustysnes-cart/src/coproc/upd77c25.rsto-dos/VERSION-PLAN.md
crates/rustysnes-*/**/*
📄 CodeRabbit inference engine (Custom checks)
For the full pull request diff against its base branch, any observable behavior change under
crates/rustysnes-<chip>/must be accompanied by an edit to the matchingdocs/<chip>.md; a crate change passes without documentation only when it does not alter observable behavior, with the non-behavioral change stated explicitly.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-savestate/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-test-harness/tests/dsp1_raster_ramp.rscrates/rustysnes-cart/src/coproc/upd77c25.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,toml}: Additive features must be default-off so shipped/native,no_std, and wasm builds remain byte-identical.
Never use or configure--all-features; validate opt-in feature combinations individually as required by the project recipe.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-savestate/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlCargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-test-harness/tests/dsp1_raster_ramp.rscrates/rustysnes-cart/src/coproc/upd77c25.rs
crates/**
⚙️ CodeRabbit configuration file
crates/**: Emulator core. Hot paths are allocation-free;unsaferequires a// SAFETY:comment
naming the invariant. Any change to save-stated fields needs aFORMAT_VERSIONbump and a
docs/adr/0006bump-log entry. Behavior changes must update the matchingdocs/<chip>.md
in the same change.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-savestate/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-test-harness/tests/dsp1_raster_ramp.rscrates/rustysnes-cart/src/coproc/upd77c25.rs
crates/*/Cargo.toml
⚙️ CodeRabbit configuration file
crates/*/Cargo.toml: The frontend pins one compatible GUI tier (currently winit 0.30 / wgpu 29 / egui 0.35 and
itsegui-wgpu/egui-winitsiblings). Flag a bump to any one of those that is not
accompanied by matching bumps to the others. Optional features are default-off by
convention so shipped, no_std and wasm builds stay byte-identical; flag a new feature added
to adefault = [...]list.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-savestate/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-core/Cargo.toml
Cargo.toml
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Cargo.toml: Keep crate dependencies one-directional; chip crates must not depend on each other.
Additive features must remain default-off so native, no_std, and wasm outputs remain byte-identical in default configurations.
Do not usecargo clippy --all-features; usecargo clippy --workspace --all-targets -- -D warnings, with explicit feature jobs when needed.
Files:
Cargo.toml
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Chip-behavior changes must update both the chip implementation and the corresponding
docs/<subsystem>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
README.mdCHANGELOG.mdcrates/rustysnes-test-harness/tests/dsp1_raster_ramp.rsdocs/cart.mddocs/STATUS.mdcrates/rustysnes-cart/src/coproc/upd77c25.rsto-dos/VERSION-PLAN.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).
Files:
README.mdCHANGELOG.mddocs/cart.mddocs/STATUS.mdto-dos/VERSION-PLAN.md
CHANGELOG.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
User-visible changes must be recorded under the
[Unreleased]section.For the full pull request diff against its base branch, modify
CHANGELOG.mdwhen user-visible behavior changes, including emulator output, frontend features, CLI flags, public APIs, or AccuracySNES cartridge contents. Do not require it for purely internal changes, tests, comments, or CI configuration.
Files:
CHANGELOG.md
crates/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
crates/**/*.rs: Preserve the master-clock lockstep timing model.
rustysnes-core::Busowns mutable machine state, and the CPU borrows&mut Bus.
Preserve determinism: seed, ROM, and input must produce bit-identical output.
Treat test ROMs as the behavioral specification; when documentation disagrees with passing ROM behavior, update the documentation.
Keepunsafeconfined to existing allowed areas, namely frontend and FFI code, and document everyunsafeblock with a// SAFETY:comment.
Files:
crates/rustysnes-test-harness/tests/dsp1_raster_ramp.rscrates/rustysnes-cart/src/coproc/upd77c25.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use Rust edition 2024 and the toolchain pinned inrust-toolchain.toml(Rust 1.96).
Runcargo fmt --all --check; Rust code must remain rustfmt-compliant.
Run Clippy withcargo clippy --workspace --all-targets -- -D warnings; warnings must not remain.
New public Rust items must have rustdoc becausemissing_docsis a workspace lint.
Do not runcargo clippy --all-features;scriptingandscript-wasmare mutually exclusive. Use explicit per-feature jobs instead.
**/*.rs: Do not introduce.unwrap(),.expect(), orpanic!()on untrusted external input—such as ROM/save-state bytes, netplay messages, Lua or scripting input, or user-supplied paths—outside#[cfg(test)]code. Use typed errors at those boundaries; locally constructed values or values immediately protected by a checked invariant are allowed.
Every newunsafe { ... }block orunsafe fnmust have an adjacent// SAFETY:comment naming the relied-on invariant and its guarantor. Unsafe code outside the frontend and FFI shims should additionally be questioned becauseunsafe_codeis a workspace lint.
**/*.rs: Use Rust edition 2024 with the pinned 1.96 toolchain; satisfy workspacepedantic,nursery,missing_docs, andunsafe_codewarnings because CI runs with-D warnings. Document every public item.
Keepunsafecode restricted to the frontend and FFI, and include a// SAFETY:justification for each use.
Keep hot paths allocation-free.
Treatrustysnes_core::Busas the owner of mutable emulator state; the CPU borrows&mut Bus.
Use the master clock at 21477270 Hz as the timing master; advance the scheduler in lockstep and run other chips on their divisors.
Maintain determinism: seed, ROM, and input must produce bit-identical audio/video; frontend rate control must not alter emulation results.
When implementing hardware behavior, pin and run the failing test ROM first; treat test ROMs as the specification.
Files:
crates/rustysnes-test-harness/tests/dsp1_raster_ramp.rscrates/rustysnes-cart/src/coproc/upd77c25.rs
docs/**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Before changing a subsystem, consult
docs/architecture.md,docs/STATUS.md,CONTRIBUTING.md, the relevant subsystem documentation, and applicable ADRs.New subsystems must add documentation under
docs/.
Files:
docs/cart.mddocs/STATUS.md
docs/**
⚙️ CodeRabbit configuration file
docs/**: Docs are the spec, not a history log. Flag claims that contradict the code, counts that
contradict the generateddocs/accuracysnes-coverage.md, and any statement of coverage that
is broader than what the corresponding test actually asserts.
Files:
docs/cart.mddocs/STATUS.md
crates/rustysnes-{cpu,ppu,apu,cart,core}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep core chip implementation changes localized to the owning chip crate and preserve the workspace crate boundaries.
Files:
crates/rustysnes-cart/src/coproc/upd77c25.rs
🔇 Additional comments (22)
Cargo.toml (1)
25-25: LGTM!crates/rustysnes-android/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-apu/Cargo.toml (1)
3-3: LGTM!crates/rustysnes-cart/Cargo.toml (1)
3-3: LGTM!crates/rustysnes-ppu/Cargo.toml (1)
3-3: LGTM!crates/rustysnes-savestate/Cargo.toml (1)
3-3: LGTM!crates/rustysnes-script/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-test-harness/Cargo.toml (1)
3-3: LGTM!README.md (1)
17-17: LGTM!.gitignore (1)
142-145: LGTM!crates/rustysnes-cheevos/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-core/Cargo.toml (1)
3-3: LGTM!crates/rustysnes-cpu/Cargo.toml (1)
3-3: LGTM!crates/rustysnes-frontend/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-gfx-shaders/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-ios/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-mobile/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-monetization/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-netplay/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-cart/src/coproc/upd77c25.rs (1)
400-423: LGTM!docs/cart.md (1)
105-113: LGTM!crates/rustysnes-test-harness/tests/dsp1_raster_ramp.rs (1)
1-73: LGTM!
| **`v1.22.0 "Horizon"` fixes the long-standing DSP-1 continuous-mode Mode-7 flat floor** (Pilotwings | ||
| flight, SMK track): the shared NEC-DSP host-sync (`run_until_rqm`) stopped one host-write too early, | ||
| so reading a host input word raised RQM before the firmware cleared DRC to 16-bit — mis-framing the | ||
| DSP-1 parameter block into a degenerate, constant-across-scanlines projection. Running the engine to | ||
| the firmware's true host-wait spin (matching ares) restores the per-scanline perspective ramp, | ||
| verified against a MesenCE reference with **zero change** to any non-flight NEC-coprocessor rendering | ||
| (DSP-1/2/4 title/attract framebuffers byte-identical before and after). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files 'docs/STATUS.md' 'CHANGELOG.md' 'to-dos/VERSION-PLAN.md' 'docs/cart.md' | cat
printf '\n== Search for dsp1_raster_ramp ==\n'
rg -n "dsp1_raster_ramp|run_until_rqm|byte-identical|zero change|zero-change|all six NEC|DSP-1/2/3/4|ST010|ST011|Revision::Upd7725" \
docs/STATUS.md CHANGELOG.md to-dos/VERSION-PLAN.md docs/cart.md .
printf '\n== Test definition locations ==\n'
rg -n "dsp1_raster_ramp" . --glob '!target' --glob '!node_modules'
printf '\n== Outline of likely test file(s) ==\n'
for f in $(rg -l "dsp1_raster_ramp" . --glob '!target' --glob '!node_modules'); do
echo "--- $f ---"
ast-grep outline "$f" --view expanded || true
doneRepository: doublegate/RustySNES
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Test file contents (bounded) ==\n'
for f in $(rg -l "dsp1_raster_ramp" . --glob '!target' --glob '!node_modules'); do
echo "--- $f ---"
wc -l "$f"
# print relevant window around the test
line=$(rg -n "dsp1_raster_ramp" "$f" | head -n1 | cut -d: -f1)
start=$(( line > 40 ? line - 40 : 1 ))
end=$(( line + 120 ))
sed -n "${start},${end}p" "$f" | cat -n
doneRepository: doublegate/RustySNES
Length of output: 18599
Narrow the no-regression claim to the DSP-1 path. dsp1_raster_ramp only supports the Pilotwings/DSP-1 fix; it does not back the byte-identical/zero-change claims for DSP-2/4 or ST010/011 in docs/STATUS.md#L28-L34, CHANGELOG.md#L28-L34, and to-dos/VERSION-PLAN.md#L1429-L1435. Trim those statements or point to separate framebuffer-hash coverage.
📍 Affects 3 files
docs/STATUS.md#L28-L34(this comment)CHANGELOG.md#L28-L34to-dos/VERSION-PLAN.md#L1429-L1435
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/STATUS.md` around lines 28 - 34, Restrict the no-regression claims tied
to dsp1_raster_ramp to the DSP-1/Pilotwings path, removing or narrowing
assertions covering DSP-2/4 and ST010/011 unless separate framebuffer-hash
evidence is cited. Apply this guidance in docs/STATUS.md lines 28-34,
CHANGELOG.md lines 28-34, and to-dos/VERSION-PLAN.md lines 1429-1435.
Sources: Coding guidelines, Path instructions
e9682d3 to
2b78a1f
Compare
Review adjudication (pushed in
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
workflow=$(fd -t f -i '^release-auto\.ya?ml$' . | head -n 1 || true)
if [ -n "$workflow" ]; then
echo "=== $workflow ==="
cat -n "$workflow"
else
echo "release-auto workflow not found"
fi
echo "=== CHANGELOG release headings ==="
rg -n -C2 '^\s*##\s+\[(Unreleased|1\.22\.0|1\.21\.0)\]' CHANGELOG.md || true
echo "=== commit 2b78a1f changed files ==="
git show --stat --oneline --no-renames 2b78a1fLength of output: 3032 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== release automation workflow candidates ==="
fd -H -t f -i 'release.*auto|auto.*release|release' .github 2>/dev/null || true
echo "=== references to changelog release parsing ==="
rg -n -H -i -C4 'CHANGELOG|Unreleased|release-auto|auto-tag|publish' .github 2>/dev/null || trueLength of output: 13909
✏️ Learnings added
|
DSP-1 games rendered a flat, perspective-less Mode-7 ground (Pilotwings flight, Super Mario Kart track) — the last major visible accuracy gap in the coprocessor set. Root-caused to the shared NEC-DSP host interface stopping the engine one host-write too early. Root cause ---------- `Upd77c25::run_until_rqm` advances the engine to its parked state after each host DR access, and stopped at the FIRST `RQM=set`. But reading a host input word (`src == 8`) raises RQM as a side effect while the firmware still has pending setup — notably clearing DRC to 16-bit for the next transfer. Stopping there deferred the DRC clear past the next host write, so Pilotwings' 7-word `cmd 0x02` projection block transferred 8-bit-misaligned and the DSP computed a degenerate, constant-across-scanlines matrix → a flat floor. The engine now runs to the firmware's genuine host-wait spin (a single-instruction `JRQM`/`JNRQM` self-loop), matching ares' continuous clock-stepping to the wait point. How it was pinned ----------------- - A MesenCE Lua tracer captured a real flight's DR command stream; RustySNES received byte-identical DSP-1 INPUT but produced constant OUTPUT (8 distinct words/frame) vs the reference's per-scanline ramp (264 distinct). - Replaying the real command stream through a fresh core reproduced the flat result deterministically, ruling out bus/game timing. - ares' readDR/writeDR, ALU/multiplier/flag logic, execLD, and the SR-write mask were verified line-for-line identical to RustySNES, isolating the single structural difference: the host-wait stepping. Verification and no-regression ------------------------------ - In-game Pilotwings flight went from 150/150 flat frames to a per-scanline ramp on every frame. - `run_until_rqm` is shared by all six NEC coprocessors (DSP-1/2/3/4, ST010/011); before/after framebuffer content-hashes are byte-identical for Pilotwings title/attract, Dungeon Master (DSP-2), and Top Gear 3000 (DSP-4). - New `dsp1_raster_ramp` regression test replays the real params and asserts the ramp — confirmed it FAILS on the pre-fix core (`[fe7f 01ff fe00 fe7f]`) and PASSES after. - `dsp1_oncart` + 156 cart-crate unit tests green; `cargo test --workspace` 527/527; all gates green (fmt, clippy default + per-feature, no_std, doc). Release ------- Cuts v1.22.0 "Horizon" (release-closeout: empty [Unreleased] + closed-out [1.22.0] section triggers release-auto.yml on merge). Version bumped across the workspace; CHANGELOG, STATUS, VERSION-PLAN, docs/cart.md §the shared NEC core updated; root .gitignore ignores the local `.codegraph/` index. The full pin-exact rewrite (scheduler-stepped DSP for cycle-accurate latency) is a scoped future effort — this fix is byte-exact at the RQM bus boundary, all DSP-1 games observe. Throwaway diagnostics stay on `fix/dsp1-mode7`, not in this release. Addresses review: documented the single-instruction-spin assumption (upd77c25.rs), swapped the test's BTreeSet for an alloc-free windows(2) check, added the "Horizon" codename to the CHANGELOG heading, and shortened the subject. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2b78a1f to
00bc097
Compare
Antigravity review (Gemini via Ultra)This PR updates the NEC µPD77C25 host-synchronization logic ( Blocking issues
Suggestions
Nitpicks
Automated first-pass review by |
1 participant
Summary
Fixes the long-standing DSP-1 continuous-mode Mode-7 flat-floor bug (Pilotwings flight, Super Mario Kart track) and cuts the v1.22.0 "Horizon" release. DSP-1 games rendered a flat, perspective-less Mode-7 ground — the last major visible accuracy gap in the coprocessor set. The fix is one function in the shared NEC-DSP host interface.
Root cause
Upd77c25::run_until_rqmadvances the NEC DSP engine to its parked state after every host DR access, and stopped at the firstRQM=set. But reading a host input word (src == 8) raises RQM as a side effect while the firmware still has pending setup — notably clearing DRC to 16-bit for the next transfer. Stopping there deferred the DRC clear past the next host write, so Pilotwings' 7-wordcmd 0x02projection block was transferred 8-bit-misaligned and the DSP computed a degenerate, constant-across-scanlines matrix → a flat floor.The engine now runs to the firmware's genuine host-wait spin (a
JRQM/JNRQMself-loop), matching ares' continuous clock-stepping to the wait point:How it was pinned
readDR/writeDR, ALU/multiplier/flag logic,execLD, and the SR-write mask were verified line-for-line identical to RustySNES, isolating the single structural difference: the host-wait stepping.Verification & no-regression
run_until_rqmis shared by all six NEC coprocessors (DSP-1/2/3/4, ST010/011). Before/after framebuffer content-hashes are byte-identical for Pilotwings title/attract, Dungeon Master (DSP-2), and Top Gear 3000 (DSP-4) — the fix only touches the flight/raster path.dsp1_raster_rampregression test replays the real params and asserts the ramp; confirmed it fails on the pre-fix core ([fe7f 01ff fe00 fe7f]) and passes after.cargo test --workspace527 passed / 0 failed,dsp1_oncartgreen, 156 cart-crate unit tests green, fmt + clippy (default & per-feature) + no_std thumbv7em +-D warningsdoc all green.Release (v1.22.0 "Horizon")
Version bumped across the workspace;
CHANGELOG.md,docs/STATUS.md,to-dos/VERSION-PLAN.md, anddocs/cart.md§the shared NEC core updated; root.gitignorenow ignores the local.codegraph/index. v1.22.0 also tags the additive[Unreleased]work already on main (WAI wake cycle, DRAM refresh).Scope notes
fix/dsp1-mode7branch and is intentionally not part of this release — this branch was cut clean frommain.🤖 Generated with Claude Code
Summary
Fixes DSP-1 continuous-mode Mode-7 rasterization so the host interface advances to the genuine firmware host-wait spin instead of stopping at the first RQM signal. This restores per-scanline projection updates in Pilotwings and Super Mario Kart; the claim would be false if raster output remained flat or other NEC-coprocessor framebuffers changed unexpectedly.
Adds the
dsp1_raster_rampregression test, asserting that consecutive raster results are not identical. Accuracy/status assertions for v1.22.0 “Horizon” were added or updated; the reported coverage denominator did not move.Also publishes the v1.22.0 release metadata and documentation updates.