Summary:
Currently, we have only one shell client type - XdgShellClient. We use
it when we are dealing with Wayland clients. But it isn't really a good
idea because we may need to support shell surfaces other than xdg-shell
ones, for example input panel surfaces.
In order to make kwin more extensible, this change replaces all usages
of the XdgShellClient class with the AbstractClient class.
Test Plan: Existing tests pass.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D27778
Summary:
The GLX backend might need a combination of swap and composite timer events for
continous painting.
The reason for that is that if the buffer age extension is not available we
fall back to copies in case not the whole screen is repainted.
The timer logic is adapted to make this possible in a lean way what cleans up
the Compositor class in several ways.
Test Plan: Tested on X11 (with/without swap events, buffer age enabled) and Wayland.
Reviewers: #kwin
Subscribers: hurikhan77, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D26216
After paint in case we have swap events the buffer swap should be pending.
This is not always the case with the X11 standalone plugin what needs to be
investigated some more. This is for now a quick fix to make sessions work
again without failing on the assert.
Summary:
When the compositor is stopped there might still be a buffer swap ongoing, in
particular when a client blocks compositing on X11.
Depending on the backend the next buffer swap event might be handled in
bufferSwapComplete (Wayland) or not be handled (X11 GLX, since a new
GLX window will be created while the swap event is sent for the old one).
With this patch the buffer swap state is reset on stop such that on later
start no outdated data might create errors and instead a new repaint can be
triggered with updated data.
BUG: 415262
Test Plan: Manually on X11 and Wayland.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D26090
Summary:
When swap events are available do not delay the next repaint by one frame
through the composite timer but directly repaint on swap event.
Test Plan: i915
Reviewers: #kwin
Subscribers: davidedmundson, zzag
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D25299
This reverts commit 9151bb7b9e.
This reverts commit ac4dce1c20.
This reverts commit 754b72d155.
In order to make the fix work, we need to redirect the client window
instead of the frame window. However, we cannot to do that because
Xwayland expects the toplevel window(in our case, the frame window)
to be redirected.
Another solution to the texture bleeding issue must be found.
CCBUG: 257566
CCBUG: 360549
Summary:
Since KDE 4.2 - 4.3 times, KWin doesn't paint window decorations on real
X11 windows, except when compositing is turned off. This leaves us with
a problem. The actual client contents is inside a larger texture with no
useful pixel data around it. This and decoration texture bleeding are
the main factors that contribute to 1px gap between the server-side
decoration and client contents with effects such as wobbly windows, and
zoom.
Another problem with naming frame pixmap instead of client pixmap is
that it doesn't quite go along with wayland. It only makes more difficult
to abstract window quad generation in the scene.
Since we don't actually need the frame window when compositing is on,
there is nothing that holds us from redirecting client windows instead
of frame windows. This will help us to fix the texture bleeding issue
and also help us with the ongoing redesign of the scene.
Test Plan: X11 clients are still composited.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D25610
Summary:
Qt has its own thing where a type might also have corresponding list
alias, e.g. QObject and QObjectList, QWidget and QWidgetList. I don't
know why Qt does that, maybe for some historical reasons, but what
matters is that we copy this pattern here in KWin. While this pattern
might be useful with some long list types, for example
QList<QWeakPointer<TabBoxClient>> TabBoxClientList
in general, it causes more harm than good. For example, we've got two
new client types, do we need corresponding list typedefs for them? If
no, why do we have ClientList and so on?
Another problem with these typedefs is that you need to include utils.h
header in order to use them. A better way to handle such things is to
just forward declare a client class (if that's possible) and use it
directly with QList or QVector. This way translation units don't get
"bloated" with utils.h stuff for no apparent reason.
So, in order to make code more consistent and easier to follow, this
change drops some of our custom typedefs. Namely ConstClientList,
ClientList, DeletedList, UnmanagedList, ToplevelList, and GroupList.
Test Plan: Compiles.
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D24950
Summary:
Compositing in X11 was done time shifted, meaning that we paint first, then
wait one vblank interval length and present on prepareRenderingFrame the
previous paint result. This is supposed to make sure we don't miss the vblank
and in case of block till retrace be able to continue issuing commands and
only shortly before next vblank present.
This is counter-intuitiv, not how we do it on Wayland or even on MESA with X.
The reason seems to be that the GLX backend was in the beginning written
against Nvidia proprietary driver which needed this but nowadays even this
driver defaults to non-blocking behavior on buffer swap.
Therefore remove this legacy anomaly fully and directly present after paint.
We then wait one refresh cycle and in the future can optimize this by delaying
the paint and present till shortly before vsync.
Test Plan: kwin_x11 tested on i915 and Nvidia proprietary driver.
Reviewers: #kwin
Subscribers: zzag, alexeymin, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D23514
Summary:
Selecting not to vsync does not make sense for an X11 compositor. In the end
we want clients to be able to present async if they want to but the compositor
is supposed to send swaps with vsync to the XServer in order to not generate
tearing artifacts.
There was also a detection logic which did some questionable things in case
vsync was not available. I don't think this is necessary at all since we can
just always run a timer to present with or without vsync.
Test Plan: kwin_x11 tested on i915.
Reviewers: #kwin, zzag
Subscribers: zzag, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D23511
Summary: getShadow is not a getter method as it doesn't return a shadow.
Test Plan: Compiles.
Reviewers: #kwin, romangg
Reviewed By: #kwin, romangg
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D24298
Summary:
Qt's metaobject is rather sensitive with scope resolution.
Foo::Bar and Bar don't always match to a Qt metaobject, even if they
refer to the same thing to a compiler. Here we register
X11Compositor::SuspendReason but Q_ARG uses SuspendReason and they don't
match. This leads to a runtime failure where the method isn't invoked.
Rather than fixing metaobject usage, port the whole thing to lambdas
which does better compile time checking and is generally nicer to read.
BUG: 412353
Test Plan:
Ran xprop to block compositing. Compositing was blocked.
Grepped source code for Q_ARG use
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D24244
Summary:
Currently each managed X11 client is represented with an instance of
Client class, however the name of that class is very generic and the
only reason why it's called that way is because historically kwin
was created as an x11 window manager, so "Client" was a sensible choice.
With introduction of wayland support, things had changed and therefore
Client needs to be renamed to X11Client in order to better reflect what
that class stands for.
Renaming of Client to X11Client was agreed upon during the last KWin
sprint.
Test Plan: Compiles, the test suite is still green.
Reviewers: #kwin, romangg
Reviewed By: #kwin, romangg
Subscribers: romangg, davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D24184
Summary:
It doesn't belong with advanced compositing settings as it's quite user
friendly, and we also want to adjust other animation speeds. May as well
do it together.
In the current form all compositing is still completely reinitiliased
like with the previous slider. Change notifications come in the form of
KConfigWatcher rather than our own bespoke update interface.
Test Plan:
Moved new slider, minimised a window.
It still behaved as expected.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, broulik, anthonyfieroni, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22887
Summary:
Rename ShellClient to XdgShellClient in order to reflect that it
represents only xdg-shell clients.
Test Plan: Compiles, tests still pass.
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D23589
Summary:
So far wayland was used by internal clients to submit raster buffers
and position themselves on the screen. While we didn't have issues with
submitting raster buffers, there were some problems with positioning
task switchers. Mostly, because we had effectively two paths that may
alter geometry.
A better approach to deal with internal clients is to let our QPA use
kwin core api directly. This way we can eliminate unnecessary roundtrips
as well make geometry handling much easier and comprehensible.
The last missing piece is shadows. Both Plasma::Dialog and Breeze widget
style use platform-specific APIs to set and unset shadows. We need to
add shadows API to KWindowSystem. Even though some internal clients lack
drop-shadows at the moment, I don't consider it to be a blocker. We can
add shadows back later on.
CCBUG: 386304
Reviewers: #kwin, davidedmundson, romangg
Reviewed By: #kwin, romangg
Subscribers: romangg, kwin
Tags: #kwin
Maniphest Tasks: T9600
Differential Revision: https://phabricator.kde.org/D22810
Summary:
Because KWin is a very old project, we use three kinds of null pointer
literals: 0, NULL, and nullptr. Since C++11, it's recommended to use
nullptr keyword.
This change converts all usages of 0 and NULL literal to nullptr. Even
though it breaks git history, we need to do it in order to have consistent
code as well to ease code reviews (it's very tempting for some people to
add unrelated changes to their patches, e.g. converting NULL to nullptr).
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson, romangg
Reviewed By: #kwin, davidedmundson, romangg
Subscribers: romangg, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D23618
Summary:
Compositor::hasScene() is redundant. Depending on use case, it can be
replaced by checking either m_state or Compositor::scene().
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D23744
Summary: Less likely to happen but still we need to handle this case.
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D23611
Summary: Overlay windows is an X11 thing.
Test Plan: Compiles.
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D23608
Summary:
Switch to Q_ASSERT in order to make code a bit more consistent. We have
places where both assert and Q_ASSERT are used next to each other. Also,
distributions like Ubuntu don't strip away assert(), let's hope that
things are a bit different with Q_ASSERT.
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: romangg, davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D23605
Summary:
Any call made to a virtual method in constructor/destructor of a base
class won't go to a derived class because the base class may access
uninitialized or destroyed resources.
For example, let's consider the following two classes
class Base {
public:
Base() { foo()->bar(); }
virtual ~Base() { foo()->bar(); }
virtual Foo* foo() const { return nullptr; }
};
class Derived : public Base {
public:
Derived() : mFoo(new Foo) {}
~Derived() override { delete mFoo; }
Foo* foo() const override { return mFoo; }
private:
Foo* mFoo;
};
When an instance of Derived class is created, constructors will run in
the following order:
Base()
Derived()
It's not safe to dispatch foo() method call to Derived class because
constructor of Derived hasn't initialized yet mFoo.
Same story with destructors, they'll run in the following order:
~Derived()
~Base()
It's not safe to dispatch foo() method call in the destructor of Base
class to Derived class because mFoo was deleted.
So, what does that weird C++ behavior has something to do with KWin? Well,
recently Compositor class was split into two classes - WaylandCompositor,
and X11Compositor. Some functionality from X11 doesn't make sense on
Wayland. Therefore methods that implement that stuff were "purified," i.e.
they became pure virtual methods. Unfortunately, when Compositor tears
down it may call pure virtual methods on itself. Given that those calls
cannot be dispatched to X11Compositor or WaylandCompositor, the only
choice that C++ runtime has is to throw an exception.
The fix for this very delicate problem is very simple - do not call virtual
methods from constructors and the destructor. Avoid doing that if you can!
This change moves Compositor::updateClientCompositeBlocking to X11Compositor
so it longer has to be a virtual method. Also, it kind of doesn't make sense
to keep it in base Compositor class because compositing can be blocked only
on X11.
BUG: 411049
Test Plan: KWin no longer crashes when running kwin_x11 --replace command.
Reviewers: #kwin, romangg
Reviewed By: #kwin, romangg
Subscribers: anthonyfieroni, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D23098
Summary:
With the split of the Compositor class in X11 and Wayland it is unnecessary
to check in X11Compositor if compositing is required since on X it never is.
Test Plan: Compiles.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D23032
Summary: Replaces foreach loops with modern for loops and improve code style overall.
Test Plan: Auto tests pass as before and manually in X and Wayland sessions.
Reviewers: #kwin
Subscribers: zzag, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D23011
Summary:
This patch is a first take at splitting up of the Compositor class into
Wayland and X11 child classes.
In this first patch we mostly deal with setup and teardown procedures.
A future goal is to further differentiate the compositing part itself too.
Test Plan: Manually X from VT and Wayland nested. Autotests pass.
Reviewers: #kwin
Subscribers: sbergeron, anthonyfieroni, zzag, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D22195
Summary:
Replace the several internal state booleans of Compositor with a single
enum to register the current state of the compositor. We register four
states of starting, started, stopping and stopped.
The goal is to replace the several different conditionals when starting
and stopping the compositor with a single well defined flow.
There are currently still some ugly conditionals and some replaced ones
might need some more work.
Test Plan: Manually in X and Wayland. Relevant auto tests pass.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D22277
Summary:
To streamline Compositor code more remove the composite reset timer. The two
times it was used we can either use a singleshot timer instead or connect the
call to a different signal in the X11 backend.
Long term goal is to have a well structured init of the Compositor such that
we can call directly instead.
Test Plan: Manually in X and Wayland nested session.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D22270
Summary:
This removes the restart function of the Compositor class and renames the
internal reinitialize function.
Instead of the restart function reinitialize can be called. Reading again
the settings in this case is fine, since it is done rarely. This reduces
the code complexity.
Test Plan: Manually on Wayland and X. 100% autotests pass.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: davidedmundson, zzag, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D22225
Summary:
The check is used to limit settings reload. But we can afford to reload
settings in case the compositing gets toggled since this happens rarely.
Removing the check reduces code complexity.
Test Plan: Manually in X and Wayland.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D22220
Summary:
As a preparation for further changes clean up the Compositor
class. First step is to use the new slot syntax and rename
some of the slots.
Includes some other minor code style improvements to the class
as well.
Test Plan: Manually in X and Wayland nested session.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: davidedmundson, zzag, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D22218
Summary:
Declare it in the source file for internal usage of ApplicationX11 and
Compositor classes. Additionally use modern style connect and do other
code style updates.
Test Plan: Manually in X session and Wayland windowed.
Reviewers: #kwin, zzag
Subscribers: davidedmundson, sbergeron, zzag, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D21655
Summary:
Compositing today is ubiquitous. There is no reason to keep compositing
specific functions of Toplevel, Client and Workspace classes in the
composite.cpp source file. Instead let these definitions be separated.
Test Plan: Compiles
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D21654
Summary:
KWin is not only an X11 compositing window manager, but also a Wayland
compositor. The Compositor class is used in both cases so in general it
would be nice to keep it as much as possible generic.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D19879
Summary:
Deleted has lifetime bounded to effects and scene.
We can't discard Deleted before EffectsHandler is destroyed because
effects that referenced it may call unrefWindow.
On the other hand, the fact that Deleted may outlive scene doesn't
make sense because Deleted exist purely for animation purposes and
nothing more.
This change arranges lifetime of Deleted so it's more reasonable.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: graesslin, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18914
Summary:
Currently, each frame callback sent by KWin has the current time in
nanoseconds, but the protocol spec states that we have to send the time
in milliseconds. This is the reason why animations that are driven by
frame callbacks are too fast.
In addition to that, m_timeSinceStart isn't actually "time since start,"
it's rather accumulated duration of all painting cycles. If there is
something to draw and it takes quite a while to compose the scene, maybe
m_timeSinceStart will be close enough to the current time. So, it has
been replaced with QElapsedTimer, this makes the current time correct
and also simplifies code a little bit.
Test Plan: The triangle in weston-subsurfaces no longer spins very fast.
Reviewers: #kwin, romangg
Reviewed By: #kwin, romangg
Subscribers: romangg, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18656
Summary:
On Wayland we cannot switch from OpenGL to QPainter compositor as this
would break any running OpenGL application. KWin registers it's
EGLDisplay to Wayland and without OpenGL this doesn't make sense any
more. We are not able to render OpenGL buffers in the QPainter
compositor.
While it's theoretically possible to switch from QPainter to OpenGL it
doesn't make any sense for the same reason. Any running OpenGL
application would be using llvmpipe and could not be switched to proper
OpenGL.
This change stores the selected compositing type in Platform and the
implementations can use it to restrict the supported compositors. On X11
we don't need this, all other Platforms implement the restriction. Thus
it's no longer possible to switch the backends at runtime.
Test Plan:
Adjusted tests run, no runtime test as gui doesn't support
switching to QPainter anyway.
Reviewers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D19084
Summary:
Adds an autotest to show that KWin fails an assertion when a client tries to
resize a sub-surface.
Since it is the first autotest dealing with sub-surfaces explicitly additional
autotest helpers are introduced to allow that.
We also add a new signal in Compositor to spy on to know when the buffer swap
has been completed.
Test Plan:
Test fails as expected:
```
QFATAL : KWin::BufferSizeChangeTest::testShmBufferSizeChangeOnSubSurface() ASSERT: "image.size() == m_size" in file /home/roman/dev/kde/src/kde/workspace/kwin/platformsupport/scenes/opengl/abstract_egl_backend.cpp, line 394
FAIL! : KWin::BufferSizeChangeTest::testShmBufferSizeChangeOnSubSurface() Received a fatal error.
Loc: [Unknown file(0)]
Totals: 4 passed, 1 failed, 0 skipped, 0 blacklisted, 367ms
********* Finished testing of KWin::BufferSizeChangeTest *********
```
Reviewers: #kwin, zzag
Subscribers: zzag, graesslin, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18452
Summary:
Currently, KWin/Wayland crashes when the compositor is reinitialized.
The reason for that is ShellClient's DecorationRenderer gets destroyed
when the scene is already gone, thus there is no current OpenGL context.
Client works around that issue by destroying scene-specific DecorationRender
in finishCompositing. Such a workaround could be applied to ShellClient
as well, but it would make code more confusing because DecoratedClientImpl
also tries to destroy DecorationRenderer.
A better approach would be to notify DecoratedClientImpl that
compositing is about to be finished, so it can destroy the decoration
renderer when the scene is still alive. This not only fixes the
previously mentioned issue in ShellClient, but also makes code a little
bit tidier.
Test Plan:
Start Plasma on Wayland session, change any compositor settings (e.g.
animation speed).
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18921
Summary:
Code that destroys all scene windows when finishing compositing is not
readable. E.g. can you tell what this piece of code is doing without
looking into the source code of Scene::windowClosed?
foreach (Client * c, Workspace::self()->clientList())
m_scene->windowClosed(c, NULL);
This change intoduces removeToplevel(as well its counterpart) method to
the Scene class. The name of the new method much better describes what
we're doing.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18210
Vlad Zahorodnii
Roman Gilg
Vlad Zahorodnii
David Edmundson
Martin Flöser