Summary:
So far we were following a bit unique and rare doxygen comment style:
/**
* Contents of the comment.
**/
Doxygen comments with this style look balanced and neat, but many people
that contribute to KWin don't follow this style. Instead, they prefer
more traditional doxygen comment style, i.e.
/**
* Contents of the comment.
*/
Reviewing such changes has been a bit frustrating for me (so selfish!)
and for other contributors.
This change switches doxygen comment style in KWin to a more traditional
style. The main reason for doing this is to make code review process easier
for new contributors as well us.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22812
Summary:
Currently code base of kwin can be viewed as two pieces. One is very
ancient, and the other one is more modern, which uses new C++ features.
The main problem with the ancient code is that it was written before
C++11 era. So, no override or final keywords, lambdas, etc.
Quite recently, KDE compiler settings were changed to show a warning if
a virtual method has missing override keyword. As you might have already
guessed, this fired back at us because of that ancient code. We had
about 500 new compiler warnings.
A "solution" was proposed to that problem - disable -Wno-suggest-override
and the other similar warning for clang. It's hard to call a solution
because those warnings are disabled not only for the old code, but also
for new. This is not what we want!
The main argument for not actually fixing the problem was that git
history will be screwed as well because of human factor. While good git
history is a very important thing, we should not go crazy about it and
block every change that somehow alters git history. git blame allows to
specify starting revision for a reason.
The other argument (human factor) can be easily solved by using tools
such as clang-tidy. clang-tidy is a clang-based linter for C++. It can
be used for various things, e.g. fixing coding style(e.g. add missing
braces to if statements, readability-braces-around-statements check),
or in our case add missing override keywords.
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, apol, romangg, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22371
Summary:
We have a mix of different doxygen comment styles, e.g.
/*!
Foo bar.
*/
/**
* Foo bar.
*/
/** Foo bar.
*/
/**
* Foo bar.
*/
/**
* Foo bar.
**/
To make the code more consistent, this change updates the style of all
doxygen comments to the last one.
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18683
Summary:
The functionality regarding triggering modifier only shortcuts is moved
out of Xkb - where it doesn't belong to - and is turned into an input
event spy listening for the changes it is interested in. Previously
the state got queried by asking e.g. for the pressed buttons, now it's
tracked directly.
The X11 side needs a larger change due to that as now pushing the events
into Xkb does not trigger modifier only shortcuts any more. Instead the
"normal" way through the platform API needs to be used which triggers the
processing of filters and spies.
The problem here is that our redirections only process events if they are
inited and that only happens on Wayland. We cannot call init on them as
that would create all the Wayland filters and spies and processing would
probably break. As an intermediate solution the spies are now processed
and there we know that it won't matter. A future solution would be to
remove the init checks completely and just send through both filters and
spies and ensure that on X11 only the supported ones are loaded.
Closes T5220
Test Plan: Tested on Wayland and X11
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Maniphest Tasks: T5220
Differential Revision: https://phabricator.kde.org/D4578
Summary:
So far KWin parsed the kxbkrc at multiple places (once in Xkb, once
in KeyboardLayout). This is now replaced by one KSharedConfigPtr hold
by kwinApp, just like the normal kwinrc. The KSharedConfigPtr is now
passed to Xkb.
As a nice side effect this makes it easier to test keyboard layout
changes as we can now properly mock the keyboard configuration. Thus
this change also comes with an autotest for loading keyboard layout
configuration. This is becoming more and more a need as we start
getting bug reports for layout specific issues like global shortcuts
not working with Greek layout.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D4315
Summary:
So far the keyboard repeat handling was triggered directly from
KeyboardInputRedirection::processKey. With the introduction of
InputEventSpies it is no longer required to be done like that, we can
split it out into a dedicated spy.
This means that processKey only has to care about processing the key
and allows us to better extend in future. So far keyboard repeat is
only functional for libinput based platforms. But it should also be
possible to use it in nested setups. By splitting it out we can
prepare for that.
Test Plan: Auto-test using repeat still passes
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D4304
Summary:
On X11 the SNI for keyboard layout is provided by the keyboard kded.
On Wayland that kded has no real access to the layouts and cannot
properly implement switching. Given that it's better to integrate the
SNI directly in KWin.
The implementation of the SNI is largly based on the existing SNI from
plasma-desktop/kcms/keyboard. The implementation so far supports:
* Switching to next layout on toggle
* Presenting all layouts in a context menu
* Switching to a specific layout through the context menu
* Opening the keyboard layout configuration module
* scroll on SNI to switch layout
* config option whether to show the SNI
Not yet supported are:
* flags and/or short text for the layouts
The last point needs more explanation. On X11 the layout name is
something like "de" or "us". This can be directly mapped to a flag and
can be added as a short note.
Xkbcommon does not provide this information directly. Instead it provides
us the full name of the layout, e.g. "German" or "English (us)". There is
no way in the API to go from "German" to "de".
Instead we need to parse the evdev.xml file to gather all information
about layouts. This is already done in the keyboard kcm to configure
layouts. The implementation needs to be split out into a small helper
library.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D4220
Summary:
So far the implementation of keyboard layout handling was split between
KeyboardInputRedirection and Xkb. KeyboardInputRedirection registered
the global shortcut and did the handling for layout switch and config
changes. Xkb did the notification on layout change.
Layout changes can nowadays be detected through an InputEventSpy. It
can only happen after a key change or an explicit layout switch. Thus
it does not need to be in Xkb anymore which allows to reduce Xkb to
only care about the Xkb keymap and state tracking.
This change introduces a new class KeyboardLayout which is an
InputEventSpy and takes over the task of the layout change notification
from Xkb and the layout management from KeyboardInputRedirection. Thus
everything related to management of keyboard layout is together in one
class.
This allows in future to add unit test to it (requires further cleanup
of Xkb to be able to use it and drop the InputRedirection dependency) and
opens the possibility to also take over keyboard layout management on X11
for the Plasma desktop.
Test Plan: Manual testing
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D4135
Summary:
Instead of emitting the key state changed and modifier state changed
signals from the right point before processing the events, let's use
an InputEventSpy to do that. The spies were introduced to be called
directly before the event processing. So the contract still holds.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D4128
Summary:
Prior to this change various event filters performed deep calls into
Xkb class to figure out the modifiers relevant for global shortcuts (aka
consumed modifiers). This shows that this is a general useful
information which should be available to all input event filters
directly.
Thus it's now added to the input events and exposed directly in
InputRedirection so that the calls into Xkb are no longer needed.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D3810
Summary:
The interactive window selection is implemented in InputRedirection
through a dedicated InputEventFilter. The InputEventFilter so far takes
care of pointer input and keyboard input. In addition it ensures that
keyboard and pointer focus is reset on start and on end.
With this change KillWindow now also works on Wayland, but only for X11
windows, as the Wayland variant is not yet implemented.
Test Plan: Tested in nested setup, auto-tests still needed
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D3365
Summary:
A little bit debug information about the current keyboard state is
useful. Thus a new tab is added to show information about xkbcommon.
It shows:
* layouts in the keymap
* currently active layout
* supported modifiers in key map
* currently active modifiers in state
* supported leds in key map
* currently active leds in state
Whenever a key is pressed/released the complete ui is updated to reflect
the latest state. That is pressing/releasing a modifier is directly
reflected in the ui.
This UI can extended as needed for further debug information about the
keymap state.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D3379
Summary:
A shortcut with e.g. shift+w could not be triggered as shift is
considered as consumed. It transforms the keysym to an uppercase variant
thus it is consumed.
This change checks for the condition that shift is pressed and is the
only consumed modifier. If the current keysym is a letter the shift is
removed from the consumed modifier again to still support the shortcut.
BUG: 370341
FIXED-IN: 5.8.2
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D3015
Summary:
The Xkb implementation starts to track the state of the LEDs in the
keymap and emits a signal whenever the LEDs change. This signal is
connected to a method in LibInput::Connection which updates the led
state on all devices and uses it to init the state of the led when a new
device gets connected.
BUG: 369214
FIXED-IN: 5.8.2
Test Plan: Connected a keyboard with LEDs and enabled NumLock and ScrollLock.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D2943
Summary:
So far KWin tried to repeat all pressed keys which should repeat. But
this is not how X11 and e.g. QtWayland handle it. There only one key -
the last one which got pressed - repeats. And this makes sense as the
key is used to generate a keysym and that one KWin caches. Thus the
logic so far resulted in incorrect keysyms to be generated during the
repeat. E.g. pressing a, pressing b, releasing b would repeat b instead
of the hold a as b was the last generated keysym.
This change addresses this problem and let's only one key repeat at a
time. When the currently repeating key gets released the repeat timer is
stopped and other hold keys won't repeat any more. This also matches the
behavior of X11 and QtWayland.
BUG: 369091
FIXED-IN: 5.8.1
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D2941
Summary:
When triggering global shortcuts we are more interested in the hold
keys than the currently active modifiers. E.g. capslock should not
be seen as "shift is hold". Similar we need to remove consumed
modifiers. Shift+5 is % and not Shift+% - the shift modifier is
consumed and needs to be removed from shortcut evaluation.
To support this we need to have the actual state directly from
xkbcommon. Thus a new method is added which exposes the modifiers
relevant for global shortcut matching. In addition on every key press
all consumed modifiers are calculated and kept so that they can be
used for shortcut matching.
In addition a workaround is added for Backtab. Similar workaround
exists in kglobalaccel for X11. The problem is that our shortcuts are
stored incorrectly: Shift+Tab instead of Backtab. Thus a mapping back
is required. To make everything worse KWin registers the wrong key
sequence "Alt+Shift+Backtab" which doesn't make any sense and is
broken on X11 at least.
The workaround supports both special cases. The one for Backtab should
be turned into Shift+Tab and also KWin's special case of adding shift
to backtab.
CCBUG: 368581
Reviewers: #kwin, #plasma_on_wayland, bshah
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D2768
Summary:
The Xkb class now creates a compose key table and a state object and
feeds all key presses through the compose state machine.
Xkb now tracks the latest keysym which is provided through new method
currentKeysym. This is now used when creating a QKeyEvent instead of
passing the key code to the xkb state. With that the keysym can also
be updated through the compose state system.
This only affects KWin internal usage where text is composed, e.g. the
present windows effect filter. Wayland clients do not gain compose key
support, though.
Minimum xkbcommon version raised to 0.5 as compose key support is new
in that version.
Test Plan: Enabled compose key support in keymap and verified through DebugConsole
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D2622
At the same time the xinput2 integration is split out of X11Cursor
and made a standalone part of the platform plugin. XInput integration
is nowadays not only used by the cursor position polling, but also
for modifier only shortcuts.
By splitting it out the modifier shortcuts start to work also when
one doesn't have anything requesting a mouse position polling.
This also simplifies the conditional builds: xinput integration is
only included if we have support for it at compile time without having
to have many ifdefs in the cursor implementation. For the inclusion of
cursor in the kcmkwin this also removes all the ifdefs.
The key events are only requested if we have xinput 2.1. Otherwise we
would not get all raw events if the input device gets grabbed.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D2473
Summary:
If caps lock is on the shift key should not trigger. Similar pressing
caps lock should neither on activation press nor on deactivation press
trigger the shortcut. Related to that are latched modifiers aka sticky
modifiers: if the modifier is still on after releasing the key the
shortcut should not trigger. We must assume the user wanted to use the
modifier to activate the modifier, not to activate the shortcut.
This change ensures that we don't track for modifier only shortcuts if
a modifier is active before press or after release.
The added test case demonstrates for caps lock, latched modifiers is
currently still untested. (Needs a way to mock it).
Test Plan: See test case for caps lock.
Reviewers: #kwin, #plasma
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D2467
Summary:
For XWayland windows the window might be activated before the Wayland
Surface is set for it. Thus the keyboard focus is not passed to the
window. Only on the next activate after the window got created the
window got keyboard focus.
This change addresses this problem by emitting a signal from Toplevel
when the surface changes. The KeyboardInput listens to this signal
for the active client and updates keyboard focus again if the surface
changes. Thus keyboard focus is properly passed to XWayland windows.
Test Plan:
Test case which creates an X11 window is adjusted to verify
the condition.
Reviewers: #plasma_on_wayland, #kwin
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D2009
Summary:
The signals emitted by LibInput::Connection carry the Device for which
the input event was received. This Device is passed to the input handlers.
Custom event classes are added which extend QMouseEvent, QKeyEvent and
QWheelEvent respectively and expose the Device. The Device is only passed
around as a forward declared pointer, so even if compiled without libinput
support, it should still compile.
Event handlers which need to get access to the Device can now just cast
the event pointer to the custom class and access it. This can be used in
future to handle device specific key codes, etc.
As we don't have a proper event classes for touch events the event
handlers do not yet have access to the Device. Here the internal API
needs to be adjusted in future.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1667
KWin starts to track which is the current layout and in case it changes
notifies the org.kde.osdService about the change through DBus. KWin has
a better knowledge about changes than the KeyboardDaemon could have, so
it's better to do in KWin. E.g. KWin can also notice changes not
triggered by the global shortcut, but by the keymap itself.
KWin registers/steals the shortcut of the "KDE Keyboard Layout Switcher"
and binds it to a new method which actually switches the layout.
The actual switcher from which the shortcut is stolen should only be a
representation on Wayland. Though how to do this is a problem for the
future. Only the active window is notified about layout changes and the
plasmoid will never get the event in time. This is of course a minor
problem compared to the fact that the KeyboardDaemon is absolutely X11
dependent.
This is the start for adding proper support for keyboard layouts. If
we have a configuration in kxkbrc the keymap is generated from that
information. This allows to have different layouts and also layout
switching is working (though not yet passed to Wayland clients properly).
Not yet working is the global shortcut for layout switching and
reconfiguring the layouts.
As a Wayland server KWin does not have to emit additional key repeat
events (unlike X11). The clients are responsible for handling this based
on the provided key repeat information.
Internally KWin needs key repeat, though. E.g. the effects need key
repeat (filtering in Present Windows), window moving by keyboard needs
repeat, etc. etc.
This change introduces the internal key repeat. For each key press a
QTimer is started which gets canceled again on the key release. If the
timer fires it invoked processKey with a new KeyboardKeyAutoRepeat state.
This is handled just like a KeyPress, but states are not updated and
the QKeyEvent has autorepeat set to true.
The event filters check for the autorepeat state and filter the event
out if they are not interested in it. E.g. the filters passing the event
to the Wayland client need to filter it out.
Currently auto-repeat is bound to using libinput. This needs to be
modified. The only backend sending repeated events is X11, thus for
other backends it should be enabled.
Whether creating a timer on each key event is a good idea is something to
evaluate in future.
Reviewed-By: Bhushan Shah
Similar to the change regarding pointer and touch a
KeyboardInputRedirection is created. The Xkb class is also moved to
the new files keyboard_input.h and keyboard_input.cpp.
Just like in the case of PointerInputRedirection no signals are added,
but the existing signals in InputRedirection are directly invoked.
