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

[19.09] nixos/libinput: apply options to all device types #73785

Merged

Conversation

@worldofpeace
Copy link
Member

worldofpeace commented Nov 19, 2019

Motivation for this change

Without this change any special config used for libinput like accelSpeed won't apply to mice because it isn't a touchpad.

This could possibly change the user experience for someone, like if they came to rely on the bug.
An example situation would be:

  • have services.xserver.libinput.accelSpeed in config
  • not notice it doesn't apply to your device that isn't a touchpad (mouse)
  • update on 19.09 and suddenly the acceleration of your mouse is different.

It's generally not good to make a change like that, but it would be nice to be able to actually configure your mouse with libinput...

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

@NixOS/backports (and myself apparently)

Instead of assign the libinput options to touchpad devices only, it
should be appied by any device using libinput.
Due to the fact that `40-libinput.conf` already defines libinput as
driver for any detected input device, we can use `MatchDriver` to appy
options.

(cherry picked from commit d616928)
@lheckemann

This comment has been minimized.

Copy link
Member

lheckemann commented Nov 21, 2019

This stops libinput from being set as the driver for all touchpads, so I think we'd need to split that part into a new InputClass section.

I don't think this should go to 19.09 though, since as you said it changes existing behaviour and configuring mice using libinput is possible on 19.09 using services.xserver.inputClassSections.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Nov 21, 2019

This stops libinput from being set as the driver for all touchpads, so I think we'd need to split that part into a new InputClass section.

Regression (I'd definitely not want to backport that)

@lheckemann

This comment has been minimized.

Copy link
Member

lheckemann commented Nov 21, 2019

Yes, and that should be fixed on master as well (has it already been merged to master?).

@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Nov 22, 2019

Yes, and that should be fixed on master as well (has it already been merged to master?).

If you look at the commit is says it has been cherry-picked.

Though what is confusing to me is that I tested the commit functioning, and llibinput list-devices still lists my touchpad etc.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Dec 3, 2019

We've established there was no issues at #72774 (comment).

@worldofpeace worldofpeace merged commit 72a2ced into NixOS:release-19.09 Dec 3, 2019
12 checks passed
12 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@worldofpeace worldofpeace deleted the worldofpeace:libinput-options-all-19.09 branch Dec 3, 2019
@thiagokokada

This comment has been minimized.

Copy link
Contributor

thiagokokada commented Dec 5, 2019

Just to leave it here since this change surprised me. I was actually reliant of the fact that those options are only applied to a touchpad, since I want to enable natural scrolling in my touchpad, however not in my mouse. It was really difficult to understand what was happening.

While it maybe just me, maybe there is more people that had this same issue.

@lheckemann

This comment has been minimized.

Copy link
Member

lheckemann commented Dec 6, 2019

This is exactly why I didn't want it backported…

@thiagokokada

This comment has been minimized.

Copy link
Contributor

thiagokokada commented Dec 6, 2019

Well, actually my setup is broken. I can't set natural scrolling to work only in my touchpad and not in mouse. It is either all devices (using services.xserver.libinput.naturalScrolling = true) or nothing. I tried both services.xserver.extraConfig and services.xserver.inputClassSection and they both don't work.

Could we revert this PR @lheckemann @worldofpeace? Or at least, there is some solution for this problem? It is really awful to have natural scrolling for both mouse and touchpad, however touchpad without natural scrolling is very strange.

@thiagokokada

This comment has been minimized.

Copy link
Contributor

thiagokokada commented Dec 6, 2019

BTW, I have a hypothesis why this is either all or nothing. It is because NaturalScrolling option for libinput is always set, so even if you don't set services.xserver.libinput.naturalScrolling it will be set to false, applying this setting to all devices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.