GitLab

Summary:
In order to help with debugging why the OpenGL scene is not loaded,
it can be really helpful to know what scenes the Compositor attempts to
load.

Test Plan: Ran kwin with QT_LOGGING_RULES="*.debug=true".

Reviewers: #kwin, davidedmundson

Reviewed By: #kwin, davidedmundson

Subscribers: kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D29261

This reverts commit b3a19f9e.

See: https://mail.kde.org/pipermail/kwin/2020-January/002999.html

This reverts commit 8d137290.

See: https://mail.kde.org/pipermail/kwin/2020-January/002999.html

This reverts commit 00bf75d0.

See: https://mail.kde.org/pipermail/kwin/2020-January/002999.html

This reverts commit ba2c0324.

See: https://mail.kde.org/pipermail/kwin/2020-January/002999.html

This reverts commit be01ba0a.

See: https://mail.kde.org/pipermail/kwin/2020-January/002999.html

This reverts commit 1e3128b0.

See: https://mail.kde.org/pipermail/kwin/2020-January/002999.html

This reverts commit af3fbf34.

See: https://mail.kde.org/pipermail/kwin/2020-January/002999.html

The assert is still not always true.

BUG: 415750

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 9151bb7b.
This reverts commit ac4dce1c.
This reverts commit 754b72d1.

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 d...

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

Vlad Zahorodnii authored Aug 31, 2019 and Vlad Zahorodnii committed Sep 23, 2019

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

Vlad Zahorodnii authored Aug 26, 2019 and Vlad Zahorodnii committed Sep 23, 2019

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: Compositor::reinitialize() already reconfigures effects.

Reviewers: #kwin

Subscribers: kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D23607

Summary: scheduleRepaint() and addRepaintFull() do nothing when m_state is not State::On.

Reviewers: #kwin, romangg

Reviewed By: #kwin, romangg

Subscribers: romangg, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D23590

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