Bug 366608 - Brightness keyboard action does not repeat while key is pressed under Wayland
Summary: Brightness keyboard action does not repeat while key is pressed under Wayland
Status: RESOLVED FIXED
Alias: None
Product: Powerdevil
Classification: Plasma
Component: general (show other bugs)
Version: 5.7.3
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Plasma Development Mailing List
URL: https://phabricator.kde.org/D2413
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-10 19:53 UTC by Matthias Fauconneau
Modified: 2016-08-16 23:31 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
attachment-26448-0.html (1.59 KB, text/html)
2016-08-11 06:19 UTC, Matthias Fauconneau
Details
attachment-28699-0.html (1.94 KB, text/html)
2016-08-11 07:20 UTC, Matthias Fauconneau
Details
attachment-23005-0.html (3.36 KB, text/html)
2016-08-16 23:31 UTC, Matthias Fauconneau
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Fauconneau 2016-08-10 19:53:41 UTC
When keeping a brightness up/down key pressed under Wayland, brightness level is changed only by one step.
PowerDevil should smoothly increment/decrement the brightness level while the key is pressed.
   

Reproducible: Always
Comment 1 Martin Flöser 2016-08-11 05:51:57 UTC
oh, on X11 the shortcuts are triggered again?
Comment 2 Martin Flöser 2016-08-11 05:58:13 UTC
Just checked, kwin explicitly ignores key repeat events to trigger global shortcuts
Comment 3 Matthias Fauconneau 2016-08-11 06:19:57 UTC
Created attachment 100537 [details]
attachment-26448-0.html

I think not repeating global shortcuts is the correct behaviour.
Still it would make sense for PowerDevil to be notified when the shortcut
is released.
I am not clear on how KGlobalAccel signals actions.
Maybe it could just send another signal when a shortcut is released.
Maybe on the condition a flag is set when setting the shortcut.

I understand that disappointingly there is no plan for desktop-agnostic
global shortcut registration.
But at least it would be useful to have the KGlobalAccel D-Bus interface
less Qt specific to allow for easier cross toolkit integration.
Also, where is the KGlobalAccel D-Bus interface documentation ?

On Thu, Aug 11, 2016 at 5:58 AM, Martin Gräßlin via KDE Bugzilla <
bugzilla_noreply@kde.org> wrote:

> https://bugs.kde.org/show_bug.cgi?id=366608
>
> --- Comment #2 from Martin Gräßlin <mgraesslin@kde.org> ---
> Just checked, kwin explicitly ignores key repeat events to trigger global
> shortcuts
>
> --
> You are receiving this mail because:
> You reported the bug.
>
Comment 4 Martin Flöser 2016-08-11 07:11:05 UTC
KGlobalAccel actions are only triggered on press events. There is no additional call when they are released.

I played a little bit with my system and there the brightness key sends press/release combos constantly when being hold.

> I think not repeating global shortcuts is the correct behaviour. Still it would make sense for PowerDevil to be notified when the shortcut is released.

I'm not sure how we can combine those two. I either make it ignore repeat events or make it always trigger. To support this we would have to extend kglobalaccel to add a flag "allow repeat".

> Also, where is the KGlobalAccel D-Bus interface documentation ?

Nowhere, it's an internal detail of the KGlobalAccel framework and not a public and stable dbus interface.
Comment 5 Matthias Fauconneau 2016-08-11 07:20:28 UTC
Created attachment 100538 [details]
attachment-28699-0.html

I think notifying release events would be a better solution since it would
allow smoother/finer brightness adjustments than the low rate of key
repeats.

So KWin/Wayland requires non-Qt applications to link with KGlobalAccel to
register shortcuts ?


On Thu, Aug 11, 2016 at 7:11 AM, Martin Gräßlin via KDE Bugzilla <
bugzilla_noreply@kde.org> wrote:

> https://bugs.kde.org/show_bug.cgi?id=366608
>
> --- Comment #4 from Martin Gräßlin <mgraesslin@kde.org> ---
> KGlobalAccel actions are only triggered on press events. There is no
> additional
> call when they are released.
>
> I played a little bit with my system and there the brightness key sends
> press/release combos constantly when being hold.
>
> > I think not repeating global shortcuts is the correct behaviour. Still
> it would make sense for PowerDevil to be notified when the shortcut is
> released.
>
> I'm not sure how we can combine those two. I either make it ignore repeat
> events or make it always trigger. To support this we would have to extend
> kglobalaccel to add a flag "allow repeat".
>
> > Also, where is the KGlobalAccel D-Bus interface documentation ?
>
> Nowhere, it's an internal detail of the KGlobalAccel framework and not a
> public
> and stable dbus interface.
>
> --
> You are receiving this mail because:
> You reported the bug.
>
Comment 6 Martin Flöser 2016-08-11 07:30:15 UTC
> So KWin/Wayland requires non-Qt applications to link with KGlobalAccel to register shortcuts ?

