-
Notifications
You must be signed in to change notification settings - Fork 45
fix: bypass X11 for scroll using CDP pixel-precise deltas #193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
60d29a0
bc33a67
582216f
b95906f
351eb3c
ee30c53
53d08cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -552,6 +552,10 @@ | |
| } | ||
|
|
||
| beforeDestroy() { | ||
| if (this._scrollFlushTimeout != null) { | ||
| clearTimeout(this._scrollFlushTimeout) | ||
| this._scrollFlushTimeout = null | ||
| } | ||
| this.observer.disconnect() | ||
| this.$accessor.video.setPlayable(false) | ||
| /* Guacamole Keyboard does not provide destroy functions */ | ||
|
|
@@ -708,7 +712,53 @@ | |
| }) | ||
| } | ||
|
|
||
| wheelThrottle = false | ||
| _scrollAccX = 0 | ||
| _scrollAccY = 0 | ||
| _scrollLastSendTime = 0 | ||
| _scrollFlushTimeout: ReturnType<typeof setTimeout> | null = null | ||
| _scrollLastClientX = 0 | ||
| _scrollLastClientY = 0 | ||
| _scrollApiUrl: string | null = null | ||
|
|
||
| _getScrollApiUrl(): string { | ||
| if (this._scrollApiUrl) return this._scrollApiUrl | ||
| // The kernel-images API is exposed on port 444 (maps to 10001 inside the | ||
| // container) in both Docker and unikernel deployments. | ||
| this._scrollApiUrl = `${location.protocol}//${location.hostname}:444/live-view/scroll` | ||
| return this._scrollApiUrl | ||
| } | ||
|
|
||
| _clearScrollFlushTimeout() { | ||
| if (this._scrollFlushTimeout != null) { | ||
| clearTimeout(this._scrollFlushTimeout) | ||
| this._scrollFlushTimeout = null | ||
| } | ||
| } | ||
|
|
||
| _sendScrollAccumulated(clientX: number, clientY: number) { | ||
| if (this._scrollAccX === 0 && this._scrollAccY === 0) { | ||
| return | ||
| } | ||
| const { w, h } = this.$accessor.video.resolution | ||
| const rect = this._overlay.getBoundingClientRect() | ||
| const sx = Math.round((w / rect.width) * (clientX - rect.left)) | ||
| const sy = Math.round((h / rect.height) * (clientY - rect.top)) | ||
|
|
||
| const dx = this._scrollAccX | ||
| const dy = this._scrollAccY | ||
| this._scrollAccX = 0 | ||
| this._scrollAccY = 0 | ||
| this._scrollLastSendTime = Date.now() | ||
|
|
||
| const url = this._getScrollApiUrl() | ||
| fetch(url, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ x: sx, y: sy, delta_x: -dx, delta_y: -dy }), | ||
| keepalive: true, | ||
| }).catch(() => {}) | ||
| } | ||
|
|
||
| onWheel(e: WheelEvent) { | ||
| if (!this.hosting || this.locked) { | ||
| return | ||
|
|
@@ -717,8 +767,6 @@ | |
| let x = e.deltaX | ||
| let y = e.deltaY | ||
|
|
||
| // Normalize to pixel units. deltaMode 1 = lines, 2 = pages; convert | ||
| // both to approximate pixel values so the divisor below works uniformly. | ||
| if (e.deltaMode !== 0) { | ||
| x *= WHEEL_LINE_HEIGHT | ||
| y *= WHEEL_LINE_HEIGHT | ||
|
|
@@ -729,26 +777,29 @@ | |
| y = y * -1 | ||
| } | ||
|
|
||
| // The server sends one XTestFakeButtonEvent per unit we pass here, | ||
| // and each event scrolls Chromium by ~120 px. Raw pixel deltas from | ||
| // trackpads are already in pixels (~120 per notch), so dividing by | ||
| // PIXELS_PER_TICK converts them to discrete scroll "ticks". The | ||
| // result is clamped to [-scroll, scroll] (the user-facing sensitivity | ||
| // setting) so fast swipes don't over-scroll. | ||
| const PIXELS_PER_TICK = 120 | ||
| x = x === 0 ? 0 : Math.min(Math.max(Math.round(x / PIXELS_PER_TICK) || Math.sign(x), -this.scroll), this.scroll) | ||
| y = y === 0 ? 0 : Math.min(Math.max(Math.round(y / PIXELS_PER_TICK) || Math.sign(y), -this.scroll), this.scroll) | ||
| this._scrollAccX += x | ||
| this._scrollAccY += y | ||
|
|
||
| this.sendMousePos(e) | ||
| if (this._scrollAccX === 0 && this._scrollAccY === 0) { | ||
| return | ||
| } | ||
|
|
||
| if (!this.wheelThrottle) { | ||
| this.wheelThrottle = true | ||
| this.$client.sendData('wheel', { x, y }) | ||
| this._scrollLastClientX = e.clientX | ||
| this._scrollLastClientY = e.clientY | ||
|
|
||
| window.setTimeout(() => { | ||
| this.wheelThrottle = false | ||
| }, 100) | ||
| const now = Date.now() | ||
| if (now - this._scrollLastSendTime < 50) { | ||
| if (this._scrollFlushTimeout == null) { | ||
| this._scrollFlushTimeout = setTimeout(() => { | ||
| this._scrollFlushTimeout = null | ||
| this._sendScrollAccumulated(this._scrollLastClientX, this._scrollLastClientY) | ||
| }, 50) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| this._clearScrollFlushTimeout() | ||
| this._sendScrollAccumulated(e.clientX, e.clientY) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Scroll sensitivity setting now silently ignoredMedium Severity The
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional — the scroll sensitivity setting controlled the max tick count for X11 discrete scroll events. With CDP pixel-precise scrolling, the browser's native WheelEvent deltas are forwarded directly, giving 1:1 scroll fidelity. The old sensitivity clamping was a workaround for X11's coarse scroll ticks and is no longer needed. The dead |
||
| } | ||
|
|
||
| onTouchHandler(e: TouchEvent) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,3 +159,66 @@ func (c *Client) SetDeviceMetricsOverride(ctx context.Context, width, height int | |
|
|
||
| return nil | ||
| } | ||
|
|
||
| // DispatchMouseWheelEvent sends a mouseWheel event to the first page target | ||
| // via CDP Input.dispatchMouseEvent. deltaX/deltaY are in CSS pixels, allowing | ||
| // sub-notch precision that X11 button events cannot express. | ||
| func (c *Client) DispatchMouseWheelEvent(ctx context.Context, x, y int, deltaX, deltaY float64) error { | ||
| targetsResult, err := c.send(ctx, "Target.getTargets", nil, "") | ||
| if err != nil { | ||
| return fmt.Errorf("Target.getTargets: %w", err) | ||
| } | ||
|
|
||
| var targets struct { | ||
| TargetInfos []struct { | ||
| TargetID string `json:"targetId"` | ||
| Type string `json:"type"` | ||
| } `json:"targetInfos"` | ||
| } | ||
| if err := json.Unmarshal(targetsResult, &targets); err != nil { | ||
| return fmt.Errorf("unmarshal targets: %w", err) | ||
| } | ||
|
|
||
| var pageTargetID string | ||
| for _, t := range targets.TargetInfos { | ||
| if t.Type == "page" { | ||
| pageTargetID = t.TargetID | ||
| break | ||
| } | ||
| } | ||
| if pageTargetID == "" { | ||
| return fmt.Errorf("no page target found") | ||
| } | ||
|
|
||
| attachResult, err := c.send(ctx, "Target.attachToTarget", map[string]any{ | ||
| "targetId": pageTargetID, | ||
| "flatten": true, | ||
| }, "") | ||
| if err != nil { | ||
| return fmt.Errorf("Target.attachToTarget: %w", err) | ||
| } | ||
|
|
||
| var attach struct { | ||
| SessionID string `json:"sessionId"` | ||
| } | ||
| if err := json.Unmarshal(attachResult, &attach); err != nil { | ||
| return fmt.Errorf("unmarshal attach: %w", err) | ||
| } | ||
|
|
||
| _, err = c.send(ctx, "Input.dispatchMouseEvent", map[string]any{ | ||
| "type": "mouseWheel", | ||
| "x": x, | ||
| "y": y, | ||
| "deltaX": deltaX, | ||
| "deltaY": deltaY, | ||
| }, attach.SessionID) | ||
| if err != nil { | ||
| return fmt.Errorf("Input.dispatchMouseEvent mouseWheel: %w", err) | ||
| } | ||
|
|
||
| _, _ = c.send(ctx, "Target.detachFromTarget", map[string]any{ | ||
| "sessionId": attach.SessionID, | ||
| }, "") | ||
|
|
||
| return nil | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicated CDP target-attach boilerplate across methodsLow Severity
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed this is good cleanup. Filed as a follow-up — extracting a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CDP scroll events lack modifier key awarenessHigh Severity
Additional Locations (1)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged — this is a known limitation of the CDP scroll path. Keyboard events go through X11/neko while scroll now goes directly through CDP, so CDP doesn't see held modifiers. In practice, Ctrl+scroll zoom is rarely used in the live view context (users use browser zoom instead). Adding modifier forwarding from the client (reading e.ctrlKey/e.shiftKey from the WheelEvent and passing them to the API) is a viable follow-up but out of scope for the initial scroll fix. |
||


Uh oh!
There was an error while loading. Please reload this page.