Ensure the server can safely handle client requests for wl_seat resource
that have become inert due to weston_seat object release and subsequent
destruction.
The clean-up involves, among other things, unsetting the destroyed
weston_seat object from the user data of wl_seat resources, and handling
this NULL user data case where required.
The list of sites extracting and using weston_seat object from wl_seat
resources which were audited for this patch are:
Legend:
N/A = Not Applicable (not implemented by weston)
FIXED = Fixed in the commit
OK = Already works correctly
== keyboard_shortcuts_inhibit_unstable_v1 ==
[N/A] zwp_keyboard_shortcuts_inhibit_manager_v1.inhibit_shortcuts
== tablet_input_unstable_v{1,2} ==
[N/A] zwp_tablet_manager_v{1,2}.get_tablet_seat
== text_input_unstable_v1 ==
[FIXED] zwp_text_input_v1.activate
[FIXED] zwp_text_input_v1.deactivate
== wl_data_device ==
[FIXED] wl_data_device_manager.get_data_device
[OK] wl_data_device.start_drag
[FIXED] wl_data_device.set_selection
[OK] wl_data_device.release
== wl_shell ==
[FIXED] wl_shell_surface.move
[FIXED] wl_shell_surface.resize
[FIXED] wl_shell_surface.set_popup
== xdg_shell and xdg_shell_unstable_v6 ==
[FIXED] xdg_toplevel.show_window_menu
[FIXED] xdg_toplevel.move
[FIXED] xdg_toplevel.resize
[FIXED] xdg_popup.grab
== xdg_shell_unstable_v5 ==
[FIXED] xdg_shell.get_xdg_popup
[FIXED] xdg_surface.show_window_menu
[FIXED] xdg_surface.move
[FIXED] xdg_surface.resize
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Properly clean up all sub-objects (e.g., weston_pointer_client objects)
when a weston_pointer object is destroyed. The clean-up ensures that the
server is able to safely handle client requests to any associated
pointer resources, which, as a consenquence of a weston_pointer
destruction, have now become inert.
The clean-up involves, among other things, unsetting the destroyed
weston_pointer object from the user data of pointer resources, and
handling this NULL user data case where required. Note that in many
sites affected by this change the existing code already properly handles
NULL weston_pointer (e.g. in init_pointer_constraint), so there is no
need for additional updates there.
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Fix init_pointer_constraint so that it creates a valid, but inert,
resource if a NULL weston_pointer value is passed in. In that case no
constraint object is associated with the resource, but this is not an
issue since affected code can already handle NULL constraint objects.
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
ivi-shell tests load their own controller plugin
for testing purposes. Tests also uses the generated
weston-ivi.in config file, which causes weston to
load hmi-controller and its helper client.
Existence of hmi-controller and its helper client
confuses test plugins. Because they are creating
surfaces and layers which are not expected by
test plugins.
We can start ivi-shell tests without config file
to solve this problem. Then, weston will not load
hmi-controller plugin.
Reported-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Acked-by: Daniel Stone <daniels@collabora.com>
Drop support for the obsolete xdg-shell v5 protocol. This clears the
path to properly support xdg-shell stable, since xdg-shell stable and
xdg-shell v5 can't currently co-exist in the same compositor, as both
define structures with the same name (such as struct
xdg_surface_interface).
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Proposed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Acked-by: Jonas Ådahl <jadahl@gmail.com>
/home/pq/git/weston/desktop-shell/shell.c: In function ‘shell_output_destroy_move_layer’:
/home/pq/git/weston/desktop-shell/shell.c:4718:24: warning: unused variable ‘output’ [-Wunused-variable]
struct weston_output *output = data;
Since the data pointer is not used for anything, decided to also set it
to NULL in the caller. This caused another variable to become unused.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Marius-Vlad <marius-cristian.vlad@nxp.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Leaks spotted by Valgrind.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Spotted by Valgrind.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Fixes the following Valgrind error:
==21607== Syscall param ioctl(generic) points to uninitialised byte(s)
==21607== at 0x5E8C787: ioctl (in /lib64/libc-2.25.so)
==21607== by 0x8220C17: drmIoctl (in /usr/lib64/libdrm.so.2.4.0)
==21607== by 0x82263CD: drmModeSetCrtc (in /usr/lib64/libdrm.so.2.4.0)
==21607== by 0x7B22095: drm_output_apply_state_legacy (compositor-drm.c:2107)
==21607== by 0x7B2335D: drm_pending_state_apply (compositor-drm.c:2539)
==21607== by 0x7B23AEB: drm_repaint_flush (compositor-drm.c:2773)
==21607== by 0x4E4A3E4: output_repaint_timer_handler (compositor.c:2500)
==21607== by 0x5081496: wl_event_source_timer_dispatch (event-loop.c:235)
==21607== by 0x5081B61: wl_event_loop_dispatch (event-loop.c:633)
==21607== by 0x50803A4: wl_display_run (wayland-server.c:1245)
==21607== by 0x409DD8: main (main.c:2644)
==21607== Address 0xffefff59a is on thread 1's stack
==21607== in frame #2, created by drmModeSetCrtc (???:)
==21607== Uninitialised value was created by a stack allocation
==21607== at 0x7B2782F: drm_output_choose_initial_mode (compositor-drm.c:4842)
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
[Pekka: switch to memset]
Reviewed-by: Daniel Stone <daniels@collabora.com>
Move it into to a new function. Following patches want to compute it
elsewhere as well.
No functional changes.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
This function needs to reset the structures to NULL, otherwise it is not
possible to re-use a once "freed" property info array.
Being able to re-use an array is useful when the memory allocation and
array lifetimes do not match. A specific example is drm_output that is
changed to allocate the CRTC on enable() and deallocate it on disable().
A drm_output might be enabled and disabled multiple times.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
I need to destroy the list from more places, so factor out the common
bits. No functional changes.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Factor out drm_output_init_crtc() and drm_output_fini_crtc(), so that
the call sites can later be moved easily.
On fini, reset scanout_plane and cursor_plane to NULL, so that in the
future when the drm_output is not longer destroyed immediately after, we
free the planes for other use.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
[Pekka: set crtc_id/pipe at top, reset both on error]
Reviewed-by: Daniel Stone <daniels@collabora.com>
Move this bit of code into its own function. The caller of this already
cluttered and origcrtc is not used for anything else.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Let drm_output_enable() remove the CRTC and the connector from the
unused id arrays.
In the future when a list of drm_heads supersedes unused_connectors
array, the usedness of a connector will be determined by the enabled
state of the output the connector (head) is attached to. The enabled
state is turned on by drm_output_enable(). If unused_crtcs array was
still updated in drm_output_repaint(), the CRTC and connector usedness
would be tracked in different places. Logically the two belong together.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
In shared-CRTC clone mode there are several wl_output globals for one
weston_output. Only one panel and background is needed per
weston_output, so the extra wl_outputs do not get their own panel and
background.
When a head is unplugged, the corresponding wl_output is removed. If
that was the wl_output associated with the background and panel
surfaces, we must transfer the ownership to a remaining wl_output that
was a clone to avoid losing the background and panel completely.
The transfer relies on desktop-shell.so implementation to register
background and panel surfaces with the weston_output, not the
weston_head, so it does not actually matter the wl_output used to bind
the surfaces is going away.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
If for some reason the helper client weston-desktop-shell would create
more than one panel surface for the same weston_output, this code would
corrupt the surface destroy listener list by adding a link already in
one list into another list.
Instead, do not store the new, redundant panel surface and do not
subscribe to its destruction. Also, tell the helper that the surface is
redundant by configuring it with a 0x0 size, so that we don't waste
memory on a panel that is never used.
(Clone mode is a valid reason why weston-desktop-shell could do that.)
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
If for some reason the helper client weston-desktop-shell would create
more than one background surface for the same weston_output, this code
would corrupt the surface destroy listener list by adding a link already
in one list into another list.
Instead, do not store the new, redundant background surface and do not
subscribe to its destruction. Also, tell the helper that the surface is
redundant by configuring it with a 0x0 size, so that we don't waste
memory on a background that is never used.
(Clone mode is a valid reason why weston-desktop-shell could do that.)
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
If for some reason the desktop-shell plugin would configure a panel with
an invalid size, just destroy the whole panel and forget about it for
this wl_output.
A following patch will cause desktop-shell to configure 0x0 panel when
it deems the panel redundant.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
If for some reason the desktop-shell plugin would configure a background
with an invalid size, just destroy the whole background and forget
about it for this wl_output.
A following patch will cause desktop-shell to configure 0x0 background
when it deems the background redundant.
Fortify weston-desktop-shell against not every output having a
background.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Since formats is an out parameter, we need to copy to the alloc'ed
memory and not over the pointer address.
Signed-off-by: Philipp Kerling <pkerling@casix.org>
Reviewed-by: Daniel Stone <daniels@collabora.com>
The cairo surface used for the icon must be completely given to the
frame as soon as said frame has been created. To prevent both the
window and the frame from sharing ownership of the icon, we set
window->icon_surface back to NULL right after creating or changing the
frame, only keeping it there when no frame has been created yet.
Fixes https://lists.freedesktop.org/archives/wayland-devel/2018-January/036655.html
Reported-by: Derek Foreman <derekf@osg.samsung.com>
Tested-by: Derek Foreman <derekf@osg.samsung.com>
Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
Setting state_invalid to true is moved together with the code adding new
unused CRTCs and connectors in drm_output_deinit(). Logically these two
operations belong together: state_invalid is required for the new unused
item to be turned off.
This does not hinder initial turning off of outputs, because on
compositor start-up, state_invalid is initialized to true, making calls
to drm_output_disable() for non-enabled outputs a no-op.
Previous changes already ensure that if a compositor does not explicitly
enable an output, the CRTC and connector will be turned off even without
an explicit disable (provided there is a at least one enabled output).
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Rather than smashing the state to disable a CRTC immediately, just
delegate that to the normal repaint cycle by setting state_invalid =
true. drm_pending_state_apply() will pick up the unused_crtcs.
A caveat here is that we have no enabled outputs at all, we will never
enter repaint, and so CRTCs do not actually get turned off until we get
at least one output to drive.
However, this should help the problem reported here:
https://lists.freedesktop.org/archives/wayland-devel/2018-January/036713.html
Arguably it is better to leave an output spuriously on in rare cases
rather than fail modeset completely in somewhat more common cases.
My personal motivation for this change is that it helps if we later move
CRTC allocation to output enable/deinit instead of create/destroy,
because then the CRTC will not be available here for initial turn-off as
the output has not been enabled to begin with.
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
Our case is when the view is the same as output being disabled/disconnected.
There's not need to check the views' output with the output being disabled
because weston_view_assign_output() already changes the output of the view when
the output has been disabled/disconnected hence the check is not needed at all.
The views' output will always be different than the output being disabled.
By the time shell_output_destroy_move_layer() gets called the views' output has
already changed to a "free" output. Tested this by unplugging/disabling the
output on purpose.
Signed-off-by: Marius Vlad <marius-cristian.vlad@nxp.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
This was used from the crash handlers, which do not exist anymore.
Nothing calls restore, so delete the dead code.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Catching an ABRT is kind of ok, catching a SEGV is russian roulette. We
have been quite lucky with it, but I've started hitting crashes inside
malloc() which causes a deadlock when our SEGV handler needs to malloc()
as well (weston_log_timestamp()).
One reason to catch SEGV and ABRT was to attempt to restore the VT on
the DRM-backend. Nowadays that job is done by logind or weston-launch.
The signal handler also printed a backtrace, which for me personally has
been extremely helpful. Arguably it's not necessary though, when we have
core files and services that catch cores. For instance, if using
systemd, 'coredumpctl gdb' is delightfully easy for getting into the
saved core.
Therefore, this code does more harm than it is useful, so remove it. We
also drop an optional dependency to libunwind.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
If XDG_PICTURES_DIR not given, it will use the current directory, as
it was before.
Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Instead of assuming the file prefix contains the path and filename
prefix, give these two items separately.
A NULL or empty string path may still be given to refer to the current
directory.
Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Instead of overwriting the 'wayland-screenshot.png' file over and
over, store each requested screenshot in a filename based on timestamp
and sequence number.
Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
Reviewed-by: Daniel Stone <daniels@collabora.com>
if the layer is in order of some screen we need to remove it
from there and mark the screen order as dirty so it will be removed
in commit_screen_list call later
layer should only be assigned to one screen at a time
Signed-off-by: Eugen Friedrich <efriedrich@de.adit-jv.com>
Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
it is assigned in weston_view_assign_outputs
Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
it is not necessary to repaint all outputs after
each commit_changes. Only outputs with modified
views has to be repainted.
We need to call weston_view_update_transform
for assigning views to outputs first.
Then, We can call weston_view_schedule_repaint
to trigger repaint for outputs.
Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
ivi_layout_layer_set_visibility has bool
as argument.
Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
commit e7fff215ad made initializing the
selection_listener conditional, but didn't make its clean-up
conditional at shutdown. Simply initializing the listener's list
link at init time makes this harmless.
To see this, run weston -Bheadless-backend.so and then connect to it
with an X client. When killing weston it will attempt shutdown but
die with a segfault.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
it has only developermode option parameter.
The parameter is only used in init_ivi_shell.
Therefore, we can basically remove the struct,
and check the option locally in the function.
Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
controller modules can be loaded as weston modules
from the main function of weston.
Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
It is better to load ivi controller modules as a
generic weston module. Then, we do not need to
have a specific ivi way of loading modules.
Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
weston loads hmi-controller as a weston module.
IVI-shell does not need to load it explicitly.
Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Put the interface into hmi_controller struct.
It is better to have it in an object.
Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Now only libshared (and libshared_cairo) requires this.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Replace every use of DATADIR to create a filename with a call to the new
function that allows overriding DATADIR with an env var at runtime.
No attention is paid to asprintf failure.
This restores make distcheck to a passing state after commit 6b58ea
began checking cairo surfaces for validity and exchanged undefined
behaviour we shouldn't have been dependent on for consistent test failure.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
[Pekka: split if-branches into two lines]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Set the env var to override the system data directory so we can run
tests with uninstalled icons.
We don't yet use the code that checks this env var, so make distcheck
will still fail.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Currently we look for png files in their install directory, and we
manufacture filenames with string pasting at compile time.
This new function will allow overriding the compile time setting with
the env var WESTON_DATA_DIR so we can do neat tricks like allow our
test suite to pass when we haven't yet installed icons system-wide.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
[Pekka: split if-branch into two lines.]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Add support for using the atomic-modesetting API to apply output state.
Unlike previous series, this commit does not unflip sprites_are_broken,
until further work has been done with assign_planes to make it reliable.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Co-authored-by: Louis-Francis Ratté-Boulianne <louis-francis.ratte-boulianne@collabora.com>
Co-authored-by: Derek Foreman <derek.foreman@collabora.co.uk>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
For atomic modesetting support, the mode is identified by a blob
property ID, rather than being passed inline. Add a blob_id member to
drm_mode to handle this, including refactoring mode destruction into a
helper function.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Set the atomic client cap, where it exists, and use this to discover the
plane/CRTC/connector properties we require for atomic modesetting.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
When leaving Weston, don't attempt to restore the previous CRTC
settings. The framebuffer may well have disappeared, and in every
likelihood, whoever gets the KMS device afterwards will be repainting
anyway.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Rather than a smattering of error handlers, use consistent jump labels
for error paths in create_output_for_connector().
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reported-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Alexandros Frantzis
Emre Ucan
Pekka Paalanen
Philipp Kerling
Emmanuel Gil Peyrot
Marius Vlad
Aleksander Morgado
Derek Foreman
Daniel Stone