Fixed volume change with hi-res mice and touchpad scrolling #1857

tsujan · 2023-01-13T06:15:15Z

Fixed volume change with hi-res mice and touchpad scrolling

An angle of 15° is required for changing the volume — as with ordinary mice.

Also,

Changing the volume by touchpad scrolling is fixed; and
Tooltip flickering on wheel rotation is prevented.

kreijack · 2023-01-13T18:53:54Z

plugin-volume/volumepopup.cpp

+
+    int delta = event->angleDelta().y();
+    if ((_delta ^ delta) < 0)
+        _delta = delta; // the wheel direction is reversed


Why this case ? Even if the user changes the wheel direction the previous "fraction" of the stroke has to be considered...
As counter example, if you in a normal mouse each tick is emitted every 15°, and you rotate for +14° up and -14° down you don't have any effect. However with your code an hires mouse you have a tick because +14 - -14 = 28 > 15°.

→ #1856 (comment) → 3

I'm afraid, you misread the code.

kreijack · 2023-01-13T18:55:46Z

plugin-volume/volumepopup.cpp

+    if (qAbs(_delta) >= QWheelEvent::DefaultDeltasPerStep) {
+        m_volumeSlider->setSliderPosition(m_volumeSlider->sliderPosition()
+                + (_delta / QWheelEvent::DefaultDeltasPerStep * m_volumeSlider->singleStep()));
+        _delta = 0;


For the same reason as above, I suggest to write this as _delta = _delta % QWheelEvent::DefaultDeltasPerStep

It would be very wrong. → #1856 (comment) → 2 and especially 4

kreijack · 2023-01-13T18:57:13Z

plugin-volume/volumepopup.h

@@ -81,6 +82,7 @@ private slots:
    QPoint m_pos;
    Qt::Corner m_anchor;
    AudioDevice *m_device;
+    QTimer *mWheelTimer; // for showing tooltip with high-resolution mice


If we are adding a member to the class, we could add also the "_delta", and remove the static variable inside the method handleWheelEvent()

→ #1856 (comment) → first paragraph

→ #1856 (comment)

An angle of 15° is required for changing the volume — as with ordinary mice. Also, * Changing the volume by touchpad scrolling is fixed; and * Tooltip flickering on wheel rotation is prevented.

kreijack · 2023-01-15T15:01:24Z

Sorry for my late replay; but I had some problem with an update of my debian/sid which break the qt library and in turn lxqt...

Anyway, I still confirm that this patch doesn't solve the "tooltip flickering". Below what I see:

hi_res_mouse_volume applied with a hires wheel mouse
hi_res_mouse_volume applied with a standard mouse

Then I tried to make a change (see below for further details), reducing the timer timeout from 350 to 0

-    mWheelTimer->setInterval(350); // "QStyle::SH_ToolTip_WakeUpDelay" is 700 by default
+    mWheelTimer->setInterval(0); // "QStyle::SH_ToolTip_WakeUpDelay" is 700 by default

Then the tooltip behaves better

0-timeout with a hires wheel mouse
0-timeout with a standard mouse

I tried this both with openbox and xfwm4..

Looking better at the videos that tsujan [https://github.com//pull/1856#issuecomment-1380977162] shared, I saw some graphical effect that can make more confusion around the issue.

The tooltip window is updated after some "phasein-phaseout" effect, when in my desktop it is done immediately:

When the mouse enter in the volumecontrol plugin, the tooltip is showed immediately:

void VolumeButton::enterEvent(QEvent *event)
{
    // show tooltip immediately on entering widget
    QToolTip::showText(static_cast<QEnterEvent*>(event)->globalPos(), toolTip());
}

The method above shows the tooltip immediately; this means that the timeout of 350ms is unneeded.

My educate guess is that kwin introduce some delay that confuses the things. Unfortunately I can't test my theory because I can't install kwin (due to the qt <-> debian related problem).

What I am sure is that in my setup setting (which is a standard debian setup, with lxqt 1.2.0) a timeout of 350 doesn't work; if tsujan see something different my hypothesis is that it is due to the kwin effects.

tsujan · 2023-01-15T15:16:53Z

You probably don't have the latest patch (changed yesterday). As the code comment says, "a wheel event immediately hides the tooltip". So, if the user changes the volume as rapidly as in your videos. the tooltip will be shown only at the end.

EDIT: Of course, provided that you don't move the mouse at all. Any mouse movement will show the tooltip, and the wheel event will hide it immediately.

this means that the timeout of 350ms is unneeded.

Again, you misread the code.

tsujan · 2023-01-15T15:37:23Z

Let me explain more.

A 350-ms timer is for distinguishing successive wheel events. If you turn the wheel rapidly, the interval between two wheel events will be less than 350 ms and the tooltip will appear only at the end. If you turn the wheel slowly, the tooltip will appear and disappear, as it always did and as it should.

As for single steps of hi-res mice, which you mentioned elsewhere, please note that we aren't limited to them. The reason is simple: touchpads can create any delta, depending on the hardware as well as the scroll speed. The code should work smoothly with them too — as it does here.

kreijack · 2023-01-15T15:58:54Z

Let me explain more.

A 350-ms timer is for distinguishing successive wheel events. If you turn the wheel rapidly, the interval between two wheel events will be less than 350 ms and the tooltip will appear only at the end. If you turn the wheel slowly, the tooltip will appear and disappear, as it always did and as it should.

Ah, ok, this now make more sense; the expected behavior is:

the mouse enter in the "button area" -> the tooltip appear
if the wheel mouse start to rotate, the tooltip disappear, and reappear at the end of the "scroll", with the new value.

My (mis)understand was that the tooltip should appear even during the scroll.

Said that, now I can confirm that what I see is what it is expected to see.

tsujan · 2023-01-15T16:12:27Z

My (mis)understand was that the tooltip should appear even during the scroll.

Once a single wheel event is received, the tooltip gets hidden. That's good with slow rotations because the user can see the steps of volume change. However, it feels like a "flickering" when the rotation is fast. The code prevents that with the 350-ms timer.

350 is chosen because it's also the delay before showing the tooltip at the end of wheel rotation. Qt's default is 700 ms, which feels a little slow.

tsujan · 2023-01-15T16:17:50Z

BTW, the WM isn't relevant here. What you saw in my attached video elsewhere was just a result of a KWin JS script I've made for various animations — KWin doesn't do that by default.

stefonarch · 2023-01-15T17:33:16Z

I tested it on the debian PC (normal mouse wheel) - it felt better before, if scrolling more than once the feedback was imminent. 350ms can't be reduced a bit?

tsujan · 2023-01-15T17:39:29Z

350ms can't be reduced a bit?

It can but would result in tooltip flickering with hi-res mice, which would defeat the purpose because the timer is added to prevent that.

tsujan · 2023-01-15T18:06:59Z

To remove any confusion, let me explain in detail how Qt works ~~(GTK works similarly)~~.

Qt immediately closes tooltips on several occasions; receiving a wheel event is one of them. That means you can't have a persistent tooltip with wheel rotation.

Without this patch, the tooltip was shown as soon as the volume was changed. With fast rotations as well as with hi-res mice, that caused several flashes because the tootip was also closed as soon as the next wheel event was received. With the patch, if the wheel is rotated fast, the tooltip will be shown only in the end. If you want to see the steps, you'll have to rotate the wheel slowly enough.

An app like Qps uses floating frames instead of tooltips in its upper bar. They look beautiful with Qps and don't disappear on turning the mouse wheel. We can't have them here because they should be drawn inside a window.

The only way of having persistent tooltips with wheel rotation for the panel is making a new kind of tooltip from bottom to top. That's the true meaning of an overkill ;)

EDIT: It seems that GTK isn't like Qt. I know Qt's behavior from its code, but I checked a GTK tooltip and saw it didn't disappear on wheel rotations; so, GTK's code should be different.

stefonarch · 2023-01-15T19:13:25Z

Thanks!
I just thought we could have both situations acceptable with a somewhat lower value than 350mx

tsujan · 2023-01-16T18:18:56Z

I just thought we could have both situations acceptable with a somewhat lower value than 350mx

One of the goals of the patch was reducing tooltip flickering. The less timeout, the more flickering.

tsujan mentioned this pull request Jan 13, 2023

plugin-volume: add support for hires wheel #1856

Open

kreijack reviewed Jan 13, 2023

View reviewed changes

Fixed volume change with hi-res mice and touchpad scrolling

45506d1

An angle of 15° is required for changing the volume — as with ordinary mice. Also, * Changing the volume by touchpad scrolling is fixed; and * Tooltip flickering on wheel rotation is prevented.

tsujan force-pushed the hi_res_mouse_volume branch from fbc327a to 45506d1 Compare January 14, 2023 16:37

tsujan closed this Mar 26, 2024

tsujan deleted the hi_res_mouse_volume branch March 26, 2024 10:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed volume change with hi-res mice and touchpad scrolling #1857

Fixed volume change with hi-res mice and touchpad scrolling #1857

tsujan commented Jan 13, 2023 •

edited

Loading

kreijack Jan 13, 2023

tsujan Jan 13, 2023

kreijack Jan 13, 2023 •

edited

Loading

tsujan Jan 13, 2023

kreijack Jan 13, 2023

tsujan Jan 13, 2023

kreijack commented Jan 15, 2023

tsujan commented Jan 15, 2023 •

edited

Loading

tsujan commented Jan 15, 2023 •

edited

Loading

kreijack commented Jan 15, 2023

tsujan commented Jan 15, 2023

tsujan commented Jan 15, 2023 •

edited

Loading

stefonarch commented Jan 15, 2023

tsujan commented Jan 15, 2023

tsujan commented Jan 15, 2023 •

edited

Loading

stefonarch commented Jan 15, 2023

tsujan commented Jan 16, 2023

Fixed volume change with hi-res mice and touchpad scrolling #1857

Fixed volume change with hi-res mice and touchpad scrolling #1857

Conversation

tsujan commented Jan 13, 2023 • edited Loading

kreijack Jan 13, 2023

Choose a reason for hiding this comment

tsujan Jan 13, 2023

Choose a reason for hiding this comment

kreijack Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

tsujan Jan 13, 2023

Choose a reason for hiding this comment

kreijack Jan 13, 2023

Choose a reason for hiding this comment

tsujan Jan 13, 2023

Choose a reason for hiding this comment

kreijack commented Jan 15, 2023

tsujan commented Jan 15, 2023 • edited Loading

tsujan commented Jan 15, 2023 • edited Loading

kreijack commented Jan 15, 2023

tsujan commented Jan 15, 2023

tsujan commented Jan 15, 2023 • edited Loading

stefonarch commented Jan 15, 2023

tsujan commented Jan 15, 2023

tsujan commented Jan 15, 2023 • edited Loading

stefonarch commented Jan 15, 2023

tsujan commented Jan 16, 2023

tsujan commented Jan 13, 2023 •

edited

Loading

kreijack Jan 13, 2023 •

edited

Loading

tsujan commented Jan 15, 2023 •

edited

Loading

tsujan commented Jan 15, 2023 •

edited

Loading

tsujan commented Jan 15, 2023 •

edited

Loading

tsujan commented Jan 15, 2023 •

edited

Loading