diff --git a/src/Spice86.Core/Emulator/Devices/Input/Keyboard/PS2Keyboard.cs b/src/Spice86.Core/Emulator/Devices/Input/Keyboard/PS2Keyboard.cs index acc891722c..74223f3162 100644 --- a/src/Spice86.Core/Emulator/Devices/Input/Keyboard/PS2Keyboard.cs +++ b/src/Spice86.Core/Emulator/Devices/Input/Keyboard/PS2Keyboard.cs @@ -8,6 +8,7 @@ namespace Spice86.Core.Emulator.Devices.Input.Keyboard; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; /// /// Emulates a PS/2 keyboard device, handling key events, scancode translation, and communication with a keyboard @@ -33,6 +34,14 @@ public partial class PS2Keyboard { private RepeatData _repeat = new() { Key = PcKeyboardKey.None, WaitMs = 0, PauseMs = 500, RateMs = 33 }; private readonly Dictionary _set3CodeInfo = new(); + /// + /// Keys the device currently believes are held down. A host key-up can be lost (window lost focus, + /// input dropped while paused, frame-buffer overflow) and the guest only ever clears a modifier on its + /// BREAK scancode, so this set is the bookkeeping that lets the device reconcile and release what it + /// still thinks is down. + /// + private readonly HashSet _pressedKeys = new(); + /// /// State of keyboard LEDs, as requested via keyboard controller /// @@ -130,6 +139,11 @@ private void EnqueueScanCodeFrame(List? scanCode) { if (_keyboardFrameBuffer.Count == BufferSizeInScancodes) { _keyboardFrameBuffer.Clear(); _bufferOverflowed = true; + // A discarded key-up would otherwise latch that key (and its modifier bits) in the guest forever. + // Reconcile the held-key set so no key is believed to be down any more. + _pressedKeys.Clear(); + _repeat.Key = PcKeyboardKey.None; + _repeat.WaitMs = 0; return; } @@ -521,6 +535,44 @@ internal void NotifyReadyForFrame() { TransferNextBufferedFrameIfReady(); } + /// + /// Releases every key the device still believes is down, emitting a proper BREAK frame for each one, + /// and stops any in-flight typematic repeat. + /// + /// + /// Call this when the host's key state can no longer be trusted: the window lost activation, the + /// emulator was paused, or the keyboard frame buffer overflowed and discarded a key-up. The guest only + /// clears a modifier on its BREAK scancode, so a lost key-up otherwise latches it forever (e.g. Alt + /// left in KB_FLAG), silently changing how every other key is translated. This method is safe to + /// call on the emulator thread only; marshal cross-thread calls onto the emulator thread first. + /// + public void ReleaseAllKeys() { + if (_pressedKeys.Count == 0) { + _repeat.Key = PcKeyboardKey.None; + _repeat.WaitMs = 0; + return; + } + + // Emit a BREAK frame for each held key in a deterministic order so the guest sees a stable sequence. + List heldKeys = _pressedKeys.OrderBy(key => key).ToList(); + foreach (PcKeyboardKey key in heldKeys) { + EnqueueScanCodeFrame(GetReleaseScanCode(key)); + } + + _pressedKeys.Clear(); + _repeat.Key = PcKeyboardKey.None; + _repeat.WaitMs = 0; + } + + private List GetReleaseScanCode(PcKeyboardKey keyType) { + return _codeSet switch { + 1 => _scancodeConverter.GetScanCode1(keyType, isPressed: false), + 2 => _scancodeConverter.GetScanCode2(keyType, isPressed: false), + 3 => _scancodeConverter.GetScanCode3(keyType, isPressed: false), + _ => _scancodeConverter.GetScanCode1(keyType, isPressed: false), + }; + } + /// /// Adds a key event to be processed by the keyboard. /// @@ -531,6 +583,16 @@ public void EnqueueKeyEvent(PcKeyboardKey keyType, bool isPressed) { return; } + if (keyType is PcKeyboardKey.None or PcKeyboardKey.Pause or PcKeyboardKey.PrintScreen) { + // Keys that are not tracked as held: Pause has no BREAK frame, PrintScreen is a momentary + // key-chord, and None is an unsupported key. Keeping them out of the held-key set means a + // later release-all does not try to emit a BREAK scancode that does not exist. + } else if (isPressed) { + _pressedKeys.Add(keyType); + } else { + _pressedKeys.Remove(keyType); + } + List scanCode = new(); switch (_codeSet) { diff --git a/src/Spice86.Core/Emulator/Mcp/EmulatorMcpServices.cs b/src/Spice86.Core/Emulator/Mcp/EmulatorMcpServices.cs index 08a55368f5..32e7233de5 100644 --- a/src/Spice86.Core/Emulator/Mcp/EmulatorMcpServices.cs +++ b/src/Spice86.Core/Emulator/Mcp/EmulatorMcpServices.cs @@ -70,7 +70,17 @@ public sealed class EmulatorMcpServices( /// /// Gets or sets the Intel 8042 PS/2 controller used for keyboard/mouse MCP automation. /// - public Intel8042Controller? Intel8042Controller { get; set; } + public Intel8042Controller? Intel8042Controller { + get => _intel8042Controller; + set { + _intel8042Controller = value; + if (value is not null) { + SubscribeKeyboardReconcileOnPause(); + } + } + } + + private Intel8042Controller? _intel8042Controller; /// /// Gets or sets the input event hub. MCP tools must post keyboard/mouse @@ -124,6 +134,26 @@ public sealed class EmulatorMcpServices( /// public Dos? Dos { get; set; } + private bool _keyboardReconcileSubscribed; + + /// + /// Subscribes the keyboard device to release all held keys whenever the emulator pauses. Key-ups + /// dropped while paused (or lost when the window deactivates) would otherwise leave a modifier + /// latched in the guest's BIOS state. The release is marshalled onto the emulator thread via + /// so it is safe to call from the UI thread. + /// + internal void SubscribeKeyboardReconcileOnPause() { + if (_keyboardReconcileSubscribed + || Intel8042Controller is null + || InputEventHub is null) { + return; + } + + InputEventHub inputEventHub = InputEventHub; + PauseHandler.Pausing += () => inputEventHub.ReleaseAllKeys(); + _keyboardReconcileSubscribed = true; + } + // Shared MCP breakpoint tracking state (survives transient tool instances) private readonly object _mcpBreakpointsLock = new(); private readonly Lock _toolsLock = new(); diff --git a/src/Spice86.Core/Emulator/VM/InputEventQueue.cs b/src/Spice86.Core/Emulator/VM/InputEventQueue.cs index f574bcf26f..9e06890ea9 100644 --- a/src/Spice86.Core/Emulator/VM/InputEventQueue.cs +++ b/src/Spice86.Core/Emulator/VM/InputEventQueue.cs @@ -59,6 +59,20 @@ private void Enqueue(Action action) { /// public void PostToEmulatorThread(Action action) => Enqueue(action); + /// + /// Thread-safe request to release every key the keyboard device still believes is down, emitting a + /// proper BREAK frame for each one. Call this when the host's key state can no longer be trusted + /// (e.g. the window lost activation and a key-up was never delivered), so the guest does not keep a + /// modifier latched. The release is enqueued and runs on the emulator thread with the rest of the input. + /// + public void ReleaseAllKeys() => + Enqueue(() => KeyboardDevice?.ReleaseAllKeys()); + + /// + /// The PS/2 keyboard device whose held-key state is reconciled by . + /// + internal PS2Keyboard? KeyboardDevice { get; set; } + /// /// Enqueues a keyboard event to be fired on the emulator thread, following /// the same path as UI keyboard events (PhysicalKey → PS2Keyboard scancode pipeline). diff --git a/src/Spice86/Spice86DependencyInjection.cs b/src/Spice86/Spice86DependencyInjection.cs index 6f31baaa6f..bcdb8806f2 100644 --- a/src/Spice86/Spice86DependencyInjection.cs +++ b/src/Spice86/Spice86DependencyInjection.cs @@ -562,6 +562,7 @@ internal Spice86DependencyInjection(Configuration configuration, MainWindow? mai configuration.FailOnUnhandledPort, loggerService, inputEventHub); emulatorMcpServices.Intel8042Controller = intel8042Controller; emulatorMcpServices.BiosKeyboardBuffer = biosKeyboardBuffer; + inputEventHub.KeyboardDevice = intel8042Controller.KeyboardDevice; BiosKeyboardInt9Handler biosKeyboardInt9Handler = new(memory, biosDataArea, stack, state, cfgCpu, dualPic, systemBiosInt15Handler, intel8042Controller, biosKeyboardBuffer, loggerService); @@ -795,6 +796,7 @@ internal Spice86DependencyInjection(Configuration configuration, MainWindow? mai if (mainWindow != null && uiDispatcher != null && hostStorageProvider != null && textClipboard != null && mainWindowViewModel != null) { + mainWindowViewModel.OnWindowDeactivated = () => inputEventHub.ReleaseAllKeys(); IMessenger messenger = WeakReferenceMessenger.Default; BreakpointsTabPlugin breakpointsTabPlugin = new(state, pauseHandler, messenger, diff --git a/src/Spice86/ViewModels/MainWindowViewModel.cs b/src/Spice86/ViewModels/MainWindowViewModel.cs index 4f94a6d4a5..d8bb23a366 100644 --- a/src/Spice86/ViewModels/MainWindowViewModel.cs +++ b/src/Spice86/ViewModels/MainWindowViewModel.cs @@ -106,6 +106,13 @@ public double? TimeMultiplier { /// Fired when this view model is being disposed so the composition root can tear down. internal event Action? Disposing; + /// + /// Thread-safe action that releases every key the keyboard device still believes is down. Invoked when + /// the window loses activation, so a key-up swallowed by the shell (e.g. Alt+Tab) does not latch a + /// modifier in the guest. Assigned by the composition root; null in headless mode. + /// + public Action? OnWindowDeactivated { get; set; } + public MainWindowViewModel( EmulatorDisplayViewModel display, EmulatorSession session, diff --git a/src/Spice86/Views/MainWindow.axaml.cs b/src/Spice86/Views/MainWindow.axaml.cs index 0bb967cc37..d84ec74d04 100644 --- a/src/Spice86/Views/MainWindow.axaml.cs +++ b/src/Spice86/Views/MainWindow.axaml.cs @@ -162,6 +162,9 @@ private void OnPointerCaptureLost(object? sender, PointerCaptureLostEventArgs e) private void OnWindowDeactivated(object? sender, EventArgs e) { ReleaseCapture(); + // The shell may swallow key-ups (e.g. Alt+Tab) while the window is inactive. Release any key the + // device still believes is down so the guest does not keep a modifier latched. + (DataContext as MainWindowViewModel)?.OnWindowDeactivated?.Invoke(); } private void OnWindowPositionChanged(object? sender, PixelPointEventArgs e) { diff --git a/tests/Spice86.Tests/Emulator/Devices/Keyboard/PS2KeyboardReleaseAllKeysTests.cs b/tests/Spice86.Tests/Emulator/Devices/Keyboard/PS2KeyboardReleaseAllKeysTests.cs new file mode 100644 index 0000000000..0fe5083e7b --- /dev/null +++ b/tests/Spice86.Tests/Emulator/Devices/Keyboard/PS2KeyboardReleaseAllKeysTests.cs @@ -0,0 +1,202 @@ +namespace Spice86.Tests.Emulator.Devices.Keyboard; + +using FluentAssertions; + +using Microsoft.Extensions.Logging; + +using NSubstitute; + +using Spice86.Core.Emulator.CPU; +using Spice86.Core.Emulator.Devices.ExternalInput; +using Spice86.Core.Emulator.Devices.Input.Keyboard; +using Spice86.Core.Emulator.IOPorts; +using Spice86.Core.Emulator.Memory; +using Spice86.Core.Emulator.VM; +using Spice86.Core.Emulator.VM.Breakpoint; +using Spice86.Core.Emulator.VM.Clock; +using Spice86.Core.Emulator.VM.DeviceScheduler; + +using System; +using System.Collections.Generic; +using System.Diagnostics; + +using Xunit; + +/// +/// Device-level tests for the PS/2 keyboard's held-key bookkeeping and its ability to reconcile +/// (release) keys whose key-up was lost. A lost BREAK scancode otherwise latches a modifier in the +/// guest forever, so releasing all held keys is the recovery path used on pause, window deactivation, +/// and buffer overflow. +/// +public sealed class PS2KeyboardReleaseAllKeysTests { + private const ushort DataPort = 0x60; + private const ushort StatusPort = 0x64; + + /// + /// A fully controllable emulated clock: the test advances time explicitly instead of the real + /// stopwatch, so the 8042's 0.3ms inter-byte delay fires deterministically. + /// + private sealed class ControllableClock : ClockBase { + private readonly Stopwatch _stopwatch = new(); + private double _offsetMs; + + public ControllableClock() : base(ClockJitter.Create(null), DateTimeOffset.UtcNow) { + _stopwatch.Start(); + } + + public override double ElapsedTimeMs => _offsetMs + _stopwatch.Elapsed.TotalMilliseconds; + protected override void OnPauseCore() => _stopwatch.Stop(); + protected override void OnResumeCore() => _stopwatch.Start(); + protected override void OnDisposeCore() => _stopwatch.Stop(); + + public void Advance(double ms) => _offsetMs += ms; + } + + private sealed class KeyboardFixture { + private readonly ControllableClock _clock = new(); + private readonly DeviceScheduler _scheduler; + + public KeyboardFixture() { + Logger = Substitute.For(); + CpuState = new State(CpuModel.ZET_86); + IOPortDispatcher dispatcher = new IOPortDispatcher(new AddressReadWriteBreakpoints(), CpuState, Logger, false); + DualPic = new DualPic(dispatcher, CpuState, Logger, false); + _scheduler = new DeviceScheduler(_clock, Logger, "test"); + Controller = new Intel8042Controller(CpuState, dispatcher, new A20Gate(true), DualPic, + _scheduler, failOnUnhandledPort: false, Logger); + } + + public ILogger Logger { get; } + public State CpuState { get; } + public DualPic DualPic { get; } + public Intel8042Controller Controller { get; } + public PS2Keyboard Keyboard => Controller.KeyboardDevice; + + /// + /// Advances the clock enough for the 8042's inter-byte delay to elapse, then pumps the scheduler + /// so the pending transfer fires. + /// + public void Pump() { + _clock.Advance(1); + _scheduler.ProcessEvents(); + } + + /// + /// Reads all keyboard bytes currently delivered by the controller (what the guest does in INT9). + /// Each read clears the pending-data flag and lets the controller hand over the next buffered + /// byte, so this drains the entire pending stream. + /// + public byte[] DrainKeyboard() { + List result = new(); + int guard = 4096; + while (guard-- > 0 && (Controller.ReadByte(StatusPort) & 0x01) != 0) { + result.Add(Controller.ReadByte(DataPort)); + Pump(); + } + + return result.ToArray(); + } + } + + [Fact] + public void ReleaseAllKeys_EmitsBreakFrameForHeldKey() { + KeyboardFixture fixture = new(); + PS2Keyboard keyboard = fixture.Keyboard; + + keyboard.EnqueueKeyEvent(PcKeyboardKey.LeftShift, isPressed: true); + fixture.Pump(); + keyboard.ReleaseAllKeys(); + + byte[] bytes = fixture.DrainKeyboard(); + + // The make frame (0x2A) plus a BREAK (0xAA) are both delivered; a release must not re-emit a make. + bytes.Should().Contain((byte)0x2A); + bytes.Should().Contain((byte)0xAA); + } + + [Fact] + public void ReleaseAllKeys_EmitsBreakFramesInDeterministicEnumOrder() { + KeyboardFixture fixture = new(); + PS2Keyboard keyboard = fixture.Keyboard; + + keyboard.EnqueueKeyEvent(PcKeyboardKey.LeftShift, isPressed: true); + keyboard.EnqueueKeyEvent(PcKeyboardKey.LeftCtrl, isPressed: true); + keyboard.EnqueueKeyEvent(PcKeyboardKey.A, isPressed: true); + fixture.Pump(); + keyboard.ReleaseAllKeys(); + + byte[] bytes = fixture.DrainKeyboard(); + + int a = Array.IndexOf(bytes, (byte)0x9E); // A break + int ctrl = Array.IndexOf(bytes, (byte)0x9D); // LeftCtrl break + int shift = Array.IndexOf(bytes, (byte)0xAA); // LeftShift break + + a.Should().BeGreaterThanOrEqualTo(0); + ctrl.Should().BeGreaterThanOrEqualTo(0); + shift.Should().BeGreaterThanOrEqualTo(0); + // Release order follows the PcKeyboardKey enum: A < LeftCtrl < LeftShift. + a.Should().BeLessThan(ctrl); + ctrl.Should().BeLessThan(shift); + } + + [Fact] + public void ReleaseAllKeys_WhenNothingHeld_EmitsNothing() { + KeyboardFixture fixture = new(); + + fixture.Keyboard.ReleaseAllKeys(); + + fixture.DrainKeyboard().Should().BeEmpty(); + } + + [Fact] + public void ReleaseAllKeys_IsIdempotent() { + KeyboardFixture fixture = new(); + PS2Keyboard keyboard = fixture.Keyboard; + + keyboard.EnqueueKeyEvent(PcKeyboardKey.LeftAlt, isPressed: true); + fixture.Pump(); + keyboard.ReleaseAllKeys(); + byte[] first = fixture.DrainKeyboard(); + + keyboard.ReleaseAllKeys(); + byte[] second = fixture.DrainKeyboard(); + + first.Should().Contain((byte)0xB8); // LeftAlt break + second.Should().BeEmpty(); // no held keys remain, so nothing more is released + } + + [Fact] + public void NormalKeyRelease_StopsBeingTrackedAsHeld() { + KeyboardFixture fixture = new(); + PS2Keyboard keyboard = fixture.Keyboard; + + keyboard.EnqueueKeyEvent(PcKeyboardKey.LeftShift, isPressed: true); + keyboard.EnqueueKeyEvent(PcKeyboardKey.LeftShift, isPressed: false); + fixture.Pump(); + byte[] released = fixture.DrainKeyboard(); + + keyboard.ReleaseAllKeys(); + byte[] afterRelease = fixture.DrainKeyboard(); + + released.Should().Contain((byte)0x2A); // normal make + released.Should().Contain((byte)0xAA); // normal key-up delivered the break + afterRelease.Should().BeEmpty(); // the key is no longer tracked as held + } + + [Fact] + public void MomentaryKeys_AreNotTrackedForRelease() { + KeyboardFixture fixture = new(); + PS2Keyboard keyboard = fixture.Keyboard; + + // Pause has no BREAK frame, so it must not be tracked as a held key. + keyboard.EnqueueKeyEvent(PcKeyboardKey.Pause, isPressed: true); + fixture.Pump(); + byte[] press = fixture.DrainKeyboard(); + + keyboard.ReleaseAllKeys(); + byte[] release = fixture.DrainKeyboard(); + + press.Should().NotBeEmpty(); // the pause make-chord is still delivered as usual + release.Should().BeEmpty(); // releasing all emits no break for the momentary pause key + } +}