Fixed stylus handling in TouchpadView#1334
Merged
Merged
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
📝 WalkthroughWalkthroughCentralizes stylus/eraser detection into a helper, routes stylus hover events via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/winlator/widget/TouchpadView.java (1)
325-325: RenameisEventTriggeredByStulustoisEventTriggeredByStylusfor clarity.This is a small typo cleanup, but it improves readability and discoverability across call sites.
Also applies to: 342-342, 391-394
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/widget/TouchpadView.java` at line 325, Rename the misspelled method isEventTriggeredByStulus to isEventTriggeredByStylus throughout TouchpadView: update the method declaration (isEventTriggeredByStulus) and all call sites (e.g., the boolean assignment currently using isEventTriggeredByStulus(event) and other usages around the affected blocks) to use isEventTriggeredByStylus(event); ensure any references (overrides, imports, or tests) are updated to the new name and rebuild to fix symbol references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/winlator/widget/TouchpadView.java`:
- Around line 396-400: The isStylusButtonPressed method currently checks only
MotionEvent.BUTTON_STYLUS_PRIMARY and MotionEvent.BUTTON_SECONDARY; update the
buttonState mask to also include MotionEvent.BUTTON_STYLUS_SECONDARY so stylus
barrel/secondary buttons reported using the stylus-specific constant are
detected across devices, keeping the existing tool type eraser check
(event.getToolType(0) == MotionEvent.TOOL_TYPE_ERASER) unchanged.
---
Nitpick comments:
In `@app/src/main/java/com/winlator/widget/TouchpadView.java`:
- Line 325: Rename the misspelled method isEventTriggeredByStulus to
isEventTriggeredByStylus throughout TouchpadView: update the method declaration
(isEventTriggeredByStulus) and all call sites (e.g., the boolean assignment
currently using isEventTriggeredByStulus(event) and other usages around the
affected blocks) to use isEventTriggeredByStylus(event); ensure any references
(overrides, imports, or tests) are updated to the new name and rebuild to fix
symbol references.
🪄 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
Run ID: e6074cf3-1181-4590-9f4d-42a55b8aa608
📒 Files selected for processing (1)
app/src/main/java/com/winlator/widget/TouchpadView.java
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/winlator/widget/TouchpadView.java (1)
372-389:⚠️ Potential issue | 🟠 MajorAdd
ACTION_CANCELhandling to stylus event path.The stylus handler skips cleanup if the event sequence is canceled after
ACTION_DOWN, potentially leaving mouse button state latched. Other input handlers in this file (handleTouchEvent,handleGestureEvent) explicitly handleACTION_CANCEL; the stylus path must do the same.Proposed fix
private boolean handleStylusEvent(MotionEvent event) { int action = event.getActionMasked(); switch (action) { case MotionEvent.ACTION_DOWN: if (isStylusButtonPressed(event)) { handleStylusRightClick(event); } else { handleStylusLeftClick(event); } break; case MotionEvent.ACTION_MOVE: handleStylusMove(event); break; case MotionEvent.ACTION_UP: handleStylusUp(event); break; + case MotionEvent.ACTION_CANCEL: + handleStylusUp(event); + break; } return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/widget/TouchpadView.java` around lines 372 - 389, The stylus event switch in TouchpadView omits MotionEvent.ACTION_CANCEL, which can leave mouse button state latched after ACTION_DOWN; add an ACTION_CANCEL case alongside ACTION_UP that calls the same cleanup path (e.g., handleStylusUp(event)) so the stylus sequence is properly terminated; update the switch in the stylus handler to include case MotionEvent.ACTION_CANCEL: handleStylusUp(event); break; referencing the existing methods isStylusButtonPressed, handleStylusLeftClick, handleStylusRightClick, handleStylusMove, and handleStylusUp.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/winlator/widget/TouchpadView.java`:
- Around line 391-404: isEventTriggeredByStylus and isStylusButtonPressed
currently call event.getToolType(0) which only inspects pointer 0; change both
to use event.getActionIndex() to check the pointer that triggered this
MotionEvent (use event.getToolType(actionIndex)), and if that index is out of
range or doesn't report stylus/eraser, fall back to scanning all pointers via
event.getPointerCount() and event.getToolType(i) to detect any stylus/eraser
pointer; for isStylusButtonPressed keep the existing buttonState bit check
(event.getButtonState() & BUTTON_STYLUS_PRIMARY | BUTTON_STYLUS_SECONDARY |
BUTTON_SECONDARY) but determine toolSetToEraser by checking the action pointer
first and then by scanning all pointers as a fallback so eraser tool detection
works in multi-pointer scenarios.
---
Outside diff comments:
In `@app/src/main/java/com/winlator/widget/TouchpadView.java`:
- Around line 372-389: The stylus event switch in TouchpadView omits
MotionEvent.ACTION_CANCEL, which can leave mouse button state latched after
ACTION_DOWN; add an ACTION_CANCEL case alongside ACTION_UP that calls the same
cleanup path (e.g., handleStylusUp(event)) so the stylus sequence is properly
terminated; update the switch in the stylus handler to include case
MotionEvent.ACTION_CANCEL: handleStylusUp(event); break; referencing the
existing methods isStylusButtonPressed, handleStylusLeftClick,
handleStylusRightClick, handleStylusMove, and handleStylusUp.
🪄 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
Run ID: 687f7977-8441-421a-a5f1-8f857471a5ad
📒 Files selected for processing (1)
app/src/main/java/com/winlator/widget/TouchpadView.java
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/src/main/java/com/winlator/widget/TouchpadView.java (1)
392-395:⚠️ Potential issue | 🟠 MajorAvoid hardcoded pointer index for stylus/eraser detection in mixed-pointer events.
At Line [393] and Line [403], using
getToolType(0)can misclassify events when the active stylus pointer is not index 0 (e.g., finger down first, stylus second). This can route stylus input to the wrong path and miss eraser/right-click behavior.Proposed fix
private static boolean isEventTriggeredByStylus(MotionEvent event) { - int toolType = event.getToolType(0); - return toolType == MotionEvent.TOOL_TYPE_STYLUS || toolType == MotionEvent.TOOL_TYPE_ERASER; + int pointerCount = event.getPointerCount(); + int actionIndex = event.getActionIndex(); + if (actionIndex >= 0 && actionIndex < pointerCount) { + int toolType = event.getToolType(actionIndex); + if (toolType == MotionEvent.TOOL_TYPE_STYLUS || toolType == MotionEvent.TOOL_TYPE_ERASER) { + return true; + } + } + for (int i = 0; i < pointerCount; i++) { + int toolType = event.getToolType(i); + if (toolType == MotionEvent.TOOL_TYPE_STYLUS || toolType == MotionEvent.TOOL_TYPE_ERASER) { + return true; + } + } + return false; } private static boolean isStylusButtonPressed(MotionEvent event) { int buttonState = event.getButtonState(); boolean stylusButtonPressed = (buttonState & (MotionEvent.BUTTON_STYLUS_PRIMARY | MotionEvent.BUTTON_STYLUS_SECONDARY | MotionEvent.BUTTON_SECONDARY )) != 0; - boolean toolSetToEraser = event.getToolType(0) == MotionEvent.TOOL_TYPE_ERASER; + boolean toolSetToEraser = false; + int pointerCount = event.getPointerCount(); + int actionIndex = event.getActionIndex(); + if (actionIndex >= 0 && actionIndex < pointerCount) { + toolSetToEraser = event.getToolType(actionIndex) == MotionEvent.TOOL_TYPE_ERASER; + } + if (!toolSetToEraser) { + for (int i = 0; i < pointerCount; i++) { + if (event.getToolType(i) == MotionEvent.TOOL_TYPE_ERASER) { + toolSetToEraser = true; + break; + } + } + } return stylusButtonPressed || toolSetToEraser; }#!/bin/bash # Verify stylus helper pointer-index usage and nearby context. rg -n -C3 'isEventTriggeredByStylus|isStylusButtonPressed|getToolType\(0\)|getActionIndex\(' app/src/main/java/com/winlator/widget/TouchpadView.javaAlso applies to: 403-404
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/widget/TouchpadView.java` around lines 392 - 395, The helper currently calls getToolType(0) which misclassifies mixed-pointer events; change isEventTriggeredByStylus to inspect the correct pointer by iterating over event.getPointerCount() (or use event.getActionIndex() when you only care about the pointer that triggered the action) and check event.getToolType(i) for MotionEvent.TOOL_TYPE_STYLUS or TOOL_TYPE_ERASER, and apply the same fix to any other places using getToolType(0) (e.g., isStylusButtonPressed) so stylus/eraser detection uses the actual pointer index rather than a hardcoded 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/src/main/java/com/winlator/widget/TouchpadView.java`:
- Around line 392-395: The helper currently calls getToolType(0) which
misclassifies mixed-pointer events; change isEventTriggeredByStylus to inspect
the correct pointer by iterating over event.getPointerCount() (or use
event.getActionIndex() when you only care about the pointer that triggered the
action) and check event.getToolType(i) for MotionEvent.TOOL_TYPE_STYLUS or
TOOL_TYPE_ERASER, and apply the same fix to any other places using
getToolType(0) (e.g., isStylusButtonPressed) so stylus/eraser detection uses the
actual pointer index rather than a hardcoded 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fb05329f-64c6-419f-a2cf-f962cb34742d
📒 Files selected for processing (1)
app/src/main/java/com/winlator/widget/TouchpadView.java
Contributor
|
You should also introduce |
Owner
|
Thanks, good fix |
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
3 participants
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.
Description
Using stylus, for example S-Pen, didn't support hovering over the screen to move the mouse, so it was impossible to read tooltips. I also enabled the right click by holding the side button and tapping
It was partially implemented, but one of the methods wasn't even used anywhere
Recording
Screen_Recording_20260429_215641_GameNative.mp4
Screen_Recording_20260429_215518_GameNative.mp4
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Fixes stylus interactions in TouchpadView. Adds hover cursor movement and reliable right‑click with stylus buttons or eraser, so tooltips and context menus work.
Bug Fixes
Refactors
android.util.Logtotimber.log.Timber.Written for commit f2b070a. Summary will update on new commits. Review in cubic
Summary by CodeRabbit