Repository navigation
fix: Restore Lemmings timing with programmed video clocks - #2406
Merged
Merged
Conversation
Use live emulated clock timing for VGA beam/input status, route MCP mouse input through the shared event hub, wire VGA handler clock injection, and tighten scheduler/CPU/PIT timing with expanded tests.
maximilien-noal
marked this pull request as draft
October 12, 2026 06:45
maximilien-noal
marked this pull request as ready for review
October 12, 2026 06:46
maximilien-noal
marked this pull request as draft
October 12, 2026 06:47
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Mouse batching can lose hardware transitions, while scheduler catch-up can replay an unbounded event backlog.
2 open findings
What changed in this PR
Synchronizes VGA status timing with the emulated clock, unifies mouse input routing, and refines CPU/device scheduling.
Changes:
- Computes VGA beam status from live clock timing.
- Routes GUI and MCP mouse events through the shared input hub.
- Adds scheduler catch-up and shorter CPU-throttling slices with tests.
| File | Description |
|---|---|
tests/Spice86.Tests/Video/VideoFramePublicationTests.cs |
Tests delayed VGA scheduler pumps. |
tests/Spice86.Tests/Video/VgaIoPortTimingTests.cs |
Tests clock-driven beam status. |
tests/Spice86.Tests/Video/CgaIntegrationTests.cs |
Supplies a clock to the VGA handler. |
tests/Spice86.Tests/McpServerToolStateTests.cs |
Tests MCP mouse delivery. |
tests/Spice86.Tests/Emulator/Devices/ExternalInput/PitModeTests.cs |
Uses cycle-based test timing. |
tests/Spice86.Tests/Emulator/Devices/ExternalInput/Pit8254Tests.cs |
Uses cycle-based test timing. |
tests/Spice86.Tests/Emulator/Devices/ExternalInput/DeviceSchedulerTests.cs |
Tests recurring-event catch-up. |
tests/Spice86.Tests/CpuCycleLimiterTests.cs |
Tests cycle-rate bounds. |
tests/Spice86.Tests/ClockTests.cs |
Tests fresh real-time clock reads. |
src/Spice86/Spice86DependencyInjection.cs |
Wires clock and shared input hub. |
src/Spice86.Core/Emulator/VM/DeviceScheduler/DeviceScheduler.cs |
Preserves periodic deadlines. |
src/Spice86.Core/Emulator/VM/CpuSpeedLimit/CpuCycleLimiter.cs |
Introduces shorter throttle slices. |
src/Spice86.Core/Emulator/VM/Clock/EmulatedClock.cs |
Removes cached clock reads. |
src/Spice86.Core/Emulator/Devices/Video/VgaTimingEngine.cs |
Exposes scanline duration internally. |
src/Spice86.Core/Emulator/Devices/Video/VgaIoPortHandler.cs |
Computes live VGA beam status. |
doc/mcp.md |
Documents timing and mouse behavior. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Retain recurring device deadlines without monopolizing a scheduler pump. Deliver throttled mouse movement and button masks on the emulated clock. Keep the stronger upstream MCP mouse test to avoid xUnit1024 in PR builds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
maximilien-noal
marked this pull request as ready for review
October 13, 2026 04:06
Keep beam status and frame scheduling on one programmed clock and epoch. Replace the fixed line rate with DOSBox-derived native VGA timings. Preserve existing rendering and cancel stale frame chains on mode changes. Refs #2406 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use a floating-point vertical counter period and an ordered zero check. Keep hardware periods unchanged while addressing CodeQL findings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kevinferrare
approved these changes
Oct 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Description of Changes
Lemmings 2 waits for video sync pulses to pace gameplay and music. Those pulses depended on rendering progress and a fixed line rate. Video timing now follows the programmed VGA clocks and registers, with one shared frame origin for status reads and frame scheduling.
No changes to scanning, pixel rendering, capture features, shaders, or rendering backends were implemented.
flowchart LR Ports["Existing CRTC / sequencer / misc writes"] --> Registers["Captured timing registers"] Registers --> Decode["VgaTimingParameters"] Decode --> Beam["Shared immutable parameters + frame epoch"] Clock["Fresh emulated clock"] --> Status["03DAh / 03BAh beam status"] Beam --> Status Status --> IRQ["Lemmings IRQ0 retrace wait"] IRQ --> Music["Music service + shared tick"] Music --> Game["Four ticks per gameplay update"] Beam --> Scheduler["Original-deadline frame scheduling"] Clock --> Scheduler Scheduler --> Pixels["Existing renderer, unchanged pixel algorithms"]The scheduler keeps original deadlines when callbacks run late, but yields after 1,024 callbacks. Clock reads stay fresh; CPU throttle waits are 25 microseconds at
--Cycles 3000, shorter than a sync pulse.Rationale behind Changes
VGA registers describe clock ticks per line and lines per frame. Both the status reader and frame scheduler now use these periods:
The half-dot flag doubles the horizontal period; the existing vertical clock-divider flag doubles vertical intervals. BIOS-programmed modes give:
In Lemmings' mode, visible display ends at line 400, but the sync pulse is only lines 412-413:
flowchart TD Visible["Lines 0-399: display interval"] --> Gap["Lines 400-411: display disabled; sync clear"] Gap --> Sync["Lines 412-413: programmed sync pulse"] Sync --> Tail["Lines 414-448: display disabled; sync clear"] Tail --> Visible DisplayEnd["Publish completed framebuffer at display end"] -.-> Gap Clock["25.175 MHz / 8; 100 character clocks per line"] --> Period["14.268123 ms frame; 70.086303 Hz"] Period --> VisibleBlanking and sync use separate wrapped counters. Status reads preserve unrelated bits and the existing attribute-register behavior.
Timing writes cancel the old frame's callbacks on the next video callback. Repeated writes, screen-off, and address-only changes keep the phase:
flowchart TD Write["Existing register write"] --> Capture["Capture timing-relevant values"] Capture --> Equal{"Same effective timing?"} Equal -->|yes| Keep["Keep parameter identity and epoch"] Equal -->|no| Publish["Publish immutable timing + write-time epoch"] Publish --> Callback["Next callback on existing video scheduler"] Callback --> Cancel["Cancel obsolete frame callbacks"] Cancel --> Restart["Start one replacement frame at current time"] Restart --> Chain["Schedule from original deadlines"]The clock/counter calculations follow DOSBox Staging's native VGA timings. This ports the timing model, not DOSBox's renderer, cropping, SVGA clocks, or adapter implementations.
Suggested Testing Steps
rtk proxy dotnet build src\Spice86.sln -c Release --no-restore -v quietrtk proxy dotnet build src\Spice86.sln -c Debug --no-restore -v quietrtk proxy dotnet test tests\Spice86.Tests -c Release --no-restore -- --filter-class '*VgaIoPortTimingTests' --filter-class '*VgaTimingParametersTests' --filter-class '*VideoFramePublicationTests' --filter-class '*CgaIntegrationTests'rtk proxy dotnet test src\Spice86.sln -c Release --no-build -- --filter-not-class '*SingleStepTest*'Tests cover counter wrap, phase, register changes, late callbacks, and unchanged framebuffer rows. CPU instruction tests were excluded locally because instruction behavior is unchanged.
Live check: Beach level at
--Cycles 3000produced 1,401 game/music ticks in 20.0191583 seconds: 69.98296 Hz, within 0.11% of DOSBox's measured 70.05972 Hz. Animation advanced and the OPL channel stayed enabled. The same IRQ counter was measured in different scenes; MCP overhead is included.xychart-beta title "Observed IRQ-maintained game/music heartbeat" x-axis ["DOSBox", "Earlier fixed-rate head", "Programmed-clock development", "Final programmed-clock replay"] y-axis "Ticks per second" 0 --> 80 bar [70.05972, 69.98379, 70.02455, 69.98296]To repeat: run Lemmings 2 with MCP enabled at 3,000 cycles, start the Beach level, and compare paused memory reads at
DS:BE58around a 20-second resumed interval. Decode the 16-bit counter difference modulo 65,536.