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:
The new connect syntax has several advantages over the old syntax:
(a) Connecting with the new syntax is faster;
(b) It is compile time checked.
There are still a few places where the old connect syntax is used, e.g.
connecting to QML buttons in the Desktop Grid effect.
Test Plan:
Have been testing this patch for ~2 weeks, haven't noticed any
regressions.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, broulik, graesslin, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18368
Summary:
Background behind some windows is not blurred because the invert effect
sets PAINT_WINDOW_TRANSFORMED. This essentially "turns off" the blur and
the background contrast effect unless we set the force roles.
Because the invert effect is a "post-processing"(not really) effect we
don't have to set that flag.
BUG: 279076
BUG: 359583
FIXED-IN: 5.14.3
Test Plan: {F6341916}
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16358
One resource is used for shader version 1.10 and one for version 1.40.
The ideas behind this change is to remove the locating of the shader
sources and also to fix that user provided shaders could be loaded
instead of the original ones (possible attack vector on Wayland).
To simplify the ShaderManager provides a new method call to load the
shader from the resource. This means the effects don't need to
duplicate the check for the shader version any more and also don't
need to duplicate the file reading functionality.
REVIEW: 126905
Without the componentDisplayName the shortcut dialog takes the name of
the application e.g. Systemsettings or "KDE Control Module", but we want
it to be KWin.
REVIEW: 124706
This removes all the hacks to add kwin4_effect_ to the name of the Effect
and adjusts the desktop files of the effect configuration's parent
component.
Note: the scripted effects still start with kwin4_effect_ prefix.
REVIEW: 117367
All KCMs and KWin core use the BuiltInEffects namespace to find and
interact with the effects. There is no information left in the desktop
file which are of usage. Thus they can be removed.
Instead of using EffectsHandler::sendReloadMessage we generate the dbus
interface in each plugin and call the reconfigure slot directly. That way
it's more type safe and we don't need to link kwineffects from the
configs.
REVIEW: 116875
There are no advantages for the effects KCM to have all the effect
config modules in one plugin.
By having a plugin per effect we can use the KPluginTrader to easily
find the configuration plugin for a given effect and load it.
To make this possible the following changes are done:
* config_builtins.cpp is deleted
* add_subdirectory is used for all effects which have a config module
* toplevel CMakeLists.txt contains the sources again for the effects
which have a config module, but effects which don't have a config
module are still included and thus the macro is still used
* plugin created for the config module, name pattern is:
kwin_effectname_config
* plugin installed to ${PLUGIN_INSTALL_DIR}/kwin/effects/configs
* desktop file adjusted to new plugin name and keyword removed
* desktop file converted to json as meta data and no longer installed
* Uses K_PLUGIN_FACTORY_WITH_JSON
* Macros for config are dropped from kwineffects.h
REVIEW: 116854
Implemented in KWin core to forward to new global shortcut system. This
method should be extended/changed once we go to Qt5/KF5 to make the usage
easier (no more KAction).
Each global shortcut in the effects makes use of this new method.
As all effects have always been compiled into the same .so file it's
questionable whether resolving the effects through a library is useful
at all. By linking against the built-in effects we gain the following
advantages:
* don't have to load/unload the KLibrary
* don't have to resolve the create, supported and enabled functions
* no version check required
* no dependency resolving (effects don't use it)
* remove the KWIN_EFFECT macros from the effects
All the effects are now registered in an effects_builtins file which
maps the name to a factory method and supported or enabled by default
methods.
During loading the effects we first check whether there is a built-in
effect by the given name and make a shortcut to create it through that.
If that's not possible the normal plugin loading is used.
Completely unscientific testing [1] showed an improvement of almost 10
msec during loading all the effects I use.
[1] QElapsedTimer around the loading code, start kwin five times, take
average.
REVIEW: 115073
Videos for the following effects are added:
* Dim Inactive
* Dim Screen for Administration Mode
* Invert
* Looking Glass
* Magnifier
* Mouse Click
* Track Mouse
* Zoom
Link for Present Windows video fixed.
Most effects had a "collection" for one action. We don't need the
action collection, all it was used for is setting the object name.
With the removal of KActionCollection the effects do not need to link
XmlGui any more, though the dependency is still pulled in through
plasma.