This is getting off-topic.
Comment 7 Martin Klapetek 2016-08-11 14:39:44 UTC
I think better use-case is volume controls - if you play a music
and your volume is too high and you don't realize, the panicky
solution is always hit the volume down key. Now pressing that
down and holding it down should imho continuously lower the
volume rather than having the finger pressed down, realizing
this doesn't do much and you need to press it repeatedly. That
is, imho, subpar.

Also all other DEs work with single press-down, we probably
shouldn't be the weird kid.
Comment 8 Martin Flöser 2016-08-12 05:48:56 UTC
Removing the key-repeat check in https://phabricator.kde.org/D2413
Comment 9 Martin Flöser 2016-08-15 15:43:54 UTC
Git commit 1111b9c98bd2b977bd68d79240ac4d60fcadc96b by Martin Gräßlin.
Committed on 15/08/2016 at 15:39.
Pushed by graesslin into branch 'master'.

Trigger global shortcuts also on key-repeat

Summary:
Restores feature parity with X11. Global shortcuts need to trigger
also for repeat events. An example is the volume key or screen
brightness.

For other shortcuts like showing yakuake it does not make sense to
trigger on repeat. Thus a long term solution is to add a flag to
global shortcuts whether the key should trigger on repeat.

Reviewers: #kwin, #plasma_on_wayland

Subscribers: plasma-devel, kwin

Tags: #plasma_on_wayland, #kwin

Differential Revision: https://phabricator.kde.org/D2413

M  +1    -1    input.cpp

http://commits.kde.org/kwin/1111b9c98bd2b977bd68d79240ac4d60fcadc96b
Comment 10 Matthias Fauconneau 2016-08-16 23:31:10 UTC
Created attachment 100640 [details]
attachment-23005-0.html

1)
Works for me with Brightness Up (F7), Volume Down (F9) and Up (F10).
But strangely Brightness Down (F6) still does not repeat.
I see no difference between the keys (libinput-debug-events) and the
shortcuts (Global Keyboard Shortcuts).
Is there a way to gain more insight in this issue ?

2)
The typically long key repeat delay still makes this suboptimal, especially
in the urgent volume down case.
If handling pressed shortcut on the client side (Powerdevil, KMix) is too
complicated, would it be possible to skip the delay and repeat directly
based on the key repeat rate ?

On Mon, Aug 15, 2016 at 5:43 PM, Martin Gräßlin via KDE Bugzilla <
bugzilla_noreply@kde.org> wrote:

> https://bugs.kde.org/show_bug.cgi?id=366608
>
> Martin Gräßlin <mgraesslin@kde.org> changed:
>
>            What    |Removed                     |Added
> ------------------------------------------------------------
> ----------------
>              Status|CONFIRMED                   |RESOLVED
>          Resolution|---                         |FIXED
>       Latest Commit|                            |
> http://commits.kde.org/kwin
>                    |
> |/1111b9c98bd2b977bd68d79240
>                    |                            |ac4d60fcadc96b
>
> --- Comment #9 from Martin Gräßlin <mgraesslin@kde.org> ---
> Git commit 1111b9c98bd2b977bd68d79240ac4d60fcadc96b by Martin Gräßlin.
> Committed on 15/08/2016 at 15:39.
> Pushed by graesslin into branch 'master'.
>
> Trigger global shortcuts also on key-repeat
>
> Summary:
> Restores feature parity with X11. Global shortcuts need to trigger
> also for repeat events. An example is the volume key or screen
> brightness.
>
> For other shortcuts like showing yakuake it does not make sense to
> trigger on repeat. Thus a long term solution is to add a flag to
> global shortcuts whether the key should trigger on repeat.
>
> Reviewers: #kwin, #plasma_on_wayland
>
> Subscribers: plasma-devel, kwin
>
> Tags: #plasma_on_wayland, #kwin
>
> Differential Revision: https://phabricator.kde.org/D2413
>
> M  +1    -1    input.cpp
>
> http://commits.kde.org/kwin/1111b9c98bd2b977bd68d79240ac4d60fcadc96b
>
> --
> You are receiving this mail because:
> You reported the bug.
>