Re: Keystates, implementing a full Plasma on Wayland

Am 2019-04-02 14:50, schrieb Aleix Pol:
> On Tue, Apr 2, 2019 at 1:59 PM Martin Flöser <[email protected]> 
> wrote:
>> 
>> Hi Aleix,
>> 
>> I would have approached the problem differently. First of all I don't
>> think we need any new protocol for transmitting key state. That's
>> already built into the wl_keyboard protocol. The "problem" with this
>> protocol is that it only sends events to the focused application, 
>> which
>> makes it not a direct solution for applications like Plasma which need
>> to show the state all the time. For all other applications it would be
>> fine. Firefox doesn't need to know the state if it's not focused.
>> 
>> Now I see two possible ways to solve this problem:
>> 1. Just send the modifier changes to all applications
>> 2. Add a protocol to be a special application to get the modifier
>> changes all the time.
>> 
>> The first solution is a minimal change in KWayland, but has the
>> disadvantage that all applications get the state changes which means 
>> we
>> are leaking information and cause useless wakeups. It's a fast 
>> solution,
>> but would work. The second solution is probably the better one.
>> 
>> Concerning the problem of kguiaddons being tier 1 I also had already
>> drafted some ideas (see also https://phabricator.kde.org/T4454 ): what
>> we need to do is the same as we did for kwindowsystem and 
>> kglobalaccel.
>> Add a small private library and implement it in kwayland-integration.
>> That way the framework can stay tier 1 and doesn't need to depend on
>> kwayland nor on QtWayland. If you run a Plasma/Wayland session it gets
>> pulled in and works.
>> 
>> Cheers
>> Martin
>> 
>> Am 2019-04-02 02:13, schrieb Aleix Pol:
>> > Hi,
>> > I've been meaning to dive a bit into how our wayland implementation
>> > works lately, mostly due to some needs that we found in KDE Connect
>> > but also because I maintain the keyboard indicator plasmoid. Since the
>> > latter problem is much smaller, I decided to give it a go at solving
>> > it, review is very welcome.
>> >
>> > https://phabricator.kde.org/D20191
>> > https://phabricator.kde.org/D20192
>> > https://phabricator.kde.org/D20193
>> >
>> > Now here I did what I it seemed to be the "normal" thing to do:
>> > Implement the protocol on kwayland then on the client (kguiaddons in
>> > this case) and on kwin.
>> >
>> > KWin implementation worked okay but the whole process of implementing
>> > it felt very boilerplate-ey.
>> >
>> > For kguiaddons I couldn't use the implementation in kwayland because
>> > kguiaddons is tier1 (although I didn't realize until after
>> > implementing it ^^') so I used QtWayland tooling to implement it
>> > there.
>> >
>> > Now to do that, I came up with a new protocol. Since kguiaddons can't
>> > depend on it, I was thinking that maybe it would make sense to have it
>> > installed somewhere by kguiaddons and kwayland when it starts
>> > generating files, it can fetch it from there. I don't know if you
>> > think that's acceptable.
>> > Also I don't know if this concept fits in another protocol, that could
>> > very well be, if that's something you think it's reasonable to pursue
>> > it could be looked into, although it seems to me that it's quite
>> > ad-hoc to how we implemented the shell separate to the different
>> > components we use.
>> >
>> > I guess this would ultimately implement this task:
>> > https://phabricator.kde.org/T4454
>> >
>> > HTH,
>> > Aleix
>> 
> 
> Wouldn't it be a bad idea to send all events to the plasma shell all
> the time too, in terms of waking it up all the time?

It's not all events, just modifier information. And that's what you 
want. You want to know the state of the caps lock modifier (which is 
actually incorrect in your patch as you look for led instead of modifier 
state).

> As I see it, we have a simple problem which is knowing about caps lock
> (and maybe numlock). We can address it and move on.

Actually no, we also need to support the state of all modifiers for the 
accessibility feature. And at that point we have the same as wl_keyboard 
modifier state event.

> 
> Otherwise we run the risk of making the problem bigger.
> 
> FWIW, there's a use-case I yesterday realized after sending the e-mail
> I should have implemented, which is changing the state (to restore the
> numlock state at boot):
> https://lxr.kde.org/source/kde/workspace/plasma-desktop/kcms/keyboard/keyboard_hardware.cpp#0100

This is already implemented in KWin. No need to add to a protocol.

Cheers
Martin