Rewrite and improve mouse emulation#531
Open
mevouc wants to merge 26 commits intoClassicOldSong:moonlight-noirfrom
Open
Rewrite and improve mouse emulation#531mevouc wants to merge 26 commits intoClassicOldSong:moonlight-noirfrom
mevouc wants to merge 26 commits intoClassicOldSong:moonlight-noirfrom
Conversation
This reverts most languages from commit 2cee67e.
…stant - Also make the Milliseconds units explicit for mouse emulation report period
- Explicitly name hardcoded values with constants - Update comment to clarify the use of cubic acceleration
Delete use of X and Y buttons when using mouse emulation with a controller due to: - Feature was unusable because it does not allow to move cursor horizontally nor vertically. When pressing X, cursor can only go in 45° diagonals - Sensitivity control by pressing Y gives no clear UX feedback and is now redundant with the "Mouse emulation sensitivity" slider in the User settings
- Hardcoded default cursor speed is in pixel/tick - Mouse emulation report period is what is also called "tick" period - Max stick axis is a raw controller value
…tionHandler
Pure structural refactor, zero behavorial change
New file MouseEmulationHandler.java:
- 3 constants (tick period, max stick axis, base speed) moved from ControllerHandler
- StickValueProvider interface for gettitng live stick values from GenericControllerContext
- State variables moved from GenericControllerContext: active, lastInputMap
- Logic scattered accross ControllerHandler and GenericControllerContext:
convertRawStickAxisToPixelMovement, sendEmulatedMouseMove, sendEmulatedMouseScroll, tick runnable, toggle(), destroy(), getMenuOptions(), handleButtonInput()
Changes to ControllerHandler.java:
- Removed 3 constants, 3 methods, unused MouseButtonPacket import
- Moved defaultContext = new InputDeviceContext() from field initializer to constructor body (so outer-class fields like conn are non-null when MouseEmulationHandler is constructed)
- GenericControllerContext now implements MouseEmulationHandler.StickValueProvider with 4 getter methods
- Replaced all mouse emulation inline code with delegation to MouseEmulationHandler
- Controller fusion, sendControllerInputPacket, and handleButtonUp all updated
…ick rate Problem: - 50 ms tick period (20 Hz rate) was not fluid at all - Cursor & scroll speed was tick-rate dependent (each tick produces a fixed pixel delta regardless of elapsed time) Changes: - Increased tick rate to 120 Hz for fluidity & responsivness - Define base speed in pixel/s = 1200 px/s, matches the old tick-dependent speed - Compute real delta time elapsed between two ticks to use real-life speed math
At 120 Hz, each tick lasts ~8ms. With the previous magnitude >= 1 threshold, any speed vector shorter than 1 px per tick was silently discarded. This corresponds to a minimum speed of 120 px/s: the bottom 10% of the speed range, and roughly the bottom 47% of the stick range due to the cubic response curve. At the old 20 Hz rate this was only 20 px/s, so the issue was minor. Moving to 120 Hz made it six times worse, turning a large portion of the stick range into a dead zone on top of the hardware dead zone. Fix: instead of discarding sub-pixel deltas, accumulate them across ticks and only send once the integer part reaches 1. The fractional remainder is carried over, so arbitrarily slow speeds are now represented accurately.
…rt calls Reuse two Vector2d instances (moveVector, scrollVector) across ticks instead of allocating on every tick. scalarMultiply() now updates magnitude directly (magnitude *= |factor|) instead of calling initialize(), which avoids a sqrt recomputation on every scale. initialize() also replaces Math.pow(x, 2) with x * x as a minor cleanup.
No behavorial change. Rewrite convertRawStickAxisToSpeedPxPerSec to make the cubic response curve explicit: speed = MAX_SPEED * (deflection/max)^3. Previous formula (normalize then multiply by magnitude^2) produced the same result but hid the cubic nature behind two opaque scalarMultiply calls
Fix a bug where toggling off (or destroying) while A or B was held left the corresponding mouse button stuck down on the host
Tests for Vector2d: initialize sets x/y/magnitude correctly, scalar multiply scales components and preserves magnitude sign rule, zero vector has magnitude zero Tests for MouseEmulationHandler: A/B flags send LMB/RMB down+up edges, handleButtonInput is a no-op when inactive, zeroed controller packet is always sent to suppress unmapped buttons, toggle-off and destroy both release any held mouse buttons (covers the Step 1 fix)
The previous summary only mentioned the toggle gesture. Users had no way to discover the button mapping without trial and error
…shoot Some controller firmwares calibrate each axis independently, so a full diagonal deflection can report both axes near max (32766, 32766). The resulting raw magnitude (~46340 instead of 32766) caused the cubic response curve to produce diagonal speeds up to 2.83× faster than cardinal speeds Fix by clamping the normalized magnitude to 1.0 before applying the curve. This is a no-op for sticks that already report circular values and has no effect on direction
…tion checkbox "Remember mouse mode" is about touchscreen mode, not mouse emulation with a controller. It must not be greyed-out when mouse emulation is disabled
- Y is used for middle click by default - If an analog stick (left or right) has been selected by user for scrolling. Emulate the analog stick click as mouse middle click as well
- ZERO is a mutable shared instance: any caller passing it to initialize() or scalarMultiply() corrupt it for others + it's unused - getNormalized() is broken because there's no division by zero handling + it's unused
Y flag sends MMB down+up edges, RS_CLK_FLAG/LS_CLK_FLAG send MMB only when the matching scrolling mode is active, stick clicks send no MMB when the wrong mode is active or when scrolling is none, toggle-off and destroy release any held middle click buttons
Replace hardcoded "Mouse emulation is: ON/OFF" Toast string with string resources for localization Include French translation
All fields were public but are only accessed from within ControllerHandler and its inner classes, which are all in the same package
Include French translation as well
|
derflacco's Artemide fork already fixed mouse emulation stuttering. It's smooth as butter, feel free to review their changes on that - clone or improve the code for this fork if needed. |
Author
|
I've just had a look at the code there, and I don't understand how it can be smoother. What change were you referring to? The report period is still 50 ms (https://github.com/derflacco/moonlight-android/blob/8157555264bf1b77eb76261755805cdbba26c3c2/app/src/main/java/com/limelight/binding/input/ControllerHandler.java#L3107). This means the tick rate is still 20 Hz, the same as the one in this repo. 20 frame per second is not smooth. That's why I went up to 120 Hz. The only change I see on derflacco's fork in |
Author
|
Nevermind, it was on a separate experimental branch. Here is the commit: derflacco@670d8d4 They indeed have changed the However, they did it very very dirty in my opinion. Since they kept the tick-rate dependent speed computation, by lowering the tick length to 10 ms they ended-up with a need to compensate the the base speed too. They fixed the shutterness, but they did not understand the core issue behind it (hardcoded tick-rate dependent speed). So they kept the issue and just built around it: it's just a quick fix. My PR goal is to introduce a "clean" rewrite of the all feature. I don't want to take that code into a "clean" rewrite. Even though the intent of increasing smoothness to 100 Hz is the same intent as mine and is a good one, it has made the code even less maintainable on their fork by doing so. |
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
2 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.
Rewrite and improve mouse emulation
In #452, @ClassicOldSong mentioned that mouse emulation is written badly and that it could be rewritten.
I have looked at the code and I fully agree. This PR is doing just that, in an attempt to make this feature a lot better in my opinion.
I've started this branch from the one for PR #529 . That's why it includes a few commits from it at the start.
#529 should be merged first, or I can merge my two PRs into one, but with the UI screenshots, I think it's better to have #529 separated.
Issues found & fixed
ControllerHandlerandGenericControllerContextpublicmethods and fields, unclear hardcoded constants, useless allocations, some dead code, unclear logic and math formulas, maintainability was terribleChanges
Behavioral/feature changes
magnitude >= 1check of mouse movement per tick + introduced fractional pixels accumulation. This fixes the "virtual" dead zone issue and allows for really slow and precise user movementRefactoring & code cleaning
MouseEmulationHandler, extracting all mouse-emulation logic fromControlerHandlerandGenericControllerContext. The only need fromMouseEmulationHandlerfrom its parent classes are the stick values,StickValueProviderlight interface introduced for thispublicfields withprivateor package-private when relevantnewand redundantsqrtcallsBug fixes
releaseHeldMouseButtonsmethod to release held buttons when toggle-off and destroyUI
Code documentation
targetSpeedcomputation andscalarMultiplyformula0values for controller to the hostTests
New unit tests for:
Note on AI usage
The PR is big because there were many issues and because I worked on it almost full-time during 3-4 days; not because AI generated slop.