All of lore.kernel.org
 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: 16+ 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  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.