Instead of checking for properties needing a modeset, do atomic tests
with ALLOW_MODESET where it makes sense, and do a second atomic test
afterwards without ALLOW_MODESET to check if the modeset can be skipped.
This should ensure that KWin always does a modeset when it needs to do one,
and not do a modeset when it's not necessary. Doing this also allows
reducing the complexity of the drm backend a bit.
The DrmOutput synchronizes the enabled state with the active state,
which makes sense on one hand, but on the other hand, that's not good.
The drm backend makes a decision that should be ideally made by either
kscreen (turn on outputs before applying an output config), user, or
kwin itself.
This would also allow kwin to control the allocation of crtcs for
non-desktop outputs, which is a minor thing, but it might be useful in
the future.
It's important for tablet devices to be able to specify to which section
of the display we'll be fitting the tablet. This setting allows to
specify this by providing some options that will do so relative to the
output size.
CCBUG: 433045
infiniteRegion() is useful not only to effects but also other kwin
components, so move it to kwinglobals.h in order to make backends stop
depending on libkwineffects
This fixes the drm backend adding hotplugged gpus that belong to other
seats and makes the udev helper depend on less stuff from the layer
above backends.
The Session can be useful not only to the platform backend but also
input backends and for things such as vt switching, etc. Therefore it's
better to have the Application own the Session.
This is a too niche feature. It also doesn't have to be implemented in
the compositor. The kernel provides a way to overwrite the edid blob,
which is not specific to the running compositor.
Platform backends are provided as plugins. This is great for
extensibility, but the disadvantages of this design outweigh the
benefits.
The number of backends will be limited, it's safe to say that we will
have to maintain three backends for many years to come - kms/drm,
virtual, and wayland. The plugin system adds unnecessary complexity.
Startup logic is affected too. At the moment, platform backends provide
the session object, which is awkward as it starts adding dependencies
between backends. It will be nicer if the session is created depending
on the loaded session type.
In some cases, wayland code needs to talk to the backend directly, e.g.
for drm leasing, etc. With the plugin architecture it's hard to do that.
Not impossible though, we can approach it as in Qt 6, but it's still
harder than linking the code directly.
Of course, the main disadvantage of shipping backends in a lib is that
you will need to patch kwin if you need a custom platform, however such
cases will be rare.
Despite that disadvantage, I still think that it's a step in the right
direction where the goal is to have multi-purpose backends and other
reusable components of kwin.
The legacy X11 standalone platform is linked directly to kwin_x11
executable, while the remaining backends are linked to libkwin.
The main motivation behind this change is to make the drm backend
multi-purpose. That's it, to make it suitable for implementing all kinds
of compositors. At the moment, there's an artificial split between
"desktop" and "non-desktop" outputs, i.e. VR headsets, which stands in
the way of that and moving the remaining wayland code out of the drm
backend for better layering and architecture.
This allows us to get more information about the outputs like vendor
and model and for example provide them to effects which might find
the extra info useful.
KWin requires surfacesless contexts, so this setSurface() is not needed.
This ensures that makeCurrent() won't make the opengl context current
against a surface that belongs to a removed output.
KWin requires surfacesless contexts, so this setSurface() is not needed.
This ensures that makeCurrent() won't make the opengl context current
against a surface that belongs to a removed output.
The main reason to drop multi-head support is that it has been simply
unmaintained for many many years. When implementing a feature, we don't
even bother checking if multi-head is broken, KCMs don't handle
multihead, window management features are written for Xinerama. KWin
is optimized for Xinerama-like operation mode in general, which is
provided out of the box.
If you use multihead for esoteric gpu stuff, consider using kwin_wayland!
Some legacy drivers either don't accept gbm buffers suitable for cursors,
or don't handle them properly. In order to work around that, always do a
CPU import with legacy and use dumb buffers instead.
BUG: 453860
CCBUG: 456306
This behavior was added in order to fix a crash reported in bug report
442990. However, the analysis was not 100% correct, kwin failed to
create a placeholder because the relevant check was incorrect. The drm
backend was checking the list of all connected outputs rather than the
list with enabled outputs to decide whether to create a placeholder output.
As a safety measure the proposed behavior makes sense, however the drm
backend is not the right layer to implement it. If the last enabled
output is disconnected, kscreen should view it as a new output
setup and re-enable outputs in order to ask user what desired output
configuration should be.
The kernel doesn't disable connector objects that represent physical ports
when the output gets removed. If KWin tries to change the output configuration
without explicitly disabling the connector, atomic commits can fail.
Removing connectors that are still powered leads to a mismatch in atomic
commits: the crtc is still powered, but the connector also still there.
If KWin tries to disable the crtc afterwards, the atomic commits fail because
the connector needs to be disabled at the same time and it's missing from the
atomic commit request.
To fix this, whenever we fail to fetch information or get wrong data from
the kernel (like 0 modes), use the cached information instead and keep the
connector.
BUG: 456298
It seems it doesn't bring much and it may backfire. Especially don't
pass GBM_BO_USE_LINEAR as it will limit a lot the buffers that can be
created and GBM_BO_USE_RENDERING use seems to be more harmful than
helpful on most cases.
When we use a shadow buffer, we always render to the whole surface - setting
the damage region is incorrect and invokes undefined behavior. On the Lima
driver this caused flickering on screen rotation.
To fix this, don't set a damage region when we use a shadow buffer, which is
effectively setting the damage region to the full surface
It can happen that the drm backend temporarily lacks permission to do atomic
commits, or that the cached drm property values become out of sync with
the real values held by the kernel. Instead of failing with both, attempt
to update property values and try the commits again at a later time.
Set the time for mouse and touch events. This is especially important
for the mouse is as the timestamp will be used to discriminate between
single and double click. Previously this was always sending the double
click event, making buttons work only every other click.
BUG: 454275
BUG: 449907
At the moment, the DrmLeaseOutput class inherits from the
KWaylandServer::DrmLeaseConnectionV1Interface class. While this works,
it's not a future-proof design. For example, kwin could also lease its
"desktop" outputs in order to let another wayland compositor run
alongside it.
Also, it's a good practice to prefer composition over inheritance.
When dpms disabled outputs get set active, they require a modeset. If after
that they are set inactive again without resetting the pipelines first,
they no longer require a modeset but still have the pending properties that
would enable a crtc - but without a framebuffer set.
To prevent this, first test the current setup as it is, and only then see
if the pipelines would work if enabled again.
It makes it very hard to debug any use-case that isn't a strip of
outputs and even then, we should have other mechanisms to arrange
outputs properly (i.e. through kscreen).
Instead of using a DmaBufAttributes instance to communicate the settings
to create a new dmabuf, use a smaller DmaBufParams class that only
contains the information we need after destroying the BO.
At the moment, dmabuf importing is scattered all over the place in kwin.
It would be great if we had one function that takes dma-buf attributes
and returns an EGLImage if successful.
As the first step, make linux-dmabuf-v1 implementation provide dmabuf
attrs compatible with KWin::DmaBufAttributes.
There are a few benefits to using smart pointers from the standard library:
- std::unique_ptr has move semantics. With move semantics, transfer of ownership
can be properly expressed
- std::shared_ptr is more efficient than QSharedPointer
- more developers are used to them, making contributions for newcomers easier
We're also already using a mix of both; because Qt shared pointers provide
no benefits, porting to standard smart pointers improves consistency in
the code base. Because of that, this commit ports most of the uses of QSharedPointer
to std::shared_ptr, and some uses of QScopedPointer to std::unique_ptr
Makes sure disabled outputs are reported as such, leaving behind the
assumption that all outputs are always enabled.
Ensures the corresponding outputEnabled/Disabled signals are emitted.
Updates the window title to reflect the output state.
In response to a XCB_CONFIGURE_NOTIFY on the output window, the new size
is set as mode and the output layer buffer is recreated.
Signed-off-by: Victoria Fischer <victoria.fischer@mbition.io>
Part-of: <https://invent.kde.org/plasma/kwin/-/merge_requests/2459>
Under kwin_wayland `kwinApp()->connection()` is for communicating with
XWayland, but in X11Windowed backend we need to talk to the host XServer.
Restore `XRenderUtils::init` and set it accordingly based on
whether we're running standalone or windowed, so that `kwin_wayland`
works running nested in an X session again.
Signed-off-by: Victoria Fischer <victoria.fischer@mbition.io>
Tested-by: Merge Service <https://invent.kde.org/plasma/kwin/-/merge_requests/2457>
Part-of: <https://invent.kde.org/plasma/kwin/-/merge_requests/2457>
The XRender backend has been removed, leaving most of KWinXRenderUtils unused.
The few features that are still used, notable `XRenderPicture` and pict format
are moved into the x11/common directory.
Signed-off-by: Victoria Fischer <victoria.fischer@mbition.io>
For some reason with legacy the cursor gets an offset when changing the
image. In order to work around this, directly issue a cursor move with the
corrected position when changing the buffer
With this only the main and the libinput threads will use realtime
scheduling, so it will be harder to leak realtime scheduling to somebody
else.
The only caveat is that kwin would need to keep CAP_SYS_NICE around,
however on the other hand, it's needed to ensure that kwin_wayland is
able to get high priority EGL contexts with some drivers, e.g. intel.
It appears that importing gbm_bo's using eglCreateImageKHR() doesn't
work on nvidia.
Another issue is that Platform::createDmaBufTexture() uses
gbm_bo_create(). The nvidia driver doesn't like that, it's preferred to
use gbm_bo_create_with_modifiers().
In order to address those issues, this change refactors how gbm_bo
objects are imported and how gbm_bo's are allocated for dmabuf textures,
so the same code can be reused in EglGbmBackend::textureForOutput()
and Platform::createDmaBufTexture().
It can happen that a gbm implementation does not support modifiers, while
the drm driver does. To prevent that from breaking KWin, fall back to creating
a gbm surface without modifiers when creating one with modifiers fails.
BUG: 453320
The first time the GBM backend's EGL context is made current after
creation, both the read and draw surfaces are set to EGL_NO_SURFACE.
This will set the GL read and draw buffers to GL_NONE in accordance with
the EGL spec.
When a real surface is later made current, however, the spec is arguably
unclear on whether the read and draw buffers should remain set to
GL_NONE or whether they should be restored to the default GL_BACK. The
Mesa driver does the latter, the NVIDIA driver does the former.
To work around this difference, Kwin has an explicit call to
glDrawBuffer in GbmSurface::makeContextCurrent. It does not have a
corresponding call to glReadBuffer, though, which can cause some desktop
effects such as background contrast to render incorrectly with the
NVIDIA driver. This change adds that missing call.
Releasing the buffers is necessary for example in the case of a GPU reset,
to make sure that the gbm surface is still properly destroyed and all buffers
with invalid content freed.
Instead of buffers being both drm framebuffers and gbm / dumb buffers, these
responsibilities are now split, which makes it possible to do zero copy
screen casting in the future.
Both framebuffers and gbm / dumb buffers also now always hold a valid underlying
resource, which simplifies code a bit.
libinput_device_get_user_data() can be used to get the associated Device
object with libinput_device. That way, we won't need to maintain a
private list of all input devices.
Format modifiers enable the graphics hardware to be much more efficient,
especially when it comes to multi-gpu transfers. With the issues regarding
bandwidth limits now solved, enable them by default to make all supported
systems benefit from them.
CCBUG: 452397
CCBUG: 452219
When explicit modifiers are used, it can happen that Mesa chooses modifiers that make
the display hardware hit bandwidth limits. In that case, atomic tests fail and the
outputs don't work, or KWin may even crash.
In order to work around that, DrmGpu now removes the used modifier whenever an atomic
test fails, and tries to find a working combination of outputs and modifiers.
While having all state be public is great for avoiding the boilerplate that
comes with setters and getters, it also exposes more state than necessary
to the rest of the backend and makes it more error-prone if more than one
part of the state needs to be changed at the same time.
Instead of passing all possible field values to the initialize()
function, pass all relevant data in a struct. With designated
initializers, it's more readable and makes code more comprehensible.
The general goal is to split Output's data in two categories - general
information about the output (e.g. edid) and mutable state (position,
mode, etc).
With this, the drm backend will be able to associate drmModeModeInfo
with Output's modes, which can be useful if there are several modes with
the same resolution and refresh rate but different flags.
This makes KWin switch to in-tree copy of KWaylandServer codebase.
KWaylandServer namespace has been left as is. It will be addressed later
by renaming classes in order to fit in the KWin namespace.
Instead of creating a gammaramp object with a fixed size, make the color
device create a color transformation object that can be used to construct
arbitrary LUTs. This is needed in order to support tiled displays well
and is useful for further color management work.
If the backend needs to apply custom logic when changing the transform,
it should override Platform::applyOutputChanges(); otherwise just update
the Output's internal transform state.
AbstractOutput is not so Abstract and it's common to avoid the word
"Abstract" in class names as it doesn't contribute any new information.
It also significantly reduces the line width in some places.
Input event flow has been refactored so all input events originate from
input devices.
The X11 backend uses InputRedirection so make it forward events to
relevant input device handlers.
Prefering alpha is needed for some interactions with video underlays; more
directly choosing formats with an alpha channel will be needed for overlay
planes, too.
The main motivation behind this change is to unify render target
representation across opengl and software renderers and avoid accessing
the render backend directory in order to get the render target.
GLRenderTarget doesn't provide a generic abstraction for framebuffer
objects, so let's call GLRenderTarget what it is - a framebuffer.
Renaming the GLRenderTarget class allows us to use the term "render
target" which abstracts fbos or shm images without creating confusion.
With these two actions being separate, RenderLoop can record the time spent
in endFrame (for example for multi-gpu transfers) without risking also recording
blocking swapbuffer calls, and endFrame can later be moved to output layer
In 5.24, the same code path is used for testing direct scanout, so that
causes false negatives. Generally though, the user setting shouldn't be
touched, it's not really proper feedback for the driver or KWin having
problems.
Some modes can have the exact frame timings but different flags.
Currently, the mode comparison function doesn't take that into account
which can result in the drm backend setting the current mode flag
incorrectly.
If the blob is fetched while there is no kernel-visible reference to it,
the driver may re-use the blob ID. When DrmProperty is created or updated,
KWin holds a reference on the blob via drmModeObjectProperties, so this
should prevent any possible issues.
CCBUG: 449285
Using the global coordinate system when specifying output layer damage
regions would be very confusing. In order to make the coordinate system
comprehensible, use the layer-local coordinate system.
The infinite region is used to tell the Compositor when it needs to
repaint the entire layer.
When modesets are necessary, they are attempted when an output on the given
GPU gets presented. With multi-gpu setups however, the situation can arise
where there is only one disabled output on a GPU; in that case KWin eternally
waits and never properly turns off the display.
In order to work around this, explicitly call DrmGpu::maybeModeset when
an output gets disabled.
BUG: 449878
FIXED-IN: 5.24.4
The .clang-format file is based on the one in ECM except the following
style options:
- AlwaysBreakBeforeMultilineStrings
- BinPackArguments
- BinPackParameters
- ColumnLimit
- BreakBeforeBraces
- KeepEmptyLinesAtTheStartOfBlocks
Virtual machines aren't properly supporting atomic mode setting yet, which
causes the cursor to be offset, and will cause more issues with overlay
planes. In order to prevent that from impacting users, fall back to legacy,
unless KWIN_DRM_NO_AMS is set.
BUG: 427060
FIXED-IN: 5.24.4
Hard to avoid as long as we don't have CI coverage yet, but that will take
a bit more time, we need KWin (and all its dependencies) to fully build
first.
The raspberry pi exposes opaque formats for the cursor plane, and interprets
them as being opaque as well... Considering that we effectively don't support
anything else with the QPainter anyways, just hardcode ARGB8888 until we paint
the cursor with OpenGl.
fbdev has been deprecated and unmaintained for a while. With Linux 5.14
including SimpleDRM driver, we can drop it. (at the time of writing this
commit message, the latest Linux version is 5.16).
[1/6] Make autotests create fake input devices
The goal of this patch set is simulating user input in unit tests via
InputDevices and no longer use the Platform to fake input. This matches
more closely with how input is processed when running a full plasma
wayland session, i.e. with the DRM and libinput backends.
Otherwise when we render it, we do so upside down and screen sharing
looks broken.
This only happens when the shadow buffer is in use, so it's not all that
common.
Instead of having the render backends manage layers, have DrmGpu and DrmPipeline
do it. This makes it possible to unify code paths for leased and normal
outputs, remove some redirection and have more freedom with assigning layers
to screens.
We already try to ensure that the surface damage is within render target
bounds. Avoid clipping surface damage in render backend, which is a bit
excessive task and perhaps it should be done an abstraction level above.
When casting from integer to pointer, promoting the integer to (u)intptr_t
will ensure that the resulting type can be converted to a pointer without
problems. These two casts changed in this commit trigger a warning when
building for CHERI-enabled architectures such as Arm Morello. This is not
just limited to CHERI, the cast from xcb_pixmap_t (uint32_t) to void*
should also be flagged by -Wint-to-void-pointer-cast when using Clang,
however, it appears that warning only handles C-style casts, and not
reinterpret_cast (https://github.com/llvm/llvm-project/issues/53964).
At this point, it's safe to assume that only X11 has weird rendering
model, which stands in the way of making rendering abstractions nice and
intuitive, so let's check operation mode. If OperationModeX11 is
dropped, this will also simplify finding X11-specific code in kwin.
The responsibilities of the Scene must be reduced to painting only so we
can move forward with the layer-based compositing.
This change moves direct scanout logic from the opengl scene to the base
scene class and the compositor. It makes the opengl scene less
overloaded and allows to share direct scanout logic.
Having a render loop in the Platform has always been awkward. Another
way to interpret the platform not supporting per screen rendering would
be that all outputs share the same render loop.
On X11, Scene::painted_screen is going to correspond to the primary
screen, we should not rely on this assumption though!
This allows us to make the GLRenderTarget a bit nicer when using it to
wrap the default fbo as we don't know what the color attachment texture
is besides its size.
This means that the responsibility of ensuring that the color attachment
outlives the fbo is now up to the caller. However, most of kwin code
has been written that way, so it's not an issue.
