Currently, the normal window lifecycle looks as follows: create Window,
wait until it's shown, add it to Workspace, wait until it's closed,
create a Deleted, copy properties from the original window to the
deleted one, destroy the original window, wait until the last deleted
window reference is dropped.
There are a couple of issues with this design: we can't nicely
encapsulate X11 or Wayland specific implementation details if they need
to be accessed for closed windows; manual copying of properties is
cumbersome and error prone and we've had a dozen of cases where effects
worked incorrectly because some properties had not been copied.
The goal of this patch is to drop Deleted and extend the lifetime of the
original window, but with a special state set: Window::isDeleted().
The main danger is that somebody can try to do something with deleted
windows that they should not do, but on the other hand, such code needs
to be guarded with relevant checks too.
When closing a window, we may need to change parent-child relationship
between windows, but we need to preserve stacking order constraints so
dialogs are placed above their parents when they're closed.
This is needed to ensure that Window::isScreenLocker() returns a good
value for closed windows. Also, it's needed to get rid of
ClientConnection::aboutToBeDestroyed() connection, which can cause
problems with window teardown.
If a transient parent is removed, X11Window::removeTransient() will try
to upgrade the transient to a group transient.
On the other hand, that code is broken because Window::removeTransient()
will unset transientFor() so we never hit the code path in the if
statement.
While we could fix it, I think it's not worth it because transient
handling code is already mental and making group transients and normal
transients disjoint will help to keep the things simpler.
Apparently CMAKE_AUTOMOC_MACRO_NAMES modifications are not visible in
sibling directories. To work around that, populate
CMAKE_AUTOMOC_MACRO_NAMES with libkwineffects plugin macros in the
top level CMakeLists.txt file.
QtConcurrent::run()'s return type is marked with [[nodiscard]], which is
not nice to code that just needs to move some tasks to a worker thread.
On the other hand, QThreadPool satisfies our needs too and Qt doesn't
need to construct futures that we won't use.
One remark about the screenshot plugin: the task lambda is mutable so it
cannot be represented using a std::function.
Workspace::outputAt() casts vectors to four rectangle corners and uses
the shortest one to decide which output is the closest to the given
point.
This works poorly on dual monitor setups where on the left side you have
a monitor with landscape orientation and one with portrait orientation
on the right hand side. In that case, outputAt() will prefer the left
monitor even though the right monitor is the closest one if you cast a
perpendicular from the given point to the right monitor.
In order to improve the handling of that case, this change makes
Workspace::outputAt() compute the closest point to the output geometry
rectangle and use the squared distance as the score.
The indirection contributes unnecessary complexity. The usage of
std::weak_ptr and std::shared_ptr complicates the things further, e.g.
![Screenshot_20230325_170226](/uploads/d8b68a9eff47c93c4463bb230b5bbe49/Screenshot_20230325_170226.png)
---
Ideally, same should be done with TabBox and TabBoxHandler, but that can be done in another MR.
"label" is already used.
Warning: The name 'label' (QLabel) is already in use, defaulting to 'label1'.
Warning: The name 'label' (QLabel) is already in use, defaulting to 'label2'.
Deleted windows are no longer destroyed using deleteLater(). They will
be destroyed immediately as Workspace goes through the window list
because the effects will be unloaded by that time.
Tabbox supports two operation modes: switching between windows and
desktops. Switching between windows is more commonly used. Desktop
switching is not exposed in user settings and it requires some advanced
knowledge of kwin's internals to enable it.
On the other hand, over the past years, we've double downed on effects
like desktop grid and overview to provide graphical means to switch
between virtual desktops.
This change drops desktop switching because it's effectively unused to
simplify the tabbox code, which can be very handy for the future
refactorings of window switching.
The `kwin_rules_dialog` executable was used to launch the rules
KCM on a specific window when called from the window menu
because we couldn't depend on the `kde-cli-tools` repo
Since now `kcmshell6` lives on KCMUtils we can call it directly
The property `flat: true`was set since the QML redesign and remained
unnoticed because it didn't actually work.
Now that the qqc2-desktop style has been fixed and correctly draws
flat (non-bordered) comboboxes let's remove it for a nicer look
(just the same as the have had all this time)
Currently, X11Window and Unmanaged call finishCompositing(), which tries
to destroy the window item and other associated compositing data.
Usually, it has no any effect on the window item and the effect window
because they are moved to the Deleted. However, it has some effect on
the XDamage handle.
If the X11 window is unmapped, it will destroy the XDamage handle. If
the X11 window is destroyed, it will do nothing. Why does it behave like
that? Because that's how the XDamage spec is written.
This change removes the call to finishCompositing() and refactors how
the XDamage is handled so Window::finishCompositing() is more generic.
If the X11 window is destroyed, SurfaceItemX11::forgetDamage() will be
called and SurfaceItemX11::~SurfaceItemX11() won't attempt to destroy
the damage handle.
If the X11 window is unmapped, SurfaceItemX11::destroyDamage() will be
called and destroyDamage() in SurfaceItemX11::~SurfaceItemX11() will
noop.
If compositing has been restarted, destroyDamage() in
SurfaceItemX11::~SurfaceItemX11() will destroy the damage handle.
Destroying a zwp_text_input_v3 object should be viewed as disabling it.
isEnabled property is cached because it cannot be computed in
_destroy_resource() handler. By that time, the resource no longer will
be in resourceMap(), so TextInputV3InterfacePrivate::isEnabled() will
erroneously return false even though we expect true.
Currently, input method relies on the fact that QObject::destroyed and
Window::closed are equivalent. But it's going to change, so make the
input method use a signal that's semantically better.
When a window is closed, a Deleted object will be constructed and the
Window's properties will be copied over to it. The long term plan is to
stop doing that, i.e. keep the Window alive but just flip a few flags to
indicate that it's been closed.
In order to unify decoration management, this change ensures that it's
okay to have decorations live as long as the Deleted.
It's needed to work around the cleanup logic of decorations.
Currently, decorations have a valid QObject parent and they're managed
using std::shared_ptr. That's not a perfect combination, but changing it
is also going to be an involved task because the QObject parent is used
to look up the Window.
In long term, it won't matter since we want to get rid of Deleted.
For what it's worth, it restores the order in which Deleted and normal
windows used to be destroyed prior to
995d509e45.
If a client owns several windows (for example it can be the case with
plasmashell) and it crashes, it's possible to encounter the following
case:
- xdg_wm_base resources are destroyed
- xdg_toplevel is destroyed
- another xdg_toplevel is destroyed
When kwin processes the destruction of the first xdg_toplevel, it may
ping the second xdg_toplevel. But the xdg_wm_base is already free()d by
that time, so kwin can access already released memory.
In order to prevent that, make the associated XdgSurfaceInterface
objects inert. Since XdgToplevelInterface and XdgPopupInterface will
become useless after destroying XdgSurfaceInterface, make them inert too.
As the spec states, it's illegal to destroy a bound xdg_wm_base object
while there are still alive xdg surfaces created by it so destroying the
surface role objects should be fine.