fix(agent-core): respect per-device thumbwheel polarity for rebound scroll actions - #640
Open
davidbudnick wants to merge 3 commits into
Open
fix(agent-core): respect per-device thumbwheel polarity for rebound scroll actions#640davidbudnick wants to merge 3 commits into
davidbudnick wants to merge 3 commits into
Conversation
Greptile SummaryThe PR normalizes thumbwheel direction using each device’s HID++ polarity and carries that polarity into the Windows native-scroll fallback.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| crates/openlogi-agent-core/src/orchestrator.rs | Publishes selected-device bindings and polarity state atomically while preserving learned per-device entries across rebuilds. |
| crates/openlogi-agent-core/src/hook_runtime.rs | Resolves native horizontal-wheel deltas against polarity and bindings from the same hook-map snapshot. |
| crates/openlogi-agent-core/src/watchers/gesture.rs | Records capture-session polarity reports under the shared hook-map lock. |
| crates/openlogi-agent-core/src/thumbwheel.rs | Introduces per-device polarity storage and selected-device resolution. |
| crates/openlogi-hid/src/gesture.rs | Normalizes diverted thumbwheel rotation and reports the detected native polarity to consumers. |
| crates/openlogi-hid/src/thumbwheel.rs | Encapsulates the HID++ default-direction interpretation with unit coverage. |
| crates/openlogi-agent/src/main.rs | Plumbs shared hook maps into the gesture watcher. |
Sequence Diagram
sequenceDiagram
participant Device as Thumbwheel device
participant Capture as HID++ capture session
participant Watcher as Gesture watcher
participant Maps as SharedHookMaps
participant Hook as Windows hook
Capture->>Device: getThumbwheelInfo
Device-->>Capture: default_dir
Capture->>Device: setThumbwheelReporting(inv_dir)
Capture->>Watcher: ThumbwheelDirection
Watcher->>Maps: Record polarity by device key
Note over Maps: Selection, bindings, and polarity<br/>share one locked snapshot
Hook->>Maps: Read selected polarity and bindings
Hook-->>Hook: Resolve native delta direction
Reviews (3): Last reviewed commit: "Merge branch 'master' into fix/thumbwhee..." | Re-trigger Greptile
…croll actions The two places that turn thumb-wheel rotation into the ThumbwheelScrollUp / ThumbwheelScrollDown bindings each hard-coded one sign convention: - the Windows native fallback (hook_runtime) mapped a positive WM_MOUSEHWHEEL delta to ThumbwheelScrollDown, calibrated on the MX Master 2S; - the HID++ diverted path (capture session + gesture watcher) assumed positive rotation means "up" and never consulted the wheel's own direction. Which physical direction a positive delta means is per-model firmware behaviour, exposed as `default_dir` by 0x2150 getThumbwheelInfo (0 = positive toward left/back, 1 = positive toward right/front). On a wheel with the opposite polarity both paths fire the forward/backward bindings swapped — with the wheel bound to previous/next tab, "next tab" does previous and vice versa (AprilNEA#457, MX Master 3 on Windows). Fix, with `default_dir` as the single source of truth: - capture sessions divert the wheel with `inv_dir` derived from `default_dir`, so the diverted rotation sign is normalised (positive = physically forward) by the firmware itself and `CapturedInput::Scroll` means the same thing on every model — the diverted dispatch path needs no per-device handling; - the session reports the wheel's polarity once at arm time (`CapturedInput::ThumbwheelDirection`) into a new shared `ThumbwheelDirs` map (selection maintained by the orchestrator, mirroring `DpiCycles`), which the OS hook's native fallback consults before interpreting the WM_MOUSEHWHEEL delta sign. A wheel whose polarity was never learned — no 0x2150 feature (MX Master 2S) or HID++ unreachable — keeps the previous behaviour on both paths. Fixes AprilNEA#457
… snapshot The polarity selection lived behind its own lock, updated by the orchestrator separately from the hook maps: a WM_MOUSEHWHEEL callback landing between the two writes of a selection switch could pair the new device's bindings with the old device's polarity and fire one tick's action inverted (review finding on AprilNEA#640). Fold ThumbwheelDirs into HookMaps, the one structure the callback already reads under a single try_read: the orchestrator republishes the selection together with the binding maps (carrying the capture watcher's learned per-device entries forward, since they come from hardware probes rather than config), and the watcher records polarity reports under the same lock. The callback-side second lock and the standalone SharedThumbwheelDirs cell are gone.
davidbudnick
force-pushed
the
fix/thumbwheel-polarity
branch
from
August 16, 2026 05:21
a4fc0c5 to
5b07c7a
Compare
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.
Summary
Symptom (#457): with the MX Master 3 thumb wheel bound to previous/next tab on Windows, the directions are swapped — "next tab" fires previous tab and vice versa (the reported "first ↔ last tab jump" is just
Ctrl+Tabwrap-around in Firefox once the wrong direction fires).Root cause: which physical direction a positive horizontal-wheel delta means is per-model firmware behaviour, exposed by HID++
0x2150getThumbwheelInfoasdefault_dir(0= positive toward left/back,1= positive toward right/front). Neither of the two sign-interpretation sites consulted it:hook_runtime::rebound_thumbwheel_action(the Windows nativeWM_MOUSEHWHEELfallback) hard-codes positive →ThumbwheelScrollDown, a convention its own doc comment says was calibrated on the MX Master 2S;ThumbwheelScrollUp) in the gesture watcher, and the capture session always diverted withinv_dir = false.A wheel whose firmware reports the opposite polarity gets its two rotation bindings swapped on whichever path serves it.
Approach — make
default_dirthe single source of truth, with the previous behaviour as the explicit fallback when it is unavailable:getThumbwheelInfowhile arming the wheel; it now derivesinv_dirfromdefault_dirand passes it tosetThumbwheelReporting, so the firmware itself normalises the diverted rotation sign to positive = physically forward.CapturedInput::Scrollthen means the same thing on every model and the diverted dispatch path needs no per-device handling.CapturedInput::ThumbwheelDirection, riding the existing input channel). The gesture watcher records it into a new sharedThumbwheelDirsmap, and the orchestrator maintains the map'sselectedkey — mirroring howDpiCyclesalready handles "the OS hook cannot attribute an event to a device, so it resolves against the selection". The nativeWM_MOUSEHWHEELfallback consults it (try_readonly, per the hook-callback locking rules) before interpreting the delta sign.0x2150on the device — e.g. MX Master 2S, whose wheel arrives via Gestures2 — or HID++ never reachable) falls back to the exact previous mapping on both paths, so already-correct devices are unchanged.Why this design: the alternative — carrying
default_dirinCapabilitiesfrom inventory probing — would change the IPC wire format (protocol bump + golden regeneration) for a value the GUI never needs, and would add an extra HID++ round-trip to every probe. Reading it where the session already callsgetThumbwheelInfocosts no extra traffic, keeps everything in-process, and reuses the establishedDpiCyclesselection pattern for the hook's device-attribution blind spot.Changes
openlogi-hid:ThumbwheelInfo::positive_is_forward()(pins thedefault_dirsemantics); capture sessions divert the wheel withinv_dirderived fromdefault_dirand report the polarity via the newCapturedInput::ThumbwheelDirectionvariant (not on the IPC wire).openlogi-agent-core: new sharedThumbwheelDirs(per-device polarity + selection, mirroringDpiCycles); the gesture watcher records session polarity reports; the orchestrator publishes the selection;rebound_thumbwheel_actiontakes the polarity and maps deltas per device, defaulting to the historical MX Master 2S convention when unknown.openlogi-agent: plumbSharedRuntime::thumbwheel_dirsinto the hook runtime and the capture watcher.Testing
cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warnings(Windows 11, stable)cargo test --workspace(Windows 11; includes new unit tests: polarity mapping both ways inhook_runtime,ThumbwheelDirsresolution/fallback, watcher recording of the polarity report,default_dirsemantics inopenlogi-hid)RUSTDOCFLAGS="-D warnings" cargo doc -p openlogi-hid --no-deps --document-private-items— fails on current master on Windows for a pre-existing, unrelated cfg-gated link intransport.rs(AsyncHidChannelonly resolves on Linux, where CI runs this gate); the changed files document cleanly.Not runtime-tested on hardware — I don't have an MX-series mouse with a thumb wheel to hand. To verify: on an MX Master 3/3S, bind the thumb wheel to previous/next tab and confirm the directions match the labels, both while the HID++ capture session is diverting the wheel and (Windows) with the agent's HID++ path unavailable so the native fallback serves the binding; on an MX Master 2S, confirm the directions are unchanged. Note the
WM_MOUSEHWHEELhalf of the fix assumes a wheel's native HID scroll sign follows the samedefault_dirconvention as its diverted rotation — this holds for the 2S/MX3 evidence in #457 but has not been confirmed against a spec.Fixes #457