fix(webrtc): preserve input on cursor hide - #596
Conversation
…ing mixed absolute/relative. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
📝 WalkthroughWalkthrough
ChangesMouse Flush Batching Fix
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0da4d861f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dy: dyServer, | ||
| timestampUs: batchTimestampUs, | ||
| }); | ||
| this.sendInputPacket(payload, INPUT_MOUSE_REL); |
There was a problem hiding this comment.
When partially reliable HID is enabled for mouse input, this second sendInputPacket can use the unordered input_channel_partially_reliable path; checked createDataChannels, where that channel is created with ordered: false. In the cursor-visible-to-hidden transition this now emits the absolute and relative parts as two separate data-channel messages, so the relative movement can arrive before the absolute packet and then be overwritten by it, recreating the dropped-first-move behavior this change is trying to fix. The mixed batch needs to be kept in one ordered payload/path, or otherwise avoid using unordered delivery for the dependent pair.
Useful? React with 👍 / 👎.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@opennow-stable/src/renderer/src/gfn/webrtcClient.ts`:
- Around line 3798-3806: The mouse flush in webrtcClient.ts currently splits the
reliable relative residue and the deferred absolute pin across different sends,
allowing the absolute packet to overtake the earlier relative flush. Update the
logic around flushMouse and the
pendingMouseDxFloat/pendingMouseDyFloat/pendingMouseAbs handling so the
INPUT_MOUSE_ABS path is sent on the reliable channel too, or ensure both packets
are emitted together in the same reliable flush.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 58b8f91b-97fc-4f8a-9e6e-d285226c21d7
📒 Files selected for processing (1)
opennow-stable/src/renderer/src/gfn/webrtcClient.ts
| if ( | ||
| Math.abs(this.pendingMouseDxFloat) >= 0.5 | ||
| || Math.abs(this.pendingMouseDyFloat) >= 0.5 | ||
| ) { | ||
| flushMouse(true); | ||
| } | ||
| this.pendingMouseDxFloat = 0; | ||
| this.pendingMouseDyFloat = 0; | ||
| this.pendingMouseAbs = abs; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether INPUT_MOUSE_ABS is eligible for the partially-reliable (unordered) channel.
fd -t f 'inputProtocol.ts' opennow-stable/src | while read -r f; do
echo "== $f =="
rg -n -C3 'isPartiallyReliableHidTransferEligible|partiallyReliableHidMaskForInputType|INPUT_MOUSE_ABS|INPUT_MOUSE_REL' "$f"
doneRepository: OpenCloudGaming/OpenNOW
Length of output: 2054
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=opennow-stable/src/renderer/src/gfn/webrtcClient.ts
echo "== flushMouse / pendingMouseAbs region =="
sed -n '3520,3825p' "$file" | cat -n
echo
echo "== sendInputPacket / channel selection =="
rg -n -C4 'function sendInputPacket|sendInputPacket\(|isPartiallyReliableHidTransferEligible|partiallyReliableHidMaskForInputType|forceReliable|mixedBatch|pendingMouseAbs' "$file"Repository: OpenCloudGaming/OpenNOW
Length of output: 19529
Deferred abs pin can overtake the reliable relative flush
This splits the rel→abs sequence across two flushes. INPUT_MOUSE_ABS can go over the partially reliable unordered channel, so the deferred abs packet can arrive before the earlier reliable relative residue and shift the cursor off the overlay. Keep the abs on the reliable channel too, or send both in the same reliable flush.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@opennow-stable/src/renderer/src/gfn/webrtcClient.ts` around lines 3798 -
3806, The mouse flush in webrtcClient.ts currently splits the reliable relative
residue and the deferred absolute pin across different sends, allowing the
absolute packet to overtake the earlier relative flush. Update the logic around
flushMouse and the pendingMouseDxFloat/pendingMouseDyFloat/pendingMouseAbs
handling so the INPUT_MOUSE_ABS path is sent on the reliable channel too, or
ensure both packets are emitted together in the same reliable flush.
* Preserve relative input during cursor visibility transitions by allowing mixed absolute/relative. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Send mixed abs+rel mouse batches over the ordered reliable channel Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Preserve raw-input deltas queued before cursor-visible transition Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
1 participant
This PR fixes a bug where raw mouse input was discarded immediately after the native cursor overlay hid. When the cursor transitioned from visible (absolute positioning) to hidden (relative positioning), the previous flush logic would send the pending absolute packet and clear the relative accumulators without sending them, causing the first movement input to be dropped.
Changes:
flushMouseinopennow-stable/src/renderer/src/gfn/webrtcClient.tsto support mixed batches: it now sends any pending absolute packet followed immediately by any pending relative packets within the same flush cycle, rather than zeroing out relative deltas.Note
Medium Risk
Changes live mouse input encoding in the WebRTC client; incorrect batching could cause cursor drift or double movement, but scope is limited to flush/queue logic.
Overview
Fixes dropped mouse movement when the native cursor overlay hides mid-batch and the client switches from absolute to relative packets.
flushMouseno longer clears pending relative deltas after sending an absolute packet or returns after only the absolute send. A single flush can emit absolute first, then relative in one cycle with a shared timestamp, matching official GFN mixed-batch ordering.queueMouseMovementnow zeros stale relative accumulators when the overlay cursor is visible and an absolute position is queued, so an absolute pin is not followed by obsolete relative deltas that would drift the server cursor off the overlay.Reviewed by Cursor Bugbot for commit d0da4d8. Configure here.
Summary by CodeRabbit