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

Enhancement: X11 local input #841

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

florianhofhammer
Copy link

This pull request targets two issues which were discovered in an environment where barrier is used to control two Linux machines with a touch screen each.
Due to the touch screen and potential other local input devices, some kind of bidirectional communication is necessary, going beyond the mainly unidirectional communication model currently employed in barrier (server -> client).

The two main problems discovered during tests of barrier as a software KVM switch and resolved by this PR are:

  • If the mouse cursor is located on the server and a local touch (or mouse) input occurs on the client, a second cursor appears on the client and both devices have focus.
    If the mouse cursor is located on the client and a touch input occurs on the server, the touch movement is translated to the client. Any touch input by the user thus does not appear at the location of the touch event but on the client display and may thus accidentally trigger unwanted behavior.
  • If the XScreensaver is not currently active, local input on the client (no matter whether through a touch screen as described before or through directly attached mouse or keyboard input devices) cannot prevent the screen saver from activating.
    If the XScreensaver is currently active, input on the server deactivates the screen saver on the server and is propagated to the client. On the other hand, input on the client (touch, mouse movement, etc.) only deactivates the screen saver on the client and is not propagated to the server (or other clients).

The changes made in this PR mainly target the Linux/X11 implementation of barrier because other operating systems were not available for tests. Still, the changes should not affect functionality in other OSs or disrupt previous interoperability between OSs.

@florianhofhammer florianhofhammer force-pushed the enhancement/x-server-client-local-input branch from deb10b2 to 68c52c4 Compare August 18, 2020 16:36
Copy link
Member

@p12tic p12tic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this.

Unfortunately the PR is unreviewable due to unnecessary changes to code formatting.

Could you revert all of the needless changes? This can be done by:

git diff -U0 -w --no-color | git apply --cached --ignore-whitespace --unidiff-zero -

@florianhofhammer
Copy link
Author

florianhofhammer commented Aug 23, 2020

Thanks for the feedback!
Would it be okay for you to rebase the branch so that the changes in formatting due to whitespace changes can be found in an additional commit or do you prefer to have any whitespace changes removed completely?
Apart from that, I'll of course resolve any merge conflicts.

@p12tic
Copy link
Member

p12tic commented Aug 24, 2020

I think the changes that remove trailing whitespace can stay as many tools strip them and it's just annoying to everyone to revert again and again. Maybe you could splice them into a separate commit and do a PR with it?

The changes that change the indentation are less useful. I think they should be reverted. Feel free to use spaces on the lines you change, but for existing code let's not change it for no reason. It breaks git blame which is very useful when investigating problems.

The same is with the fixes to alignment in variable declarations. Feel free to not align variable names among themselves in different lines. This style requires constant realignments which is just waste of time, so let's avoid it for new code.

Allows to switch from client to server and vice versa if
local input is detected, e.g. a touchscreen input or a
mouse click if clients also have input devices
Allows to deactivate the screensaver and postpone screensaver
activation if local input is detected on a client. This fixes
the screensavers getting out of sync if a client has a local
input device such as a touchscreen
@florianhofhammer
Copy link
Author

I removed most of the whitespace changes and reworked my commits. I also resolved merge conflicts by rebasing onto the current master branch.
If you have any questions concerning the code or run into problems, please let me know.

@tattsan
Copy link

tattsan commented Nov 7, 2020

I use barrier with this patch. This is a nice feature, and very comfortable.
However, depending on the nature of the work, I somtimes want to disable this feature.
So I would like to be able to enable or disable this feature in the configuration file.

@florianhofhammer
Copy link
Author

@tattsan Thanks for the positive feedback, I'm glad to hear that!

What exactly do you mean by disabling the feature? In my experiments and usage scenario, this patch was more seen as a bug fix than a feature because without this patch, barrier sometimes behaves unpredictably, for example when it comes to screensavers activating and deactivating out of sync due to touchscreen inputs on the client.
Could you describe your usage scenario to me? If I understand correctly what your plans are, I might think about implementing your request (given that I can find the time to do so of course). However, as I don't have too much experience with barrier's source code, it may take me some time. Additionally, I currently don't have access to two Linux/X11 machines with touchscreens, so I might have to rely on your feedback for functionality tests.

If you'd like to support me with additional information and maybe some testing, I'd be glad to hear from you!

@tattsan
Copy link

tattsan commented Nov 7, 2020

I sometimes want to leave the barrier cursor on the server and use the pen tablet on the client.

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

3 participants