linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Harry Cutts <hcutts@chromium.org>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Jiri Kosina <jkosina@suse.cz>,
	linux-input@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Logitech high-resolution scrolling..
Date: Tue, 30 Oct 2018 12:53:03 -0300	[thread overview]
Message-ID: <20181030125303.0f94b6e0@coco.lan> (raw)
In-Reply-To: <CAHk-=wjisf8Bmgbtf3y0W+Lu58t3nSnvKGc0J=Zo=rmz3eA+Cw@mail.gmail.com>

Em Sun, 28 Oct 2018 14:08:31 -0700
Linus Torvalds <torvalds@linux-foundation.org> escreveu:

> On Sun, Oct 28, 2018 at 12:13 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > So the recent change to enable the high-res scrolling really seems a
> > bit *too* extreme.
> >
> > Is there some middle ground that turns the mouse from "look at it
> > sideways and it starts scrolling" to something slightly more
> > reasonable?  
> 
> Actually, I think the bug may be in the generic HID high-resolution
> scrolling code, and I only notice because the Logitech support means
> that now I see it.
> 
> In particular, if you look at hid_scroll_counter_handle_scroll(),
> you'll notice that it tries to turn a high-res scroll event into a
> regular wheel event by using the resolution_multiplier.
> 
> But that code looks really broken. It tries to react to a "half
> multiplier" thing:
> 
>         int threshold = counter->resolution_multiplier / 2;
>    ..
>         counter->remainder += hi_res_value;
>         if (abs(counter->remainder) >= threshold) {
> 
> and that's absolutely and entirely wrong.
> 
> Imagine that the high-res wheel counter has just moved a bit up (by
> one high-res) tick, so now it's at the half-way mark to the
> resolution_multiplier, and we scroll up by one:
> 
>                 low_res_scroll_amount =
>                         counter->remainder / counter->resolution_multiplier
>                         + (hi_res_value > 0 ? 1 : -1);
>                 input_report_rel(counter->dev, REL_WHEEL,
>                                  low_res_scroll_amount);
> 
> and then correct for it:
> 
>                 counter->remainder -=
>                         low_res_scroll_amount * counter->resolution_multiplier;
> 
> now we went from "half resolution multiplier positive" to "half negative".
> 
> Which means that next time that the high-res event happens by even
> just one high-resolution tick in the other direction, we'll now
> generate a low-resolution scroll event in the other direction.
> 
> In other words, that function results in unstable behavior. Tiny tiny
> movements back-and-forth in the high-res wheel events (which could be
> just because either the sensor is unstable, or the wheel is wiggling
> imperceptibly) can result in visible movement in the low-res
> ("regular") wheel reporting.
> 
> There is no "damping" function, in other words. Noise in the high
> resolution reading can result in noise in the regular wheel reporting.
> 
> So that threshold handling needs to be fixed, I feel. Either get rid
> of it entirely (you need to scroll a *full* resolution_multiplier to
> get a regular wheel event), or the counter->remainder needs to be
> *cleared* when a wheel event has been sent so that you don't get into
> the whole "back-and-forth" mode.
> 
> Or some other damping model. I suspect there are people who have
> researched what the right answer is, but I guarantee that the current
> code is not the right answer.
> 
> I suspect this also explains why I *sometimes* see that "just moving
> the mouse sends wheel events", and at other times don't. It needs to
> get close to that "half a resolution multiplier" stage to get into the
> bad cases, but then tiny tiny perturbations can cause unstable
> behavior.
> 
> I can't be the only person seeing this, but I guess the Logitech mouse
> is right now the only one that uses the new generic HID code, and I
> guess not a lot of people have been *using* it.

I remember I submitted in the past some patches adding a different event
for the high scroll mode. I have myself a MX Anywhere 2, and even wrote
some patches for Solaar[1] in order to allow selecting between low
res and high res wheel modes.

The problem I faced, on that time, was similar to yours: when the
high res wheel was enabled, it was very hard to control the mouse,
specially when using the wheel in "free" mode (with I do).

I remember that the patchset I sent was not actually applied, but I
didn't followed what happened after that (got sidetracked by
something else).

[1] https://github.com/pwr/Solaar

Thanks,
Mauro

  reply	other threads:[~2018-10-30 15:53 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-28 19:13 Logitech high-resolution scrolling Linus Torvalds
2018-10-28 21:08 ` Linus Torvalds
2018-10-30 15:53   ` Mauro Carvalho Chehab [this message]
2018-10-29 13:18 ` Jiri Kosina
2018-10-29 15:16   ` Linus Torvalds
2018-10-29 18:32     ` Linus Torvalds
2018-10-29 19:17       ` Harry Cutts
2018-10-29 21:11         ` Linus Torvalds
2018-10-29 21:42           ` Harry Cutts
2018-10-29 22:00             ` Linus Torvalds
2018-10-29 23:03               ` Harry Cutts
2018-10-30  6:26                 ` Peter Hutterer
2018-10-30 16:29                   ` Linus Torvalds
2018-10-30 17:48                     ` Harry Cutts
2018-10-31 13:47                       ` Nestor Lopez Casado

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181030125303.0f94b6e0@coco.lan \
    --to=mchehab+samsung@kernel.org \
    --cc=benjamin.tissoires@redhat.com \
    --cc=hcutts@chromium.org \
    --cc=jkosina@suse.cz \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).