This makes the behavior of virtual and x11 backend consistent
with the drm and the wayland backend and reduces the number of users of
the Platform::outputEnabled signal.
The current code has multiple index-based for loops iterating over items we get from XCB
Dealing with raw indices is error-prone and not pretty
By using std::span we can replace these loops with range-based for loops
Also remove some intermediate containers that aren't neccesary
Two things about the code were wrong:
1. m_current is used in updateBlob() but was only updated afterwards
2. the assumption that the property having the same ID means it has the
same blob contents is not always true
BUG: 449285
Since the screen number is well-known, we can look up the default
screen on demand. Note that xcb_get_setup() is pretty cheap as it
simply returns a const pointer to pre-allocated data.
This breaks cyclic dependency between Compositor and DRM backend, there
are still indirect dependencies though. However, fewer cyclic
dependencies should make the architecture more cleaner and easier to
tweak.
Switches to a shared pointer rather than a raw one. It would leak, which
in general is not a big problem but it would flood the output when
running tests with ASAN. Also it's the right thing to do.
The wayland backend changes the dpms mode when the enabled state of the
output changes. This creates problems when destroying all outputs on
shut down. The wayland backend calls InputRedirection's methods, but
InputRedirection is already destroyed by that time.
While the crash can be fixed by guarding input() in createDpmsFilter()
with an if statement, changing dpms mode in updateEnablement() doesn't
seem like a good idea because Output changes unrelated states (enabled !=
dpms mode), so let's get rid of it.
At the moment, a platform should provide two output lists - one that
lists all available outputs, and the other one that contains only
enabled outputs. In general, this amounts to some boilerplate code and
forces backends to be implemented in some certain way, which sometimes
is inconvenient, e.g. if an output is disabled or enabled, it will be
simpler if we only change Output::isEnabled(), otherwise we need to
start accounting for corner cases such as the order in which
Output::isEnabled() and Platform::enabledOutputs() are changed, etc.
This can be used to make our backends more multi-purpose. At the moment,
new outputs are enabled by default, but it makes sense to do otherwise.
For example, if an output is disabled by default, it would be possible
to delegate initial output configuration to layer above, to kwin.
In long term, the drm backend would need to scan connectors, create an
Output for every one of them, kwin sees new outputs and tries to apply
the initial output configuration, which includes the enabled status.
OutputMode provides a more robust way to refer to outputs. A mode can
have flags and things as such that are not taken into account with mode
+ refresh rate.
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.
