Currently, the surface destroy listener in pointer constraints is redundant,
since surface destruction already handles pointer constraints destruction (see
libweston/compositor.c:weston_surface_unref()).
Signed-off-by: Sergio Gómez <sergio.g.delreal@gmail.com>
(cherry picked from commit 64da736d37a7df8b3bd6fd43746ac513bae72748)
When a view is destroyed then the views of subsurfaces remain until the view
list is rebuilt for the next repaint.
During that time view->parent_view contains an invalid pointer and weston will
crash when it tries to access the view.
This happens for a surface with subsurfaces with views on two different outputs
with the ivi-shell:
When the surface is destroyed then the destroy handler of the ivi-shell
(shell_handle_surface_destroy()) may be called first. It will (indirectly)
destroy the view of the main surface with weston_view_destroy().
Next the surface destroy handler of the subsurfaces
(subsurface_handle_parent_destroy() is called. It will unmap the first view of
the subsurface. Here weston_surface_assign_output() is called which tries to
find the output of the second view and accesses the now invalid
view->parent_view in the process.
There are probably other ways to trigger similar crashes.
To avoid this, clear view->parent_view when the parent view is destroyed.
Fixes 0669d4de4f ("libweston: Skip views without a layer assignment in
output_mask calculations")
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
(cherry picked from commit 39796f88e6ed4a33a42c74b743e999294b3e4651)
Starting with commit 4cde507be6 "backend-drm: fix plane sorting" the
plane list will have a descending order of the planes rather than ascending.
This reversed order had the side-effect of exposing the fact that we
don't set-up a plane index when creating the drm_plane using the DRM
virtual API. Without settting a plane index for that drm_plane we
effectively overwrite the plane index which has the 0 (zero) entry.
This wasn't an issue before commit 4cde507be6 "backend-drm: fix
plane sorting" as it seems we never picked up that plane index as
being a suitable one due to the fact that those were assigned to primary
planes, but after that commit, the cursor plane will be one getting
the 0 (zero) plane index.
Finally, this would trip over because we attempt to place a (cursor)
view on a primary plane (where it would've normally be a cursor
plane) and we end up with no framebuffer ref.
This is fixed trivially by assigning a plane index, different than the
ones already created by create_spirtes().
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit 27ce9dadd8865b266f72f848b784d61aeaf8b228)
This adds a destroy listener on the SHM buffer provided by our client.
It will unregister the frame notify listener in case our buffer is
destroyed before the frame signal is emitted and thus avoid a memcpy
to invalid memory.
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
(cherry picked from commit 0afd3428dc899c426d37650192f828541f70e390)
As clipboard_find_supported_format_by_mime_type() can return -1 if it
can't find out an index avoid trying to print outside of the array.
Fixes the following warnings triggered when enabling FORTIFY_SOURCE
combined with optimizations (-O)
../libweston/backend-rdp/rdpclip.c:1114:17: error: array subscript -1 is below array bounds of ‘uint32_t[5]’ {aka ‘unsigned int[5]’} [-Werror=array-bounds]
1114 | weston_log("RDP %s (%p:%s) specified format \"%s\" index:%d formatId:%d is not supported by client\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1115 | __func__, source, clipboard_data_source_state_to_string(source),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1116 | mime_type, index, source->client_format_id_table[index]);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../libweston/backend-rdp/rdpclip.c:131:18: note: while referencing ‘client_format_id_table’
131 | uint32_t client_format_id_table[RDP_NUM_CLIPBOARD_FORMATS];
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit 9455ad7c7c07fdb8218330f449c97be73f695742)
As things are, even when mode=current is specified on the .ini file,
a full modeset is needed (and done), which causes a very noticeable
screen blinking. That is because setting the max_bpc on a connector
needs full modesetting.
The idea here is that if mode=current on the .ini, no modesetting
should be done, so the current max_bpc is programmed into the
connector.
But if a custom max-bpc=... is specified, that will be used instead,
even if mode=current on the .ini
Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/660
Signed-off-by: vanfanel <redwindwanderer@gmail.com>
(cherry picked from commit 3240ccc69d1488003c1cfc36d23750145d4f13f7)
Currently the frame event gets lost: The touch focus is removed in the 'up'
event. So the focus is gone when the frame event arrives so it is never sent to
the clients.
To avoid this, keep the touch focus until the frame is handled.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
(cherry picked from commit 5448580111b5ff992ce2603cb6e99b9f54db7ad8)
This has undergone a change to avoid an ABI break, so rather than
hooking up a pending_touch boolean in weston_touch, keep a local list of
weston_touch_devices and have a pending_touch with each device to check
upon.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Surface views that are not assigned to a layer are not going to be
rendered, and thus should not participate in determining the outputs the
surface is on.
There are other view properties that may determine if the view should be
considered in output_mask calculations, e.g., is_mapped, but checking
for this currently breaks tests. Such additional checks are left for
future fixes or reworkings of the view infrastructure.
Fixes#646
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Tracking correctly previous events shouldn't corrupt the surface destroy
signal list. This enforces that by ensuring that we wouldn't have
a .notify wl_listener still being set (which shouldn't happen if we do
eventually get a focus_in event that clears it out).
Suggested-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Rather than doing it with a local variable, track any previous events by
hanging it out of the x11 backend and use it to handle keymap notify
events.
In this way we avoid corrupting the surface destroy signal list, in
notify_keyboard_focus_out(), ultimately leading to a crash.
Fixes#649, #650
Suggested-by: Daniel Stone <daniel.stone@collabora.com>
Reported-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
The planes in the plane_list must be sorted from largest zpos_max to smallest.
Currently the plane order is only correct when the planes are already ordered
and added starting with the smallest zpos_max. This works accidentally in most
cases because the primary plane is usually first and there is often only one
overlay plane or the zpos is sufficiantly configurable.
To fix this, insert a new plane before the first plane with a smaller zpos_max.
And if none is found, insert it at the end of the list.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
With the help of a newly introduced function, weston_desktop_surface_set_orientation(),
this patch adds missing tiled states from the xdg-shell protocol.
The orientation state is passed on as a bitmask enumeration flag, which the
shell can set, allowing multiple tiling states at once.
These new states are incorporated the same way as the others, retaining
the set state, but also avoiding sending new configure events if nothing
changed since previously acked data.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Changing the mode will destoy the GBM surface for the output. As a result all
corresponding BOs are deleted regardless of the drm_fb refcount.
While a commit is pending, the last_state may contain a reference to such a BO.
So delay the mode switch until the commit is finished and the reference is
release.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
weston_output_set_position() currently assumes the output is enabled, but
we could be using weston_output_move() to configure an output that hasn't
yet been enabled.
If that's the case, we don't want to send signals or perform setup that
will eventually happen when the output is enabled anyway.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Make sure we don't enable an output that overlaps with other enabled
outputs.
We should probably do something similar when moving outputs, but we can't
realistically do that right now, so at least leave a comment explaining
why we're ignoring that case.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This is pretty counter-intuitive, and should probably happen outside of
the core in the front end while configuring the outputs.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
When an atomic commit fails then the output will be stuck in
REPAINT_AWAITING_COMPLETION state. It is waiting for a vblank event that was
never scheduled.
If the error is EBUSY then it can be expected to be a transient error. So
propagate the error and schedule a new repaint in the core compositor.
This is necessary because there are some circumstances when the commit can fail
unexpectedly:
- With 'state_invalid == true' one commit will disable all planes. If another
commit for a different output is triggered immediately afterwards, then this
commit can temporarily fail with EBUSY because it tries to use the same
planes.
- At least with i915, if one commit enables an output then a second commit for a
different output immediately afterwards can temporarily fail with EBUSY. This
is probably caused by some hardware interdependency.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
As wayland-backend is blitting the output decorations into the output
buffer itself, it pretends towards the pixman-renderer that there is no
decorations area. The pixman_image_create_bits() call wraps the
previously allocated buffer with an offset so that pixman-renderer will
paint in the right position.
The bug is that this pixman image was using the original buffer width
and height, instead of the composited area width and height. So the
pixman image looks too big to pixman-renderer, but the renderer didn't
care. The image being too big does risk access out of bounds in
pixman-renderer.
I found this when I was making renderers explicitly aware of the
frambuffer size and resizing, added asserts, and they surprisingly
failed. This fixes that.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The GL format and type are already recorded with pixel_format_info, use
that instead of a switch on Pixman formats.
Less special-casing, less dependency on Pixman formats.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Everywhere we are standardising to drm_fourcc.h pixel format codes, and
using struct pixel_format_info as a general handle that allows us to
access the equivalent format in various APIs. In the name of
standardisation, convert weston_compositor::read_format to
pixel_format_info.
Pixman formats are defined CPU-endian, while DRM formats are defined
always little-endian. OpenGL has various definitions. Correctly mapping
between these when the CPU is big-endian is an extra chore we can
hopefully offload to pixel-formats.c.
GL-renderer read_format is still defined based on Pixman format, because
of the pecualiar way OpenGL defines a pixel format with
GL_UNSIGNED_BYTE. That matches the same Pixman format on big-endian but
not the same drm_fourcc.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Sometimes you will have a pixman_image_t and you need the corresponding
drm_fourcc format.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
A following patch is going to need the introduced 'area' and 'fb_size'
variables. Until then though, a little hack is needed to avoid no-gl
builds failing with error: variable 'fb_size' set but not used.
While starting to use struct weston_geometry, convert also the input and
opaque regions to use it. This shortens and simplifies the code, as we
can drop the roughly duplicate code of doing stuff for with vs. without
a frame.
No change in behavior, this is pure refactoring.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Pixman image formats are CPU-endianess dependent while drm_fourcc are
not. Standardise around drm_fourcc because DRM-backend uses them anyway.
This also makes Pixman-renderer use the same format as GL-renderer will
prefer on headless.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This allows for setting a buffer offset without having to make it part
of the wl_surface.attach request. This is useful for e.g. setting a DND
surface icon hotspot offset when using Vulkan; or doing the same with
EGL without having to use wl_egl_window_resize().
Signed-off-by: Jonas Ådahl <jadahl@gmail.com>
In the future we'll have multiple output support, which makes storing
the peer list on an output rather tricky.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
The paint_node_z_order_list contains all views, not just the ones visible on the
current output. So all views are moved to the primary plane when one output
does not support planes.
This will be relevant with multiple backends: When an output without plane
support is rendered then the views of all other outputs are removed from
the current planes and the corresponding outputs will be repainted
unnecessarily.
So only reset the plane if the view is actually on the current output.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
It is only enabled by a debug key binding, currently not tested at all,
and is seems it doesn't really work, so let's remove it. This also
removes it from the man page.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
A head may have its output protection set before it is attached to an
output. Recompute the output protection whenever a head is attached to
make sure it correctly set in output.
Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
This skips over xdg-shell v4, which can be implemented with no changes
as it's just another optional event.
v5 adds a capabilities event, which we send to inform clients of the
window manager's capabilities.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This protocol allows clients to create single-pixel RGBA buffers. Now
that we have proper support for these buffers internally within Weston,
we can expose them to clients.
This bumps the build container version, as we now depend on
wayland-protocols v1.26.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This patch acts as bandaid in the core compositor to avoid the renderer
doing a flush after the buffer has been released. Flushing after release
can happen due to problems in the internal damage tracking, is violating
the protocol, and causes visible glitches.
A more proper fix would be to handle compositor side damage correctly.
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Acked-by: Daniel Stone <daniel.stone@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Since b38b735e20, 'backend-drm: Remove Pixman conditional
for keep_buffer' the Pixman renderer keeps its own reference to buffers
when attached to surfaces, rather than flipping keep_buffer variable for
the surface. Problem is that when switching from the Pixman render to
the GL would not work and could result in a crash upon first repaint.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
When an output is destroyed then the output state is freed immediately. In this
case, the plane state is only partially destroyed because it is the currently
active state. This includes the buffer reference.
Without the output, the plane will not be updated any more until it is used by a
different output (if possible) or the output returns and the plane is used
again.
As a result, the buffer reference is kept for a long time. This will cause some
applications to stall because weston now keeps two buffers (the one here and
another one for a different output where the application is now displayed).
To avoid this, do a synchronous commit that disables the output. The output
needs to be disabled anyways and this way the current state contains no
buffers that would remain.
`device->state_invalid = true` in drm_output_detach_crtc() is no longer
needed, because drm_output_detach_crtc() is called only when initialization
failed and the crtc was not yet used or in drm_output_deinit() when the
crtc was already disabled with the new synchronous commit.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
Introduced with e0a858a5f2, commit 'weston-debug: Introduce
weston_log_subscription and weston_log_subscriber objects'. We don't
really return a weston_log_subscription so let's remove it.
Some newer doxygen detects this and we are treating warning as errors.
Fixes#594
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This was introduced in a partial MR, where the later commits in the new
multi-GPU MR fully fix it, but the initially cherry-picked ones don't.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We were only destroying these when the parent display removed the output
global. Do it on shutdown too, so we can avoid leaking it.
Signed-off-by: Daniel Stone <daniels@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>
noop-renderer needs to actually access the buffer content, to ensure
that the bad-buffer test works. This was previously done using a
volatile variable, but clang rightly pointed out that the variable
access had no effect (since the volatile stack variable was never read
from, and the source is not volatile), so 9b0b5b57dd changed it to be
explicitly marked it as unused to suppress the compiler warning.
Unfortunately suppressing the warning still leaves the compiler free to
optimise out the access.
Replace the variable decorations with actually using the result of the
read, so we can be really sure that it's never going to be optimised
away.
Signed-off-by: Daniel Stone <daniels@collabora.com>
ca9bb01fe6 made it so that we already set shm_buffer, width, height,
etc, in the core. There's no need for the renderer to do this.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Our positioning of override redirect windows falls apart when an
app is on the fullscreen layer, because we end up putting its
menus and tooltips beneath it. This patch raises the special
override redirect layer to be just below things like on-screen
keyboards (and, unfortunately, above things like panels).
There is no perfect way to deal with this problem, especially
for content like tooltips that don't come with transience hints.
In some cases override redirect menus could be better placed by
using the parenting/transience information provided with them
at map time, and we should probably do that at some point, but
that would still leave us with tooltips below full screen
applications, and the need for this layer change.
based on a patch
Co-authored-by: Hideyuki Nagase <hideyukn@microsoft.com>
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
I changed the layer position and the comments, so:
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>
A view shouldn't be mapped if a surface isn't mapped, and it shouldn't
be in the scene graph if it isn't mapped either. Print when this happens
so you can see more from the debug.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Currently the idle_repaint_source is removed when the output is destroyed.
This covers the most common case: When a monitor is unplugged then the
corresponding DRM output is destroyed and not just disabled.
However, outputs can be explicitly disabled by the shell. In this case the
output is not removed and idle_repaint() may be called for a removed
output.
Remove the idle_repaint_source in weston_compositor_remove_output() to fix
this. And reset the variable to ensure that the source can be created
again.
Removing the source in weston_output_release() is now no longer necessary
since it calls weston_compositor_remove_output().
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>