Skip to content

Add wheel scroll settings#94

Open
klovinad wants to merge 6 commits into
AprilNEA:masterfrom
klovinad:proposal/wheel-scroll-settings
Open

Add wheel scroll settings#94
klovinad wants to merge 6 commits into
AprilNEA:masterfrom
klovinad:proposal/wheel-scroll-settings

Conversation

@klovinad

@klovinad klovinad commented Jun 2, 2026

Copy link
Copy Markdown

Summary

This proposes two related mouse-control improvements:

  • Change the default horizontal gesture-button swipes to switch macOS desktops/spaces:
    • Left → Previous Desktop
    • Right → Next Desktop
  • Add app-wide scroll wheel preferences:
    • invert wheel direction
    • scroll strength multiplier
    • tactility/chunking

Implementation notes

  • Scroll preferences are persisted in AppSettings and mirrored into the hook runtime through a shared ScrollSettings value.
  • The hook transforms captured scroll events only when non-default scroll settings are active; default settings pass through natively.
  • Synthetic transformed scroll events are posted at the session tap to avoid being re-captured by OpenLogi's own HID event tap.
  • Added unit coverage for scroll axis ordering, inversion, and chunking.

Verification

Ran locally on macOS:

cargo fmt --all
cargo check -p openlogi-gui
cargo test -p openlogi-gui -p openlogi-core

Results:

  • openlogi-core: 34 passed
  • openlogi-gui: 14 passed

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Minor suggestions inline — one observation about the scroll-settings default check.

Reviewed changes — Adds three scroll-preference controls (invert, strength, tactility) to the Settings window and changes default horizontal gesture swipes to desktop/Space switching.

  • Default gesture direction updatePrevTab/NextTab replaced with PreviousDesktop/NextDesktop.
  • ScrollSettings shared stateArc<RwLock<ScrollSettings>> mirrors the persisted config to the hook runtime.
  • Scroll event transformationtransform_scroll/quantize_scroll apply inversion, strength, and chunking to captured wheel events.
  • Session-tap re-injection — Transformed scroll events are posted at CGEventTapLocation::Session to avoid re-capture by OpenLogi's HID tap.
  • Settings UI — Invert switch, strength slider (1–10), and tactility slider (0–10) in a new "Scroll" group box.
  • Generalized setting_row — Signature changed from control: Switch to control: impl IntoElement to accept sliders alongside switches.
  • Expanded native-click passthroughBackBrowserBack and ForwardBrowserForward added.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Big Pickle (free) (credentials for Anthropic not configured) | 𝕏

Comment thread crates/openlogi-gui/src/hook_runtime.rs Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Prior feedback addressed — no new issues found.

Reviewed changes — Fixed the ScrollSettings::default() mismatch with AppSettings defaults by replacing the derived Default with a manual impl whose strength: 1 matches the app config.

  • Manual Default impl for ScrollSettings — Replaced #[derive(Default)] with a manual impl Default where strength: 1 aligns with AppSettings::wheel_strength. This restores the identity fast-path in hook_runtime.rs.

Pullfrog  | View workflow run | Using Big Pickle (free) (credentials for Anthropic not configured) | 𝕏

@averypelle

Copy link
Copy Markdown

This should close #126

@phuocleoceo

Copy link
Copy Markdown

I'm really interested in the "invert wheel direction" feature. Thanks for bringing it up in this Pull Request, I hope it gets merged into the software soon!

@shawnflanagan

Copy link
Copy Markdown

Let's get these conflicts resolved so we can merge this. Invert wheel direction is sorely needed in the app

@averypelle

Copy link
Copy Markdown

for anyone looking for an interim fix, https://pilotmoon.com/scrollreverser/ is compatible with this app

@klovinad klovinad force-pushed the proposal/wheel-scroll-settings branch from 1813205 to 86fdb0d Compare June 14, 2026 09:09
@greptile-apps

greptile-apps Bot commented Jun 14, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds app-wide scroll-wheel preferences (invert direction, strength multiplier, and tactility/chunking) to OpenLogi, along with the implementation of PreviousDesktop/NextDesktop actions using the macOS CGS symbolic hotkey SPI. It also consolidates Action::execute() into openlogi-core.

  • Scroll settings are persisted in AppSettings and mirrored into the hook runtime via SharedScrollSettings. On macOS the original CGEvent is transformed in-place; on Linux/Windows the original event is suppressed and a new one is re-injected (the WH_MOUSE_LL hook already drops re-injected events via LLMHF_INJECTED, preventing feedback loops).
  • Continuous/trackpad events are excluded from transformation via the new is_continuous field on MouseEvent::Scroll.
  • The quantize_scroll helper is duplicated between hook_runtime.rs and macos.rs with the same algorithm but different numeric types.

Confidence Score: 5/5

