From b6f59f5000d7e8c3e6a602b57397f5c383a8db33 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 6 Jul 2026 18:29:33 +0200 Subject: [PATCH] =?UTF-8?q?fix(apple):=20resolve=20macOS=20modifiers=20by?= =?UTF-8?q?=20keyCode=20=E2=80=94=20Control=20was=20silently=20dropped?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Modifier keys arrive only as flagsChanged, and the direction was recovered by diffing the device-dependent L/R bits (NX_DEVICE*KEYMASK) alone. Those bits are undocumented and some keyboards omit them (only the class bit, e.g. NX_CONTROLMASK, is set), so the diff saw no transition and the key never reached the host — no Ctrl shortcuts. SDL/Moonlight key off the event's keyCode for exactly this reason; do the same: keyCode names the changed key, the class bit says up, the device bits (when present) pick the side, and a tracked-held-state flip covers keyboards without them. PUNKTFUNK_INPUT_DEBUG=1 now also logs every flagsChanged (keyCode + raw flags) so a field report is diagnosable from client logs. Co-Authored-By: Claude Fable 5 --- .../PunktfunkKit/Input/InputCapture.swift | 90 +++++++++---------- .../Sources/PunktfunkKit/Input/KeyMaps.swift | 18 ++++ .../PunktfunkKit/Views/StreamView.swift | 7 +- .../ModifierResolveTests.swift | 87 ++++++++++++++++++ 4 files changed, 154 insertions(+), 48 deletions(-) create mode 100644 clients/apple/Tests/PunktfunkKitTests/ModifierResolveTests.swift diff --git a/clients/apple/Sources/PunktfunkKit/Input/InputCapture.swift b/clients/apple/Sources/PunktfunkKit/Input/InputCapture.swift index 0f4dca7..dfaa89f 100644 --- a/clients/apple/Sources/PunktfunkKit/Input/InputCapture.swift +++ b/clients/apple/Sources/PunktfunkKit/Input/InputCapture.swift @@ -84,15 +84,6 @@ public final class InputCapture { /// its Esc suppression need it in both states). private var cmdKeysDown: Set = [] - #if os(macOS) - /// Previous raw `NSEvent.modifierFlags.rawValue` (LOW 16 bits intact — those carry the - /// device-dependent L/R bits). Modifier keys never fire keyDown/keyUp on macOS; they - /// arrive as flagsChanged, which doesn't carry down-vs-up — we recover that by diffing - /// this snapshot. Resynced (not diffed) while forwarding is off so a modifier held - /// across a capture toggle can't produce a phantom transition on re-engage. - private var prevModFlags: UInt = 0 - #endif - /// While true, mouse/keyboard flow to the host and key NSEvents are swallowed /// locally; while false the user is interacting with the local UI (dragging the /// window, clicking the HUD) and nothing is forwarded. Main-queue only. @@ -279,12 +270,6 @@ public final class InputCapture { residualY = 0 residualScrollX = 0 residualScrollY = 0 - #if os(macOS) - // Drop the modifier snapshot too: a flagsChanged transition can be missed if focus - // leaves mid-chord, and the next handleFlagsChanged resyncs from a clean slate (it - // resyncs while released anyway, but this keeps stuck state from outliving a blur). - prevModFlags = 0 - #endif } /// Release any held MOUSE buttons host-side, leaving keyboard state untouched. Used when @@ -359,39 +344,52 @@ public final class InputCapture { } /// NSEvent modifier path (macOS): modifier keys never fire keyDown/keyUp — they arrive - /// as flagsChanged, which carries no down-vs-up. We diff the raw flags against the prior - /// snapshot to recover each transition, and the changed key's L/R identity from the - /// device-dependent bits in the LOW 16 bits (the .deviceIndependentFlagsMask the ⌘⎋ - /// monitor uses deliberately strips exactly these — do NOT pre-mask here). Each side maps - /// to the same L/R modifier VK `hidToVK` already emits, so the host needs no change. - /// Fed `UInt(event.modifierFlags.rawValue)`. - public func handleFlagsChanged(_ rawFlags: UInt) { - // While released we only resync the snapshot, so a modifier held across a capture - // toggle doesn't show up as a spurious transition the moment forwarding re-engages. - guard forwarding else { - prevModFlags = rawFlags - return + /// as flagsChanged, which carries no down-vs-up. `keyCode` names the key that changed + /// (kVK_Control & co., already L/R-specific); `resolveModifier` recovers the direction + /// from the flags. Fed `event.keyCode` + `UInt(event.modifierFlags.rawValue)` — LOW 16 + /// bits intact, they carry the device-dependent L/R bits (the .deviceIndependentFlagsMask + /// the ⌘⎋ monitor uses deliberately strips exactly these — do NOT pre-mask here). + public func handleFlagsChanged(keyCode: UInt16, rawFlags: UInt) { + if inputDebug { + inputLog.debug( + "flagsChanged keyCode \(keyCode, privacy: .public) flags 0x\(String(rawFlags, radix: 16), privacy: .public) forwarding \(self.forwarding, privacy: .public)") } - // (device-dependent mask, VK). LOW-16-bit masks from IOLLEvent.h (NX_DEVICE*MASK): - // Lshift 0x2 Rshift 0x4 | Lctrl 0x1 Rctrl 0x2000 | Lalt 0x20 Ralt 0x40 | Lcmd 0x8 Rcmd 0x10. - let table: [(UInt, UInt32)] = [ - (0x2, 0xA0), (0x4, 0xA1), // VK_LSHIFT / VK_RSHIFT - (0x1, 0xA2), (0x2000, 0xA3), // VK_LCONTROL / VK_RCONTROL - (0x20, 0xA4), (0x40, 0xA5), // VK_LMENU / VK_RMENU (left/right alt-option) - (0x8, 0x5B), (0x10, 0x5C), // VK_LWIN / VK_RWIN (left/right command) - ] - for (mask, vk) in table { - let now = (rawFlags & mask) != 0 - let was = (prevModFlags & mask) != 0 - guard now != was else { continue } - // Keep cmdKeysDown in step (the ⌘⎋ toggle + Esc suppression read it); sendKey - // adds the VK to pressedVKs so releaseAll/blur flushes a held modifier cleanly. - if vk == 0x5B || vk == 0x5C { - if now { cmdKeysDown.insert(vk) } else { cmdKeysDown.remove(vk) } - } - sendKey(vk, down: now) + guard forwarding else { return } + guard let (vk, down) = Self.resolveModifier( + keyCode: keyCode, rawFlags: rawFlags, isDown: { pressedVKs.contains($0) }) + else { return } // Fn / Caps Lock / unknown — nothing the host consumes on this path + // Keep cmdKeysDown in step (the ⌘⎋ toggle + Esc suppression read it); sendKey + // adds the VK to pressedVKs so releaseAll/blur flushes a held modifier cleanly. + if vk == 0x5B || vk == 0x5C { + if down { cmdKeysDown.insert(vk) } else { cmdKeysDown.remove(vk) } } - prevModFlags = rawFlags + sendKey(vk, down: down) + } + + /// Resolve one flagsChanged transition to (Windows VK, down). The changed key is + /// `keyCode`; the direction comes from the flags. The device-dependent L/R bits (LOW + /// 16 bits, NX_DEVICE*KEYMASK) disambiguate the two same-class keys, but some + /// keyboards ship flagsChanged WITHOUT them — only the device-independent class + /// bit (NX_CONTROLMASK & co.) is set. A pure diff of the device bits silently drops + /// those keys (seen live: Control never forwarded), so this is keyCode-driven with the + /// flags as evidence: class bit clear → the key went up; device bits present → they + /// say which side is held now; class bit set with NO device bits → flip the held state + /// we track (`isDown`, from pressedVKs — SDL ships the same fallback). Each keyCode + /// maps to the L/R modifier VK `hidToVK` already emits, so the host needs no change. + /// Returns nil for modifiers the host doesn't consume on this path (Fn, Caps Lock). + static func resolveModifier( + keyCode: UInt16, rawFlags: UInt, isDown: (UInt32) -> Bool + ) -> (vk: UInt32, down: Bool)? { + guard let mod = modifierBits[keyCode] else { return nil } + let down: Bool + if rawFlags & mod.classMask == 0 { + down = false + } else if rawFlags & (mod.deviceBit | mod.siblingBit) != 0 { + down = rawFlags & mod.deviceBit != 0 + } else { + down = !isDown(mod.vk) + } + return (mod.vk, down) } #endif diff --git a/clients/apple/Sources/PunktfunkKit/Input/KeyMaps.swift b/clients/apple/Sources/PunktfunkKit/Input/KeyMaps.swift index 6ccceab..5103bfe 100644 --- a/clients/apple/Sources/PunktfunkKit/Input/KeyMaps.swift +++ b/clients/apple/Sources/PunktfunkKit/Input/KeyMaps.swift @@ -98,5 +98,23 @@ extension InputCapture { m[0x47] = 0x90 // KP clear sits where NumLock is → VK_NUMLOCK. (KP equals 0x51 dropped.) return m }() + + /// NSEvent.keyCode of each modifier key (kVK_Shift & co. — modifiers arrive only as + /// flagsChanged) → its Windows VK plus the `NSEvent.modifierFlags` bits that describe + /// it: `classMask` is the device-INDEPENDENT NX_*MASK for the modifier class, + /// `deviceBit`/`siblingBit` the device-dependent bits (LOW 16 bits, NX_DEVICE*KEYMASK + /// in IOLLEvent.h) for this key and its opposite-side twin. Consumed by + /// `resolveModifier`, which explains why both kinds of bit are needed. + static let modifierBits: + [UInt16: (vk: UInt32, classMask: UInt, deviceBit: UInt, siblingBit: UInt)] = [ + 56: (0xA0, 0x2_0000, 0x2, 0x4), // left shift → VK_LSHIFT + 60: (0xA1, 0x2_0000, 0x4, 0x2), // right shift → VK_RSHIFT + 59: (0xA2, 0x4_0000, 0x1, 0x2000), // left control → VK_LCONTROL + 62: (0xA3, 0x4_0000, 0x2000, 0x1), // right control → VK_RCONTROL + 58: (0xA4, 0x8_0000, 0x20, 0x40), // left option → VK_LMENU + 61: (0xA5, 0x8_0000, 0x40, 0x20), // right option → VK_RMENU + 55: (0x5B, 0x10_0000, 0x8, 0x10), // left command → VK_LWIN + 54: (0x5C, 0x10_0000, 0x10, 0x8), // right command → VK_RWIN + ] #endif } diff --git a/clients/apple/Sources/PunktfunkKit/Views/StreamView.swift b/clients/apple/Sources/PunktfunkKit/Views/StreamView.swift index 441211c..27a428d 100644 --- a/clients/apple/Sources/PunktfunkKit/Views/StreamView.swift +++ b/clients/apple/Sources/PunktfunkKit/Views/StreamView.swift @@ -346,10 +346,13 @@ public final class StreamLayerView: NSView { super.keyUp(with: event) } /// Modifier keys (shift/control/option/command) arrive ONLY as flagsChanged on macOS, - /// never keyDown/keyUp — InputCapture diffs the raw flags to recover each L/R down/up. + /// never keyDown/keyUp — the changed key is `event.keyCode`; InputCapture resolves the + /// down-vs-up direction from the flags (diffing the device-dependent flag bits alone + /// proved unreliable — some keyboards omit them, which silently dropped Control). public override func flagsChanged(with event: NSEvent) { if captured, let inputCapture { - inputCapture.handleFlagsChanged(UInt(event.modifierFlags.rawValue)) + inputCapture.handleFlagsChanged( + keyCode: event.keyCode, rawFlags: UInt(event.modifierFlags.rawValue)) return } super.flagsChanged(with: event) diff --git a/clients/apple/Tests/PunktfunkKitTests/ModifierResolveTests.swift b/clients/apple/Tests/PunktfunkKitTests/ModifierResolveTests.swift new file mode 100644 index 0000000..4494d3c --- /dev/null +++ b/clients/apple/Tests/PunktfunkKitTests/ModifierResolveTests.swift @@ -0,0 +1,87 @@ +#if os(macOS) +import XCTest + +@testable import PunktfunkKit + +/// Pins the macOS flagsChanged → modifier-VK resolution (InputCapture.resolveModifier). +/// Modifier keys arrive only as flagsChanged, which carries no down-vs-up: the changed key +/// is the event's keyCode, and the direction is recovered from the flag bits — with a +/// held-state fallback for keyboards that omit the device-dependent L/R bits (the gap that +/// used to silently drop Control when the transition was diffed from those bits alone). +final class ModifierResolveTests: XCTestCase { + /// Resolve with a fixed already-held answer for the fallback path. + private func resolve( + keyCode: UInt16, rawFlags: UInt, held: Bool = false + ) -> (vk: UInt32, down: Bool)? { + InputCapture.resolveModifier(keyCode: keyCode, rawFlags: rawFlags) { _ in held } + } + + // MARK: Keyboards that report the device-dependent L/R bits (the common case) + + func testControlPressAndReleaseWithDeviceBits() { + // Real left-Control down: NX_CONTROLMASK | NX_DEVICELCTLKEYMASK (+ misc low bits). + let down = resolve(keyCode: 59, rawFlags: 0x4_0101) + XCTAssertEqual(down?.vk, 0xA2) // VK_LCONTROL + XCTAssertEqual(down?.down, true) + // Release: the class mask is gone entirely. + let up = resolve(keyCode: 59, rawFlags: 0x100) + XCTAssertEqual(up?.vk, 0xA2) + XCTAssertEqual(up?.down, false) + } + + func testRightControlUsesItsOwnDeviceBit() { + let down = resolve(keyCode: 62, rawFlags: 0x4_2000) + XCTAssertEqual(down?.vk, 0xA3) // VK_RCONTROL + XCTAssertEqual(down?.down, true) + } + + func testReleasingOneOfTwoHeldControls() { + // Left goes up while right stays held: class mask still set, right device bit + // still set, LEFT device bit cleared — the left key must resolve as UP. + let leftUp = resolve(keyCode: 59, rawFlags: 0x4_2000, held: true) + XCTAssertEqual(leftUp?.vk, 0xA2) + XCTAssertEqual(leftUp?.down, false) + } + + func testEverySideMapsToItsOwnVK() { + XCTAssertEqual(resolve(keyCode: 56, rawFlags: 0x2_0002)?.vk, 0xA0) // VK_LSHIFT + XCTAssertEqual(resolve(keyCode: 60, rawFlags: 0x2_0004)?.vk, 0xA1) // VK_RSHIFT + XCTAssertEqual(resolve(keyCode: 58, rawFlags: 0x8_0020)?.vk, 0xA4) // VK_LMENU + XCTAssertEqual(resolve(keyCode: 61, rawFlags: 0x8_0040)?.vk, 0xA5) // VK_RMENU + XCTAssertEqual(resolve(keyCode: 55, rawFlags: 0x10_0008)?.vk, 0x5B) // VK_LWIN + XCTAssertEqual(resolve(keyCode: 54, rawFlags: 0x10_0010)?.vk, 0x5C) // VK_RWIN + for (_, down) in [56, 60, 58, 61, 55, 54].compactMap({ + self.resolve(keyCode: UInt16($0), rawFlags: 0xFF_FFFF) + }) { + XCTAssertTrue(down) + } + } + + // MARK: Keyboards that DON'T report the device bits (the bug this resolver fixes) + + func testControlPressWithoutDeviceBitsFallsBackToHeldState() { + // Only NX_CONTROLMASK, no low bits at all: a flag diff of the device bits sees no + // transition and drops the key — the fallback must infer DOWN from "not held yet". + let down = resolve(keyCode: 59, rawFlags: 0x4_0000, held: false) + XCTAssertEqual(down?.vk, 0xA2) + XCTAssertEqual(down?.down, true) + // And the mirror release (class cleared) still resolves as UP. + let up = resolve(keyCode: 59, rawFlags: 0, held: true) + XCTAssertEqual(up?.down, false) + } + + func testClassBitStillSetButKeyAlreadyHeldResolvesUp() { + // Device-bit-less keyboard, second same-class key still holding the class bit: + // the best available answer for the key that changed is to flip its held state. + let up = resolve(keyCode: 59, rawFlags: 0x4_0000, held: true) + XCTAssertEqual(up?.down, false) + } + + // MARK: Modifiers the host doesn't consume on this path + + func testFnAndCapsLockResolveToNothing() { + XCTAssertNil(resolve(keyCode: 63, rawFlags: 0x80_0000)) // Fn / Globe + XCTAssertNil(resolve(keyCode: 57, rawFlags: 0x1_0000)) // Caps Lock + } +} +#endif