We don't need to be setting this because it isn't (yet) used anywhere.
It will get set when it is used.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Rather than juggling our layers manually, just use
weston_view_move_to_layer. This obviates the need for manually marking
the view as mapped.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We must have (from || to), because if (!from && !to), we would've caught
the if (from == to) early-return case.
Signed-off-by: Daniel Stone <daniels@collabora.com>
However many times you thought this might reasonably be called when
setting up for fullscreen, there were more.
Signed-off-by: Daniel Stone <daniels@collabora.com>
The only place we ever set shsurf->fullscreen.black_view is when we're
going into fullscreen, and we destroy it when we're going out of
fullscreen. Hence if we do ever have a fullscreen black view, we should
move it out of the way.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Merge shell_configure_fullscreen() and
shell_ensure_fullscreen_black_view() into shell_set_view_fullscreen().
Signed-off-by: Daniel Stone <daniels@collabora.com>
If we're asked to set a view as fullscreen, assume we're not doing so
with the surface not being mapped.
Signed-off-by: Daniel Stone <daniels@collabora.com>
weston_shell_utils_center_on_output() already handles the case where we
don't have an output to aim at.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Remove any attempt to reposition windows when handling an output resize.
This had some side-effects on x11 and wayland backends that keep moving
windows at random positions when manually the output.
A more smarter approach would be to move them only if they're actually
affected by a resize -- like no longer in the output's viewing area, but
that remains for another day.
Fixes: #793
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Reported-by: Philipp Zabel <p.zabel@pengutronix.de>
The output resize handler we only accounts for the background and panel
surfaces, so this handles all other regular shsurfs.
This patch would reuse any previously saved position, or reposition the
surfaces to avoid placing them outside of the output area. For maximized
or fullscreen type of surfaces, issue a new configure event to let
clients react to the new output dimensions.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
We'd need to go over them when handling output resize so use
desktop_shell to hang of the list of shsurfs.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This introduces two helpers, shsurf_is_max_or_fullscreen() and
set_shsurf_size_maximized_or_fullscreen() to handle
maximized/unmaximized fullscreen/unfullscreen transitions.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
When we're rotating a surface, only mark the geometry as dirty after
we've actually updated the transformations. Then we can restrict our
repaint to just the view itself, not the full compositor.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Use the new helper instead of open-coding part of it. This removes a
comment about not marking a surface as mapped until it has a buffer: the
surface->width == 0 check already guarantees that we have a buffer.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Instead of pre-creating a fade-out view that's sort of left half-mapped
around in the scene graph, create the view only when we need it, and use
the helpers to make sure that the damage is correctly handled.
Signed-off-by: Daniel Stone <daniels@collabora.com>
When we're tabbing away from fullscreen views, use the new helper rather
than open-coding layer switching.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Preserve the same order as desktop-shell for handling view (un)mapping,
so we can move these into a shared helper. These should have no
functional effect but provide a helpful bisect point.
Signed-off-by: Daniel Stone <daniels@collabora.com>
shell_surface_update_layer() is the thing which moves our views around
into layers. Since we want to keep an invariant that a view is mapped if
it is on a layer, and unmapped if it is not on a layer, handle mapping
along with the layer moves.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Try to stick to a pattern of first mapping a surface, then inserting a
view into a layer together with marking it as mapped.
Signed-off-by: Daniel Stone <daniels@collabora.com>
weston_view_geometry_dirty() won't automatically clear out the old
region, so manually damage underneath the view before we mark the
geometry dirty.
Signed-off-by: Daniel Stone <daniels@collabora.com>
There is no way for shell_surface_calculate_layer_link() to give us a
NULL layer, so don't pretend it can and silently drop out without
removing it from the old layer.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Until now we've only had the unadorned arithmetic functions, but they're
easy to abuse and tedious to use.
For now, we just add weston_coord_global_add/sub functions and use them
where appropriate.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This is stored as an unadorned weston_coord internally, but with getter
functions we can put together the appropriate global or surface
coordinate.
Use them where appropriate.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Truncating a weston coord to integer values is something we do
frequently enough to warrant a helper function.
Use this in the kiosk and desktop shells where appropriate.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This is similar to commit 'libweston: Update view transforms more often', where
we update the view's transform when getting the panel dimensions.
Fixes#740
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
The addition in a former commit of the flags field in the activation data will
let us pass the reason for activation to the constraint logic. We use that
reason here to unconditionally enable constraints in the recently 'fullscreened'
surface.
Signed-off-by: Sergio Gómez <sergio.g.delreal@gmail.com>
When a toplevel xdg surface is changing from non-fullscreen to fullscreen upon a
client request, activate its corresponding shell surface.
This will let us use the activation mechanism to enforce the enabling of
pointer confinement.
Signed-off-by: Sergio Gómez <sergio.g.delreal@gmail.com>
When the position or size of a fullscreen surface changes (e.g., due to change
in the output), the surface commit logic in desktop_surface_committed() resets
the fullscreen by first calling unset_fullscreen() and later on calling
shell_configure_fullscreen(). One part of this reset is the recreation of the
black view curtain (destroyed in unset_fullscreen() and created again in
shell_configure_fullscreen()).
In the upcoming commit we will replace the call to shell_configure_fullscreen()
with a call to activate() (since we want to activate this shell surface, and
activate() already has a call to shell_configure_fullscreen() in it).
The code in activate() by default lowers the fullscreen layer of the shell
surface through lower_fullscreen_layer(), which is called before
shell_configure_fullscreen(). This lowering function assumes that the fullscreen
shell surface has a valid black view curtain, which as said before was removed
in the call to unset_fullscreen(). Add the check to guard against this case.
Signed-off-by: Sergio Gómez <sergio.g.delreal@gmail.com>
Upon connector reconnect/disconnect we seem to ignore any kind of window
state we might have previously to disconnect so this takes that into
account and avoids a change in the state in case we detect one set-up
previously.
Fixes: #731
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Based on patches from:
Peter Hutterer <peter.hutterer@who-t.net>
Lyude Paul <thatslyude@gmail.com>
Bastian Farkas <bfarkas@de.adit-jv.com>
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
When it comes to a window frame, a tablet tool and cursor act almost
identical; they click things, drag things, etc. The tool type and extra
axes don't serve any use in the context of a window frame, so tablet
pointers share the frame_pointer structures used for the mouse pointer.
Co-authored-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Lyude Paul <thatslyude@gmail.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
Signed-off-by: Bastian Farkas <bfarkas@de.adit-jv.com>
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
Popups should have keyboard focus when active, but the toplevel window
should still appear "active". Make sure this is the case by changing the
"active" tracking to see whether any child surface has keyboard focus.
Signed-off-by: Jonas Ådahl <jadahl@gmail.com>
I also snuck in a trivial change to drag_surface_configure at the same
time to avoid yet another micro patch.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
When compositor is active, we cannot make sure that the output power
state is normal, if the output power was forced off, there is nothing can
display on the output. Therefore, only do fade animations when the output
power state is normal
Signed-off-by: Vinh Nguyen Trong <Vinh.NguyenTrong@vn.bosch.com>
The desktop_surface object is destroyed first so it can happen that the shsurf
still exists but desktop_surface is already NULL. So expand the check to make
sure the desktop_surface is still available in the resize callbacks.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
Convert the bare x,y coordinates into struct weston_coord and update all
users.
We keep the surface position in wl_fixed_t for now so it still exactly
matches the position most recently sent to clients.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This reverts commit 4eea291512.
There were some details and cases that I've missed when writing this
commit, resulting in some weird behaviors. Trying to cover all of them
became a nightmare, and the function got really hard to read.
So it's better to revert this commit and think about other possible
solutions for the issue.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
During interactive resizes, we progressively change the size of the
client surface and send config events with these sizes to the client.
After that, the toplevel->pending.size keeps the size of the last config
event that we've sent, i.e. the surface size after the resize is over.
Later, if the client spontaneously resize (by attaching a buffer with a
different size or setting the viewport destination, for instance), their
surface size will change, but toplevel->pending.size continues being
that old size from after the resize. If something happens and Weston
decides to send a config event, clients may re-allocate to that old
size, resulting in a sudden resize.
This does not happen when a client goes from fullscreen/maximized to
windowed mode because in such cases we are resetting
toplevel->pending.size to zero. So in the next config event that clients
receive they are allowed to attach buffers with the size that they
prefer.
So do the same after a resize: set the pending config size to zero.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
Every shells have an implementation of weston_desktop_api structures.
It includes some callbacks to listen to xdg_surface signals. Committed callback
is one of them.
Currently, the xdg_shell don't invoke the weston_desktop_api->committed
callback when the xdg_surface is initial stage or no surface content.
In the case: the client attached a null buffer after the valid buffer,
the shell isn't going to invoke to xdg_surface committed, and don't know
the surface is disappeared. If the surface is fullscreen, we will get
a black background on the screen until a new valid frame come. That
should not happen.
Signed-off-by: Tran Ba Khang(MS/EMC31-XC) <Khang.TranBa@vn.bosch.com>
Update users of the old coordinate space conversion functions that take
x, y pairs to the new weston_coord versions.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
In commit d611ab24fd "libweston: Update
view transforms more often", a call to weston_view_update_transform()
was introduced to desktop_surface_committed(). It was added between the
point in which we call unset_fullscreen() and
shell_configure_fullscreen(), right after the view geometry dirty bit is
set.
There's a scenario with dual displays in which this change resulted in
the surface being alternated between two outputs:
---
Dual display configuration:
1st display: DP1, with scale 1 - origin 0, 0
2nd display: DP2, with scale 2 - origin 1920, 0
We start the app with the cursor on DP2. Function
desktop_surface_committed() gets called a few times, and it ends up
setting shsurf->saved_x and shsurf->saved_y to the origin of DP2.
Application wants to become fullscreen on DP1, so when the surface gets
committed again and desktop_surface_committed() gets called, we have the
following sequence:
desktop_surface_committed():
was_fullscreen = shsurf->state.fullscreen
is_fullscreen = weston_desktop_surface_get_fullscreen()
if (!weston_surface_is_mapped(desktop_surf))
map(shell, desktop_surf)
return;
/* POINT A, this is important for understanding the issue. */
if (shsurf size didn't change and
fullscreen state didn't change)
return;
if (was_fullscreen)
/* This function calls weston_view_set_pos(saved_x,
* saved_y), and the saved position is the origin of
* DP2. Then it invalidates the saved position */
unset_fullscreen(shsurf)
if (is_fullscreen && !shsurf->saved_position_valid)
/* Saves the position (as it just have been
* invalidated), which will be the origin of DP2
* again. */
shsurf->saved_x = shsurf->view->geometry.x
shsurf->saved_y = shsurf->view->geometry.y
shsurf->saved_position_valid = true
/* This function calls weston_view_assign_output(), which then
* calls weston_surface_assign_output(). The effect of these two
* functions is that the view gets assigned to an output, and to
* choose the output it takes into consideration the position in
* which it is and the area that it occupies on the output. As
* the view has been moved to the origin of DP2, it gets
* assigned to this output. Then Weston sends the enter/leave
* surface events. */
weston_view_update_transform()
if (is_fullscreen)
/* This function positions the view on DP1, because
* that's the output in which the wine app wants to
* become fullscreen. */
shell_configure_fullscreen(shsurf)
/* Now we call weston_view_update_transform() again to each view
* of the surface, and so we end up sending enter/leave surface
* events. But notice that now we are positioned on DP1. */
wl_list_for_each(view, &surface->views, surface_link)
weston_view_update_transform(view);
The next time the surface gets committed and desktop_surface_committed()
gets called, the same sequence will happen. So we'll continue in this
weird loop.
The reason why the surface size changes and we don't return in POINT A
in this scenario is because the application uses a viewport, and then
when its surface moves to the output with scale 2 it sets the surface
size to half its size. That happens for apps that want to keep a
reasonable DPI on scaled displays.
This only happens after the change that introduced the call to
weston_view_update_transform() in this function. Without this call we'd
not reposition the view on DP2 and send enter/leave events at that
point.
---
So in order to avoid that, be more careful before calling
unset_fullscreen() and then shell_configure_fullscreen(). Only do that
when:
- the surface was not fullscreen, and now it becomes.
- the surface was fullscreen, but now it becomes fullscreen on a
different output.
In order to be consistent, do something similar to the maximized state.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
This doesn't really belong into shell-utils, so better move it out to
shared/config-parser. Renamed to weston_config_get_binding_modifier
to maintain the same namespace.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
These shell utils functions are potentially useful to other shells as
well, so make them widely available.
Renamed all functions to weston_shell_utils namespace.
No functional change, copied ad litteram.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Ever since d611ab24 the panel surface's frame callback gets lost and the
panel stops updating after its first draw.
Fix this by dirtying the geometry in configure_static_view() after it
changes the layer list, since the layer list is considered part of
geometry.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
After commit 2dc8680d71 this started crashing. We need to update the
view transform here too.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
These places all eventually lead to calling weston_view_to_global_float()
or weston_view_from_global_float() on a view with a dirty transform.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This introduces a few getters to retrieve the pending state from
libweston-desktop, now just libweston, and makes use of it,
specifically get_pending_maximized to avoid sending invalid
dimensions to the client in the particular use case
set_maximized/unset_fullscreen.
These pending state getters are useful to query/poke a not-applied
yet state, and could be useful where we don't have a buffer attached
where the client might be set-up as maximized, but internally libweston
hasn't yet applied that pending state.
Fixes#645
Suggested-by: Morgane Glidic <sardemff7+git@sardemff7.net>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Sometimes callers don't want them, and sometimes (when view is NULL) the
coordinate is invalid.
Waste a tiny bit of time calculating them as needed in the callers
instead.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
We're always passing pointer->x, y converted to surface coordinates, or
garbage if view is NULL. Let's just stop passing those coordinates
entirely and calculate them in the function.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This is arguably a little nicer without calling the pixman functions
directly.
In the future when we have different datatypes for coordinates in different
spaces, this test will only be valid on global coordinates, so this change
is also a precursor to stronger type validation.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
With commit 'Move libweston-desktop into libweston' we've moved out
libweston-desktop DSO into libweston. Move also the header to
libweston/desktop.
This removes removes the libweston-desktop pc file and bumps libweston
major version to 12.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This properly handles transition states to, and from, maximized,
fullscreen, surface movement and resizing.
Specifically for surface movement and resizing we unset any
(previously set) tiled information we might have. The same happens for
maximized and fullscreen but additionally we attempt re-install the
orientation if we had one previously.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Patch adds KEY_UP/KEY_DOWN for tiled top and bottom positioning,
KEY_LEFT/KEY_RIGHT correspondingly, for left and right positioning.
It also modifies the man page to include these new bindings, But also with
commit 'compositor: Remove desktop zoom' we no longer have zoom effects
so removed them.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
In case shsurfs are migrated/moved or started on different outputs other
than the default one, it causes fullscreen views to never being demoted
to a lower stacking level, due to the fact we never update
the view's output whenever that has changed.
Synchronize the desktop shell output's with the view's output in the
transform_handler.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Daniel Stone <daniel.stone@collabora.com>
We're going to need this to properly send xwayland events later, so add
API to get the current x,y co-ordinates of a shell surface and add it to
the kiosk and desktop shells.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
It's not really useful to have libweston without libweston-desktop. It's
also very little code.
Merging both into the same DSO will allow us to cut out a bunch of
indirection and pain.
Signed-off-by: Daniel Stone <daniels@collabora.com>
It's not the most code ever, but it does make desktop-shell somewhat
more complicated for questionable (i.e. no) end-user benefit.
When desktop-shell is back in more healthy shape it could potentially be
reintroduced, but for now it's just making it more difficult to reason
about desktop-shell and fix it.
Signed-off-by: Daniel Stone <daniels@collabora.com>
The only caller of map() then manually sets is_mapped = true. Just do it
in the function which makes you think that's what it would do.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This fixes the following leaks for
weston_curtain/weston_buffer_create_solid_rgba when shutting down the
compositor:
#0 0x7f9170372987 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7f915bfeb8b7 in zalloc ../include/libweston/zalloc.h:38
#2 0x7f915bfec71d in weston_curtain_create ../shell-utils/shell-utils.c:150
#3 0x7f915bfd9e51 in shell_ensure_fullscreen_black_view ../desktop-shell/shell.c:2082
#4 0x7f915bfda2a9 in shell_configure_fullscreen ../desktop-shell/shell.c:2118
#5 0x7f915bfdc72d in desktop_surface_committed ../desktop-shell/shell.c:2538
#6 0x7f915bfa3ef5 in weston_desktop_api_committed ../libweston-desktop/libweston-desktop.c:159
#7 0x7f915bfae778 in weston_desktop_xdg_toplevel_committed ../libweston-desktop/xdg-shell.c:746
#8 0x7f915bfb0d45 in weston_desktop_xdg_surface_committed ../libweston-desktop/xdg-shell.c:1374
#9 0x7f915bfa7382 in weston_desktop_surface_surface_committed ../libweston-desktop/surface.c:174
#10 0x7f916fe628a6 in wl_signal_emit /home/mvlad/install-amd64/include/wayland-server-core.h:481
#11 0x7f916fe7c0e2 in weston_surface_commit_state ../libweston/compositor.c:4062
#12 0x7f916fe7c161 in weston_surface_commit ../libweston/compositor.c:4068
#13 0x7f916fe7c6ef in surface_commit ../libweston/compositor.c:4146
#14 0x7f916fc847e9 (/usr/lib/x86_64-linux-gnu/libffi.so.8+0x77e9)
#0 0x7f9170372987 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7f916fe62aa3 in zalloc ../include/libweston/zalloc.h:38
#2 0x7f916fe7398d in weston_buffer_create_solid_rgba ../libweston/compositor.c:2603
#3 0x7f915bfec879 in weston_curtain_create ../shell-utils/shell-utils.c:162
#4 0x7f915bfd9e51 in shell_ensure_fullscreen_black_view ../desktop-shell/shell.c:2082
#5 0x7f915bfda2a9 in shell_configure_fullscreen ../desktop-shell/shell.c:2118
#6 0x7f915bfdc72d in desktop_surface_committed ../desktop-shell/shell.c:2538
#7 0x7f915bfa3ef5 in weston_desktop_api_committed ../libweston-desktop/libweston-desktop.c:159
#8 0x7f915bfae778 in weston_desktop_xdg_toplevel_committed ../libweston-desktop/xdg-shell.c:746
#9 0x7f915bfb0d45 in weston_desktop_xdg_surface_committed ../libweston-desktop/xdg-shell.c:1374
#10 0x7f915bfa7382 in weston_desktop_surface_surface_committed ../libweston-desktop/surface.c:174
#11 0x7f916fe628a6 in wl_signal_emit /home/mvlad/install-amd64/include/wayland-server-core.h:481
#12 0x7f916fe7c0e2 in weston_surface_commit_state ../libweston/compositor.c:4062
#13 0x7f916fe7c161 in weston_surface_commit ../libweston/compositor.c:4068
#14 0x7f916fe7c6ef in surface_commit ../libweston/compositor.c:4146
#15 0x7f916fc847e9 (/usr/lib/x86_64-linux-gnu/libffi.so.8+0x77e9)
We do not migrate the weston_curtain destruction from
desktop_surface_removed() to desktop_shell_destroy_surface() because
we'd want have the curtain removed before the animation starts.
If we were to move the black view destruction, *and* only handle it from
desktop_shell_destroy_surface() the animation runs but the black curtain
will be removed right at the end, effectively diminishing the effect of
the animations.
To this end, we keep both of the two worlds, if the client terminates on
its own, we keep the same animation effect, but if the compositor is
shutting down we destroy it immediately.
We remove wl_list_for_each_safe() and instead loop each time to avoid
using a stale pointer iterator which could cause a UAF as the shsurf
would be free'ed.
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This fixes the following weston_view leak at compositor shutdown:
#0 0x7f4250247987 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7f424fd37aa3 in zalloc ../include/libweston/zalloc.h:38
#2 0x7f424fd3a05f in weston_view_create ../libweston/compositor.c:386
#3 0x7f423be7be6a in weston_desktop_surface_create_desktop_view ../libweston-desktop/surface.c:364
#4 0x7f423be7c0a8 in weston_desktop_surface_create_view ../libweston-desktop/surface.c:404
#5 0x7f423beae91d in desktop_surface_added ../desktop-shell/shell.c:2273
#6 0x7f423be77db1 in weston_desktop_api_surface_added ../libweston-desktop/libweston-desktop.c:138
#7 0x7f423be80c73 in weston_desktop_xdg_toplevel_ensure_added ../libweston-desktop/xdg-shell.c:362
#8 0x7f423be8207a in weston_desktop_xdg_toplevel_committed ../libweston-desktop/xdg-shell.c:697
#9 0x7f423be84d45 in weston_desktop_xdg_surface_committed ../libweston-desktop/xdg-shell.c:1374
#10 0x7f423be7b382 in weston_desktop_surface_surface_committed ../libweston-desktop/surface.c:174
#11 0x7f424fd378a6 in wl_signal_emit /home/mvlad/install-amd64/include/wayland-server-core.h:481
#12 0x7f424fd510e2 in weston_surface_commit_state ../libweston/compositor.c:4062
#13 0x7f424fd51161 in weston_surface_commit ../libweston/compositor.c:4068
#14 0x7f424fd516ef in surface_commit ../libweston/compositor.c:4146
#15 0x7f424fb597e9 (/usr/lib/x86_64-linux-gnu/libffi.so.8+0x77e9)
With commit 'libweston, desktop-shell: Add a wrapper for weston_surface
reference' we've removed an explicit weston_view_destroy() call due to a
UAF which would've happen if we had close animations enabled, upon
terminating a client. In that patch I've incorrectly wrote this happened
when animations are off, but in fact it happened when they're on, see the
following trace:
READ of size 8 at 0x616000026498 thread T0
#0 0x7f757fba8797 in weston_signal_emit_mutable ../shared/signal.c:52
#1 0x7f757fb4bba1 in weston_view_destroy ../libweston/compositor.c:2269
#2 0x7f756bca89c0 in desktop_shell_destroy_surface ../desktop-shell/shell.c:275
#3 0x7f756bcb379e in fade_out_done_idle_cb ../desktop-shell/shell.c:2228
#4 0x7f757faec1da in wl_event_loop_dispatch_idle ../src/event-loop.c:969
#5 0x7f757faec31d in wl_event_loop_dispatch ../src/event-loop.c:1032
#6 0x7f757faea114 in wl_display_run ../src/wayland-server.c:1408
#7 0x7f757ff777ba in wet_main ../compositor/main.c:3589
#8 0x55f765c8d17d in main ../compositor/executable.c:33
#9 0x7f757fd997fc in __libc_start_main ../csu/libc-start.c:332
#10 0x55f765c8d099 in _start (/home/mvlad/install-amd64/bin/weston+0x1099)
0x616000026498 is located 24 bytes inside of 608-byte region [0x616000026480,0x6160000266e0)
freed by thread T0 here:
#0 0x7f758004c4d7 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127
#1 0x7f757fb4bdc8 in weston_view_destroy ../libweston/compositor.c:2295
#2 0x7f757fb4c14d in weston_surface_unref ../libweston/compositor.c:2334
#3 0x7f756bca898b in desktop_shell_destroy_surface ../desktop-shell/shell.c:273
#4 0x7f756bcb379e in fade_out_done_idle_cb ../desktop-shell/shell.c:2228
#5 0x7f757faec1da in wl_event_loop_dispatch_idle ../src/event-loop.c:969
This patch re-introduces it to avoid leaking the view upon compositor
shutdown, but it does it in tandem with weston_desktop_surface_unlink_view(),
(which was added in a later patch) and before weston_surface_unref() call.
This way we should be safe to terminate/close clients with additional views
created by libweston-desktop (pop-ups), but also in other different situations.
Verified it in the following circumstances:
- terminating a client with close animation on
- terminating a client with close animations off
- shutting down the compositor with clients running, with and
without close animations
- terminating top-level clients with additional pop-ups with both with
and without close animations
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Fix an apparent copy and paste error in resize code. I'm not sure anything
sets the relevant callback that would lead to height being different than
width, so there's no easy way to demonstrate a bug, but this change
appears to rectify the intent of the code.
Reported-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This documents the fact that other views are handled implictly by
libweston-desktop, and we shouldn't attempt to destroy indiscriminately
views that aren't created by desktop-shell.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This patch fixes the following trace:
#0 0x7f07d1bcecfa in weston_desktop_surface_get_surface ../libweston-desktop/surface.c:585
#1 0x7f07d1bfc9b8 in move_grab_motion ../desktop-shell/shell.c:1499
#2 0x7f07e539f841 in notify_motion ../libweston/input.c:1794
#3 0x7f07e1e8ace4 in handle_pointer_motion ../libweston/libinput-device.c:132
#4 0x7f07e1e8cad5 in evdev_device_process_event ../libweston/libinput-device.c:535
#5 0x7f07e1e89311 in udev_input_process_event ../libweston/libinput-seat.c:208
#6 0x7f07e1e8932f in process_event ../libweston/libinput-seat.c:218
#7 0x7f07e1e8935f in process_events ../libweston/libinput-seat.c:228
#8 0x7f07e1e8940a in udev_input_dispatch ../libweston/libinput-seat.c:239
#9 0x7f07e1e89437 in libinput_source_dispatch ../libweston/libinput-seat.c:249
#10 0x7f07e53122b1 in wl_event_loop_dispatch ../src/event-loop.c:1027
#11 0x7f07e5310114 in wl_display_run ../src/wayland-server.c:1408
#12 0x7f07e579c7ba in wet_main ../compositor/main.c:3589
#13 0x555611d6b17d in main ../compositor/executable.c:33
#14 0x7f07e55be7fc in __libc_start_main ../csu/libc-start.c:332
#15 0x555611d6b099 in _start (/home/mvlad/install-amd64/bin/weston+0x1099)
A highly unlikely, but still valid operation, is to close/destroy the
window while still having it grabbed and moved around, basically having
an in-flight destruction of grabbed moving window. Another situation
would be that the client terminates abruptly (crashing for instance),
while being dragged which might take down the compositor.
This could happen for both touch/pointer grab operations and could cause
a NULL pointer access while accessing desktop_surface when being used
to retrieve the underlying weston_surface.
With this patch we check for a valid desktop_surface and return early
if that's not the case.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Moving weston_desktop_surface_unlink_view() to
desktop_shell_destroy_surface() makes sure we don't leak the underlying
weston_desktop_view when tearing/shutting down the compositor.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Creates a distinct view, separated from the one created by
libweston-desktop, in order to avoid a potential ownership fight with
libweston-desktop upon destroying the view. Upon weston_desktop_surface
destruction, libweston-desktop inflicts damage on the view it creates,
so we need the view to be alive at that time.
This wasn't such an issue before because we had different destruction paths but
with commit 'desktop-shell: Do not leave views in layers upon shell
destruction' all of the destruction paths now land in the same spot
+ handle compositor tear down.
Note as we still use the same weston_surface we'll keep the previous
construct where we were taking a reference to keep it alive.
The original view is destroyed when releasing the ownership, while for
the view created in this patch we handle the destruction directly upon
compositor tear down.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Similar to how we do it with drm_fb ref counts, increase a reference
count and return the same object.
Plug-in in desktop-shell when we map up the view in order to survive a
weston_surface destruction.
Astute readers will notice that this patch removes weston_view_destroy()
while keeping the balance between removing and adding a
weston_surface_unref() call in desktop_shell_destroy_surface().
The reason is to let weston_surface_unref() handle destruction on its
own. If multiple references are taken, then weston_surface_unref()
doesn't destroy the view, it just decreases the reference, with
a latter call to weston_surface_unref() to determine if the view
should be destroyed as well. This situation happens if we have
close animation enabled, were we have more than one reference taken: one
when mapping the view/surface and when when the surface itself was created,
(what we call, a weak reference).
If only a single reference is taken (for instance if we don't have close
animations enabled) then this weston_surface_unref()
call is inert as that reference is not set-up, leaving libweston to
handle the view destruction.
Following that with a weston_view_destroy() explicit call would cause a
UAF as the view was previous destroyed by a weston_surface_unref() call.
A side-effect of not keeping the weston_view_destroy() call would
happen when tearing down the compositor. If close animations are enabled,
weston_surface_unref() would not destroy the view, and because
weston_view_destroy() no longer exists, we would still have the
view in the other layers by the time we check-up if layers
have views present.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Make it obvious that weston_surface has a reference counting happening
and destruction of the weston_surface happens when the last
weston_surface reference has been accounted for.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
