Skip to content
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

Force mouse source when --forward-all-clicks #3579

Merged
merged 1 commit into from Dec 22, 2022
Merged

Force mouse source when --forward-all-clicks #3579

merged 1 commit into from Dec 22, 2022

Conversation

rom1v
Copy link
Collaborator

@rom1v rom1v commented Nov 13, 2022

Right click and middle click require the source device to be a mouse, not a touchscreen). Therefore, the source device was changed only when a button other than the primary button was pressed (see adc547f).

However, this led to inconsistencies between the ACTION_DOWN when a secondary button is pressed (with a mouse as source device) and the matching ACTION_UP when the secondary button is released (with a touchscreen as source device, because then there is no button pressed).

To avoid the problem in all cases, force a mouse as source device when --forward-all-clicks is set. As a consequence, pinch-to-zoom using a virtual finger is disabled in this case.

Concretely, in --forward-all-clicks mode:

  • device source is set to InputDevice.SOURCE_MOUSE;
  • motion event toolType is set to MotionEvent.TOOL_TYPE_MOUSE;
  • pinch-to-zoom simulation is disabled.

Otherwise:

  • left-click mouse event simulates a first virtual finger;
  • pinch-to-zoom simulation uses a second virtual finger.

For all (virtual or not) finger events:

  • device source is set to InputDevice.SOURCE_TOUCHSCREEN;
  • motion event toolType is set to MotionEvent.TOOL_TYPE_FINGER.

Fixes #3568

TODO:

  • test that nothing is broken on a computer with a touchscreen with --forward-all-clicks
  • test that nothing is broken on a computer with a touchscreen without --forward-all-clicks

@rom1v
Copy link
Collaborator Author

rom1v commented Nov 22, 2022

Given this remark (we could use 2 mice for pinch-to-zoom), I adapted the implementation to also support pinch-to-zoom if --forward-all-clicks is enabled.

cc @paweljasinski

@paweljasinski
Copy link
Contributor

Here are the test results with --forward-all-clicks done on pixel 6 pro with stock android 13.

  1. play store - unable to scroll up/down with mouse as a touch, but roller works - no worry
  2. aFreeRDP - pinch works
  3. termux - pinch works
  4. RD Client (microsoft) - the app does not implement pinch, in addition pressing ctrl triggers conflicting functionality
  5. aRDP Free - pinch emulation doesn't work, pinch works with real touch

For 2,3,4 and 5 the right click works.

@rom1v
Copy link
Collaborator Author

rom1v commented Nov 24, 2022

And for all of them, pinch work without --forward-all-clicks on this branch?

@paweljasinski
Copy link
Contributor

And for all of them, pinch work without --forward-all-clicks on this branch?

  1. yes
  2. yes
  3. yes*
  4. same problems
  5. yes
  • you have to carefully pick the location where you initiate pinch. If the emulated finger gets over the gui keyboard the app interprets it as dragging over the keyboard. This is consistent with real finger touch.

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 2, 2022

Thank you for your feedback.

So it seems to me that this PR is acceptable:

  • it guarantees that primary and secondary clicks are consistent when --forward-all-clicks is set
  • in addition, pinch-to-zoom is not disabled even in --forward-all-clicks (by simulating two mice, even if it is not always handled as expected by the app, it's better than disabling it completely).

I'm not sure if it is good enough, or if we can do better. Maybe what you suggested here could be better, but I'm a bit afraid of side effects (and complexity). What do you think?

@paweljasinski
Copy link
Contributor

During last testing session discovered problems with middle mouse button. Let me first figure out what is going on.

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 21, 2022

OK.

FYI, I will probably release a new version (1.25) soon, especially for #3497.
I plan to merge this one too (it improves the behavior, even if it's not perfect), unless there are major problems with it.
It could be changed later if necessary.

Right click and middle click require the source device to be a mouse,
not a touchscreen. Therefore, the source device was changed only when a
button other than the primary button was pressed (see
adc547f).

However, this led to inconsistencies between the ACTION_DOWN when a
secondary button is pressed (with a mouse as source device) and the
matching ACTION_UP when the secondary button is released (with a
touchscreen as source device, because then there is no button pressed).

To avoid the problem in all cases, force a mouse as source device when
--forward-all-clicks is set.

Concretely, for mouse events in --forward-all-clicks mode:
 - device source is set to InputDevice.SOURCE_MOUSE;
 - motion event toolType is set to MotionEvent.TOOL_TYPE_MOUSE;

Otherwise (when --forward-all-clicks is unset, or for real touch
events), finger events are injected:
 - device source is set to InputDevice.SOURCE_TOUCHSCREEN;
 - motion event toolType is set to MotionEvent.TOOL_TYPE_FINGER.

Fixes #3568 <#3568>
PR #3579 <#3579>

Co-authored-by: Romain Vimont <rom@rom1v.com>
Signed-off-by: Romain Vimont <rom@rom1v.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants