Summary:
When destroying a SurfaceInterface all callbacks are getting destroyed.
This used to iterate over the callbacks and performing
wl_resource_destroy on them. This triggered the destroy handler which
removes the resource from the callback list. Which means removing from
the list we are iterating on. This could result in a double delete or
accessing invalid memory.
This change copies all callbacks to a temporary list and clears the
normal lists. So the destroy handler does no longer modify the lists
currently being iterated on.
Test Plan: Added a test case which crashed with previous code
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1677
The ShellSurface might have the resource destroyed, before being deleted,
so there is a short time frame where resource might be null.
Crash was caught by KWin.
Reviewed-By: sebas and notmart
A downstream KWin test shows a possible heap-use-after-free if we
access the wl_client pointer. Basically we get the client disconnected
just before the focused surface gets unbind. Thus for a short moment
the ClientConnection pointer is gone. This needs to be extended with
a test case, but for the moment it should be good enough to get KWin
green again.
Summary:
So far the server component performed manual cleanup in some cases
when a client disconnects. But this is not needed: the Wayland library
calls the static unbind methods which do cleanup. If we cleanup ourselves
this can result in double deletes in the worst case, so let's only use
the Wayland functionality.
Adjusted:
* RegionInterface
* SurfaceInterface
* ShellSurfaceInterface (doesn't take a parent anymore)
* DpmsInterface
* QtSurfaceExtensionInterface
* KeyboardInterface
* PointerInterface
* TouchInterface
* DataOfferInterface
* PlasmaShellSurfaceInterface
For each adjusted case a test case is added to verify that the cleanup
works. Exceptions are DpmsInterface as the actual Resource is not exposed
at all in the Server component and DataOfferInterface as that is server
side created.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1640
From time to time the test is failing on build.kde.org with an ASAN
heap-use-after-free error. From my investigation this seems to be caused
by the OutputDevice being constructed on the stack and being destroyed
while handling Wayland events, but before all are handled.
The test mostly operates on the changed signal. There is also a done
signal emitted later on. Wayland sends the done after a set of changes
is transmitted. Thus the test is adjusted to wait for done instead of
changed. So we can ensure that all events are handled before the object
gets destroyed.
I have never been able to reproduce the problem locally, so I cannot
guarantee that the issue is solved for good. If it still happens more
investigation will be needed.
Summary:
When the model gets destroyed the lambda connections were still invoked
and could cause crashers.
Test Plan: Test case added and each one verified that it crashed
Reviewers: #plasma, hein
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1624
Summary:
There is a possibility that a PlasmaWindow is unmapped when the
PlasmaWindowModel gets created. In this situation the unmapped
PlasmaWindow will be deleted in the next event cycle. So far
PlasmaWindowModel didn't handle this situation and the model might
hold deleted objects due to this.
This change addresses this potential problem and ensures the model
gets updated when a PlasmaWindow is deleted.
Test Plan: Test case which exposes the problem is added
Reviewers: #plasma, hein
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1622
Summary:
So far when the active PlasmaWindow got unmapped or destroyed, the
PlasmaWindowManagement didn't update the activeWindow. This means it
could expose a deleted object through it's API which could result in
a crash.
This change addresses the problem by updating the active window when
a window gets unmapped or destroyed.
Test Plan: Tests added which exposed the problem
Reviewers: #plasma, hein
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1621
Summary:
The protocol is extended by a dedicated destructor request. When a
PlasmaWindow is umapped we no longer destroy the resource directly,
but only send the unmap. The client is then supposed to clean up
(which it already did in that case) and will invoke the destructor.
The PlasmaWindowInterface object will be automatically deleted after
the unmap once all resources bound for it are destroyed.
The tests are extended by two new test cases which triggered protocol
errors on the client side prior to this change.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1594
Summary: To use pid_t one should include sys/types.h -- else the build fails on FreeBSD.
Reviewers: graesslin
Reviewed By: graesslin
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1579
Coverity complains (in CID 1335150) about `Generator::parseInterface()`
since the default copy-ctor for `Interface` will end up copying
uninitialized memory (the unset `m_factory` member). Fixed by
initializing the m_factory.
REVIEW:127836
Summary:
Exposes closable state in the window model and adds tests. This was
included in the protocol and interface, but missing from the model.
Test Plan: Autotest extended.
Reviewers: graesslin
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1435
Summary:
Analogous to NET::ActionChangeDesktop.
KWindowInfo::actionSupported(NET::ActionChangeDesktop) is hardcoded
to return true in kwin, but that's not how it should be; as this will
be fixed later the Wayland protocol needs this state bit as well for
parity.
Test Plan: PlasmaWindowModel test is extended.
Reviewers: graesslin
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1448
Summary:
Adds states and request methods to determine whether a window can be put into
move or resize modes, and request move and resize modes, respectively.
request* naming pre-discussed with Martin. I chose to add *Mode to be more
explicit as well as avoid namespace conflicts with possible other Move/Resize
methods in the future.
Since these are not toggleable states, there is no requestToggle* methods.
Protocol version is not bumped (also pre-discussed with Martin) since we have
pending changes already bumping to 3.
Depends on D1417.
Test Plan: PlasmaWindowModel test extended.
Reviewers: graesslin
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1432
Summary:
This adds support for 'shadable' and 'shaded' states to the protocol and
to the client and server classes, as well as the window model.
Test Plan: The PlasmaWindowModel test has been extended to test the new states.
Reviewers: graesslin
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1417
Summary:
Surface/SubSurface and Shadow handle the case that an attached buffer
gets destroyed by the client. So far we didn't have this code covered,
but it's rather important as incorrect reference counting can hit
asserts.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1403
Summary:
KWayland does not have a client implementation of the QtSurfaceExtension
protocol. Thus the test is different: it starts a helper binary which
creates a QWindow. The test closes that one which should terminate the
started applications.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1388
Summary:
Basic functionality is covered. Changing of shadow elements not covered,
there seems to be lacking server API for that - no change signal.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1398
Summary:
Now also tests:
* requesting mode from client side
* update supported change on server side
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1357
If the focused pointer and keyboard surface is the same we use pointer
clicks as a hint to which child surface should have keyboard focus.
Keyboard focus handling for sub surfaces is rather limited overall.
We just don't have a good model on how to determine which child surface
should get the keyboard focus. When passing focus to a surface there
is no way to know which of the sub-surfaces should get the focus.
Ideally the client should handle this, but that's just not the case.
The best we have is a reference through the pointer. But that's of
course also limited. Keyboard focus passed to the surface for another
reason (Alt+Tab) cannot select the proper sub-surface without interaction
from another input device.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1330
Summary:
The idea behind this change is to make the existance of sub-surfaces
an implementation detail for pointer events. The user of the library
does not need to care about on which sub-surface the pointer is on.
It only needs to care about the main surface and passes the focus to
the main surface.
Internally the PointerInterface takes care of sending the enter to
the sub-surface at the current pointer position. Also whenever the
pointer position changes, the PointerInterface evaluates whether it
triggered a change for the focused sub-surface and sends enter/leave
events accordingly. If the focused sub-surface does not change, it
sends motion events as normally, but of course under consideration
of the sub-surface position.
Overall this means that from pointer usage perspective a user of the
library doesn't need to care about the fact that there are sub-surfaces
at all. The library does the correct thing for it.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1329
Summary:
The new method returns the (child) surface at a given surface position
taking care of stacking order, whether surfaces are mapped, etc.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1319
Summary:
This change introduces a damage tracking feature in SurfaceInterface.
So far the SurfaceInterface only exposes the damage compared to the
last attached buffer. But this is not always usefull for the user of
the library. E.g. if:
* server renders
* client damages buffer and commits
* client damages buffer and commits
* server wants render
In this situation the last damage information is not helpful to the
user of the library. It would need the combined damage information over
all attached buffers.
The new API combines the damage of the two commits in the example above.
The user of the library can then call resetTrackedDamage once it
processed the current damage (e.g. by updating the OpenGL texture).
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1281
Summary:
QtWayland doesn't map the parent sub-surfaces in a sub-surface tree.
According to the spec this would mean also the child sub-surface is not
mapped. But being strict according to the spec will make applications
like SystemSettings fail badly. Embedded child windows will not be
rendered and QtWayland is going to hard freeze. This is not acceptable,
thus we need to workaround this QtWayland bug till it's fixed.
It's worth mentioning that Weston as the reference compositor also
doesn't handle this situation according to spec and renders the
sub-surface. See https://bugs.freedesktop.org/show_bug.cgi?id=94735
The difficult part for the workaround is to determine whether a surface
should be considered unmapped. E.g. when the parent gets unmapped we need
to really unmap it. But what's the difference between an unmapped parent
surface which should be considered mapped and an unmapped parent surface
which should be considered unmapped?
The implementation goes with considering a new sub-surface always as
mapped - independently of whether it ever got a buffer attached. As soon
as it had a buffer attached and it gets unmapped again, it will go back
to a standard conform way.
The behavior now is not standard conform, thus the autotest is adjusted
to have QEXPECT_FAIL for the now no longer standard conform areas.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1250
Summary:
If a Surface doesn't have a buffer attached and a null buffer gets
attached the buffer state doesn't really change. Thus neither the
unmapped signal nor the damaged signal should not be emitted.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1261
Summary:
New test case which verifies the behavior when a surface is considered
mapped or unmapped in a sub-surface tree.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1248
Summary:
In a SubSurface tree a Surface is only considered mapped if the Surface
has a buffer applied and the parent Surface is mapped.
The added method implements this check. It's useful for the compositor
to easily check this condition as it allows to easily figure out whether
a SubSurface needs to be rendered and it's also useful for implementing
the input handling as a not mapped sub-surface should not get any input
events.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1247
Summary:
This is a workaround for https://bugreports.qt.io/browse/QTBUG-52118
It should take effect whenever the parent surface's state is applied,
but QtWayland never commits the parent surface. Thus the position is
always wrong.
Having a workaround for that in our server code is not good, but better
than completely broken applications such as Systemsettings.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1212
Summary:
From spec:
If the wl_surface associated with the wl_subsurface is destroyed,
the wl_subsurface object becomes inert. Note, that destroying either
object takes effect immediately.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1211
Summary: Fixes errors in API of methods which should be const not being const.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1214
Summary:
See: https://bugreports.qt.io/browse/QTBUG-52092
Freeze happens if a sub-surface is rendered to before the main surface
is rendered. The compositor has no chance to know that this is a window
which needs to be rendered, thus the application might freeze without
ever becoming visible.
Famous example applications being affected: all kcms with a nested
QQuickView. E.g.: kcmshell5 kwineffects
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1208
Summary:
Recursively go up to the main surface, that is the top level surface
which doesn't have a parent.
This is useful to know to which surface tree a sub-surface belongs to.
Reviewers: #plasma
Subscribers: plasma-devel
Projects: #plasma
Differential Revision: https://phabricator.kde.org/D1207
QtWayland doesn't commit the parent surface when creating a sub-surface.
This results in a QtWayland application to freeze as it renders to the
surface and waits for the frame rendered, which it will never get as the
Compositor waits for the commit on the parent prior to mapping the
sub-surface.
To work around this behavior, we apply the adding/removing directly.
The behavior around this is actually not fully documented, so QtWayland
is not wrong per se. See:
https://lists.freedesktop.org/archives/wayland-devel/2016-March/027540.html
Once this is properly clarified and implemented in the Client, we should
revert this change.
Differential Revision: https://phabricator.kde.org/D1191
