From ee2ab6426bc862324c03e9cb478198c8b92252be Mon Sep 17 00:00:00 2001 From: berkaymuvaffak Date: Sat, 22 Aug 2026 02:33:46 +0300 Subject: [PATCH 1/4] feat(hid): gesture from the DPI button where no gesture control exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MX Vertical ships no dedicated gesture button. Its `0x1b04` list carries no `0x00c3` at all, its `0x00d7` (virtual gesture button) is listed but never emits an event, and `0x00fd` — the DPI/ModeShift switch — is the only control there flagged `raw-xy`. That is also the control Logitech's own software gestures from on this model, corroborated by the device's asset metadata, which ships `SLOT_NAME_GESTURE_*_BUTTON` markers next to `SLOT_NAME_DPI_BUTTON`. Add the DPI/ModeShift family to `GESTURE_SOURCE_BUTTONS` and to `ButtonId::is_hidpp_gesture_source`, so a gesture map bound to `ButtonId::DpiToggle` arms a raw-XY divert and dispatches swipes. DPI cycling remains the out-of-the-box behavior: `default_binding_for` returns `Single` for that button and a `Single` shape drops out of the gesture-map lookup, pinned by a test so it cannot regress silently. Also guard the capture session's DPI pass against re-arming a CID already armed as a raw-XY gesture source. That write carries no raw-XY and would strip the reporting the hold depends on — reachable exactly on a device whose DPI button *is* its gesture source. Verified on MX Vertical over Bluetooth-direct. Before: ten presses gave 19 `button=DpiToggle action=Cycle DPI Presets` events and no gestures. After: `gesture_sources=1 dpi_buttons=0`, and all four directions plus click dispatch their bound actions. --- crates/openlogi-core/src/binding/button.rs | 20 ++++++++++++------ crates/openlogi-core/src/binding/tests.rs | 11 ++++++++++ crates/openlogi-device/src/session/gesture.rs | 21 ++++++++++++++++--- .../src/session/gesture/tests.rs | 13 ++++++++++++ 4 files changed, 56 insertions(+), 9 deletions(-) diff --git a/crates/openlogi-core/src/binding/button.rs b/crates/openlogi-core/src/binding/button.rs index c3183a6ac..739e99521 100644 --- a/crates/openlogi-core/src/binding/button.rs +++ b/crates/openlogi-core/src/binding/button.rs @@ -118,14 +118,22 @@ impl ButtonId { ) } - /// Whether this button is a HID++ gesture source — a control that is - /// captured over HID++ raw-XY diversion (never the OS hook) and can - /// therefore own the gesture role with swipe directions: the dedicated - /// gesture button, or the MX Master 4 haptic panel. The capture layer maps - /// each to its control ID. + /// Whether this button is a HID++ gesture source — a control captured over + /// raw-XY diversion (never the OS hook) and so able to own the gesture role + /// with swipe directions: the dedicated gesture button, the MX Master 4 + /// haptic panel, or the DPI/ModeShift button on a device that ships neither. + /// The capture layer maps each to its control ID. + /// + /// [`ButtonId::DpiToggle`] qualifies because on such a device it is the only + /// raw-XY-capable control (MX Vertical's `0x00fd`). It only *acts* as one + /// once a gesture map is bound to it: its canonical default is a single + /// DPI-cycle action, which drops out of the gesture-map lookup. #[must_use] pub fn is_hidpp_gesture_source(self) -> bool { - matches!(self, ButtonId::GestureButton | ButtonId::HapticPanel) + matches!( + self, + ButtonId::GestureButton | ButtonId::HapticPanel | ButtonId::DpiToggle + ) } /// Human-readable label for popovers and tooltips. diff --git a/crates/openlogi-core/src/binding/tests.rs b/crates/openlogi-core/src/binding/tests.rs index 6b854f07a..9a5c91916 100644 --- a/crates/openlogi-core/src/binding/tests.rs +++ b/crates/openlogi-core/src/binding/tests.rs @@ -544,3 +544,14 @@ fn scroll_actions_lower_to_unit_direction() { Effect::Scroll { dx: 1, dy: 0 } ); } + +#[test] +fn dpi_toggle_is_a_gesture_source_yet_defaults_to_a_single_action() { + // The `Single` default is what keeps DPI cycling the behavior every existing + // device still gets: it drops out of the gesture-map lookup. + assert!(ButtonId::DpiToggle.is_hidpp_gesture_source()); + assert!(matches!( + default_binding_for(ButtonId::DpiToggle), + Binding::Single(_) + )); +} diff --git a/crates/openlogi-device/src/session/gesture.rs b/crates/openlogi-device/src/session/gesture.rs index bb82640b6..4a324a89a 100644 --- a/crates/openlogi-device/src/session/gesture.rs +++ b/crates/openlogi-device/src/session/gesture.rs @@ -142,13 +142,22 @@ pub const DIVERTABLE_STANDARD_BUTTONS: [(u16, ButtonId); 3] = [ ]; /// HID++ gesture sources: the `0x1b04` control ID and the [`ButtonId`] it -/// delivers — the dedicated gesture button on most MX mice, and the Haptic -/// Sense Panel on MX Master 4 (two distinct physical controls). Each source in +/// delivers — the dedicated gesture button, the MX Master 4 Haptic Sense Panel, +/// and the DPI/ModeShift button on a model that ships neither. Each source in /// gesture mode is diverted with raw-XY; one with a non-default single binding /// instead is plain-diverted like a standard button. -pub const GESTURE_SOURCE_BUTTONS: [(u16, ButtonId); 2] = [ +/// +/// The DPI/ModeShift family is listed here *and* in the plain DPI capture +/// because on a device without a gesture button it is the only raw-XY-capable +/// control there is (MX Vertical: no `0x00c3`, `0x00d7` never emits, `0x00fd` +/// is `raw-xy`). It only gestures once a map is bound to +/// [`ButtonId::DpiToggle`], and the divert loop skips CIDs a device lacks. +pub const GESTURE_SOURCE_BUTTONS: [(u16, ButtonId); 5] = [ (reprog_controls::GESTURE_BUTTON_CID, ButtonId::GestureButton), (reprog_controls::HAPTIC_PANEL_CID, ButtonId::HapticPanel), + (reprog_controls::DPI_MODE_SHIFT_CIDS[0], ButtonId::DpiToggle), + (reprog_controls::DPI_MODE_SHIFT_CIDS[1], ButtonId::DpiToggle), + (reprog_controls::DPI_MODE_SHIFT_CIDS[2], ButtonId::DpiToggle), ]; /// Which of one device's controls a capture session should divert. @@ -478,6 +487,12 @@ async fn arm_controls_into( } } for &cid in &reprog_controls::DPI_MODE_SHIFT_CIDS { + // This write carries no raw-XY, so re-arming a CID already armed as + // a gesture source would strip the reporting its hold depends on — + // the same hazard the `divert_buttons` loop guards against below. + if armed.gesture_cids.contains(&cid) { + continue; + } if controls.iter().any(|c| c.cid == cid && c.is_divertable()) { let reporting = arm_reprog_control(&rc, cid, false).await?; armed.reporting.push(reporting); diff --git a/crates/openlogi-device/src/session/gesture/tests.rs b/crates/openlogi-device/src/session/gesture/tests.rs index 8925f5759..97901216d 100644 --- a/crates/openlogi-device/src/session/gesture/tests.rs +++ b/crates/openlogi-device/src/session/gesture/tests.rs @@ -618,3 +618,16 @@ fn contact_without_rotation_or_a_tap_carries_no_input() { None ); } + +#[test] +fn dpi_cids_are_gesture_sources_dispatching_as_the_dpi_toggle() { + // An unresolved CID is dropped rather than misattributed, so a device that + // gestures from its DPI button would never begin a hold. + for &cid in &reprog_controls::DPI_MODE_SHIFT_CIDS { + assert_eq!( + gesture_source_button(cid), + Some(ButtonId::DpiToggle), + "cid {cid:#06x} must dispatch as the DPI toggle" + ); + } +} From ff6ef8b44fef322f632e17afae1deeb73e218d87 Mon Sep 17 00:00:00 2001 From: berkaymuvaffak Date: Sat, 22 Aug 2026 02:33:46 +0300 Subject: [PATCH 2/4] fix(gui): map the DPI-button slot name MX Vertical reports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `map_slot_name` knew only `SLOT_NAME_MODESHIFT_BUTTON`, the MX Master line's name for that control. MX Vertical calls the same button `SLOT_NAME_DPI_BUTTON`, so it produced no hotspot: the device rendered with no editable control for its only extra button and could not be bound from the GUI at all. The five `SLOT_NAME_GESTURE_*_BUTTON` markers that model also ships stay unmapped on purpose — they mark the control the DPI marker already covers, this builder does not dedupe, and the picker renders swipe directions from the binding's gesture map. Both halves are pinned by tests. --- .../src/features/mouse/geometry.rs | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/crates/openlogi-desktop/src/features/mouse/geometry.rs b/crates/openlogi-desktop/src/features/mouse/geometry.rs index 3395543a4..e44a23146 100644 --- a/crates/openlogi-desktop/src/features/mouse/geometry.rs +++ b/crates/openlogi-desktop/src/features/mouse/geometry.rs @@ -183,9 +183,18 @@ fn map_slot_name(name: &str) -> Option { "SLOT_NAME_MIDDLE_BUTTON" => Some(MouseControlId::Button(ButtonId::MiddleClick)), "SLOT_NAME_BACK_BUTTON" => Some(MouseControlId::Button(ButtonId::Back)), "SLOT_NAME_FORWARD_BUTTON" => Some(MouseControlId::Button(ButtonId::Forward)), - "SLOT_NAME_MODESHIFT_BUTTON" => Some(MouseControlId::Button(ButtonId::DpiToggle)), + // Two names for one control: the MX Master line calls it ModeShift, a + // model whose only extra button *is* the DPI switch names it directly. + "SLOT_NAME_MODESHIFT_BUTTON" | "SLOT_NAME_DPI_BUTTON" => { + Some(MouseControlId::Button(ButtonId::DpiToggle)) + } "SLOT_NAME_THUMBWHEEL" => Some(MouseControlId::ThumbwheelRotation), "SLOT_NAME_GESTURE_BUTTON" => Some(MouseControlId::Button(ButtonId::GestureButton)), + // Deliberately unmapped: a model with no gesture button also ships a + // marker per swipe direction (`SLOT_NAME_GESTURE_*_BUTTON`), all marking + // the control its `SLOT_NAME_DPI_BUTTON` already covers. Mapping them + // would stack five duplicate hotspots — this builder does not dedupe — + // and the picker renders directions from the binding's gesture map. // The MX Master 4 Haptic Sense Panel. Logi names the slot after its // Options+ default assignment (the radial Actions Ring menu), but the // marker is the panel itself. @@ -223,6 +232,37 @@ mod tests { ); } + #[test] + fn both_dpi_slot_names_resolve_to_the_dpi_toggle() { + // Missing either name leaves that button with no hotspot at all. + for name in ["SLOT_NAME_MODESHIFT_BUTTON", "SLOT_NAME_DPI_BUTTON"] { + assert_eq!( + map_slot_name(name), + Some(MouseControlId::Button(ButtonId::DpiToggle)), + "{name} must resolve to the DPI toggle" + ); + } + } + + #[test] + fn per_direction_gesture_slots_do_not_become_hotspots() { + // They mark the control the DPI marker covers, and the builder does not + // dedupe, so mapping them would stack duplicate hotspots. + for name in [ + "SLOT_NAME_GESTURE_UP_BUTTON", + "SLOT_NAME_GESTURE_DOWN_BUTTON", + "SLOT_NAME_GESTURE_LEFT_BUTTON", + "SLOT_NAME_GESTURE_RIGHT_BUTTON", + "SLOT_NAME_GESTURE_CLICK_BUTTON", + ] { + assert_eq!( + map_slot_name(name), + None, + "{name} must not become a hotspot" + ); + } + } + #[test] fn labels_track_hotspots_and_avoid_crossing() { let hotspots = default_hotspots(true); From 7442d9735985992c781b42ff0ccbe24e890c189e Mon Sep 17 00:00:00 2001 From: berkaymuvaffak Date: Sat, 22 Aug 2026 02:33:46 +0300 Subject: [PATCH 3/4] feat(hook): invert scroll in software when firmware cannot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scroll inversion was gated on HID++ `0x2121` (HiResWheel) reporting an invert capability, so a device without that feature showed a permanently "Unavailable" toggle. MX Vertical reports 30 features and none is `0x2121`, putting the setting out of reach on that hardware. Add `EventDisposition::InvertScroll`. macOS negates the `CGEvent`'s delta fields in place, so the event keeps its phase, momentum and pixel precision and no synthetic replacement re-enters the tap. Only fields the event actually carries are negated: writing an axis the device left empty would introduce a delta an app reading that field would honour. The hook rewrites only devices in `HookMaps::invert_scroll`, which the orchestrator builds from the config while excluding natively-capable ones — for those the setting goes to the firmware, and rewriting on top would invert twice and cancel out. Trackpad scroll is never touched, since macOS already applies its own natural-scrolling preference there. Inversion is keyed per device rather than scoped to the selected one, so disabling the selected device empties the button maps without stopping another device's inversion. Both properties are covered by tests. evdev and `WH_MOUSE_LL` have no rewrite path yet; both pass the new disposition through rather than swallowing the scroll. --- .../openlogi-agent-core/src/hook_runtime.rs | 33 ++++++++-- .../openlogi-agent-core/src/orchestrator.rs | 41 ++++++++++-- .../src/orchestrator/tests.rs | 64 +++++++++++++++++++ crates/openlogi-desktop/src/state/scroll.rs | 22 +++++-- crates/openlogi-hook/src/lib.rs | 10 +++ crates/openlogi-hook/src/linux.rs | 6 +- crates/openlogi-hook/src/macos.rs | 30 +++++++++ crates/openlogi-hook/src/windows.rs | 12 +++- 8 files changed, 199 insertions(+), 19 deletions(-) diff --git a/crates/openlogi-agent-core/src/hook_runtime.rs b/crates/openlogi-agent-core/src/hook_runtime.rs index 8eafe377f..a4629c9f7 100644 --- a/crates/openlogi-agent-core/src/hook_runtime.rs +++ b/crates/openlogi-agent-core/src/hook_runtime.rs @@ -6,7 +6,7 @@ //! and gesture events. use std::cell::RefCell; -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::sync::mpsc; use std::sync::{Arc, Mutex, PoisonError, RwLock}; use std::thread; @@ -89,10 +89,16 @@ pub struct HookMaps { /// Per-button single action — the single-action dispatch path. pub bindings: BTreeMap, /// Per-direction maps for the OS-hook gesture buttons (Middle/Back/Forward in - /// gesture mode), so a hold+swipe resolves to a bound action. The dedicated - /// HID++ gesture button (0x00c3) uses the gesture watcher's separate map - /// instead — it never reaches the OS hook. + /// gesture mode), so a hold+swipe resolves to a bound action. The HID++ + /// gesture sources use the gesture watcher's separate map instead — a + /// diverted control never reaches the OS hook. pub gestures: BTreeMap>, + /// `(vendor_id, product_id)` of each device whose wheel is inverted in + /// software, for firmware reporting no native HID++ inversion (`0x2121`). + /// Keyed by OS-level identity because that is all the hook sees — a scroll + /// event carries an [`openlogi_hook::EventDevice`], not a config key — and a + /// set rather than one bool is what keeps the inversion per-device. + pub invert_scroll: BTreeSet<(u32, u32)>, } /// Shared, atomically-published [`HookMaps`], threaded between the config owner @@ -406,8 +412,23 @@ pub fn start( EventDisposition::PassThrough } MouseEvent::Scroll { - delta_x, delta_y, .. + delta_x, + delta_y, + from_trackpad, + device, } => { + // Trackpad scroll is never rewritten: macOS already applies + // its own natural-scrolling preference there, so inverting + // on top would fight the OS. + if !from_trackpad + && let Some(ids) = + device.as_ref().and_then(|d| d.vendor_id.zip(d.product_id)) + && hooks + .try_read() + .is_ok_and(|maps| maps.invert_scroll.contains(&ids)) + { + return EventDisposition::InvertScroll; + } #[cfg(not(target_os = "windows"))] let _ = (delta_x, delta_y); #[cfg(target_os = "windows")] @@ -796,6 +817,7 @@ mod tests { (ButtonId::ThumbwheelScrollDown, Action::PrevTab), ]), gestures: BTreeMap::new(), + ..Default::default() }; assert_eq!( rebound_thumbwheel_action(&maps, 1.0), @@ -822,6 +844,7 @@ mod tests { ), ]), gestures: BTreeMap::new(), + ..Default::default() }; assert_eq!(rebound_thumbwheel_action(&maps, 1.0), None); assert_eq!(rebound_thumbwheel_action(&maps, -1.0), None); diff --git a/crates/openlogi-agent-core/src/orchestrator.rs b/crates/openlogi-agent-core/src/orchestrator.rs index 8eadbb5b6..ddae4e803 100644 --- a/crates/openlogi-agent-core/src/orchestrator.rs +++ b/crates/openlogi-agent-core/src/orchestrator.rs @@ -10,7 +10,7 @@ //! [`DpiCycleState::capabilities`] stays `None` and presets cycle at their raw //! (still valid) values — exactly the GUI's "window never opened" behaviour. -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, RwLock}; @@ -241,18 +241,49 @@ impl Orchestrator { /// so they're built together here and published under one lock — keeping /// `rebuild` and `set_current_app` from drifting into a half-populated write. fn hook_maps_for(&self, key: Option<&str>, app: Option<&str>) -> HookMaps { - // A disabled selected device gets empty maps: the OS hook then passes - // its events through untouched instead of applying remaps to a device - // the user asked OpenLogi to leave alone. + // A disabled selected device gets empty button maps so the hook stops + // remapping it. Inversion is exempt: it is keyed per device, so muting + // this one must not stop inverting another's wheel (and the builder + // already drops a device that is itself disabled). if key.is_some_and(|k| !self.config.device_enabled(k)) { - return HookMaps::default(); + return HookMaps { + invert_scroll: self.software_scroll_inversion(), + ..Default::default() + }; } HookMaps { bindings: bindings_for(&self.config, key, app), gestures: oshook_gestures_for(&self.config, key, app), + invert_scroll: self.software_scroll_inversion(), } } + /// `(vendor_id, product_id)` of every device with scroll inversion on but no + /// native HID++ inversion to carry it — the set the OS hook rewrites deltas + /// for. Not app-scoped, unlike the button maps: inversion belongs to the + /// device, not the foreground app. + /// + /// Natively-capable devices are excluded because their setting goes to the + /// firmware (`scroll_settings_for`); rewriting on top would invert twice. + /// Only [`DeviceRoute::Direct`] devices qualify — a receiver-paired device + /// reports the receiver's ids, which would match every device on that + /// dongle. + fn software_scroll_inversion(&self) -> BTreeSet<(u32, u32)> { + self.devices + .iter() + .filter(|dev| self.config.device_enabled(&dev.config_key)) + .filter(|dev| !dev.capabilities.is_some_and(|caps| caps.scroll_inversion)) + .filter(|dev| self.config.invert_scroll(&dev.config_key)) + .filter_map(|dev| match dev.route { + Some(DeviceRoute::Direct { + vendor_id, + product_id, + }) => Some((u32::from(vendor_id), u32::from(product_id))), + _ => None, + }) + .collect() + } + /// The keyboard key-capture spec for the first known keyboard, or `None` /// when no keyboard is paired or none of its capturable keys carries a /// real binding (an unbound key must never be diverted). diff --git a/crates/openlogi-agent-core/src/orchestrator/tests.rs b/crates/openlogi-agent-core/src/orchestrator/tests.rs index 4c25ccd83..60f98cca3 100644 --- a/crates/openlogi-agent-core/src/orchestrator/tests.rs +++ b/crates/openlogi-agent-core/src/orchestrator/tests.rs @@ -775,3 +775,67 @@ fn app_switch_republishes_capture_plans() { orch.set_current_app(Some("com.example.editor".into())); assert_eq!(published_back_binding(&orch), Some(Action::Undo)); } + +/// A directly-attached mouse with inversion configured, either reporting native +/// HID++ inversion or not. +fn direct_dev(key: &str, product_id: u16, native_inversion: bool) -> AgentDevice { + AgentDevice { + route: Some(DeviceRoute::Direct { + vendor_id: 0x046d, + product_id, + }), + capabilities: Some(Capabilities { + scroll_inversion: native_inversion, + ..Capabilities::default() + }), + ..dev(key, DIRECT_DEVICE_INDEX, true) + } +} + +#[test] +fn software_scroll_inversion_skips_native_and_receiver_paired_devices() { + use std::collections::BTreeSet; + + let mut config = Config::default(); + for key in ["native", "software", "receiver"] { + config.set_invert_scroll(key, true); + } + let mut orch = orchestrator(config); + orch.devices = vec![ + // Native inversion goes to the firmware; rewriting on top would cancel. + direct_dev("native", 0xb034, true), + direct_dev("software", 0xb020, false), + // A receiver route reports ids shared by every device on that dongle. + dev("receiver", 1, true), + ]; + + assert_eq!( + orch.software_scroll_inversion(), + BTreeSet::from([(0x046d_u32, 0xb020_u32)]) + ); +} + +#[test] +fn disabling_the_selected_device_keeps_another_devices_software_inversion() { + use std::collections::BTreeSet; + + // Inversion is keyed per device, so muting one cannot stop inverting another. + let mut config = Config::default(); + config.set_invert_scroll("other", true); + config.set_device_enabled("selected", false); + let mut orch = orchestrator(config); + orch.devices = vec![ + direct_dev("selected", 0xb034, false), + direct_dev("other", 0xb020, false), + ]; + + let maps = orch.hook_maps_for(Some("selected"), None); + assert!( + maps.bindings.is_empty(), + "a disabled selection remaps nothing" + ); + assert_eq!( + maps.invert_scroll, + BTreeSet::from([(0x046d_u32, 0xb020_u32)]) + ); +} diff --git a/crates/openlogi-desktop/src/state/scroll.rs b/crates/openlogi-desktop/src/state/scroll.rs index 070e281ec..ac9af4c6e 100644 --- a/crates/openlogi-desktop/src/state/scroll.rs +++ b/crates/openlogi-desktop/src/state/scroll.rs @@ -17,19 +17,29 @@ impl AppState { .and_then(DeviceRecord::persistent_config_key) .is_some_and(|key| self.config.invert_scroll(key)) } - /// Whether the active device reports native HID++ wheel inversion support. + /// Whether the active device's wheel can be inverted at all — natively over + /// HID++ `0x2121`, or in software where the hook can rewrite scroll deltas. + /// + /// macOS rewrites the `CGEvent`, so inversion is offered for any pointing + /// device there, including firmware with no `0x2121` (MX Vertical, whose + /// toggle would otherwise read "Unavailable" forever). evdev and + /// `WH_MOUSE_LL` have no rewrite path, so there this stays gated on the + /// native capability. No capability snapshot yet means `false` everywhere. #[must_use] pub fn current_scroll_inversion_supported(&self) -> bool { self.current_record() .and_then(|record| record.capabilities) - .is_some_and(|capabilities| capabilities.scroll_inversion) + .is_some_and(|capabilities| { + capabilities.scroll_inversion || (cfg!(target_os = "macos") && capabilities.pointer) + }) } - /// Set the active device's scroll-wheel inversion, persist it, and reload - /// the agent so it writes the device's native HID++ wheel inversion. No-op - /// when no device is selected or the active device does not report support. + /// Set the active device's scroll-wheel inversion, persist it, and reload the + /// agent so it either writes the device's native HID++ wheel inversion or + /// republishes the hook's software-inversion set. No-op when no device is + /// selected or the active device does not support inversion either way. pub fn commit_invert_scroll(&mut self, invert: bool) { if !self.current_scroll_inversion_supported() { - debug!("active device does not support native scroll inversion"); + debug!("active device supports neither native nor software scroll inversion"); return; } let Some(key) = self diff --git a/crates/openlogi-hook/src/lib.rs b/crates/openlogi-hook/src/lib.rs index a30b7838e..af2e251da 100644 --- a/crates/openlogi-hook/src/lib.rs +++ b/crates/openlogi-hook/src/lib.rs @@ -192,6 +192,16 @@ pub enum EventDisposition { PassThrough, /// Drop the event; the target application never sees it. Suppress, + /// Negate this scroll event's deltas in place, then let it continue — + /// software wheel inversion for firmware with no native HID++ inversion + /// (`0x2121`). Only meaningful for [`MouseEvent::Scroll`]; any other class + /// treats it as [`Self::PassThrough`]. + /// + /// macOS rewrites the `CGEvent` rather than suppressing and re-posting, so + /// the scroll keeps its phase, momentum and pixel-precision fields and no + /// synthetic event re-enters the tap. Linux and Windows have no rewrite path + /// yet and pass it through. + InvertScroll, } /// Where in the event stream a tap is inserted (macOS `CGEventTapLocation`). diff --git a/crates/openlogi-hook/src/linux.rs b/crates/openlogi-hook/src/linux.rs index 4a5ef0739..11fcc1d16 100644 --- a/crates/openlogi-hook/src/linux.rs +++ b/crates/openlogi-hook/src/linux.rs @@ -519,7 +519,11 @@ fn device_thread( None => EventDisposition::PassThrough, }; match disposition { - EventDisposition::PassThrough => pending.push(event), + // No evdev rewrite path for `InvertScroll` yet; pass it + // through rather than silently eating the scroll. + EventDisposition::PassThrough | EventDisposition::InvertScroll => { + pending.push(event); + } EventDisposition::Suppress => {} } } diff --git a/crates/openlogi-hook/src/macos.rs b/crates/openlogi-hook/src/macos.rs index 209e1eac8..d849967dc 100644 --- a/crates/openlogi-hook/src/macos.rs +++ b/crates/openlogi-hook/src/macos.rs @@ -446,6 +446,27 @@ fn usable_scroll_delta(event: &CGEvent, axis: ScrollAxisFields) -> f64 { event.get_integer_value_field(axis.line) as f64 } +/// Negate whichever of one axis's delta fields the event carries — the in-place +/// half of [`EventDisposition::InvertScroll`]. +/// +/// All three must be touched (see [`ScrollAxisFields`]), but only where already +/// non-zero: writing a field the device left empty would *introduce* a delta on +/// an axis it never scrolled, which an app reading that field would honour. +fn negate_scroll_axis(event: &CGEvent, axis: ScrollAxisFields) { + let point = event.get_double_value_field(axis.point); + if point != 0.0 { + event.set_double_value_field(axis.point, -point); + } + let fixed = event.get_double_value_field(axis.fixed); + if fixed != 0.0 { + event.set_double_value_field(axis.fixed, -fixed); + } + let line = event.get_integer_value_field(axis.line); + if line != 0 { + event.set_integer_value_field(axis.line, -line); + } +} + const CALLBACK_WATCHDOG_POLL_INTERVAL: Duration = Duration::from_millis(20); const LIFECYCLE_WATCHDOG_POLL_INTERVAL: Duration = Duration::from_millis(100); const FREEZE_HAZARD_EXIT_CODE: i32 = 78; @@ -689,6 +710,15 @@ fn run_tap_callback( match cb(hook_event) { EventDisposition::PassThrough => CallbackResult::Keep, EventDisposition::Suppress => CallbackResult::Drop, + // Guarded on the type because only a scroll has deltas to negate; + // any other class degrades to a plain pass-through. + EventDisposition::InvertScroll => { + if matches!(etype, CGEventType::ScrollWheel) { + negate_scroll_axis(event, VERTICAL); + negate_scroll_axis(event, HORIZONTAL); + } + CallbackResult::Keep + } } })); if let Ok(disposition) = result { diff --git a/crates/openlogi-hook/src/windows.rs b/crates/openlogi-hook/src/windows.rs index af1a3c0fe..4afae1bb0 100644 --- a/crates/openlogi-hook/src/windows.rs +++ b/crates/openlogi-hook/src/windows.rs @@ -317,7 +317,11 @@ unsafe extern "system" fn mouse_proc(code: i32, wparam: WPARAM, lparam: LPARAM) cb(HookEvent::Mouse(event)) }); match disposition { - EventDisposition::PassThrough => call_next(code, wparam, lparam), + // `InvertScroll` has no WH_MOUSE_LL implementation yet (see the + // variant's docs) — pass through rather than swallow the scroll. + EventDisposition::PassThrough | EventDisposition::InvertScroll => { + call_next(code, wparam, lparam) + } EventDisposition::Suppress => 1, } } @@ -454,7 +458,11 @@ unsafe extern "system" fn keyboard_proc(code: i32, wparam: WPARAM, lparam: LPARA cb(HookEvent::Key(event)) }); match disposition { - EventDisposition::PassThrough => call_next(code, wparam, lparam), + // `InvertScroll` has no WH_MOUSE_LL implementation yet (see the + // variant's docs) — pass through rather than swallow the scroll. + EventDisposition::PassThrough | EventDisposition::InvertScroll => { + call_next(code, wparam, lparam) + } EventDisposition::Suppress => 1, } } From 0e4418c51533a7a9537f8490758cfd901b820c53 Mon Sep 17 00:00:00 2001 From: berkaymuvaffak Date: Sat, 22 Aug 2026 11:08:33 +0300 Subject: [PATCH 4/4] fix(agent): don't invert scroll where devices share one identity The hook matches a scroll event by vendor/product id, which two identical directly-attached mice share, so it cannot rewrite one without rewriting the other. Collapsing them applied one device's setting to its twin. Every reason to skip a device is now a refusal *on its identity* rather than an omission, and a single refusal drops the identity with a warning: a disabled device must stay untouched, and a natively-capable one already carries the setting in firmware, where rewriting on top would invert twice. An identity whose devices all agree still yields one entry. Reported by Greptile on #783. --- .../openlogi-agent-core/src/orchestrator.rs | 61 ++++++++++++---- .../src/orchestrator/tests.rs | 72 +++++++++++++++++++ 2 files changed, 120 insertions(+), 13 deletions(-) diff --git a/crates/openlogi-agent-core/src/orchestrator.rs b/crates/openlogi-agent-core/src/orchestrator.rs index ddae4e803..733f6e773 100644 --- a/crates/openlogi-agent-core/src/orchestrator.rs +++ b/crates/openlogi-agent-core/src/orchestrator.rs @@ -263,23 +263,58 @@ impl Orchestrator { /// for. Not app-scoped, unlike the button maps: inversion belongs to the /// device, not the foreground app. /// - /// Natively-capable devices are excluded because their setting goes to the - /// firmware (`scroll_settings_for`); rewriting on top would invert twice. /// Only [`DeviceRoute::Direct`] devices qualify — a receiver-paired device /// reports the receiver's ids, which would match every device on that /// dongle. + /// + /// The hook rewrites *by identity* and two identical directly-attached mice + /// share one, so every device behind an identity has to agree. Anything + /// short of "enabled, inversion on, no native support" counts as a refusal + /// and drops the identity: a disabled device must stay untouched, and a + /// natively-capable one already has the setting in firmware + /// (`scroll_settings_for`), where rewriting on top would invert twice. + /// Dropping is the safer failure — inverting a wheel nobody asked for is + /// untraceable from the GUI. fn software_scroll_inversion(&self) -> BTreeSet<(u32, u32)> { - self.devices - .iter() - .filter(|dev| self.config.device_enabled(&dev.config_key)) - .filter(|dev| !dev.capabilities.is_some_and(|caps| caps.scroll_inversion)) - .filter(|dev| self.config.invert_scroll(&dev.config_key)) - .filter_map(|dev| match dev.route { - Some(DeviceRoute::Direct { - vendor_id, - product_id, - }) => Some((u32::from(vendor_id), u32::from(product_id))), - _ => None, + // Per identity: whether a device sharing it wants the rewrite, and + // whether another refuses. + let mut by_identity: BTreeMap<(u32, u32), (bool, bool)> = BTreeMap::new(); + for dev in &self.devices { + let Some(DeviceRoute::Direct { + vendor_id, + product_id, + }) = dev.route + else { + continue; + }; + // Every reason to skip a device is a refusal *on its identity*, not + // an omission: skipping it silently would let a twin's setting drag + // it along. + let wants = self.config.device_enabled(&dev.config_key) + && self.config.invert_scroll(&dev.config_key) + && !dev.capabilities.is_some_and(|caps| caps.scroll_inversion); + let entry = by_identity + .entry((u32::from(vendor_id), u32::from(product_id))) + .or_default(); + if wants { + entry.0 = true; + } else { + entry.1 = true; + } + } + by_identity + .into_iter() + .filter_map(|(identity, (wanted, refused))| { + if wanted && refused { + warn!( + vendor_id = identity.0, + product_id = identity.1, + "devices sharing this identity disagree on scroll inversion; \ + the hook cannot tell them apart, so neither wheel is inverted" + ); + return None; + } + wanted.then_some(identity) }) .collect() } diff --git a/crates/openlogi-agent-core/src/orchestrator/tests.rs b/crates/openlogi-agent-core/src/orchestrator/tests.rs index 60f98cca3..137d37b55 100644 --- a/crates/openlogi-agent-core/src/orchestrator/tests.rs +++ b/crates/openlogi-agent-core/src/orchestrator/tests.rs @@ -815,6 +815,78 @@ fn software_scroll_inversion_skips_native_and_receiver_paired_devices() { ); } +#[test] +fn identical_devices_disagreeing_on_inversion_invert_neither() { + use std::collections::BTreeSet; + + // The hook sees only vendor/product, so it cannot honour one setting without + // also applying it to the twin. Inverting a wheel nobody asked for is worse. + let mut config = Config::default(); + config.set_invert_scroll("left-hand", true); + let mut orch = orchestrator(config); + orch.devices = vec![ + direct_dev("left-hand", 0xb020, false), + direct_dev("right-hand", 0xb020, false), + ]; + + assert_eq!(orch.software_scroll_inversion(), BTreeSet::new()); +} + +#[test] +fn a_disabled_twin_blocks_its_siblings_software_inversion() { + use std::collections::BTreeSet; + + // The hook cannot rewrite one twin without the other, so a device the user + // switched off must not be dragged along by its sibling's setting. + let mut config = Config::default(); + config.set_invert_scroll("enabled", true); + config.set_device_enabled("switched-off", false); + let mut orch = orchestrator(config); + orch.devices = vec![ + direct_dev("enabled", 0xb020, false), + direct_dev("switched-off", 0xb020, false), + ]; + + assert_eq!(orch.software_scroll_inversion(), BTreeSet::new()); +} + +#[test] +fn a_natively_capable_twin_blocks_software_inversion() { + use std::collections::BTreeSet; + + // Its setting is already in firmware, and the hook would rewrite it too — + // inverting that wheel twice. + let mut config = Config::default(); + config.set_invert_scroll("software", true); + config.set_invert_scroll("native", true); + let mut orch = orchestrator(config); + orch.devices = vec![ + direct_dev("software", 0xb020, false), + direct_dev("native", 0xb020, true), + ]; + + assert_eq!(orch.software_scroll_inversion(), BTreeSet::new()); +} + +#[test] +fn identical_devices_agreeing_on_inversion_yield_one_entry() { + use std::collections::BTreeSet; + + let mut config = Config::default(); + config.set_invert_scroll("left-hand", true); + config.set_invert_scroll("right-hand", true); + let mut orch = orchestrator(config); + orch.devices = vec![ + direct_dev("left-hand", 0xb020, false), + direct_dev("right-hand", 0xb020, false), + ]; + + assert_eq!( + orch.software_scroll_inversion(), + BTreeSet::from([(0x046d_u32, 0xb020_u32)]) + ); +} + #[test] fn disabling_the_selected_device_keeps_another_devices_software_inversion() { use std::collections::BTreeSet;