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:
QImage::byteCount() was deprecated in Qt 5.10. It is advised to use
QImage::sizeInBytes() method instead.
Reviewers: #kwin, apol
Reviewed By: apol
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22355
Summary:
QRegion::rects was deprecated in Qt 5.11. It is advised to use begin()
and end() methods instead.
Reviewers: #kwin, romangg
Reviewed By: #kwin, romangg
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22353
Summary:
The screenlock fails on X11 if it can't grab the keyboard.
We can't nicely solve the generic case. We can solve the common case of
a kwin effect being active.
It's not critical, arguably not even desirable to have these effects
persist after the screen is locked through an external trigger. We can
just close the effect early.
Key grabs have to be relased early before the close animation completes
so that the locker doesn't have a race based on animation times.
It's not ideal, but no worse than the current state for not much work.
BUG: 234153
Test Plan:
locked screen on a timer
opened various effects
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: ngraham, zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D20890
Summary:
libkwineffects in master is no longer ABI compatible with 0.227 because
order of virtual methods in EffectsHandler and EffectWindow had been
changed.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D20652
Summary:
Window open/close animation effects should not animate the outline
because the end result is a bit awkward.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D19886
Summary:
We need AniMap only for test purposes so it would be better to make the
typedef protected (and mark as internal as well).
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D17833
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:
So far the ThumbnailItem in TabBox mode used the window id for finding
the window it should render a thumbnail on. In the Wayland world this is
not unique. The window id could be either an X11 window or a wayland
window. We don't guarantee that there are no conflicting ids.
With the internal id we have a way to properly identify the windows, so
this element should use them.
To support this the property changed the type to QUuid and the
clientmodel also provides the QUuid. As in TabBox the way to get the
window is through the model this should be compatible for all themes.
It's tested and verified with the Breeze switcher.
For declarative KWin scripts the ThumbnailItem also provides the
AbstractClient as a property, so there should not be any script which
uses wid. If it does, this could break, but well the script should use
the intended API.
Test Plan: ctest passes, manual testing of Breeze alt-tab switcher
Reviewers: #kwin
Differential Revision: https://phabricator.kde.org/D18405
Summary:
KWindowSystem provides a plugin interface to have platform specific
implementations. So far KWin relied on the implementation in
KWayland-integration repository.
This is something I find unsuited, for the following reasons:
* any test in KWin for functionality set through the plugin would fail
* it's not clear what's going on where
* in worst case some code could deadlock
* KWin shouldn't use KWindowSystem and only a small subset is allowed
to be used
The last point needs some further explanation. KWin internally does not
and cannot use KWindowSystem. KWindowSystem (especially KWindowInfo) is
exposing information which KWin sets. It's more than weird if KWin asks
KWindowSystem for the state of a window it set itself. On X11 it's just
slow, on Wayland it can result in roundtrips to KWin itself which is
dangerous.
But due to using Plasma components we have a few areas where we use
KWindowSystem. E.g. a Plasma::Dialog sets a window type, the slide in
direction, blur and background contrast. This we want to support and
need to support. Other API elements we do not want, like for examples
the available windows. KWin internal windows either have direct access
to KWin or a scripting interface exposed providing (limited) access -
there is just no need to have this in KWindowSystem.
To make it more clear what KWin supports as API of KWindowSystem for
internal windows this change implements a stripped down version of the
kwayland-integration plugin. The main difference is that it does not use
KWayland at all, but a QWindow internal side channel.
To support this EffectWindow provides an accessor for internalWindow and
the three already mentioned effects are adjusted to read from the
internal QWindow and it's dynamic properties.
This change is a first step for a further refactoring. I plan to split
the internal window out of ShellClient into a dedicated class. I think
there are nowadays too many special cases. If it moves out there is the
question whether we really want to use Wayland for the internal windows
or whether this is just historic ballast (after all we used to use
qwayland for that in the beginning).
As the change could introduce regressions I'm targetting 5.16.
Test Plan:
new test case for window type, manual testing using Alt+Tab
for the effects integration. Sliding popups, blur and contrast worked fine.
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18228
Summary: Connecting with the new syntax is faster and also type checked.
Test Plan: Scripted effects still work as expected.
Reviewers: #kwin, graesslin
Reviewed By: #kwin, graesslin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18393
Summary: This change reformats the documentation so it's more easier to read it.
Test Plan:
Generated Doxygen documentation, started Python's Simple HTTP server, navigated
to AnimationEffect's documention, haven't noticed any serious issues.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: graesslin, davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D17832
Summary:
7834bec52a missed to port minimize and
unminimize to dynamic dispatch. In addition to that, we don't benefit
from QMetaObject::invokeMethod so port addRepaint and addLayerRepaint
to dynamic dispatch as well.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18049
Summary:
Currently, KWin is not able to detect Radeon GPUs with GCN architecture.
This patch tries to address that.
Reviewers: #kwin, graesslin
Reviewed By: #kwin, graesslin
Subscribers: graesslin, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D17715
Summary:
Recently, libkwineffects got some changes that break ABI compatibility,
so we have to bump the API version.
Test Plan:
When using a binary effect, which is made against 226 version, KWin no
longer crashes.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D17444
Summary:
EffectWindow proxies its properties from the client/deleted's
properties.
QObject::property(char*) is a slow string search. It's a loop
of string comparisons not a hash lookup!
QML's use of properties is different, there's a property cache.
It's fetched multiple times for every window in every paint of some
effects (such as blur). Hotspot shows this as a significant amount of
the render pass (X11) with nothing in kwin animating.
This patch replaces the macro that does
parent()->property("propertyName")
with a macro calling the relevant function directly without metaobjects.
This also improves type safety for future changes.
Test Plan:
Existing unit tests
Ran it for a bit
Reviewers: #kwin, graesslin
Subscribers: graesslin, zzag, broulik, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16602
Summary:
Effects that prefer to manipulate direction of animations sometimes need
to create animations in some particular state so later on they can be
played backward (swapping from and to is not enough and it would be wrong).
The proposed complete function lets such effects to fast-forward animations to
to the target position so they can be played backwards later on.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16450
Summary:
Consider current implementation of the Squash effect: if a window was
minimized, an animation will be started; if the window is unminimized
and the animation is still active (that can happen when user clicks on
app's icon really fast), the animation will be stopped and a new one will
be created. Such behavior can lead to rapid jumps in the observed
"animation".
A better approach would be first try to **reverse** the already active
animation, and if that attempt wasn't successful, start a new animation.
This patch introduces a new function to the scripted effects API that
lets JavaScript effects to control direction of animations. The
prototype of the function looks as follows:
redirect(<animation id(s)>, <direction>, [<termination policy>])
the first argument is an animation id or a list of animation ids, the
second argument specifies the new direction of the animation or
animations if a list of ids was passed as the first argument. The
third argument specifies whether the animation(s) should be terminated
when it(they) reaches the source position, currently it's relevant only
for animations that are created with set() function. The termination
policy argument is optional, by default it's Effect.TerminateAtSource.
We can use this function to fix issues with rapid jumps in the Squash
effect. Also, redirect() lets us to write effects for simple animations
in slightly different style: first, we have to start the main animation
(e.g. for the Dialog Parent effect, it would be dimming of main windows)
and then change direction of the animation depending on external events,
e.g. when the Desktop Cube effect is activated.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, abetts, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16449
Summary:
TimeLine has nice API for controlling its direction that can be re-used
later by AnimationEffect.
Test Plan: The existing tests for scripting effects still pass.
Reviewers: #kwin, davidedmundson, graesslin
Reviewed By: #kwin, davidedmundson, graesslin
Subscribers: davidedmundson, abetts, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16448
Summary:
The redirect modes control behavior of the timeline when its direction
is changed at the start or target position. For example, consider the
following piece of code:
TimeLine timeLine(1000ms, TimeLine::Forward);
timeLine.setDirection(TimeLine::Backward);
What should happen when the direction of the timeline was changed to go
backward? Should the current value of the timeline go from 1 to 0, or
should the timeline stop its "execution"?
In the relaxed mode, the timeline will go from 1 to 0.
In the strict mode, the timeline will stop its execution.
Different effects may prefer different modes for source and target
positions. For example, most C++ effect would prefer relaxed mode for
source position, and strict mode for target position. On the other side,
scripted effects(AnimationEffect) would prefer strict mode for source
position, and relaxed mode for target position(because of set).
Reviewers: #kwin, graesslin
Reviewed By: #kwin, graesslin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16447
Summary:
As setDesktop was changed to "move" this left unSetDesktop non-symetric.
This replaces it with explicit API to enter/leave.
This also moves new API to the new object based API rather than still
using ints.
Where numbers are used it has been tidied up so that desktop IDs are
uint, which should be used when we have a list of desktops.
int is used only when we have either a desktop ID or NET::OnAllDesktops
(-1)
Effects API cleared up to use this and use a set of x11 IDs, which
avoids any potential complications of handling add and removes any
ambiguity with what happens if you leave all desktops and such.
Test Plan:
testVirtualDesktops passes (with pending kwayland patch)
Moving a window in the desktop grid on X11 behaves
Moving a window in the desktop grid on wayland behaves
Reviewers: #kwin, zzag
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16704
Summary:
If the cross fade animation is cancelled, we are not gracefully
unreference the previous window pixmap.
This change addresses that issue by using RAII approach to
reference/unreference the previous window pixmap.
Test Plan: Manually. The Maximize and the Morphing Popups effect still work.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16391
Summary:
With the plasma shell protocol, a ShellClient gets its role/window type
after it's created, so the cached value of the popupWindow property in
EffectWindow can be wrong.
In addition to that, the value of the popupWindow property still can be
wrong because plasma shell surfaces don't have xdg-popup role, so we need
to check NetWM-based window type in ShellClient::isPopupWindow.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16835
Summary:
Type of the x11DesktopIds property was changed from QList<int> to
QVector<int> in KWin core, but not in libkwineffects. That resulted in
having effects like slide not working because Qt can't implicitly
convert QVector<int> to QList<int>.
Test Plan: Slide effect works again.
Reviewers: #kwin, graesslin
Reviewed By: #kwin, graesslin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16743
Summary:
implement virtual desktop support for Wayland.
use the new virtual desktop protocol from D12820
The VirtualDesktopManager class needed some big change in order
to accomodate it, which is where most changes are.
Other than that, it's mostly connections to wire up
VirtualDesktopsManager and VirtualDesktopsManagement(the wayland protocol impl)
Depends on D12820
Other notable detail, is the client visibility updated to reflect the presence
of the client in the plasmavirtualdesktop.
(and the unSetDesktop concept)
Test Plan: used a bit a plasma session together with D12820, D13748 and D13746
Reviewers: #plasma, #kwin, graesslin, davidedmundson
Reviewed By: #plasma, #kwin, davidedmundson
Subscribers: hein, zzag, davidedmundson, kwin
Tags: #kwin
Maniphest Tasks: T4457
Differential Revision: https://phabricator.kde.org/D13887
Summary:
862bf0f153 introduced a regression(sorry
for that): if you close a confirmation dialog(with the Scale effect and the
Dialog Parent effect being enabled), the main window(s) will flicker.
That flickering happens because the C++ version of the Scale effect and
AnimationEffect unref deleted windows in different places. AnimationEffect
unrefs deleted windows in prePaintScreen, which btw is not the best
place to do that. The C++ version unrefed windows in postPaintScreen.
So, when the Scale effect has finished animating the main windows, it
will unref them, but windowDeleted signal won't be emitted immediately.
Which means that the Dialog Parent won't be able to delete corresponding
animations on its side and main windows will be painted for a single
frame.
This change addresses flickering by adjusting prePaintWindow in
AnimationEffect so deleted windows won't be painted if none of
animations keeps them alive.
Test Plan:
(make sure that both the scale and the dialog parent effect are enabled)
* Go to System Settings > Application Style > Window Decorations;
* Select a decoration that is different from the current;
* Close the system settings window (don't click "Apply" button);
* When a dialog is shown, click "Discard" or "Apply" button.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16542
Summary: windowType is not used anywhere so delete it.
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16389
Summary:
The constant is a leftover after
2569768074.
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16373
Summary:
In some cases, the out transition is false-triggered because we don't
react to changes in the keep-above and the full screen state.
Test Plan:
* Set the keep-above state on a window;
* Click on the desktop;
* (the window didn't "flicker")
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, abetts, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16270
Summary:
In order to check whether a window is on some particular virtual
desktop, one has to use isOnDesktop.
Also, this change adds implicit support for the new virtual desktop protocol.
Test Plan:
Switched between virtual desktops with a Konsole window being on all
desktops.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16103
Summary:
If a modal window is closed and some alternative effect that animates
the disappearing of windows is enabled(e.g. the Glide effect, or the
Scale effect), the Dialog Parent effect can cause flickering of the
parent window because its animation duration doesn't match duration of
those alternative effects.
Also, if the Fade effect, the Glide effect, and the Scale effect are
disabled, the Dialog Parent will keep the parent window alive for no
good reason.
This change addresses that problem by adding keepAlive property to
`animate` function so scripted effects have more control over lifetime
of animated windows.
If both a modal window and its parent window are closed at the same time
(and there is no effect that animates the disappearing of windows), the
Dialog Parent will stop immediately(because windowDeleted will be
emitted right after windowClosed signal).
If both a modal window and its parent window are closed at the same time
(and there is effect that animates the disappearing of windows), the
Dialog Parent won't reference the latter window. Thus, it won't cause
flickering. I.e. it will "passively" animate parent windows.
BUG: 355036
FIXED-IN: 5.15.0
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D14919
Summary:
This code has several bugs, it's untested, it's not used anywhere, it
adds unnecessary complexity -> delete it.
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16099
Summary:
The fullscreen effect lock is purely an implementation detail of the
AnimationEffect, we don't need to have it in the public API.
Test Plan: KWin still compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16075
Summary:
The Scale effect and the Glide effect have to animate only ordinary
windows(i.e. the ones that are considered to be apps).
On X11, in order to distinguish ordinary windows from combo box popups,
popup menus, and other popups, those effects check whether given window
is managed.
On Wayland, there is no concept of managed/unmanaged windows.
XDG Shell protocol defines 2 surface roles:
* xdg_toplevel;
* and, xdg_popup.
The former can be used to implement typical windows, the ones that can
be minimized, maximized, etc.
The latter can be used to implement tooltips, popup menus, etc. Thus,
that's a good criteria to filter popup windows.
CCBUG: 398100
Reviewers: #kwin, graesslin, davidedmundson
Reviewed By: #kwin, graesslin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D15117
