fix: dynamic cursor visibility based on input source#656
Open
jeremybernstein wants to merge 3 commits intoutkarshdalal:masterfrom
Open
fix: dynamic cursor visibility based on input source#656jeremybernstein wants to merge 3 commits intoutkarshdalal:masterfrom
jeremybernstein wants to merge 3 commits intoutkarshdalal:masterfrom
Conversation
cursor hidden in touchscreen mode on touch, shown on external mouse or controller-as-mouse movement. removes touchscreen mode check that was preventing cursor from showing when app window first appears.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughShow cursor when external mouse/controller input occurs and hide it for touch in touchscreen mode; guard XServer null-safety and ensure renderer cursor is visible before injecting pointer-move deltas; pass mouseEnabled into PhysicalControllerHandler. Changes
Sequence Diagram(s)sequenceDiagram
participant Touchpad as "TouchpadView / InputControlsView"
participant Controller as "PhysicalControllerHandler"
participant TouchMouse as "TouchMouse"
participant XServer as "XServer (renderer)"
Touchpad->>Touchpad: detect input (touch or external mouse)
alt External mouse or controller-as-mouse
Touchpad->>XServer: if cursor hidden -> setCursorVisible(true)
Touchpad->>Controller: forward pointer delta (if mouseEnabled)
Controller->>XServer: xServer?.let -> ensure cursor visible -> injectPointerMoveDelta
TouchMouse->>XServer: onExternalMouseEvent ensures cursor visible then handle buttons/scroll
else Touch input in touchscreen mode
Touchpad->>XServer: if cursor visible -> setCursorVisible(false)
end
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. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/com/winlator/widget/InputControlsView.java">
<violation number="1" location="app/src/main/java/com/winlator/widget/InputControlsView.java:292">
P2: The new timer tick unconditionally re-shows the cursor, which overrides touchscreen-mode hiding because the timer runs continuously once created.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![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: 3
🤖 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/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt`:
- Around line 153-157: The current PhysicalControllerHandler path forcibly calls
it.renderer.setCursorVisible(true) whenever controller simulates mouse; change
this to respect the disabled-mouse policy by checking the screen/container flag
before toggling visibility: retrieve the mouse policy (or a boolean like
isMouseEnabled) from the screen where container flags live and only call
setCursorVisible(true) when mouse input is enabled; update the method in
PhysicalControllerHandler (the block that calls it.injectPointerMoveDelta) to
accept or access that policy and guard the setCursorVisible call accordingly.
In `@app/src/main/java/com/winlator/inputcontrols/TouchMouse.java`:
- Around line 304-307: In TouchMouse, the renderer cursor code
(xServer.getRenderer().isCursorVisible() / setCursorVisible(true)) must be moved
into the branch that confirms the event came from an external mouse-capable
device; locate the branch that checks the input device/type (the mouse-device
handling block in TouchMouse where the code discriminates pointer/mouse devices)
and place the cursor-show logic inside that block so the cursor is only made
visible when the event originates from an external mouse.
In `@app/src/main/java/com/winlator/widget/TouchpadView.java`:
- Around line 609-612: The cursor is being made visible unconditionally; change
the logic in TouchpadView around the xServer.getRenderer().isCursorVisible()
check to only call xServer.getRenderer().setCursorVisible(true) when the
incoming event is an actual external mouse event (verify the input source/type
on the MotionEvent/InputDevice) and when the local "disabled-mouse" mode is not
set; in other words, add a guard that checks the event's input source is a mouse
(external) and that the disabled-mouse flag (use the existing field controlling
local mouse disablement) is false before toggling visibility.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/inputcontrols/TouchMouse.javaapp/src/main/java/com/winlator/widget/InputControlsView.javaapp/src/main/java/com/winlator/widget/TouchpadView.java
app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt (1)
154-157: Guard pointer movement injection when mouse input is disabled.Line 156 injects pointer movement even when
mouseEnabled == false. For consistency with line 155 (which guards cursor visibility) and the intended semantics of disabling mouse input entirely (per PrefManager comment: "prevents external mouse events"), pointer movement injection should also be blocked when mouse is disabled.🔧 Suggested change
xServer?.let { - if (mouseEnabled && !it.renderer.isCursorVisible) it.renderer.setCursorVisible(true) - it.injectPointerMoveDelta(deltaX, deltaY) + if (!mouseEnabled) return@let + if (!it.renderer.isCursorVisible) it.renderer.setCursorVisible(true) + it.injectPointerMoveDelta(deltaX, deltaY) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt` around lines 154 - 157, The pointer move is being injected regardless of mouseEnabled; update the guard around xServer usage in PhysicalControllerHandler (the block handling xServer?.let { ... }) so that injectPointerMoveDelta(deltaX, deltaY) is only called when mouseEnabled is true (same condition used for renderer.setCursorVisible), i.e. check mouseEnabled before calling injectPointerMoveDelta on the xServer instance to prevent external pointer movement when mouse input is disabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt`:
- Around line 154-157: The pointer move is being injected regardless of
mouseEnabled; update the guard around xServer usage in PhysicalControllerHandler
(the block handling xServer?.let { ... }) so that injectPointerMoveDelta(deltaX,
deltaY) is only called when mouseEnabled is true (same condition used for
renderer.setCursorVisible), i.e. check mouseEnabled before calling
injectPointerMoveDelta on the xServer instance to prevent external pointer
movement when mouse input is disabled.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/inputcontrols/TouchMouse.javaapp/src/main/java/com/winlator/widget/TouchpadView.java
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/winlator/widget/TouchpadView.java
- app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
- app/src/main/java/com/winlator/inputcontrols/TouchMouse.java
Review suggested guarding setCursorVisible behind disableMouseInput checks, but that flag means "don't let touch generate mouse events" not "disable all pointing devices." External mouse and controller- as-mouse should show cursor regardless. Reverted touchscreenMouseDisabled guard in TouchpadView, removed mouseEnabled param from PhysicalControllerHandler. Kept: SOURCE_MOUSE check in TouchpadView and isMouseDevice check in TouchMouse — cursor only shows for actual mouse device events.
jeremybernstein
force-pushed
the
jb/cursor-visibility
branch
from
e53d6df to
f44523a
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt (1)
153-156:⚠️ Potential issue | 🟠 MajorController-mouse movement still bypasses disabled-mouse visibility policy.
This block always re-shows the cursor during controller-as-mouse movement. It should be gated by a mouse-enabled policy so disabled-mouse mode keeps the cursor hidden.
🔧 Suggested fix
class PhysicalControllerHandler( private var profile: ControlsProfile?, private val xServer: XServer?, + private val isMouseInputEnabled: () -> Boolean = { true }, private val onOpenNavigationMenu: (() -> Unit)? = null ) { @@ val deltaX = (mouseMoveOffset.x * 10 * cursorSpeed).toInt() val deltaY = (mouseMoveOffset.y * 10 * cursorSpeed).toInt() xServer?.let { - if (!it.renderer.isCursorVisible) it.renderer.setCursorVisible(true) + if (isMouseInputEnabled() && !it.renderer.isCursorVisible) { + it.renderer.setCursorVisible(true) + } it.injectPointerMoveDelta(deltaX, deltaY) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt` around lines 153 - 156, The code inside xServer?.let always calls it.renderer.setCursorVisible(true) which re-enables the cursor during controller-as-mouse motion; wrap the visibility change in a check against the app's mouse-enabled policy (e.g., an isMouseEnabled()/mouseEnabled flag) so that you only call it.renderer.setCursorVisible(true) when mouse mode is allowed, while still calling it.injectPointerMoveDelta(deltaX, deltaY) unconditionally; update the block around xServer, renderer.isCursorVisible, setCursorVisible and injectPointerMoveDelta to gate visibility changes by the policy check.
🤖 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/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt`:
- Around line 153-156: The code inside xServer?.let always calls
it.renderer.setCursorVisible(true) which re-enables the cursor during
controller-as-mouse motion; wrap the visibility change in a check against the
app's mouse-enabled policy (e.g., an isMouseEnabled()/mouseEnabled flag) so that
you only call it.renderer.setCursorVisible(true) when mouse mode is allowed,
while still calling it.injectPointerMoveDelta(deltaX, deltaY) unconditionally;
update the block around xServer, renderer.isCursorVisible, setCursorVisible and
injectPointerMoveDelta to gate visibility changes by the policy check.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.ktapp/src/main/java/com/winlator/inputcontrols/TouchMouse.javaapp/src/main/java/com/winlator/widget/TouchpadView.java
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/winlator/widget/TouchpadView.java
Contributor
Author
@coderabbitai enable mouse mode isn't supposed to prevent all mouse activity, just touchscreen mouse events. The code is correct. |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
|
rename to "Disable Touchscreen" with description clarifying it only prevents touch-generated mouse events. external mouse and controller still work. touchscreen mode preference disabled when touchscreen is disabled since it has no effect.
Contributor
Author
|
Added: renamed "Disable Mouse Input" to "Disable Touchscreen" with description clarifying it only prevents touch-generated mouse events. Touchscreen Mode preference is now disabled (greyed out) when touchscreen is disabled. |
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
1 participant
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
isTouchscreenModecheck that blocked cursor visibility on app window startFixes #655
Test plan
disableMouseInputON: cursor stays hidden regardlessSummary by cubic
Make cursor visibility adapt to the input source. Touch hides the cursor in touchscreen mode; external mouse, controller-as-mouse, and on-screen controls show it. Cursor appears on first app window unless Disable Touchscreen is on.
Bug Fixes
Refactors
Written for commit 1fe1e0f. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Settings / UI