Currently, we use glFinish() to ensure that stream consumers don't see
corrupted or rather incomplete buffers. This is a serious issue because
glFinish() not only prevents the gpu from processing new GL commands,
but it also blocks the compositor.
This change addresses the blocking issue by using native fences. With
the proposed change, after finishing recording a frame, a fence is
inserted in the command stream. When the native fence is signaled, the
pending pipewire buffer will be enqueued.
If the EGL_ANDROID_native_fence_sync extension is not supported, we'll
fall back to using glFinish().
Besides being unused, we should avoid making OpenGL contexts current
against the EGLSurface of the first output because it's a slippery road
that may end up in leaking context-specific resources in the mid of a
compositing restart.
On Wayland, internal windows that use OpenGL are rendered into fbos,
which are later handed over to kwin. In order to achieve that, our QPA
creates OpenGL contexts that share resources with the scene's context.
The problems start when compositing has been restarted. If user changes
any compositing settings, the underlying render backend will be
reinitialized and with it, the scene's context will be destroyed. Thus,
we no longer can accept framebuffer objects from internal windows.
This change addresses the framebuffer object sharing problem by adding
a so called global share context. It persists throughout the lifetime of
kwin. It can never be made current. The scene context and all contexts
created in our QPA share resources with it.
Therefore we can destroy the scene OpenGL context without affecting
OpenGL contexts owned by internal windows, e.g. the outline visual or
tabbox.
It's worth noting that Qt provides a way to create a global share
context. But for our purposes it's not suitable since the share
context must be known when QGuiApplication attempts to instantiate a
QOpenGLContext object. At that moment, the backend is not initialized
and thus the EGLDisplay is not available yet.
BUG: 415798
Every time Platform::supportsQpaContext() is called, we go through the
list of supported extensions and perform a string comparison op. This is
not really cheap.
Commit e459c8bf54 added a sanity check to
prevent recomputing the texture matrix if the y-inverted hint hasn't been
changed, which is totally reasonable!
However, code that initializes dmabuf textures implicitly assumes that
calling setYInverted() always results in updating the matrix. But it may
be not the case if the passed value matches current isYInverted().
This change adds missing calls to force updating the texture matrix.
Note that we don't need to check the buffer size every time the dmabuf
image has been modified externally because the window pixmap is going to
be re-created if the dimensions of the attached buffer have changed.
I've seen some reports on the internet about Firefox displaying garbage
instead of videos. 99% that bug is caused by this issue. But it seems
like Firefox no longer displays corrupted videos on my machine, so it's
hard to tell.
Uses a setter and clear method pattern rather than having the code
repeated.
Instead of keeping a QPointer, now we are a QObject and we get notified
about destruction intention directly, so we can clear the pointer when
necessary.
Summary:
Notify the driver about the parts of the screen that will be repainted.
In some cases this can be benefitial. This is especially useful on lima
and panfrost devices (e.g. pinephone, pinebook, pinebook pro).
Test Plan:
Tested on a pinebook pro with a late mesa version.
Basically I implemented it, then it didn't work and I fixed it.
Maybe next step we want to look into our damage algorithm.
The main advantage of SPDX license identifiers over the traditional
license headers is that it's more difficult to overlook inappropriate
licenses for kwin, for example GPL 3. We also don't have to copy a
lot of boilerplate text.
In order to create this change, I ran licensedigger -r -c from the
toplevel source directory.
The wp_viewporter compositor extension allows clients to crop and scale
their surfaces. It can be especially useful for applications wishing to
reduce their power consumption, e.g. video players, etc.
Given that there is no any direct relationship between the surface size
and the buffer size anymore, we have to use specialized helper methods
for converting coordinates from the surface-local space to buffer pixel
space and vice versa.
The recommended way to get all EGL extension defines is to include
EGL/eglext.h. EGL/eglmesaext.h is a private header that compositors
should not use.
BUG: 422131
Summary:
Removes a few TODO items mentioning that some code was exactly the same
in a couple of different places.
drm: Remove unnecessary cast
Test Plan:
Using it right now
Will push in 2 different commits
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D28708
Summary:
Add a small getter to query information internally if the backend supports
swap events. Defaults to true as it is the default in the GBM Wayland backend.
Test Plan: i915
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D25298
Summary:
We leak memory if we do not destroy the dmabuf implementation on EGL backend
going down.
Also this makes sure everything is cleaned up on shutdown.
FIXED-IN: 5.17.4
BUG: 413637
Test Plan: Compiles, settings change and shutdown ok.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D25577
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:
It is not clear what the advantage of triple buffering is for KWin. An X11
compositor is meant to swap buffers once every monitor cycle. For that triple
buffering is not necessary.
The functionality is not maintained, does not reliably work as displayed by
the existence of an environment variable to force some behavior, pollutes
our code and every compositing-related problem that might be mitigated with
triple buffering should find a simpler and more fitting solution with other
means.
There is one caveat which is if we shall block for retrace. We set it
currently according to the result of the swap profiler and in the most common
case with double buffering it is set to true. But on Nvidia systems this might
be actual the wrong behavior. Instead of trying to work around this ignore
the issue for now and move the overall architecture to something less complex
by presenting after paint how we do it in the Wayland DRM backend and with
double buffering on GLX (although this is at the moment also borken because
we actually present then twice).
Test Plan: kwin_x11 tested on i915.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, fredrik, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D23504
Summary:
The EGL platform might go away at any time through reconfiguration or because
of a graphic error. KWin then resets the graphics. The dmabuf implementation
must respect that and recover from a graphics reset by recreating all EGL
images for existing buffer.
This assumes that we won't change our graphics API mid-session and that
supported plane and modifier configuration stays constant.
In practise we remember all current dmabufs in a single map and only remove
them if the client did destroy the resource.
BUG: 411980
CCBUG: 413403
FIXED-IN: 5.17.2
Test Plan: Applied screenedge configuration without crash.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: fvogt, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D24954
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:
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:
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: This adds support for LinuxDmabufUnstableV1Interface in kwin.
Test Plan: Session starts. `weston-simple-dmabuf-egl` and `weston-simple-dmabuf-drm` execute without errors.
Reviewers: #kwin, #plasma, davidedmundson, mart, graesslin, fredrik
Subscribers: meven, zzag, romangg, anthonyfieroni, plasma-devel, kwin
Tags: #kwin
Maniphest Tasks: T8067
Differential Revision: https://phabricator.kde.org/D10750
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:
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:
Some of these variables have been declared protected but can apparently
also be private.
Other minor code style improvements.
Test Plan: Compiles, Wayland nested session runs.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22109
Summary: Shm textures via Gl must be reloaded in case their size changes.
Test Plan: testShmBufferSizeChangeOnSubSurface passes.
Reviewers: #kwin, graesslin
Reviewed By: #kwin, graesslin
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18453
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:
This patch implements using EGL_IMG_context_priority to request
high-priority rendering contexts if the extension is available.
EGL_IMG_context_priority is currently used in this fashion by
e.g. Android's SurfaceFlinger (RenderEngine.cpp) and libweston
(gl-renderer.c) and seems promising given this widespread
acceptance.
Reviewers: #kwin, graesslin, romangg, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D11572
Summary: `abstract_egl_backend.cpp` includes several kwayland headers, but the include paths aren't added for kwayland. This patch solves the problem, by adding kwayland as requirement for the opengl scene plugin.
Reviewers: #kwin, graesslin
Reviewed By: #kwin, graesslin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D10373
Summary:
Unfortunately a rather large change which required more refactoring than
initially expected. The main problem was that some parts needed to go
into platformsupport so that the platform plugins can link them. Due to
the rather monolithic nature of scene_opengl.h a few changes were
required:
* SceneOpenGL::Texture -> SceneOpenGLTexture
* SceneOpenGL::TexturePrivate -> SceneOpenGLTexturePrivate
* texture based code into dedicated files
* SwapProfiler code into dedicated files
* SwapProfiler only used in x11 variants
* Safety checks for OpenGL scene moved into the new plugin
* signal declared in SceneOpenGL moved to Scene, so that we don't need
to include SceneOpenGL in composite
Test Plan: Nested OpenGL compositor works
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D7740
This change is similar to D7232 and moves the scene_qpainter into a
dedicated plugin. Compared to the XRender case it's more complicated as
the platform plugins need to implement a platform specific backend.
The base implementation for this part used to be in scene_qpainter. As
the idea is to completly move it away from KWin core it would be point
less to still have the backend definition in KWin core, but it cannot
be in the scene plugin as otherwise all platforms need to link the
plugin.
To solve this a new platformsupport subdirectory is added which contains
the scene platform backend as a static library. For the OpenGL scene such
a static library will also be required.
Test Plan: SceneQPainter test still passes, nested compositor still works
Reviewers: #kwin, #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D7259
