Increase values for touchpad acceleration that can be set from kcm libinput UI
ClosedPublic

Authored by kurmikon on May 2 2020, 7:47 PM.

Details

Summary

Plasma System settings can set only 11 values for touchpad acceleration. The option accept a float number between -1 and 1 but, because of the slider limitation, only 11 values in between multiple of 0,2 con be set.

I think this is a great limitation since some touchpad models need specific values to give the user the proper comfort in handling the pointer. Libinput has already less options than the old synaptic, at least let's give the user the ability to set the acceleration with more precision.

In example, the best value for my touchpad is 0,15, but I can't set it, and even if 0,2 is the nearest, I feel better with 0,15 and want to set it. Unfortunately I can't from the kcm UI and I have to modify ~/.config/touchpadxlibinputrc config manually. I think this is unacceptable for a DE like Plasma that claims to be highly customizable.

So, in order to increase values that can be set with the slider, I suggest to modify the formula in touchpad.qml to pick 41 values in the middle, multiple of 0,05.

Diff Detail

Repository
R119 Plasma Desktop
Lint
Lint Skipped
Unit
Unit Tests Skipped
kurmikon created this revision.May 2 2020, 7:47 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 2 2020, 7:47 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
kurmikon requested review of this revision.May 2 2020, 7:47 PM

I agree with the premise that 0.2 steps are much too coarse, and have periodically groused about this myself. However I wonder if 0.05 is too fine? A slider with 41 ticks is screaming out "you used the wrong control for this!" Perhaps it should be a spinbox instead, or it should just be wider so the tickmarks don't look so closely spaced. VDG people, opinions?

kurmikon updated this revision to Diff 81847.EditedMay 4 2020, 8:38 AM

The spinbox would be a better solution.

https://doc.qt.io/qt-5/qml-qtquick-controls2-spinbox.html

Of course the problem with a spinbox is that by displaying numbers, it demands some kind of legend or label. What does a value of 100 mean? Does a value of 0 make any sense? etc. The value displayed to the user must make some sense or explained with context. In this case, perhaps a percentage would work, with the default value being "100%", and the max and min values displaying 50% and 200%, or something, or whatever the scale actually is.

kurmikon added a comment.EditedMay 4 2020, 7:20 PM

Of course the problem with a spinbox is that by displaying numbers, it demands some kind of legend or label. What does a value of 100 mean? Does a value of 0 make any sense? etc. The value displayed to the user must make some sense or explained with context.

Well, I posted the slider with more ticks, you said it wasn't good. Maybe is it better to increase the width? I will check it out to get a wider size.

In this case, perhaps a percentage would work, with the default value being "100%", and the max and min values displaying 50% and 200%, or something, or whatever the scale actually is.

-100 the slowest acceleration and 100 the fastest. Maybe a legend would be good. I don't think libinput has a specific scale. From -100% to 100% with 0% default would be good also?

I think both solutions are better than the actual one since they are more accurate.

Another solution. Spinbox 0% to 100%. -1 is 0% and 1 is 100%. Libinput default 0 will set 50%.

// Acceleration
RowLayout {
    Controls.SpinBox {
        Kirigami.FormData.label: i18nd("kcm_touchpad", "Pointer speed:")
        id: accelSpeed

        from: 0
        to: 100
        stepSize: 1

        function load() {
            enabled = touchpad.supportsPointerAcceleration
            if (!enabled) {
                value = 50
                return
            }
            // transform libinput's pointer acceleration range [-1, 1] to spinbox range [0, 100] %
            value = 50 + touchpad.pointerAcceleration / 0.02
        }

        onValueChanged: {
            if (touchpad != undefined && enabled && !root.loading) {
                // transform spinbox range [0, 100] to libinput's pointer acceleration range [-1, 1]
                touchpad.pointerAcceleration = Math.round(((value-50) * 0.02) * 100) / 100
                root.changeSignal()
            }
        }
    }

    QtControls.Label {
        text: i18n("%")
    }
}

Hmm, 0% as a lower value would imply no movement speed, which is obviously not possible here.

Hmm, 0% as a lower value would imply no movement speed, which is obviously not possible here.

As far as I know, libinput manages acceleration rather speed. Let's change the label to Pointer acceleration.

ndavis added a subscriber: ndavis.EditedMay 5 2020, 1:29 AM

Of course the problem with a spinbox is that by displaying numbers, it demands some kind of legend or label. What does a value of 100 mean? Does a value of 0 make any sense? etc. The value displayed to the user must make some sense or explained with context. In this case, perhaps a percentage would work, with the default value being "100%", and the max and min values displaying 50% and 200%, or something, or whatever the scale actually is.

Didn't we solve this already with Slider + Spinbox in the Display Configuration KCM?

Of course the problem with a spinbox is that by displaying numbers, it demands some kind of legend or label. What does a value of 100 mean? Does a value of 0 make any sense? etc. The value displayed to the user must make some sense or explained with context. In this case, perhaps a percentage would work, with the default value being "100%", and the max and min values displaying 50% and 200%, or something, or whatever the scale actually is.

Didn't we solve this already with Slider + Spinbox in the Display Configuration KCM?

With percentage there's no need to set a slider in my opinion. It's intuitive that 0 and 100 and the limits. That implementation would be good with the real values libinput handles for "AccelSpeed" option, showing the slider moves between -1 and 1, see here.

But if that case @ngraham will say the ticks are too many for the slider and we lose accuracy. There's no way to build a slider like the brightness one (plus a spinbox) inside power management kcm with real values and without ticks? A format function to round on at least two decimal will be given.

ndavis added a comment.May 5 2020, 9:11 AM

I had a look at the documentation for libinput pointer acceleration (https://wayland.freedesktop.org/libinput/doc/latest/pointer-acceleration.html). For anyone following this conversation:

  • 0 is the default
  • when greater than 0, acceleration starts sooner and pointer movement increases by larger factors
  • when less than 0, acceleration starts later and pointer movement increases by smaller factors until pointer movement does not increase with mouse movement speed. The point where that happens is somewhere in the range of greater than -0.75 and less than -0.5.

With percentage there's no need to set a slider in my opinion. It's intuitive that 0 and 100 and the limits. That implementation would be good with the real values libinput handles for "AccelSpeed" option, showing the slider moves between -1 and 1, see here.

Since understanding the way values match acceleration speeds is kind of tricky here, I don't think making the value a percentage would be very helpful. Only people who read the documentation will actually know what to expect from the values, so I think it would be best to show values that match the documentation.

But if that case @ngraham will say the ticks are too many for the slider and we lose accuracy. There's no way to build a slider like the brightness one (plus a spinbox) inside power management kcm with real values and without ticks? A format function to round on at least two decimal will be given.

In the Display Configuration KCM, the slider and spin box in the screenshot have different increments. The slider has increments of 25% to quickly give values that should work fine for most people. The spinbox has increments of 6.25% for those that want finer control.

I had a look at the documentation for libinput pointer acceleration (https://wayland.freedesktop.org/libinput/doc/latest/pointer-acceleration.html). For anyone following this conversation:

  • 0 is the default
  • when greater than 0, acceleration starts sooner and pointer movement increases by larger factors
  • when less than 0, acceleration starts later and pointer movement increases by smaller factors until pointer movement does not increase with mouse movement speed. The point where that happens is somewhere in the range of greater than -0.75 and less than -0.5.

Indeed it's acceleration, not speed, also the label has to be changed.

Since understanding the way values match acceleration speeds is kind of tricky here, I don't think making the value a percentage would be very helpful. Only people who read the documentation will actually know what to expect from the values, so I think it would be best to show values that match the documentation.

Good idea. Something like this:

// Acceleration
Controls.SpinBox {
    Kirigami.FormData.label: i18nd("kcm_touchpad", "Pointer acceleration:")
    id: accelSpeed

    from: -100
    to: 100
    stepSize: 10
    editable: true

    function load() {
        enabled = touchpad.supportsPointerAcceleration
        if (!enabled) {
            value = 0
            return
        }
        value = Math.round(touchpad.pointerAcceleration * 100)

    }

    onValueChanged: {
        if (touchpad != undefined && enabled && !root.loading) {
            touchpad.pointerAcceleration = value / 100
            root.changeSignal()
        }
    }
    
    textFromValue: function(value, locale) {
        return Number(value / 100).toLocaleString(locale, 'f', 2)
    }

    valueFromText: function(text, locale) {
        return Math.round(Number.fromLocaleString(locale, text) * 100)
    }
}

Stepsize of 10 so you won't complain to navigate too much, but we have the accuracy of 2 decimal numbers.

I like @ndavis's idea of using a slider and a spinbox. The slider can be used for coarse adjustment, while the spinbox can be used for fine-tuning and have a very fine step size. I think that would work.

I like @ndavis's idea of using a slider and a spinbox. The slider can be used for coarse adjustment, while the spinbox can be used for fine-tuning and have a very fine step size. I think that would work.

[-1, 1] range of values?

Yep, exposing the real values there is probably fine.

kurmikon updated this revision to Diff 82323.May 8 2020, 9:34 PM

So I worked on this.

It's not easy to me to do this stuff since the slider and the spinbox have to synchronize themselves on the other one.

Making symply and update from one to another would lead to infinite loops.

On first attempt I succeeded to synchronize them, but failed to show the right values when the kcm was started, maybe a race condition on the load method was causing an issue.

Second attempt, I made it with this diff. It's working, tested several times on my system. Test on yours, please.

Don't know if there's an easier way to make it. Let me know.

Indeed, the code here is a catastrophe. I'm actually in the middle of a rewrite to make everything more declarative which should make UI changes much easier. If you'd like to wait a few days until that's done, it might make your patch a lot simpler.

Indeed, the code here is a catastrophe. I'm actually in the middle of a rewrite to make everything more declarative which should make UI changes much easier. If you'd like to wait a few days until that's done, it might make your patch a lot simpler.

Okay, but I don't get it. Will you rewrite touchpad.qml so this can be closed and a new revision will be opened or will you just change on this one?

The latter, if you don't mind. I appreciate your patience! I hope to have the rewrite submitted tomorrow.

Actually sorry, I won't get to that refactor soon so I don't want to block your work.

Is this reviewable in its current state?

Actually sorry, I won't get to that refactor soon so I don't want to block your work.

Is this reviewable in its current state?

Can you explain how I can improve this code? Basically I did that way to avoid update loops. The hard part is that the spinbox and the slider have to be synchronized.

Has qml something native to do that instead or relying on custom property flags and control methods? I didn't find anything related in the documentation, but maybe I'm missing something.

Typically would would bind both to the same backend value and everything would just magically work. However the code in this KCM is terrible, and is quite imperative, without using bindings properly. So you'll have to manually set both anywhere the slider's value is currently set, and read the value of both anywhere the slider's value is read.

Typically would would bind both to the same backend value and everything would just magically work. However the code in this KCM is terrible, and is quite imperative, without using bindings properly. So you'll have to manually set both anywhere the slider's value is currently set, and read the value of both anywhere the slider's value is read.

If I have to read from both, I have to use the same piece of code on value change, but which one I have to choose? Since the spinbox is more accurate I will prefer the spinbox, but who knows which one was changed?

And if I set both, the "values change" will trigger the function another time. Isn't this going in loop? Or, if not in loop, it will write the value more than one time.

Unfortunately I'm not able to bind the widgets to a backend. I'll see if I can study something on this, but I don't know if I will make it.

kurmikon updated this revision to Diff 82966.EditedMay 15 2020, 5:54 PM

Diff updated to a better version. So, I'm explaining what happens here.

A RowLayout is placed where the old slider was. This contains a new slider and an additional spinbox.

The spinbox gets float values by the user as reported by libinput documentation: [-1, 1]. Displayed value is converted in the internal integer value [-100, 100] with a stepsize of 1.

The slider is basically the same as before, range [1, 11], only values multiple of 0,2 can be selected. An additional property AccelSpeedValue is set to convert the internal value to the format used by the spinbox.

What happens when one control is modified. If the spinbox is modified, its new internal value is passed to onAccelSpeedChanged method of the RowLayout. Il the slider is modified, it's new AccelSpeedValue is passed to onAccelSpeedChanged method of the RowLayout.

What onAccelSpeedChanged does. It takes the argument and compares it three times.

  1. with the slider accelSpeedValue; if it's different, the slider has been updated
  2. with the spinbox internal value; if it's different, the spinbox has been updated
  3. the argument divided by 100 with the libinput pointer acceleration; if it's different, the libinput property is updated and root.changeSignal() is invoked.

Surely the method is called another time because of the update of the other control, but all checks will result false and nothing happens, avoiding loops, double writings and other issues.

Compiled and tested on my system, it's working. Please, test on yours. Can't do better than that with my poor knowledge.

Thanks for your work. Will re-review soon.

@ngraham is there an admin that can review this or should I abandon and make a MR on gitlab?

Sorry for the delay in reviewing this. It's on my to-do list. I'll try to make some time today (it is a national holiday in my country).

Overall it works very well given the current plumbing here! I'll accept it after one little change:

kcms/touchpad/kcm/libinput/touchpad.qml
294

Can you set Layouts.Layout.minimumWidth: Kirigami.Units.gridUnit * 4 on this so it doesn't change in width when the numberincreases in size?

ngraham accepted this revision.Jun 4 2020, 2:50 PM

I'll fix that for you and land this.

This revision is now accepted and ready to land.Jun 4 2020, 2:50 PM
ngraham closed this revision.Jun 4 2020, 3:02 PM