Safe to merge. The scroll transform paths are well-bounded, the Windows hook guards against re-injected events via LLMHF_INJECTED, and continuous/trackpad events are correctly excluded.

The core transform logic is unit-tested. The zero-delta passthrough concern from the prior review is now correctly handled. The binding.rs refactor compiles clean on all targets.

binding.rs carries the CGS symbolic-hotkey SPI for space switching — worth a careful read if macOS space-switching is observed to be unreliable.

Important Files Changed

Filename Overview
crates/openlogi-agent-core/src/hook_runtime.rs Adds ScrollSettings / SharedScrollSettings types and wires them into the scroll handler. Zero-value transforms correctly return PassThrough, not Suppress.
crates/openlogi-core/src/binding.rs Large refactor moving Action::execute() into openlogi-core; adds platform helpers and PreviousDesktop/NextDesktop via CGS symbolic hotkey SPI.
crates/openlogi-hook/src/macos.rs Adds transform_scroll_event for in-place CGEvent field modification and a local quantize_scroll for tactility chunking.
crates/openlogi-hook/src/linux.rs Adds is_continuous: false to scroll events and updates passthrough logic to !matches!(Suppress).
crates/openlogi-hook/src/windows.rs Adds is_continuous: false to scroll events; existing LLMHF_INJECTED guard prevents feedback loops from re-injected events.
crates/openlogi-gui/src/windows/settings.rs Adds invert toggle, strength slider (1-10), and tactility slider (0-10) to the general settings page.
crates/openlogi-core/src/config.rs Adds wheel_inverted, wheel_strength, and wheel_tactility to AppSettings with proper serde defaults.

Reviews (7): Last reviewed commit: "Merge master into wheel scroll settings" | Re-trigger Greptile

Comment thread crates/openlogi-agent-core/src/hook_runtime.rs Outdated
Comment thread crates/openlogi-core/src/binding.rs Outdated
@klovinad klovinad force-pushed the proposal/wheel-scroll-settings branch from 86fdb0d to 183eb59 Compare June 14, 2026 12:57

@AprilNEA AprilNEA left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — invert-scroll is clearly in demand (#126 plus the comments here). Before merging I want to flag some concerns with the current approach, because the suppress-and-reinject design has a few real problems on macOS, and one of them is that it doesn't actually solve #126 as written.

1. It doesn't solve #126 (per-device inversion)

#126 specifically asks to keep the trackpad on natural scrolling and invert only the mouse. The hook taps at CGEventTapLocation::HID (crates/openlogi-hook/src/macos.rs:257) and captures every ScrollWheel event regardless of source, and transform_scroll never looks at kCGScrollWheelEventIsContinuous. So enabling invert flips the trackpad too — exactly the native-setting behavior #126 wants to avoid. We shouldn't close #126 with this as-is.

2. Modifier flags are dropped

post_scroll_delta builds a fresh CGEvent::new_scroll_event(...) and never copies the original event's flags. Modifier+scroll gestures that apps read off the event flags (zoom, etc.) will stop working while non-default settings are active. (System-level Ctrl+scroll screen zoom reads live key state and may be unaffected — worth verifying on-device.)

3. Momentum / pixel precision are lost

We suppress the original and re-emit ScrollEventUnit::LINE from the rounded AXIS_1/AXIS_2 line deltas. That drops scroll-phase/momentum and ignores the pixel POINT_DELTA_* fields, so continuous input (trackpad, MX free-spin high-res wheel) becomes coarse. Micro-deltas round to 0 → PassThrough, so on a free-spinning wheel small movements aren't inverted while fast ones are — inconsistent. Given our primary devices are MX-class, this matters.

4. Per-event cost on the hottest path

The macOS tap must stay lock-light or it stalls the whole input stream (see the comments in macos.rs). This adds an RwLock read per scroll event and, when active, a fresh CGEventSource::new + CGEvent allocation per event. On a high-rate free-spin stream that risks TapDisabledByTimeout / scroll stutter.

Suggested approach

Instead of suppress + reinject, mutate the event in place and return Keep. We already do in-place field writes elsewhere (crates/openlogi-core/src/binding.rs:1312,1327), and the same set_*_value_field works on the callback's &CGEvent. Negate/scale AXIS_1/AXIS_2 plus the POINT_DELTA_* and fixed-point fields and keep the event — that preserves momentum, pixel precision, flags, and continuity, with zero allocation and no re-entry concern. For #126, branch on kCGScrollWheelEventIsContinuous to apply inversion to discrete (mouse) scroll only. This is essentially how Scroll Reverser does it.

Unrelated change

The default gesture remap (PrevTab/NextTabPreviousDesktop/NextDesktop) is a separate UX decision — please split it into its own PR so each can be reviewed/reverted independently.

The config plumbing, clamping, and default fast-path are clean; the concern is purely the macOS transform path. Happy to help iterate on the in-place version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants