Page MenuHomeFreeBSD

Add natural scrolling support to psm.
ClosedPublic

Authored by nyan_myuji.xyz on May 28 2019, 5:12 PM.

Details

Summary

Natural scrolling is one of the features that is not covered in https://wiki.freebsd.org/SynapticsTouchpad with moused.

This patch enables natural scrolling in vertical and horizontal direction with two finger scroll enabled and when user is using a trackpad (mouse and trackpoint are not affected) by adding two sysctl that can each enabled/disable natural scrolling in one of the direction with using trackpad.

Test Plan

Tested on Thinkpad X1 Extreme.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

nyan_myuji.xyz created this revision.May 28 2019, 5:12 PM
lwhsu added a subscriber: lwhsu.May 28 2019, 5:25 PM
lwhsu added a reviewer: wulf.May 28 2019, 5:34 PM
lwhsu added inline comments.May 28 2019, 7:16 PM
sys/dev/atkbdc/psm.c
5753

I think we should have a /* FALLTHROUGH */ here (and following line) per https://man.freebsd.org/style/9

wulf added a comment.May 28 2019, 9:04 PM

Hi Nyan,
Could you upload the diff as fullcontext diff next time. See https://wiki.freebsd.org/Phabricator

git diff -U999999 other-branch > change.diff
git show -U999999 <commit-hash> > change.diff
svn diff --diff-cmd=diff -x -U999999 > change.diff`

It is not necessary to reupload current diff

sys/dev/atkbdc/psm.c
4336

Are there any reasons to limit axis inversion to two finger scrolling only?

4337

IMO this operation can be simplified with using of logical XOR:
ms->button |= ((dyp > 0) != (two_finger_scroll && natural_ver_scroll)) ?

5753

"Do nothing" fallthrough cases does not require explicit /* FALLTHROUGH */ . See style(9) getopt() example, case '?':

lwhsu added inline comments.May 28 2019, 9:38 PM
sys/dev/atkbdc/psm.c
5753

Oh that's right, sorry for the noise.

In D20447#441492, @wulf wrote:

Hi Nyan,
Could you upload the diff as fullcontext diff next time. See https://wiki.freebsd.org/Phabricator

git diff -U999999 other-branch > change.diff
git show -U999999 <commit-hash> > change.diff
svn diff --diff-cmd=diff -x -U999999 > change.diff`

It is not necessary to reupload current diff

Ah my bad, will pay attention next time.

nyan_myuji.xyz added inline comments.May 29 2019, 5:25 AM
sys/dev/atkbdc/psm.c
4336

This is because when two finger scrolling is disabled, virtual scrolling from dedicated area, which has an expected behavour of "move to the direction the scroll bar goes",

When the option is activated but without two finger scrolling enabled, the normal virtual scroll from dedicated area will get inverted as will, which implies that the scrolling direction from the dedicated area will be opposite to the direction of the scroll bar.

wulf accepted this revision.May 29 2019, 8:28 PM
This revision is now accepted and ready to land.May 29 2019, 8:28 PM
wulf added a comment.Jun 1 2019, 12:46 PM

One more question:

Why ver and hor sysctls are added separately?

I think single "natural_scroll" is enough.

In D20447#442373, @wulf wrote:

One more question:
Why ver and hor sysctls are added separately?
I think single "natural_scroll" is enough.

There are not much good reasons but was just because I was thinking that this will be a bit more flexible.

This revision was automatically updated to reflect the committed changes.
wulf added a comment.Jun 3 2019, 10:09 AM

Given the fact that horizontal scrolling with sysmouse protocol is not supported by userland software out of boх, I joined both hor & ver sysctls into common "natural_scroll" one to not confuse users.