We seem to be using at least mesa 21.1.1 since Weston 10, but we never
explicitly asked for it.
Fixes: #790
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit 0713ea7ee6216e45ebdb67cef63bcef7961d1d4e)
The override-redirect window will not be assigned a shell_surface
object. If it is used as a parent window, it will cause a crash
when calling the set_parent function.
The EWMH specification does not describe the behavior of an
override-redirect window as a parent window, so we should ignore
this case.
Signed-off-by: Liu, Kai1 <kai1.liu@intel.com>
(cherry picked from commit b468687dd2663240d1613bf4a917f049ef09af46)
Currently, if a head is detached, the entire state of the device is invalidated
to make sure that the connector is disabled on the next atomic commit. Side
effect of the invalid state is that all planes are disabled on the next commit.
This includes planes that are used with a different head that is not part of the
next atomic commit. Disabling the planes of unrelated outputs causes a blanking
of these outputs until output is repainted and the plane is reenabled.
Store the detached heads in a list on the output and disable the connectors for
all heads in this list in the next atomic commit.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
(cherry picked from commit bcacd9ec5a924317416eabb65a6cd6767d5bfb94)
We need only check that the region is not empty. If either the input region or
the constraint region have degenerate extents, the intersection from the
previous instruction will set confine_region->data to pixman_region_empty_data.
Fixes: b6423e59
Signed-off-by: Sergio Gómez <sergio.g.delreal@gmail.com>
(cherry picked from commit 1ed88f60c0125988cf1d952f0dabf568bfd82a13)
Since the logic of pointer constraints assumes a valid view throughout, add a
signal to disable constraints when its current view is unmapped by Weston.
The assumption that a previously unmapped view is valid already leads to the
constraints code crashing. This can happen when attaching a NULL buffer to the
surface and commiting, which effectively unmaps the view with the side effect of
clearing the surface's input region, which is then assumed valid inside
maybe_warp_confined_pointer().
Fixes: #721
Signed-off-by: Sergio Gómez <sergio.g.delreal@gmail.com>
(cherry picked from commit e3079393c400e3dc6498234d1d092f3072fa8b44)
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)
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>
(cherry picked from commit 06365e602bd7599760a9a1414d12d6c26ca9c445)
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)
A config event with width == 0 or height == 0 from the shell is a hint
to the client to choose its own dimensions. Since X11 clients don't
support such hints we make a best guess by trying to use the last saved
dimensions or, as a fallback, the current dimensions.
This hint is mainly used by libweston/desktop shells when transitioning
to a normal state from maximized, fullscreen or after a resize [1].
Without support for this hint the aforementioned transition causes
xwayland surfaces to be configured to a 1x1 size.
To be able to use the last saved dimensions with xwayland surface, the
shell needs to first set the maximized/fullscreen state and only then
set the new size, which is currently the case for desktop-shell.
Otherwise, if the new size is set first, then the last saved dimensions
will be set to the fullscreen/maximized values and won't be useful when
restoring to a normal window size.
[1] Recently we've introduced ba82af938a87ff088b4aacff3b8ac1b6bb461be2
"desktop-shell: do not forget to reset pending config size after
resizes". As we were not handling the 0x0 size hint, resizing X
applications started to fail. This patch fixes that.
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
Co-authored-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
(cherry picked from commit 2acd2c74891cd0548c1ff410ccfe81952bed27b3)
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>
(cherry picked from commit ba82af938a87ff088b4aacff3b8ac1b6bb461be2)
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)
Similarly to remoting plug-in in commit "Check virtual outputs/remoting
instance" this avoids touching the pipewire instance, and with it, the
pipewire output.
Includes a note to point up what should be done about by
checking out https://gitlab.freedesktop.org/wayland/weston/-/issues/591.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit 6617deebec5586c4d8c61097b7e51dd53c4e4624)
Seems like we are missing destroying the pipewire outputs on the shutdown
path; this follow-ups with remoting plug-in as well.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit 278fe4d7d47c7828d42047f4c910f1d815d32b80)
This happens when shutting the compositor, and follows-up with the
remoting plug-in.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit aa78da24659a97b82bcf1e28b985ea4f9fce4499)
Similarily to what the remoting plug-in does, explicitly call
weston_release_head() before removing the output list entry.
We do that to avoid lookup_pipewire_output() returning NULL and still
find out the pipewire_output.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit 5db6d19e6bb4c72085104fcae9abf2f632d5f45a)
With commit aab722bb, "backend-drm: prepare virtual output API for
heterogeneous outputs", we now call the virtual destroy function,
but when shutting the compositor we no longer have a remoting instance
available.
When searching out for a remoting output verify if the remoting instance is
still available before attempting to search for a remoting output.
Addresses the following crash at shutdown:
0x00007fd430a1d347 in lookup_remoted_output (output=0x557163d5dad0) at ../remoting/remoting-plugin.c:515
0x00007fd430a1d746 in remoting_output_destroy (output=0x557163d5dad0) at ../remoting/remoting-plugin.c:635
0x00007fd439e11ab9 in drm_virtual_output_destroy (base=0x557163d5dad0) at ../libweston/backend-drm/drm-virtual.c:265
0x00007fd43a8635d0 in weston_compositor_shutdown (ec=0x557163239530) at ../libweston/compositor.c:8271
0x00007fd439e029d4 in drm_destroy (backend=0x557163240ae0) at ../libweston/backend-drm/drm.c:2713
0x00007fd43a863e07 in weston_compositor_destroy (compositor=0x557163239530) at ../libweston/compositor.c:8626
Includes a note to point up what should be done about by
checking out https://gitlab.freedesktop.org/wayland/weston/-/issues/591.
Fixes aab722bb "backend-drm: prepare virtual output API for
heterogeneous outputs"
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit ca52c79c51088ca4c724b34e54c3bb97a9a51c67)
This re-orders the disable/destroy shutdown sequence such that
lookup_remoted_output(), in remoting_output_disable(), would find a
remoting output.
Otherwise, without this, lookup_remoted_output() wouldn't find a
remoting output available when shutting down the compositor, ultimately
leading to a crash.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit c3270e887bc07bb9c7d0e58e25d25e3a65145d2e)
If we don't xcb_flush() when we set the selection owner, we end up with
a ridiculous corner case where we can run use a command line X client
like 'xclip -i -selection clipboard' to crash weston.
Start weston, ensure Xwayland is running (set a selection with xclip), set
the clipboard from a wayland client, then set the clipboard with xclip
again.
Since xclip doesn't do anything xwm notices except set the clipboard, it
won't provoke a flush on our selection ownership change. xclip will take
ownership, then we call xcb_convert_selection(), and THEN we flush, sending
out our pending ownership change and the xcb_convert_selection() request.
The ownership change takes place first, we attempt to get our own selection
and weston explodes in a mess.
Stop this from happening with a flush when changing selection ownership.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
(cherry picked from commit 11bcad116f5cc1eb76c2de83d8c39af0cdb71a81)
It's possible to set the clipboard with no seat present - one way is to
use the RDP backend and then run 'xclip -i -selection clipboard' locally
without making an RDP connection.
Check if seat is NULL to prevent this from crashing.
Fixes#698
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
(cherry picked from commit bb993df236766178df95579217171bce5b166031)
To avoid the following UAF:
Invalid read of size 8
at 0x4AE5EFF: weston_desktop_get_display (libweston-desktop.c:110)
by 0x4AEB2C9: weston_desktop_xdg_surface_schedule_configure (xdg-shell.c:1160)
by 0x4AEA77A: weston_desktop_xdg_toplevel_set_size (xdg-shell.c:711)
by 0x4AE839D: weston_desktop_surface_set_size (surface.c:504)
by 0x63F7D43: ivi_layout_surface_set_size (ivi-layout.c:1599)
by 0x63F949F: transition_move_resize_view_destroy (ivi-layout-transition.c:311)
by 0x63F9397: layout_transition_destroy (ivi-layout-transition.c:259)
by 0x63F8E0B: ivi_layout_remove_all_surface_transitions (ivi-layout-transition.c:121)
by 0x63F4BC1: ivi_layout_surface_destroy (ivi-layout.c:258)
by 0x63F38AF: layout_surface_cleanup (ivi-shell.c:162)
by 0x63F3D2D: shell_destroy (ivi-shell.c:359)
by 0x4AF059A: weston_signal_emit_mutable (signal.c:62)
Address 0x174202d0 is 0 bytes inside a block of size 152 free'd
at 0x484617B: free (vg_replace_malloc.c:872)
by 0x4AE5EDC: weston_desktop_destroy (libweston-desktop.c:97)
by 0x63F3CF2: shell_destroy (ivi-shell.c:355)
by 0x4AF059A: weston_signal_emit_mutable (signal.c:62)
by 0x4ACBC2C: weston_compositor_destroy (compositor.c:8629)
by 0x4864A4B: wet_main (main.c:3908)
by 0x10915D: main (executable.c:33)
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit eb755cd81aad6f958629475a0429272aef3147b0)
Shutting down the compositor gives us:
Invalid write of size 8
at 0x4B1AEDB: wl_list_remove (wayland-util.c:56)
by 0x4AF05BF: weston_signal_emit_mutable (signal.c:66)
by 0x4ACBC2C: weston_compositor_destroy (compositor.c:8629)
by 0x4864A4B: wet_main (main.c:3908)
by 0x10915D: main (executable.c:33)
Address 0x17435f20 is 224 bytes inside a block of size 384 free'd
at 0x484617B: free (vg_replace_malloc.c:872)
by 0x17718C7E: hmi_controller_destroy (hmi-controller.c:761)
by 0x4AF059A: weston_signal_emit_mutable (signal.c:62)
by 0x4ACBC2C: weston_compositor_destroy (compositor.c:8629)
by 0x4864A4B: wet_main (main.c:3908)
by 0x10915D: main (executable.c:33)
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit cfbf2b0ab2ac7c50d8a1ea7645be3a9f3927825b)
The mouse button and touch bindings to activate a surface shouldn't
use the binding modifier.
Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/679
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
(cherry picked from commit 723709aa073fb740c3443fae8e370306d0738675)
The ivi-shell keeps track of its surfaces by adding them to the ivi_surface_list
to be able to remove them on shutdown. It also creates an ivi_layout_surface for
a desktop surface, but does not keep track of these surfaces.
During compositor shutdown, libweston prints the following message:
BUG: finalizing a layer with views still on it.
Fix it by adding the created ivi_layout_surface to the ivi_surface_list to
remove the surfaces from the layer during shutdown.
Furthermore, remove the ivi_layout_surface from the desktop surface and free it
when the desktop surface is destroyed.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
(cherry picked from commit 266e2e1d4866ef7c39cff0b77f1e404d0dc96b55)
Resolved small conflict which arose due to commit cbf476f208fbdcefe.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
If a controller requests the layers under a surface that has no views attached,
Weston crashes since it tries to free the array that would be used to return the
found layers, but has not been allocated before.
Free the ppArray only if it was allocated in ivi_layout_get_layers_under_surface
before and no layers were found.
While at it, make it obvious that checking the length is an integer comparison
by comparing it to 0.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
(cherry picked from commit c56e69bc850540f243ebb87c5bcc38713ef1862a)
The activation of a view implies, among other things, a change in the
associated view layer which is initially unset. In order for this change
to be reflected in the corresponding surface's output mask, and hence
allow surface damage to trigger output repaints, we need to update the
view transform.
Fixes#674
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
(cherry picked from commit 341d09d232d652c0001441cce55beb874fb3ba36)
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)
Currently we can't tell the difference between a window intentionally
created at 0,0 and a window that we can place anywhere.
Check the size hints to see if the flags indicating the placement
is intentional are present.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
(cherry picked from commit 1cb46994e3808e8000300ed9ae9dcaa0b76bff28)
Fixes the following warnings when building with _FORTIFY_SOURCE
and optimizations enabled:
../shared/xalloc.h:49:9: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
49 | write(STDERR_FILENO, oommsg, strlen(oommsg));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
or
../compositor/main.c:427:25: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
427 | write(STDERR_FILENO, fail_seteuid,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
428 | strlen(fail_seteuid));
| ~~~~~~~~~~~~~~~~~~~~~
../compositor/main.c:434:25: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
434 | write(STDERR_FILENO, fail_cloexec,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
435 | strlen(fail_cloexec));
| ~~~~~~~~~~~~~~~~~~~~~
../compositor/main.c:442:25: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
442 | write(STDERR_FILENO, fail_exec, strlen(fail_exec));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit 8c4cdd782e17aa587bfccb6746998571ddc90dd7)
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 seen previous times, with newer doxygen version we seem to be
generating warnings and with it to stop generating documentation
entirely as we have enabled warning as error in the doxygen
configuration file.
By default meson werror build option is not enabled, so users can still
generate documentation when building regularly, and when we'll update to
the same doxygen version we should be able to catch those errors up if
any of them will pile up in between.
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
(cherry picked from commit 402d9a81c9a42b0dcfc457c80382c36f5cecc6e6)
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>
All these Doxygen configuration directives raise a deprecation warning
with Doxygen 1.9.5.
Since we have WARN_AS_ERROR = YES, this causes the build to fail.
Remove these deprecated directives.
I have checked the differences by first building from scratch without
this patch, and then building from scratch with this patch, and
in the latter builddir checking
$ diff -ru -x '*.md5' -x '*.pdf' ~/tmp/weston-doc-before doc
The only differences are the Doxygen config file and one .pickle file.
So it seems the generated docs are identical with Doxygen 1.9.1.
Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/661
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We have no guarantee that we can create a surface for the pointer at the
instant we receive a seat that will (probably eventually) need one.
Hold off until we receive an enter event before creating this - at that
point we know with certainty that wl_compositor is available, since we've
used it to create the surface that was entered.
Fixes#659
Signed-off-by: Derek Foreman <derek.foreman@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 commit 62ab6891db, 'clients/simple-egl: Handle buffer
scale and transform' we changed the way we resized the client, by
encapsulating the resize in update_buffer_geometry() function.
we didn't correct that when creating the EGL window, which might be
problematic if you attempt to start the window with different a
different state, like maximized.
Fixes 62ab6891db
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
If weston-simple-egl is run with the "-b" flag, it will attempt to set
the swap interval to 0 during create_surface. However, at that point, it
will not have made its EGLContext current yet, causing the
eglSwapInterval call to have no effect. To fix this, wait until the
EGLContext has been made current in init_gl before updating the swap
interval.
Signed-off-by: Erik Kurzinger <ekurzinger@nvidia.com>
We want atomic hotspot updates - this can't happen with
wl_pointer_set_cursor. So if we have a surface that already has a cursor
role, just update the hotspot when attaching new content.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This reverts commit 992ee045f1.
Recreating the surface for every cursor change causes flickering
cursors on some compositors, and is not the best way to achieve
atomic cursor updates
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This reverts commit f079f43658.
This only partially fixed a problem introduced in
992ee045f1
I'm reverting that commit in favor of a different fix, so this
broken fix needs to go first.
Signed-off-by: Derek Foreman <derek.foreman@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>
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>
The README for the IVI-shell is completely outdated.
Update the documentation, add some more information on the IVI-shell use cases
and explain how to use and customize the IVI-shell. Also convert the file to rst
and move it to doc directory next to the kiosk-shell documentation.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The at.projects.genivi.org domain redirects to wiki.covesa.global and the
referenced wiki entry does not exist anymore. Remove it from the comment.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The ivi_layout_screen is internal to the IVI shell and not used by any
controllers. Controllers use weston_output directly.
Remove it from the exported header to avoid confusion.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
We are missing debug keybinds in kiosk-shell so install them. Adds
the binding-modifier like in desktop-shell in a helper.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
WM_TAKE_FOCUS requires a valid timestamp that isn't XCB_TIME_CURRENT. To
get one, we set a property on the window and wait for the notification
that it was set - this notification comes with a valid timestamp.
Once we have that timestamp, delete the property, and fire off the slightly
delayed WM_TAKE_FOCUS client request.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
We've been doing this when clicking on windows, even if they're
already activated. This leads to sending extra WM_TAKE_FOCUS events
as well as re-rendering the decor every mouse click.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This should be XCB_EVENT_MASK_NO_EVENT, but was not.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
According to https://tronche.com/gui/x/icccm/sec-4.html#s-4.1.7 we should
send this focus notification only if a client has WM_TAKE_FOCUS set in
their WM_PROTOCOLS property. We've been sending it unconditionally.
Rather, we've been not-sending it unconditionally because the event mask
is wrong, but that will be fixed in a future commit. Fixing the event
mask first would break some clients (such as xterm).
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
When the gl-renderer is not enabled, weston fails to start, as it
doesn't automatically fallback to the pixman renderer, which is
always enabled.
This commit changes the drm-backend to set by default the --use-pixman
option to true when the gl-renderer is disabled (BUILD_DRM_GBM is not
defined).
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
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>
This was using read_format for the read_pixels() call, and then using a
hardcoded format for interpreting the data received from read_pixels().
That works only by accident, read_format being the same as the hardcoded
format.
Use read_format for the interpreting too. This should guarantee the read
pixels are processed correctly.
Found by code inspection.
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>
It's entirely possible, if ridiculous, for an X11 client to change a
window's override redirect flag while it's mapped. If this changes from
true to false we will start receiving Configure requests for the window.
That leads us to a crash when we try to query the window's current
position from the shell to send a configure notify event, as the shell
doesn't know about the surface.
Instead of trying to cleverly handle this, mostly go back to the behaviour
these clients would've seen before commit cf5aca5a and don't send them
a synthetic configure notify.
We also specifically check in weston_wm_handle_configure_request for
the same condition, and early return there, bypassing a couple of
other things we would've done previously.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Fullscreen-shell forgot to mark the weston_surface as mapped when
mapping the surface and view. With
f962b48958 that means no surface from
fullscreen-shell clients is eveer shown. Most notably this broke
screen-share plugin, which is maybe the only "real" user of
fullscreen-shell.
Fix this oversight. Now screen-share works again with RDP-backend.
Fixes: f962b48958
"compositor: Only create paint nodes for mapped surfaces/views"
(currently unreleased)
Signed-off-by: Pekka Paalanen <pekka.paalanen@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>
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>
Commit b18f788e2e76 broke motif applications by ensuring they could never
focus their menus - since then any attempt by an application to focus any
window would be met by the window manager immediately refocusing the
currently active toplevel window.
Later we loosened the restriction in 9e07d25a1b to allow clients that
received focus from a grab to do so - but motif applications like nedit
don't set focus in this way, and remain broken.
This patch further loosens our restrictions, now only reverting a focus
change to an inactive top level. This will hopefully prevent any
confusing input routing without breaking reasonable clients.
This restores functionality to motif menus.
Fixes#636
Fixes b18f788e2e
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
The text_input_manager might be destroyed upon a compositor shutdown, so
verify if it's still set-up before attemping to use it to avoid a UAF.
Signed-off-by: Marius Vlad <marius.vlad@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>
This is so the systemd-notify module, if used, will notify readiness after
we're ready to accept X connections, instead of before.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This is awkward and long deprecated, and makes us load xwayland after all
the other modules so we know if we have to load it or not. Let's remove it.
We do still need to prevent loading the module the wrong way, though.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
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>
It seems we've missed an update from 3 to 4 (bounds events). With it,
this updates to version 5 which sends the capabilities event. Stubs, as
we're not using them.
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>
We want to support staging protocols which have a version too, so don't
assume that anything versioned is unstable.
Signed-off-by: Daniel Stone <daniels@collabora.com>
There are many reasons why trying to handle malloc() returning NULL by
any other way than calling abort() is not beneficial:
- Usually malloc() does not return NULL, thanks to memory overcommit.
Instead, the program gets SIGSEGV signal when it tries to access the
memory.
- Trying to handle NULL will create failure paths that are impractical
to test. There is no way to be sure the compositor still works once
such path is actually taken.
- Those failure path will clutter the code, increasing maintenance and
development burden.
- Sometimes there just isn't a good way to handle the failure.
For more discussion, see the issue link below.
Closes: https://gitlab.freedesktop.org/wayland/weston/-/issues/631
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The definition of zalloc is trivial, so let's just have it here instead
of loading libweston/zalloc.h.
Now xalloc.h does not depend on any libweston header, which makes me
feel slightly better. It's more clean.
Who knows, maybe one day libweston/zalloc.h will be removed.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This file relied on shared/xalloc.h to include <libweston/zalloc.h>.
That would be a problem if xalloc.h stopped doing that.
Just use xzalloc().
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Recently I learnt that fprintf() is not async-signal-safe. Maybe it also
attempts to allocate memory sometimes. Hence, using it when we
presumably are out of memory is wishful thinking.
Therefore replace that with async-signal-safe code. If you have to check
pointers from traditional signal handlers, now you could do that too!
While doing this, we also lose the string formatting for line number. I
would argue that printing file and line number is not that useful, if
the system really is out of memory. If not out of memory, a core dump
would give us much more detailed information about what went wrong.
clients/window.c had some calls to fail_on_null() and these are simply
replaced. They were used for checking that creating new wl_proxy by
issuing a protocol request worked, and IIRC that only fails on
out-of-memory, so the same rationale applies here.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Drop the even more home-grown alloc wrapper and use the xalloc.h
wrappers directly.
xcalloc() is added and used, because calloc() will detect integer
overflows in the size multiplication, while doing a simple
multiplication in the caller is subject to overflows which may result in
allocating not what was expected, subjecting to out-of-bounds access.
All MEM_ALLOC() calls that had a meaningful multiplication in them were
converted to xcalloc(), the rest to xzalloc().
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
If we leave xwayland in weston's process group, it can receive
signals from the controlling TTY intended for weston.
The easiest way to see this is to launch weston under gdb, start an
X client, and hit ctrl-c in the gdb session. The Xwayland server
will also catch the SIGINT, and the X client will be disconnected.
Instead, let's call setsid() when launching Xwayland, like we do
for launched clients.
Suggested-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
[common equivalent of 77cf8cb006 in Xwayland from Pekka Paalanen; its
commit message follows]
Between fork() and exec() in the child process it is only safe to use
async-signal-safe functions. weston_log() definitely is not one, it
allocates memory and does whatnot.
weston_log() is also inappropriate for other reasons: the child process
has its own stream buffers and flight-recorder. No-one looks into the
child process' flight recorder, so messages would be lost there. The
logging machinery might also attempt to write into debug streams,
meaning both parent and child could be writing simultaneously.
It seems that the best we can do is to pre-bake an error message and
only write() it out if exec() fails. There is no mention that even
strerror_r() might be safe to call, so we don't.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Use the custom_env framework we added for Xwayland when forking to
execute clients. This avoids calling the unsafe getenv in between fork
and exec.
Signed-off-by: Daniel Stone <daniels@collabora.com>
It was only a small function, and inlining it will allow us to make it
more safe without having to duplicate a ton of stuff.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This doesn't actually stop us from calling setenv() in between fork()
and exec() when starting clients, but gets us closer to Xwayland's safe
implementation by reusing one of the helpers it added.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Use the arg handling added in the previous commit so that the
environment is completely encapsulated inside the custom env.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Rather than open-coding our own implementation of parsing a string to
construct an envp and an argp, just use custom_env's implementation.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Users like desktop-shell want to parse a provided string containing a
combination of environment and arg, e.g.: ENV=stuff /path/to/thing --good
Add support to custom-env for parsing this, with tests, so we can delete
the custom implementation inside desktop-shell.
Signed-off-by: Daniel Stone <daniels@collabora.com>
execve() takes the same form for arguments as environment: an array of
constant pointers to mutable strings, terminated by a NULL.
To make it easier for users who want to build up their own argument
strings to pass to execve, add support for argument arrays to custom_env.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Test the basic stuff: initialising from a known environment, setting a
new variable, overwriting a previous variable, and getting the resulting
array to pass to execve.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Rename the bits handling environment variables (currently, all of it),
so we have room to handle args as well.
No functional changes.
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>
Between fork() and exec() in the child process it is only safe to use
async-signal-safe functions. Painfully, setenv() is not listed as such.
Therefore we must craft our own custom environment, and we get no help
from libc with that.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Constructing argv before-hand is a little easier to look at, but this is
mostly just anticipating more changes to how Weston spawns processes in
general.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We are already using pipe2() in many places, even in libweston, so let's
simplify the code here as well - not to mention avoid a theoretical
race.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Between fork() and exec() in the child process it is only safe to use
async-signal-safe functions. Surprisingly, snprintf() is not such
function. See e.g. https://stackoverflow.com/a/6771799 and how snprintf
is not listed in signal-safety(7) manual.
Therefore we must prepare the fd argument strings before fork(). That is
only possible if we also do not dup() fd in the child process. Hence we
remove the close-on-exec flag instead in the child process which has
copies of the parent's file descriptors. Fortunately fcntl() is safe.
struct fdstr is helping to reduce code clutter a bit.
Additionally, if fork() fails, we now clean up the fds we created.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This function will be used between fork() and exec() to remove the
close-on-exec flag. The first user will be compositor/xwayland.c.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
fcntl(2) manual says the return type is int, and that F_SETFD takes an
int. So use int.
Noticed by code inspection.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Between fork() and exec() in the child process it is only safe to use
async-signal-safe functions. weston_log() definitely is not one, it
allocates memory and does whatnot.
weston_log() is also inappropriate for other reasons: the child process
has its own stream buffers and flight-recorder. No-one looks into the
child process' flight recorder, so messages would be lost there. The
logging machinery might also attempt to write into debug streams,
meaning both parent and child could be writing simultaneously.
It seems that the best we can do is to pre-bake an error message and
only write() it out if exec() fails. There is no mention that even
strerror_r() might be safe to call, so we don't.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Doing any kind of memory allocation calls between fork() and exec() in
the child process is prone to deadlocks and explosions. In general, only
async-signal-safe functions are safe there.
Move the config access to the parent process before fork() to avoid
problems.
See also:
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/941#note_1457053
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This reverts commit 4aa885d4af.
Turns out the problem was not about dupping fds at all, but calling
non-async-signal-safe functions like strdup() between fork() and exec()
in the child process.
For more discussion, see:
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/941#note_1457053
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This patch fixes the following:
AddressSanitizer:DEADLYSIGNAL
=================================================================
==528956==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x7fbc5d66bdd7 bp 0x7ffd465573c0 sp 0x7ffd46557398 T0)
==528956==The signal is caused by a WRITE memory access.
==528956==Hint: address points to the zero page.
#0 0x7fbc5d66bdd7 in wl_list_remove ../../git/wayland/src/wayland-util.c:56
#1 0x7fbc5cb8869e in wxw_compositor_destroy ../../git/weston/compositor/xwayland.c:357
#2 0x7fbc5baf3ca6 in weston_signal_emit_mutable ../../git/weston/shared/signal.c:62
#3 0x7fbc5ba4d6f9 in weston_compositor_destroy ../../git/weston/libweston/compositor.c:8639
#4 0x7fbc5cb7a5f2 in wet_main ../../git/weston/compositor/main.c:3772
#5 0x55bd13de2179 in main ../../git/weston/compositor/executable.c:33
#6 0x7fbc5be61d09 in __libc_start_main ../csu/libc-start.c:308
#7 0x55bd13de2099 in _start (/home/pq/local/bin/weston+0x1099)
The problem is triggered by configuring a bad path to Xwayland in
weston.ini, which causes exec() to fail. The fork() succeeded though,
which means the weston_process was already on the watch list, and the
watch can be handled, making sigchl_handler() leave the link
uninitialized.
Making sure the link remains removable fixes this.
Fixes: 18897253d4
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
While developing the new color management, keeping these old plugins
working would require extra work. Let's deprecate these to see if anyone
cares about them, pending removal after the Weston 11.0.0 release.
CI will keep building these in the "Full build" configuration only. Doc
and no-GL builds are no different for these plugins, so there these are
no longer built.
See https://gitlab.freedesktop.org/wayland/weston/-/issues/634
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
There are two versions of WM_NORMAL_HINTS: the original pre-ICCCM
version (standardised by Xlib itself?) provides 15 elements of 32 bits
each, with the ICCCM v1 extending this by 3 additional elements.
Since the flags are enough to identify which elements are present, and
the structure is append-only, we only need to read the minimum length
between what the user provided and what we support.
Fixes a heap overrun found with ASan.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Setting up the arguments may consume some of the arguments, e.g. if we
provide a config file or extra modules, then the test harness is
expected to be responsible for freeing those arguments.
Checking the requirements and bailing first means that we never do that,
and thus skipped tests result in leaks. Flip the order so we set up the
args first and skip last, so we can consistently take ownership of all
the provided setup parameters.
Signed-off-by: Daniel Stone <daniels@collabora.com>
ZUC's default mode is to fork for every test case. Unfortunately on
AArch64, fork in an ASan-traced program usually takes around 3.6 entire
seconds. This was leading to the config-parser test timing out.
As none of our remaining ZUC tests even need to fork, just remove all
support for it.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This commit is truly horrible.
We want to run ASan with leak checking enabled in CI so we can catch
memory leaks before they're introduced. This works well with Pixman, and
with NIR-only drivers like iris or Panfrost.
But when we run under llvmpipe - which we do under CI - we start failing
because:
- Mesa pulls in llvmpipe via dlopen
- llvmpipe pulls in LLVM itself via DT_NEEDED
- initialising LLVM's global type/etc systems performs thread-local
allocations
- llvmpipe can't free those allocations since the application might
also be using LLVM
- Weston stops using GL and destroys all GL objects, leading to Mesa
unloading llvmpipe like it should
- with everything disappearing from the process's vmap, ASan can no
longer keep track of still-reachable pointers
- tests fail because LLVM is 'leaking'
Usually, an alternative is to LD_PRELOAD a shim which overrides
dlclose() to be a no-op. This is not usable here, because when
$LD_PRELOAD is not empty and ASan is not first in it, ASan immediately
errors out. Prepending ASan doesn't work, because we run our tests
through Meson (which also invokes Ninja), leading to LSan exploding
over CPython and Ninja, which is not what we're interested in.
It would be possible to inject _both_ ASan and a dlclose-does-nothing
shim DSO into the LD_PRELOAD environment for every test, but that seems
even worse, especially as Meson strongly discourages globbing for random
files in the root.
So, here we are, doing what we can: finding where swrast_dri.so (aka
llvmpipe) lives, stashing that in an environment variable, and
deliberately leaking a dlopen handle which we never close to ensure that
neither llvmpipe nor LLVM leave our process's address space before we
exit.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Unfortunately just adding suppressions isn't enough; the build of Expat
we have in our CI system does not have frame pointers, so ASan's fast
unwinder can't see through it. This means that the suppressions we've
added won't be taken into account.
For now, disable the fast unwinder for the Xwayland test only. Disabling
it globally is not practical as it massively increases the per-test
runtime, so here (to avoid polluting the build system), we use a
per-test wrapper to selectively choose the unwinder.
Signed-off-by: Daniel Stone <daniels@collabora.com>
For some reason, the Debian CI setup leaks fontconfig allocations in a
way which it does not for me on Fedora. On the assumption that this has
been fixed between our older CI Debian and Fedora 36, suppress the leak
warning, because we do already call FcFini() which should free it.
Signed-off-by: Daniel Stone <daniels@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>
This way the backends will the actual outputs. And at that point the backend
knows the compositor is shutting down so it can handle this differently if
necessary.
Afterwards wet_compositor_destroy_layout() just deletes the remaining
datastructures.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
Replace an oft-duplicated pattern with a trivial helper function. In
doing so, we observe that the one special case (displayfd 'didn't need
to be CLOEXEC') was wrong, because the X server does fork itself
internally, so there is nothing wrong with setting CLOEXEC.
Signed-off-by: Daniel Stone <daniels@collabora.com>
CLASS_DIAGRAM has been obsolete in newer version of doxygen, and
it's enabled if HAVE_DOT and CLASS_GRAPH are set.
This increase DOT_GRAPH_MAX_NODES to avoid dot complaning,
and include dot/graphviz for doxygen.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Same as LaTeX, RTF is being made obsolete in newer version of doxygen.
Also, we weren't really using it so there's no harm in removing it
entirely.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
LaTeX has become obsolete in newer doxygen version, and we weren't using
it at all so remove it entirely.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
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 is adding basically a copy of alpha-blending-test.c. The difference
is that here we use ICC files to set up the output color profile, and
then test light-linear blending only. BLOCK_WIDTH is set to 1 to fit
inside the output size used by the fixture setup, which is smaller than
in the original, but does not change the results.
The test is aimed at testing how color-lcms module succeeds in
linearizing the output of different ICC output profiles. Incorrect
linearization should cause changes in blending results.
The tolerance is taken from the currently achieved error statistics
(1.40908) and rounded up a little to achieve a suitable margin.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
It was binding to any advertised version, but it can't actually work
with version 4 (because it doesn't handle the new configure_bounds
event).
Other sample clients in the tree are hard-coding version 1, so do the
same here.
Fixes: 6d9fda7156 ("clients/presentation-shm: use xdg_shell instead of wl_shell")
Signed-off-by: Michel Dänzer <mdaenzer@redhat.com>
Switch from per-channel max error tolerance to max two-norm (Euclidean
distance) error. Geometrically this means that previously the accepted
volume was a +/- tolerance cube around the reference point, and now it
is a sphere with tolerance radius.
The real benefit is simplifying the code.
The error tolerance are also changed to float. Integers cannot represent
values between 1 and 2, and the jump from 1 to 2 would have been too
much. AdobeRGB tolerance gets relaxed a bit, while BT2020 tolerance
becomes stricter. The new tolerance values are the reported achieved
two-norm max errors plus a bit of margin.
Surprisingly the sRGB case tolerances remain strictly at zero, and
that's no bug in the test.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
compare_float() was an ad hoc max error logger with optional debug
logging.
Now that we have rgb_diff_stat, we can get the same statistics and more
with less code. It looks like we would lose the pixel index x, but that
can be recovered from the dump file line number.
This patch takes care to keep the test condition exactly the same as it
was before. The statistics print-out has more details now.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Switch from per-channel max error tolerance to max two-norm (Euclidean
distance) error. Geometrically this means that previously the accepted
volume was a +/- tolerance cube around the reference point, and now it
is a sphere with tolerance radius. This makes the check slightly
stricter.
The real benefit is simplifying the code.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
compare_float() was an ad hoc max error logger with optional debug
logging.
Now that we have rgb_diff_stat, we can get the same statistics and more
with less code. It looks like we would lose the pixel index x, but that
can be recovered from the dump file line number.
This patch takes care to keep the test condition exactly the same as it
was before. The statistics print-out has more details now.
The recorded dump position is the foreground color as that varies while
the background color is constant.
An example Octave function is included to show how to visualize the
rgb_diff_stat dump.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This is a special case of scalar_stat dumps to record all of two-norm
and RGB differences on the same line in the dump file.
This makes the dump file easier to handle when you want full RGB errors
recorded.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The recently introduced rgb_diff_stat value dumping feature logs the
"position" where the value or error was measured. The reference value
was used as the position, but the problem with the reference value is
that it is an output value and not an input value. Therefore mapping
that back to which input values promoted the error is not easy.
Fix that problem by passing the position explicitly into
rgb_diff_stat_update(), just like it is already passed in to
scalar_stat_update().
Currently the only user simply passes the reference value as position,
because there the input value is also the reference value. This is not
true for future uses of rgb_diff_stat.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The new field in struct scalar_stat allows recording all tested values
into a file. This is intended to replace ad hoc dumping code like in
alpha-blending-test.c.
To make it easy to set up, also offer a helper to open a writable file
whose name consists of a custom prefix and test name.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Seems it will be common to print all four min/max/avg sets of errors, so
move the printing code into a shared place.
While 0.0-1.0 is the natural range for color values, people are often
accustomed to working with 8-bit or even 10-bit pixel values. An error
of +/- 1 in 8-bit is more intuitive than +/- 0.004 in floating-point.
Hence 'scaling_bits' is added so the caller can determine the value
scaling. This will scale both the reported error numbers, and the
recorded error positions (rgb-tuples), so they are all comparable.
I'm happy to get rid of those two macros as well.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
More tests are going to need this.
The API is changed to work by copy in and copy out to match the other
color_util API. Hopefully this makes the caller code easier to read.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We treat the argv we pass into the compositor as its to mangle, just as
it is free to do so for POSIX argv. To support this, we stash argv away
and free the saved copy later so as to not leak.
This works perfectly, except when we never call the compositor at all,
and have no saved array to free. Make sure we free the args in this
case, which can be seen as a leak of any generated args when a test
skips on preflight checks, e.g. drm-smoke when not running in CI.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Pango, Cairo, and fontconfig, all want to leave thread-global data
hanging around in order to maintain a cache. Try to clean up as much of
it as we possibly can on exit, apart from the Pango language string
which appears to be unfreeable, so has been added to LSan suppressions.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Rework PangoCairo context initialisation, so we don't leak either the
Pango layout, or any of the derived objects it creates.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This code was all dead, since neither cairo-glesv2 nor the sample nested
compositor ever made it to the Meson build.
Signed-off-by: Daniel Stone <daniels@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>
Many programs use this information to help position pop-ups properly, and
without it funny things happen. For example, nedit and tkinter apps will
position their menus incorrectly either all the time or after an initial
window move, firefox may position right-click pop-ups incorrectly
depending on other internal state.
https://tronche.com/gui/x/icccm/sec-4.html#s-4.1.5 has much detail on
how this should work, and the Advice to Implementors section shows that
common client practices will break in the face of our miserly handling
of ConfigureNotify events.
Instead of trying to send it only for configure requests received when a
client is in a fullscreen state, send them much more frequently.
Fixes#619
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Currently weston_wm_window_send_configurenotify is only called for
fullscreen clients, and it is written to be correct only in that case.
Fix it up to handle other cases properly so we can use it for them in a
later commit. Synthetic Configure Notify events are relative to the
root window, so this means adding our window co-ordinate when
necessary.
Signed-off-by: Derek Foreman <derek.foreman@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 doesn't need to be using desktop-shell; trying to use it is not
sensible as it will try to bind to all the devices we're repeatedly
creating and destroying, sometimes lose the race, and fail the test
because desktop-shell has died too early.
Signed-off-by: Daniel Stone <daniels@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>
Make fail_on_null static inline and put it in xalloc.h so we can use the
header exclusively instead of having to link with the library for it.
This is so we can use xalloc in places (like the RDP backend) without
having to bring in libshared.
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>
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>
weston_compositor_reflow_outputs() assumes that all output are positioned from
left to right with no gaps in the same order in which they where created.
If the shell moves an output with weston_output_move() then this assumption is
no longer true. So stop reflowing the outputs in the case. The shell is now
responsible for positioning all outputs as needed.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
Particularly important was _XWAYLAND_ALLOW_COMMITS atom which caused
some annoying flicker when resizing or hoovering over buttons.
This was introduced with 'shared/xcb-xwayland: Split into common
helpers' and somehow I missed those atoms.
Fixes 49d6532254
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This is another followup to ffc011d6a3
("backend-drm: check that outputs and heads are in fact ours") which missed
some places.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
https://specifications.freedesktop.org/wm-spec/1.4/ar01s05.html says
"The Window Manager MUST set _NET_FRAME_EXTENTS to the extents of the
window's frame", so this is probably something we should be doing.
Some programs (such as some versions of Firefox) expect this to be present,
and will render popups in wrong locations if it's not.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
We need these values to calculate frame extents to properly set
_NET_FRAME_EXTENTS, but we don't want to calculate them twice.
Break out these bits from frame_resize_inside, and update it to use
the new function.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Instead of closing the window directly by calling close_handler() use a
deferred task to do that instead.
That way we avoid a potential invalid access on a link which was
previously removed, due to the fact both window_destroy() and
touch_handle_up() traverse the same list.
This is an alternative to 841.
Fixes: #607.
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Reported-by: He Yong <hyyoxhk@163.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
By moving the application of view_alpha after pre-multiplication we can
simplify main() considerably.
The cost is that for straight-alpha input or color_pipeline() we might
be doing three multiplications more than before. However,
a) the cost of running color_pipeline() probably dominates anyway, and
b) to get straight-alpha input you have to use a future Wayland
extension that probably won't be advertised without color management.
So we keep the optimization for the simple case (no color management)
while potentially incurring a small cost on the heavy case (with color
management).
Thanks to Pierre-Yves Mordred for the inspiration in
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/889#note_1411774
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Now that we have the if-else ladder to call color_pipeline() only when
necessary, and since only color_pipeline() needs undo-premult, move
undo-premult into color_pipeline().
This is a small step towards improving code readability.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We always talk about "view alpha", so the name variable in the fragment
shader the same. Now it's clear without the comments, making the code
easier to read overall.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Avoid duplication of atom retrieval. This is particuarly useful
if one would one to reuse atom retrival in other parts, like tests.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Daniel Stone <daniel.stone@collabora.com>
According to the wm-spec we must keep the _NET_WM_STATE property updated
to reflect the current state of the window.
This has been biting me when firefox starts maximized, then I click the
maximize button to toggle to unmaximized state. The next time I mouse over
the maximize button (which causes the frame to be re-rendered with the
maximized button in a highlighted state) we re-read the window state and
weston then believes the window is maximized even though it is being
rendered in a not-maximized state.
Update the state when we change maximized status so this doesn't happen.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
When we leave fullscreen or maximized mode we restore a saved window
position. This is expected, but that saved position was saved when window
geometry was set to have shadows rendered.
Since we restore a saved position that had shadow geometry, we don't want
to move the window when we set geometry again, or we'll move away from the
intended saved position.
I guess this is a counter-proposal to !614Fixes#454
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
If a client starts off maximized, clicking the unmaximize button would
result in a 0x0 window - basically a blob of decor with no content.
Instead, use 512x512 as a totally random default value.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
There is missing dependency on linux-dmabuf-unstable-v1-server-protocol.h
header file in backend-headless, backend-drm and backend-x11. That files
do not depend on that header, in fact. But by this moment they've had
that implicit dependency due to linux-dmabuf.h header.
With specific set of meson configure options the protocol header is not
generated at the right time, what causes build error in 100% cases using
small amount of building threads (from -j1 to -j8).
Signed-off-by: Ivan Nikolaenko <ivan.nikolaenko@unikie.com>
This is a followup to ffc011d6a3
("backend-drm: check that outputs and heads are in fact ours") which missed
some places.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
This uses the legacy DRM API it incomplete and no longer works anyways.
At this point, weston is no longer DRM master, so these calls fail with
"Permission denied".
So just remove the corresponding code.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
If a surface or a view is not mapped, then we should not be trying to
paint it. Check if this is the case and ensure that we only insert
paint nodes for mapped surfaces & views.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Fixes: #621
Keep the surface map state in sync with the buffer state: the surface
can be mapped it has a valid buffer, and not if it doesn't.
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>
Now we've got a wrapper which tells us whether or not the surface has
valid content, use it.
The 'XXX' comment was removed because it's the same pattern as every
other surface-role implementor: if the surface is not mapped but does
have valid content, then map it.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Simplify the code by using ready-made helpers.
This also change the loop to draw the image row by row rather than
column by column. Row by row is more natural as it is linear with the
memory layout. No other change in behaviour.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Make use of the shared code instead of open-coding everywhere. This
should make the code easier to read, and reduce the chance of typos if
changes are needed in the future.
No change in behaviour.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
These are the last places in weston-test-client-helper.c where using
image_header_from() will reduce the code line count and simplify the
code a little.
No change in behaviour.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Move the struct image_header and get_image_prop() into a header where we
can share these useful helpers between more test code. While doing that,
drop the unused field 'depth', rename into image_header_from(), and
introduce a helper to get u32 pointer to the beginning of a row. These
helpers should make pixel iterating code easier to read and safer (less
room for mistakes in address computations, and asserts).
Use the shared 'struct image_header' instead of the local one. The
intention is to make the code easier to read by using the same helpers
everywhere.
Width, height and stride use type 'int' because Pixman API uses that
too.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
For working around hardware limitations as explained in the man page.
Now added for completeness' sake without knowing if anyone will ever
need this.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
"max bpc" property is meant for working around faulty sink hardware.
Normally it should be set to the maximum possible value so that the
kernel driver has full freedom to choose the link bpc without being
artificially forced to lower color precision.
The default value is 16 because that is a nice round number and more
than any link technology I've heard is using today which would be 12.
Also offer an API set the value, so that weston.ini could be used in the
next patch for sink workaround purposes.
Closes: https://gitlab.freedesktop.org/wayland/weston/-/issues/612
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
I will be needing it in a new test, so let's share it.
For convenience, this also adds client back-pointer in struct surface so
that I don't need to pass client explicitly.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
skip() is a left-over from an old test harness design, the comment even
refers to automake. Calling skip() cannot do anything good anymore,
because it wouldn't print the skips in the TAP report, so it would
probably be considered a failure.
Delete this unused and nowadays incorrect function, so it doesn't
confuse people.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Commit 62ab6891db intended to change the angle calculation
so that the a time delta since the first frame would be used
instead of the absolute time. That was done in order to ensure
the angle would always start with the same value, allowing users
to differentiate left and right, which again is needed when
testing flipped transforms.
However, the `benchmark_time` variable is unsuitable for that
purpose as it gets reset on each benchmark interval, abruptly
changing the angle.
Thus introduce a dedicated variable.
Fixes 62ab6891db
Signed-off-by: Robert Mader <robert.mader@collabora.com>
Add RDP to the list of backends we can set as default for use
when weston is launched without display/socket environment vars
set.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
Move gen_ramp_rgb() down in the file where the TEST() specific code
begins. This way we first have a big block of fixture setup code which
creates an ICC profile, and the next big block is the actual test client
code. gen_ramp_rgb() belongs with the latter.
This makes the file structure slightly more logical.
This is a pure code move, no changes at all.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This name describes better what this test does. In the future another
TEST() for alpha blending will be added. Both of them will be using
matrix-shaper and cLUT output profiles.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The new name better matches the contents of the test.
Currently the test creates output ICC profiles with matrix-shaper and
cLUT forms, and tests that basic color conversion from input to output
color space is correct.
The common theme in this test program is to create ICC profiles to be
used as output profiles. In the future this can include more kinds of
testing, e.g. linear blending. OTOH, this test program will always be
limited to SDR because HDR testing probably will not use ICC files.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
To support heterogeneous outputs, the output must be created by the
same backend as the head(s) it is created for. Solve this by always
creating an output with a first head to attach that determines the
backend to use. Skip already attached first heads in drm_try_attach().
See: https://gitlab.freedesktop.org/wayland/weston/-/issues/268
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Add a struct weston_head parameter to weston_compositor_create_output()
and fold weston_compositor_create_output_with_head() into it.
See: https://gitlab.freedesktop.org/wayland/weston/-/issues/268
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
As a first step towards heterogeneous outputs, ignore other backends'
heads and outputs. This is done by checking the destroy callbacks for
heads and outputs.
See: https://gitlab.freedesktop.org/wayland/weston/-/issues/268
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
As a first step towards heterogeneous outputs, ignore other backends'
heads and outputs. This is done by checking the destroy callbacks for
heads and outputs.
See: https://gitlab.freedesktop.org/wayland/weston/-/issues/268
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
As a first step towards heterogeneous outputs, ignore other backends'
heads and outputs. This is done by checking the destroy callbacks for
heads and outputs.
See: https://gitlab.freedesktop.org/wayland/weston/-/issues/268
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
As a first step towards heterogeneous outputs, ignore other backends'
heads and outputs. This is done by checking the destroy callbacks for
heads and outputs.
See: https://gitlab.freedesktop.org/wayland/weston/-/issues/268
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
As a first step towards heterogeneous outputs, ignore other backends'
heads and outputs. This is done by checking the destroy callbacks for
heads and outputs.
See: https://gitlab.freedesktop.org/wayland/weston/-/issues/268
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Stop plugins from overwriting the struct weston_output::destroy vfunc,
as that will be used by backends to recognize their outputs.
Instead, pass a plugin-specific destroy callback when creating the
virtual output.
See: https://gitlab.freedesktop.org/wayland/weston/-/issues/268
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
As a first step towards heterogeneous outputs, add an opaque pointer
weston_head::backend_id that will be used by backends to identify
their own heads.
See: https://gitlab.freedesktop.org/wayland/weston/-/issues/268
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
The get file descriptor functions are being deprecated and a two step
process of getting handles and then getting the descriptors for the
handles is being used instead.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Update to a newer FreeRDP release so we can start cleaning up
some of our usage of things that will be deprecated in the next
major release.
For this, I've simply picked the newest version currently in
our CI images.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This doesn't actually fix a bug - cairo refcounts this. But I
really don't like the look of code that drops a reference then
continues to use it.
While we're here, set a different pattern when we're done so the
one we allocated loses its last reference.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This reverts commit 6914064066.
This is a follow-up change of b623fd2a ("drm-backend: stop parsing IN_FORMATS
blobs, use libdrm instead"). Weston now has a hard-requirement on libdrm
2.4.108, clean up remaining and unnecessary conditional code. Change 69140640
("backend-drm: add HDR_OUTPUT_METADATA definitions") is no longer needed
and stop including libdrm-updates.h from kms-color.c.
Signed-off-by: Luigi Santivetti <luigi.santivetti@imgtec.com>
Before this change the drm-backend in Weston did the work of parsing DRM
blobs in order to query IN_FORMATS data, if available. This is also the
case for other DRM/KMS clients that use IN_FORMATS (i.e. X).
libdrm 2.4.108 with e641e2a6 ("xf86drm: add iterator API for DRM/KMS
IN_FORMATS blobs") introduced a dedicated API for querying IN_FORMATS data.
Bump the minimum required version to 2.4.108, stop parsing IN_FORMATS in
Weston and start using DRM iterators. In addition, remove fallback code for
libdrm <2.4.107.
Signed-off-by: Luigi Santivetti <luigi.santivetti@imgtec.com>
libdrm with version 2.4.108 provides new functionality for parsing
IN_FORMATS blobs. Weston can make use of it and avoid implementing
its own logic. At present CI uses Debian 11 (bullseye) which comes
with an older version (2.4.104), so we build it from source.
Signed-off-by: Luigi Santivetti <luigi.santivetti@imgtec.com>
This patch makes sure we have a gl_buffer_state present when using
direct-display protocol extensions (which forbids any GL imports, and
assumes a direct path with the display unit to perform a KMS import).
Without this patch we would basically have no gl_buffer_state at repaint
time because we never manged to create one, as direct-display code path
will return much early.
Partially fixes gitlab.freedesktop.org/wayland/weston/-/issues/621.
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Added cLUT profile creation to validate linearization algorithm
for DToB3 tag (direction dev to PCS). The 3DLUT is
built by using raw matrix conversion from dev to XYZ and reverse
(XYZ to device).
The test uses floating point pipeline, known as unbounded mode of LCMS.
The details are described in ICCSpecRevision_02_11_06_Float.pdf
The purpose of these new test cases is to keep the GL-renderer 3D LUT
path tested even after color-lcms and GL-renderer start using
specialized matrix-shaper paths.
These also exercise build_eotf_from_clut_profile() in color-lcms, but do
not actually verify it. These cases only test that the recovered EOTF
and its inverse produce an identity mapping together.
BT.2020 is not used in these tests, because the RGB-XYZ conversion
matrix does not stay inside [0.0, 1.0] in either direction, which would
be a problem for the 3D LUT element in the multiProcessingElement
pipelines. Handling that would have been possible, but testing with
AdobeRGB color space should suffice while keeping the test code from
being even more complicated.
roundtrip_verification() tests that we succeed in creating cms
pipelines correctly in both directions so that the resulting ICC file is
better behaved. The Weston test itself only cares about the BToD
direction.
Credits to:
Vladimir Lachine <vladimir.lachine@amd.com>
Graeme Gill <graeme@argyllcms.com>
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We will want to run the same color spaces with different types of ICC
profiles. To help with that:
1. Let struct lcms_pipeline define the test color space and
transformations and move the tolerance into a new per test case
structure.
2. Added profile type: PTYPE_MATRIX_SHAPER, PTYPE_CLUT.
PTYPE_MATRIX_SHAPER is the previously implemented type.
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This function sets some basic text tags to make an ICC file better
formed.
The code is taken from LittleCMS, https://github.com/mm2/Little-CMS.git
git revision
lcms2.13.1-28-g6ae2e99 (6ae2e99a3535417ca5c95b602eb61fdd29d294d0)
file src/cmsvirt.c.
Suggested-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This adds a new test helper library that depends on LittleCMS 2.
For starters, the library implements conversion from enum transfer_fn to
ICC multiProcessingElements compatible LittleCMS curve object.
That conversion allows encoding transfer funtions in ICC files and
LittleCMS pipelines with full float32 precision instead of forcing a
conversion to a 1D LUT which for power-type curves is surprisingly
imprecise.
This also adds CI tests to make sure the conversion matches our
hand-coded transfer functions.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
These helpers allow collecting color difference statistics easily.
To be used in color-shaper-matrix-test.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
When defining a color space with a transfer function, this looks up the
inverse transfer function without needing to store that separately.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This will be useful to make a curve in a color pipeline pass-through
without needing to special-case skipping the curve.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Fix up whitespace and document what this array is for.
For the sake of slightly better readability.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Re-use color_float_apply_curve() instead of open-coding it.
Maybe makes reading the code a little easier.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Make process_pixel_using_pipeline() slightly easier to read by
extracting a meaningful function.
Pure refactoring, no behavioral changes.
Compared to previous, flip the scalar multiplication around, so that it
matches the mathematical order of matrix-vector multiplication.
Also document the layout conventions for lcmsVEC3 and lcmsMAT3. These
follow the convention used in LittleCMS for cmsVEC3 and cmsMAT3, and are
necessary to understand to review the matrix-vector multiplication for
correctness.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Make process_pixel_using_pipeline() slightly easier to read by
extracting a meaningful function.
Pure refactoring, no changes.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Looks like this was forgotten, and I managed to get compiler errors
about redeclaring all enums.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
It's bad form to set the same variable in multiple places, and not all
of them were even equivalent.
Move lcms2 finding to the root level build file only. It is still an
optional dependency like before, and the if-not-found checks are still
in place where actually needed.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
When color management is disabled, the fragment shader was still first
ensuring straight alpha and then immediately just going back to
pre-multiplied. This is near-impossible for a shader compiler to
optimize out, I guess because of the if-statement to handle division by
zero. Having view alpha applied in between certainly didn't make it
easier.
That causes extra fragment computations that are unnecessary. In the
issue report this was found to cause a notable performance regression.
Fix the performance regression by introducing special-case paths for
when straight alpha is not needed. This skips the unnecessary
computations.
Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/623
Fixes: 9a6a4e7032
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Currently, the opaque is discarded for all transformations other than a simple
translation, because correctly transforming the opaque area is not possible in
general.
However, there is one simple case that is probably the most common one: A fully
opaque surface that is translated and scaled. In this case the opaque area is
simply the new bounding box. So set the transformed opaque area accordingly.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
If surface->is_opaque is set then we can assume that the whole surface is
opaque. In the trivial case (no transformation or translation only) this means
that transform.boundingbox is exactly the view area and is fully opaque. So it
can be used for transform.opaque.
This is important because damage calculation uses transform.opaque. Without
this, anything underneath a surface without an explicit opaque region but a
pixel format without alpha channel is drawn unnecessarily.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
The drm_device is initialized as a side effect of the (badly named)
drm_device_is_kms function. Explicitly pass the drm_device to be able to
initialize kms devices that are not the main drm device of the drm backend.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
If we have multiple drm devices, we cannot use the drm device from the backend,
because we would only get the primary device and not the device of the output.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
If Weston receives a hotplug event, it has to check if the hotplug device
actually belongs to the drm device before updating the heads of the device. The
hotplug event should only remove heads that belong to the device and must not
change heads of other devices.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The compositor lists the heads from all devices, but we must only disable the
connectors that belong to the current device. Therefore, other heads must be
ignored.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The outputs, heads, crtcs, and connectors are specific to a drm device and not
the backend in general.
Link them to the device that they belong to to be able to retrieve the
respective device.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The commits happen per device instead of per backend. The pending state is
therefore per device as well. Allow to retrieve the device from the pending
state.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The scanout format for the dma-buf feedback are specific to the kms device that
is used for scanout. Therefore, we have to pass the device of the output when
retrieving the scanout formats.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The fbs are specific to the device on which they will be displayed. Therefore,
we have to tell which device shall be used when we are creating the fb.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The atomic commit is device specific. If we have multiple kms devices, we need
to know which device was used for the atomic commit.
Pass the device instead of the backend through the atomic commit.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Extract the kms device from the backend to allow a better separation of the
backend and the kms device. This will allow to handle multiple kms devices with
a single drm backend.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Get the backend at the beginning of the function instead of retrieving it from
another object in the debug statement. This simplifies refactoring, as the debug
statement is not affected by changes how the backend is retrieved.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The gbm_format is the same as the drm format used by the pixel format.
Print the format name using the pixel format in the error message to make the
error message easier to understand for humans.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The fb already contains a DRM fd for later use. So just use that one instead of
fetching it from the backend.
This is necessary if the fbs are allocated on different devices, since otherwise
the wrong device might be used to get the fd of the passed fb.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Looks like we missed this one during the conversion to
weston_signal_emit_mutable.
Found by running weston under valgrind and running/killing
weston-simple-dmabuf-egl
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This reverts commit 1618697dc3.
The original commit was a workaround for
https://gitlab.freedesktop.org/mesa/mesa/-/issues/2219 which was fixed
in Mesa:
- c7617d8908a970124321ce731b43d5996c3c5775 released as 20.1.0-rc1
- a0e6341fe4417e41cda0b19e4fa7f8bbe4e1dba1 released as 19.3.5
- f27e5d9df5bc9c85d45c2cb1f2a4997b453365fe released as 20.0.0
This workaround should not be necessary anymore, we don't use it in our
CI, and it was manual to begin with. Therefore remove it.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The RDP spec says we can trust x, y position on all messages except
PTR_FLAGS_WHEEL and PTR_FLAGS_HWHEEL, so let's do that to ensure
proper sync with the RDP client.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
And use it to get a feedback event for when adding scanout tranche.
With this change, I get back a feedback event for dmabuf-feedback
on VC4:
���� tranche: target device /dev/dri/card0, scanout
� ���� format ABGR2101010, modifier LINEAR (0x0)
� ���� format XBGR2101010, modifier LINEAR (0x0)
� ���� format ARGB8888, modifier LINEAR (0x0)
� ���� format ABGR8888, modifier LINEAR (0x0)
� ���� format XRGB8888, modifier LINEAR (0x0)
� ���� format XBGR8888, modifier LINEAR (0x0)
� ���� format RGB565, modifier LINEAR (0x0)
� ���� format YUV420, modifier LINEAR (0x0)
� ���� format YUV422, modifier LINEAR (0x0)
� ���� format YVU420, modifier LINEAR (0x0)
� ���� format YVU422, modifier LINEAR (0x0)
� ���� format NV12, modifier LINEAR (0x0)
� ���� format NV12, modifier BROADCOM_SAND128 (0x700000000000004)
� ���� format NV16, modifier LINEAR (0x0)
� ���� end of tranche
Besides that, it can place a fullscreen state of simple-egl on the
primary plane, which without this change wasn't possible.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
I guess this reverts commit 73bdc0ce85
"xwm: Fix fd leak in weston_wm_send_data()"
That commit closes the send half of a pipe in weston_wm_send_data,
claiming that it's dup()licated later, and we'll leak the fd if
we don't close it.
That may have been true at the time? But currently that fd is only
duplicated by wl_event_loop_add_fd() in its normal operation, and
closing our original before that fd handler ever fires results
in an EBADF on write, and the data never reaching its intended
destination.
Worse, by the time that handler is called there might be another
use for that fd, and we could push data into it and close it.
To provoke the problem, launch an app like FireFox over Xwayland,
cut something to the clipboard, then close the app (this is the
path where the wm has stored the clipboard contents and the
app has gone away). relaunch it and paste the clipboard content
back in. clipboard_client_data() will EBADF on write, and the
data won't be pasted.
Reported-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This a new matrix inversion test written from scratch to be suitable for
running in CI: quick to run and automatically detects success/failure.
This all is a result of what I learnt while working on
https://gitlab.freedesktop.org/pq/fourbyfour
Computing the residual error with infinity norm comes straight from
fourbyfour documentation on how to evaluate matrix inversion error.
Most of the hard-coded test matrices have been generated with fourbyfour
project as well, as it contains the generator code. The matrices are
hard-coded here also to make testing faster, but primarily because the
generator code needs BLAS and LAPACK, and having those as Weston
dependencies would be far too much just for this.
Now, if someone wants to modify weston_matrix stuff, we should at least
detect matrix inversion and multiplication bugs.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This #define was used only by the matrix-test program, which was removed
in the previous commit.
Remove it as unused and fold away MATRIX_TEST_EXPORT.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This test program was useful a decade ago when weston_matrix_invert()
was being developed. It was a manual test program that ran for a certain
number of seconds and required human interpretation of numbers to see if
results were acceptable or not. Hence it was foundamentally unsuitable
for CI.
The way it generated random matrices for inversion testing was also very
naive, and it used the determinant value to determine invertability
which is completely bogus. This made it also a bad test for correctness.
Much better speed and correctness testing is implemented in
https://gitlab.freedesktop.org/pq/fourbyfour
with documented testing procedures. It has a copy of the weston_matrix
implementation.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Buffer scale is common enough in the modern desktop space to
expect average GL clients to handle it. Thus lets include it into
our main example client.
While on it, also handle buffer transforms. It's essentially free
for GL clients in terms of computing power but may increase the
chance that Wayland compositors are able to hit scanout fast paths.
Thus having an example client for it is likely valueabel for client
and compositor developers.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
Setting the opaque region correctly is common source of error for
clients that simply want to express that a whole surface is opaque.
This is especially true once buffer_scale and buffer_transform come
into play, as unlike for damage, where buffer_damage is the
encouraged and user friendly way today, opaque regions are always
in logical coordinates.
As faulty opaque regions don't have a visual impact in these cases
but only increase resource consumption, these errors often remain
for long times. See
1e2bc68171
for one of many examples.
Give an easy example how to set the opaque region in a conformant
and reliable way.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
Obviously the first allocation is always leaked, there is a second
zalloc() right below. Fix the leak.
Found by code inspection.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
As we could have situations where dmabuf import failed when attempting
to figure it the framebuffer is scanout-capable, make sure we also have
a way to store that information. Otherwise, we could end up
NULL-dereferencing, as we don't provide a valid storage for it.
Further more, with this, we also print out the reason why it failed, to
aid in further debugging.
Observed on platforms where GBM_BO_HANDLE failed + in combination w/
direct-display proto extension.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Output color profile may be changed in flight. Output basic color
characteristics and EOTF mode cannot yet be changed in flight, but it is
reasonable to assume they could in the future. Therefore the color
outcome data may change in flight as well, which is the basis for HDR
metadata, which needs to be updated as well.
Track the changes to color outcome data with a serial number.
DRM-backend checks the serial number to see if it needs to re-create the
HDR metadata blob. This allows the changes to propagate all the way to
KMS.
The code added here is more of a reminder of what should happen than a
tested path.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Forward the HDR Static Metadata Type 1 to the video sink. This makes the
sink aware of our video content parameters and may be able to produce a
better picture. This type of metadata is used only with the ST 2084 HDR
mode a.k.a PQ.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This creates a new file for KMS related color code, to avoid making
drm.c even longer.
The moved code was just added in 5151f9fe9e
so the new file copyrights are written based on that.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
'color_characteristics_config_error' test ensures that all code paths in
parse_color_characteristics() and wet_output_set_color_characteristics()
get exercised. The return value and logged error messages are checked.
Other cases test the weston_hdr_metadata_type1 validation.
These are for the sake of test coverage, but also an example of how to
test a function from main.c, and how to capture messages from
weston_log().
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Check that weston.ini settings to eotf-mode and basic color
characteristics are correctly parsed.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This is the beginnings of creating composited content HDR metadata for
the ST2084 HDR mode. The immediate goal is to allow essentially setting
the HDR metadata from weston.ini, so that it can be experimented with.
Setting an output ICC profile will stop weston.ini metadata from taking
effect, but using an ICC profile in HDR mode is an open question anyway.
maxDML, maxCLL, and minDML are set based on the assumption that we want
to make use of the full sink/monitor dynamic range.
This also adds several TODOs about how we should handle output profiles,
basic output color characteristics, and HDR metadata. Implementing these
properly will take more thought and effort.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This adds hdr_meta field in weston_output_color_outcome. This field is
intended to be set by color manager modules, and read by backends which
will send the information to the video sink in SMPTE ST 2084 mode a.k.a
Perceptual Quantizer HDR system.
Such metadata is essential in ST 2084 mode for the video sink to produce
a good picture.
The validation of the data and the group split is based on the HDR
Static Metata Type 1 definition in CTA-861-G specification.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This adds an option to program basic display color characteristics from
weston.ini. In the future there will be a way to set this information
from EDID, but because EDID is unreliable that will probably not be the
default. An ICC profile will likely override most or all of this. The
main reason to add this option is to be able to characterise HDR
monitors.
An 'output' section can have a key 'color_characteristics' (string)
set to a name. The name refers to any 'color_characteristics' section
with 'name' set to the same string.
The 'name' key of a 'color_characteristics' section cannot contain a
colon ':'. Names with colon in 'output' section key
'color_characteristics' value are reserved for future use, e.g. to
indicate that the metadata is to be taken from EDID instead of a
weston.ini section.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This adds color_chracteristics field in weston_output. This field is
intended to be set by compositor frontends and read by color managers.
Color managers can use this information when choosing the output color
space and dynamic range, particularly when no ICC profile has been set.
This is most useful for HDR outputs, where the HDR static metadata for
PQ mode or the display luminance parameters for HLG mode can be based on
color_characteristics.
The fields of weston_color_characteristics mirror the information
available in EDID. However, using EDID information as-is has several
caveats, so the decision to use EDID for this is left for the frontend
and ultimately to the end user.
There are no defined ranges or validity checks for this data. The color
manager will have to validate the values against whatever it is using
them for.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Allow the front end to register audio setup and teardown functions. These
functions should use FreeRDP's rdpsnd_server_context or
audin_server_context and set up their own handler threads.
The backend remains mostly ignorant to any audio details beyond setting up
and tearing down.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Instead of a meson option or hidden define, just run these checks always.
It is not Weston's style to add build options for specific asserts, and
currently weston's codebase is expected to always run with asserts
enabled.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
It is used in Mesa. Lets switch to it as well in order to provide
good examples and encourage proper API usage.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
Compositors may choose to send multiple scanout or non-scanout
tranches. So instead of assuming that the first respective tranche
contains the format/modifier we're looking for, check all tranches.
While on it, make sure that in case a compositor sends scanout
tranches on the initial feedback, `pick_format_from_scanout_tranche()`
does not unintentionally pick `INITIAL_BUFFER_FORMAT`.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
It doesn't and can't build, because it depends on cairo-gl. We already
have simple-egl which shows how to use EGL/GLESv2 on Wayland.
Signed-off-by: Daniel Stone <daniels@collabora.com>
It's three planes, not two.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Fixes: 8b167a1703 ("gl-renderer: Store EGL buffer state in weston_buffer")
There's just no good reason to do this.
The query entrypoints already tell us if we need to use
GL_TEXTURE_EXTERNAL_OES for a particular format/modifier. We also have
RGB -> YUV fallbacks which should be able to work well with TEXTURE_2D.
TEXTURE_EXTERNAL pessimises quite hard, forcing GPU-side reloads as well
as bad filtering. Allowing multi-planar formats to use TEXTURE_2D should
thus result in performance and quality improvements.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Now that we can pull everything we need from pixel-formats, go one step
further and reuse the same YUV format descriptors we use to emulate
dmabuf/EGLImage imports for SHM.
This eliminates all special-case YUV/SHM handling.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Add a new hide_from_clients flag which, if set, specifies that the
format is only for internal information and processing, and should not
be advertised for clients.
This will be used for formats like R8 and GR88, which are not useful for
client buffers, but are used internally to implement YUV -> RGB
conversion.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We support this as an explicit YUV fallback path in gl-renderer's dmabuf
EGLImage import path, so might as well support it in the SHM path, given
it's just YUV420 with no subsampling.
Signed-off-by: Daniel Stone <daniels@collabora.com>
If we're doing partial uploads from SHM buffers, we need to use the
vertical subsampling factor rather than the horizontal for secondary
planes.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We already had these with effective width and height, but they're useful
externally as well. Pull them out to a helper.
Signed-off-by: Daniel Stone <daniels@collabora.com>
'depth' isn't actually used to determine the bit depth of meaningful
components generally, but specifically to determine whether we can use
the legacy drmModeAddFB (pre-AddFB2) with those formats.
Rename the member to make it more clear what it's used for.
Signed-off-by: Daniel Stone <daniels@collabora.com>
pixel-formats already stores the gl_format, at least for single-planar
formats; use that instead of storing our own copies.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Instead of checking for each format whether we need compatibility
workarounds for GL implementations not supporting ES3.x or when
GL_EXT_texture_rg isn't present, have each format declare the ideal case
and fix it up later.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Rather than checking all the pixel-format components which are currently
duplicated inside gl-renderer, just check for equality of the pixel
format itself, which will become useful as we remove some of the
duplicate content.
This means that the texture storage will now be reallocated when clients
switch between pixel formats which could've had compatible GL storage
(e.g. XRGB <-> ARGB) on the same surface. However this does not seem
like a case worth optimising, and simplifies the code somewhat.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We've got a nice shiny ARRAY_COPY macro, so use it rather than memcpy or
hand-unrolled assignments.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Allow clipboard pasting in and out of an RDP session.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
RDP exposes certain features (audio, clipboard, RAIL) through a facility
called "virtual channels". Set up the communications framework for using
these.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
FreeRDP has some features that start new threads and run
callback functions in them.
We need a way to punt work from these threads back to the
compositor thread.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
Log EGL features similar to how GL ES features are logged: listing just
the ones weston tests for.
This replaces some log messages from gl-renderer.c that become
redundant or belong with EGL better.
has_native_fence_sync and has_wait_sync are not logged, because missing
them already logs warnings.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Feels like this might be nice to log.
The failure case is not fatal, so say it's a warning only.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This is a human readable replacement for printing out the list of all
available GL extensions that doesn't happen anymore by default.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Print all EGL and OpenGL extension lists into a new log scope
"gl-renderer" instead of the usual log.
These lists cluttered the log while they were very rarely actually
useful. Sometimes they might be interesting, so make them still
available through the new log scope.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Plumb struct gl_renderer all the way through to
gl_renderer_log_extensions(). In the future, the extension lists will be
printed into a debug scope specifically, and it will get the debug scope
from gr.
Signed-off-by: Pekka Paalanen <pekka.paalanen@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>
Now that the gl_buffer_state owns everything related to buffers, move
the textures from there rather than living on the surface, to join the
EGLImage and/or SHM params.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Now that EGLImages are strongly associated with a gl_buffer_state, which
has a lifetime strictly bounded by a weston_buffer, we don't need to
have an egl_image wrapper having its own separate refcounting anymore.
Signed-off-by: Daniel Stone <daniels@collabora.com>
... apart from SHM.
EGL and dmabuf buffers already have a gl_buffer_state created for them
when we first attach the weston_buffer. By turning
gl_surface_state::buffer into a pointer, we can just reference rather
than inline the gl_buffer_state.
SHM buffers are special, in that we don't keep individual copies of them
within the GL renderer. Instead, the GL surface has a texture allocated
with a shadow copy of the most up-to-date surface content. Handle this
by allocating and destroying gl_buffer_state every time we need to
respecify textures or somehow meaningfully change the parameters.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Similarly to EGL buffers, store the gl_buffer_state for a dmabuf buffer
inside weston_buffer, rather than on the linux_dmabuf_buffer. This
slightly simplifies our gl_buffer_state handling, and will be used later
to eliminate the egl_image refcounting.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Introduce a renderer_private hook for weston_buffer, and use this to
store a copy of the gl_buffer_state for EGL buffers (i.e. non-dmabuf, via
EGL_WL_bind_wayland_display).
As part of this, we create the EGLImage along with the weston_buffer
information, and just take a reference to it each time it is attached.
If you have bisected a failure to update surface content to this commit,
it very likely means that your EGL implementation requires images to be
recreated rather than only rebound in order to have their content
updated, which is contrary to specification.
Signed-off-by: Daniel Stone <daniels@collabora.com>
At the moment, attach_shm() will modify the gl_buffer_state in place,
then compare it and see if it differs enough to require a new one. That
rather mixes up the old and new worlds, so quite explicitly build up a
shadow gl_buffer_state with variables first before we change the one
which already exists.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Now that we can reliably access buffer dimensions from weston_buffer,
and gl-renderer isn't doing strange things with buffer widths, just use
that. The renderer interface is now unused and can be deleted.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This was only used for what was presumably an attempt at an
optimisation, to force the texture's pitch in pixels to match the SHM
buffer. This is really unlikely to have ever made a difference, given
the alignments GPUs demand.
Signed-off-by: Daniel Stone <daniels@collabora.com>
It's just a shadow of weston_buffer.buffer_origin, which also has a
slightly more descriptive name.
Signed-off-by: Daniel Stone <daniels@collabora.com>
If we can reuse the textures we already have, just return early, rather
than putting all the work in a large indented body.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Allow the various attach handlers to access the existing buffer, only
referencing the new buffer when they have successfully attached.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Deduplicate the no-buffer and the import-fail case, and try to fall
through where we can. This will make it easier to shift the buffer
reference change later, so the attach subhandlers can reference the old
buffer when checking for compatibility.
Signed-off-by: Daniel Stone <daniels@collabora.com>
It's good to know if we succeeded or failed to import our buffers. This
will also later make for a more smooth transition when we start
returning a gl_buffer_state from them.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This only happens for the legacy renderer, but still, might as well
clean up after ourselves when we can't import a secondary plane.
Signed-off-by: Daniel Stone <daniels@collabora.com>
gl_surface_state has a bunch of members which are tied to the input
buffer, rather than the surface per se.
Split them out into a separate gl_buffer_state member as a first step
towards sanitising its use and lifetime.
Signed-off-by: Daniel Stone <daniels@collabora.com>
No-one should be implementing an external renderer, so move the
interface out of the public header.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This value is always `NULL`.
Silences:
`warning: ‘%s’ directive argument is null [-Wformat-overflow=]`
Signed-off-by: Robert Mader <robert.mader@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>
clang-13 complains about bitwise xor assigments like the following:
../libweston/noop-renderer.c:62:25: warning: variable 'unused' set but
not used [-Wunused-but-set-variable] volatile unsigned char unused = 0;
Use the __attribute__((unused)) instead.
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
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>
Calling weston_surface_unref() one time too many could be a sign we
haven't correctly increased the ref count for it.
Also, if we don't have a surface being passed, do no attempt to
use it.
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>
Unconditionally creating a surface feedback for each surface
creates unnecessary overhead and noise in the logs. Thus
create it when the first surface feedback resource for a
surface is requested and delete it again once all those
resources have been destroyed.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
Weston uses a timeout of 2 seconds before it sends scanout
tranches to clients in order to not trigger excessive buffer
reallocations in clients.
`simple-dmabuf-feedback` in turn counts redraws (200) before
exiting. That doesn't work on e.g. 144Hz screens, thus use a
timer here as well.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
For a surface that is already fullscreen making it maximized means to
exit fullscreen then set to it maximized. Instead of doing it, refuse to
do anything until the user explicitly performs that operation.
With this approach we follow other DE (desktop environments) which would
not perform any operation until the user exits fullscreen state.
Fixes#321
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
To me, the use of setup_search_param() makes the code harder to
understand than it needs to be. Replacing that function with open-coding
the struct cmlcms_color_transform_search_param initialization makes it
more clear that:
- get_surface_color_transform is the only one that actually uses a
surface to initialize it
- get_blend_to_output does not use an input profile at all
- get_sRGB_to_output and get_sRGB_to_blend hardcode the sRGB profile
like they should
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
I am going to need to add yet another output property to be set by a
color manager: HDR Static Metadata Type 1. With the old color manager
API design, I would have needed to add the fourth function pointer to be
called always in the same group as the previous three. This seemed more
convoluted than it needs to be.
Therefore collapse the three existing function pointers in the API into
just one that is resposible for setting up all three things.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This new struct collects all the things that a color manager needs to
set up when any colorimetry aspect of an output changes. The intention
is to make the color manager API less verbose.
In this first step, the new struct is added and replaces the fields in
weston_output.
The intention is for the following color manager API changes to
dynamically allocate this structure. Unfortunately, until that actually
happens, we need a temporary way to allocate it. That is
weston_output::colorout_, which will be removed in the next patch. This
keeps the patches more palatable for review at the cost of some
back-and-forth in code changes.
This is a pure refactoring, no functional changes.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
- Use more consistent style, e.g. the tree structure uses
the same indentation level throughout
- Swap format name and code for consistency with modifiers
- Use constants for ASCII art (taken from drm_info)
Signed-off-by: Simon Ser <contact@emersion.fr>
Trying to do HDR with XRGB8888 is a bit like using RGB565 on SDR: you
get visible color quantization and banding in gradients (without dithering
which Weston does not implement yet, and might not work too well for HDR
anyway).
Therefore, on any HDR mode, default output framebuffer format to 10 bpc
instead of 8 bpc.
Ideally we'd also optionally try 16F or 16 bpc formats, but automatic
fallbacks for those are more complicated to arrange. You can still
configure 16F or 16 bpc manually.
This patch also moves the default format setting from
drm_output_set_gbm_format() to drm_output_enable(), because setting the
default now requires eotf_mode. Frontends may call set_gbm_format()
first and set eotf_mode next. This does create an awkward situation for
outputs that a frontend disables and re-enables. This patch here makes
sure that the old output configuration remains, but changing eotf_mode
may not change the default format. One needs to call
set_gbm_format(NULL) to re-evaluate the default format. Resetting the
format on drm_output_deinit() would lose the current setting.
DRM_FORMAT_INVALID was introduced in libdrm 2.4.95 which we already
hard-depend on.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Program the connector property HDR_OUTPUT_METADATA based on the EOTF
mode of the output.
For now, this changes only the EOTF. The colorimetry and luminance are
left undefined, to be filled in by later patches. This should still be
enough to put a video sink into HDR mode, albeit the response is
probably unknown.
drm_output keeps track of the currently existing blob id. If the blob
contents need to be re-created, this blob would be destroyed and the
field set to zero. In this patch, there is no provision for runtime
changing of HDR metadata, so there is no code doing that.
Destroying the blob at arbitrary times is not a problem, because the
kernel keeps a reference to the data as long as the blob id remains with
KMS.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Check whether HDR_OUTPUT_METADATA property exists on a KMS connector. If
yes, pretend that EDID claims support for all EOTF modes and update the
head supported EOTFs mask accordingly. If not, then only SDR is
possible.
Parsing EDID to take monitor capabilities into account is left for
later.
HDR mode cannot be set without HDR_OUTPUT_METADATA.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
These are fallback definitions in case libdrm is not new enough.
They are copied from libdrm 2.4.107.
struct hdr_output_metadata defines the contents of the blob to be used
with the connector property "HDR_OUTPUT_METADATA".
This is needed for programming a HDR mode in KMS.
This headers need to be excluded from Doxygen, because Doxygen chokes on
the kerneldoc markup.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
A reminder that this variable needs to be taken into account when
crafting color transformations.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The no-op color manager will not support any other EOTF mode than SDR.
Other modes would require it to set up color transformations.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The headless backend does not display to anything, so it doesn't care
what the EOTF mode is. To allow testing compositor internal behavior,
claim to support all EOTF modes.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This is the switch to turn HDR mode on.
The values in the enumeration come straight from CTA-861-G standard.
Monitors advertise support for some of the HDR modes in their EDID, and
I am not aware of any other way to detect if a HDR mode actually works
or not. Different monitors may support different and multiple modes.
Different modes may look different. Therefore the high-level choice of
how to drive a HDR video sink is left for the Weston frontend to decide.
All the details like what HDR metadata to use are left for the color
manager.
This commit adds the libweston API for backends to advertise support and
for frontends to choose a mode. Backend and frontend implementations
follow in other commits.
The frontend API does not limit the EOTF mode to the supported ones to
allow experimentation and overriding EDID.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Rather than setting the fullscreen/maximized before initial
wl_surface.commit, make it part of the initial window state.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Rather than creating the wl_egl_window at the same time as wl_surface,
do it after we get the first configure event.
With it, we also defer eglMakeCurrent() as according to the spec, the
first time a OpenGL or OpenGL ES context is made current, the viewport
and scissor dimensions are set to the size of the draw surface.
This is particulary important when attempting to start simple-egl either
as fullscreen or as maximized, as not doing so will either incorrectly
commit a buffer with the original dimensions, and later on to resize to
the correct dimensions (which is the case for fullscreen), or it will
terminate the wayland connection abruptly due to xdg-shell protocol
violation, with a mismatch for the client's geometry (the case for
maximized).
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Daniel Stone <daniel.stone@collabora.com>
display->wm_base is checked right after handling registry object, and
with it the globals, so there's no to perform and additional check for
xwm_base.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
A previous patch modified this for fullscreen, but missed out the
maximized state. As we check the geometry rather than the buffer
dimensions use the same terminology.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Just like start as fullscreen, let us add a start as maximized as well.
It tests out the maximized state and with clients geometry checks.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Even if the weston_buffer_reference is still alive in situations like
when we have closing animations, the underyling buffer (wl_shm_buffer)
is no longer available. Call the appropriate destroy handler to
invalidate the pixman image and avoid touch the shm_buffer.
Fixes: #613
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Synchronize events carry keylock status, so we should update it.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
When RDP indicates that a Japanese keyboard layout is used without
a Japanese 106/109 keyboard (keyboard type 7), use the "us" layout,
since the "jp" layout in xkb expects the Japanese 106/109 keyboard
layout.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
Korean keyboards are keyboard type 8:
https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getkeyboardtype
While type 8 is not explicitly mentioned in the RDP documentation,
it can be sent over the wire. Let's support the variants we can.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
This code will eventually be used by RAIL as well, so let's
split it out now.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
Use the common abnt2 variant, instead of the niche nativo one.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
Exposay was done as a showcase for what we could do with Weston and an
efficient compositing pipeline. This was mostly with the old
vendor-specific Raspberry Pi backend which could actually process that
many surfaces bypassing the GPU.
Given enough bitrot, Exposay is now pretty exemplary as what _not_ to do
in a Weston shell - particularly the way it manipulates existing
weston_views rather than create its own non-destructive stack.
As it's annoying technical debt, a terrible example to others, and not a
very compelling showcase in 2022, just delete it.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We move this into a function for when we add horizontal wheel support
later.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
We currently hardcode a 60Hz update rate for the rdp backend.
In some cases it may be useful to override this to increase the rate
for a faster monitor, or to decrease it to reduce network traffic.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
Instead of hard coding a 16ms refresh interval, use the refresh rate
from the current mode to determine when the next frame should be.
Currently, we still hard code the refresh rate, so this will end up
with roughly the same value we've been using, but in the future
we'll allow setting it via command line.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
We already have a way for a single RDP client connection to be
passed from a parent process to a child using a combination
of environment variable (RDP_FD) and env var (--env-socket)
This patch allows a bound socket fd (as opposed to a client
connection) to be established in a parent process and provided
to the rdp backend. WSLg uses this to set up an AF_VSOCK
socket for communication between a Windows RDP client and a
weston compositor running under a hypervisor.
Co-authored-by: Hideyuki Nagase <hideyukn@microsoft.com>
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
These events carry the 4th and 5th mouse buttons, so
we should propagate them. We also need to use pointer
frames to ensure the buttons are properly paired with
the pointer co-ordinates.
Unfortunately, there is no way in RDP to determine if
a mouse event and an extended mouse event should be in
the same pointer frame, so this is the best we can do.
We also enable extended mouse events so they'll be used.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
Print a message when presentation switches to/from zero-copy mode.
This makes it easier to understand whether the compositor DMA-BUF
feedback was effective.
Signed-off-by: Simon Ser <contact@emersion.fr>
This fixes an issue when running simple-dmabuf-feedback:
"wl_display@1: error 1: invalid arguments for wl_surface@3.attach".
As we are not using create_immed request from zwp_linux_dmabuf_v1, we
can't start to use a dma-buf buffer before we process compositor's event
telling us that the creation succeeded.
This was causing problems in the following scenario:
1. buffer is marked to be recreated (because of dma-buf feedback);
2. in buffer_release() event, we destroy the buffer and recreate it;
3. after we recreate it, roundtrip is not called, as we don't want to
block during the drawing loop;
4. buffer status is not being properly tracked, so we are trying to
use a buffer before receiving the event from the compositor telling
us that the creation succeeded.
To fix this, this patch improves buffer status tracking. Now we only
pick a buffer in the drawing loop when it is available. Also, if we have
no buffers available we perform a roundtrip and try again, as we may
have recreated all of them but still didn't have the chance to process
compositor's events telling us that creation succeeded.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
This is a minor re-work of how we de-activate and activate the surfaces
in desktop-shell. As activate() is being used for handling keyboard
input events, this basically rectifies that such that activation
happens only if the passed surface is different from the currently
focused surface.
Fixes: #593
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Rather than punching through to set the surface as a solid colour,
attach an actual weston_buffer to it instead.
This becomes the first user of attaching non-client-generated buffers
to a weston_surface. As a result, it is necessary to introduce a
function which will allow compositors and shells to attach a buffer to a
surface. weston_surface_attach_solid() is therefore introduced as a
special-purpose helper which will attach a solid buffer to a
weston_surface.
It is not intended as a general-purpose mechanism to allow compositors
to attach client-generated buffers to surfaces, as doing so would have
unknown effects on this core part of the compositor itself.
Signed-off-by: Daniel Stone <daniels@collabora.com>
When we're checking to see if a view is suitable to go on a plane, check
for (and reject) solid-colour buffers.
Signed-off-by: Daniel Stone <daniels@collabora.com>
The Pixman renderer keeps its own reference to buffers when attached to
surfaces, through its surface state: just use that instead.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Refactor the buffer-type check slightly so we can handle solid-color
buffers, which we do exactly nothing with.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Currently solid-colour displays (e.g. the background for fullscreen
views) is implemented by a special-case weston_surface which has no
buffer attached, with a special punch-through renderer callback to set
the colour.
Replace this with a weston_buffer type explicitly specifying the solid
colour, which helps us eliminate yet more special cases in renderers and
backends.
This is not handled yet in any renderer or backend, however it is also
not used by anything yet. Following commits add support to the renderers
and backends.
Signed-off-by: Daniel Stone <daniels@collabora.com>
When the renderer/backend indicate that they do not need a surface's
buffer content to be preserved, most often because they have copied it,
simply downgrade the buffer reference to 'will not access', rather than
drop the buffer reference altogether.
Signed-off-by: Daniel Stone <daniels@collabora.com>
In the original conception, a weston_buffer_reference indicated that the
underlying contents of the wl_buffer would or could be accessed, so
wl_buffer.release must not be sent until the last reference was
released, as the compositor may still use it.
This meant that renderers or backends which copied the buffer content -
such as the GL renderer with SHM buffers - could only send a buffer
release event to the client by 'losing' the buffer reference altogether.
The main side effect is that `weston-debug scene-graph` could not show
any information at all about SHM buffers when using the GL renderer, but
it also meant that renderers and backends grew increasingly exotic
structures to cache information about the buffer.
Now that we have an additional buffer-reference mode (still referring to
the weston_buffer/wl_buffer, but not going to access its content), we
can allow the weston_buffer_reference and weston_buffer to live as long
as the buffer itself, even if we do send a release event.
This will enable a bunch of backend and renderer deduplication, as well
as finally making scene-graph more useful.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Add a mode argument to weston_buffer_reference which indicates whether a
buffer's storage may/will be accessed, or whether the underlying storage
will no longer be accessed, e.g. because it has been copied. This will
be used to retain a pointer to the weston_buffer whilst being able to
send a release event to the client.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Keep the weston_buffer alive for as long as at least one of the
underlying wl_buffer or a backend usage exists.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Make sure we don't die if we're asked to flush the damage on a SHM
buffer which has subsequently been destroyed.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We currently allow a weston_buffer to outlive the underlying wl_buffer
iff the renderer/backend has cached it. Currently the 'is this buffer
valid?' test relies on looking for the validity of the weston_buffer
itself; shift these tests to looking at the validity of the underlying
resource.
Signed-off-by: Daniel Stone <daniels@collabora.com>
y_inverted meant that the buffer's origin was (0,0), and non-inverted
meant that the buffer's origin was (0,height). In practice, every buffer
was 'inverted' into our natural co-ordinate space that we use
everywhere.
Switch to using an explicit origin enum to make this more clear.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Rather than calling accessors (wl_shm_buffer_get etc) to figure out
which type our buffer is, just look in the structure.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Rather than open-coding various resource -> type accessors, just stick a
type enum in the buffer struct.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Rather than only filling weston_buffer information when we first come to
use it, add an explicit hook so we can fill the dimensions the first
time the buffer's attached.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Make sure we only import dmabufs where the underlying pixel_format is
known: if we can't reason about the buffer content, we're not entirely
likely to be able to display it well.
Signed-off-by: Daniel Stone <daniels@collabora.com>
When we first see a buffer attached, we create a weston_buffer for it.
The weston_buffer doesn't contain any useful information in and of
itself; that's left to renderers to populate later.
Switch this to doing it in the core at the first opportunity, at least
for SHM and dmabuf buffers; EGL buffers will follow in the next commit.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We already have the buffer in the caller, and every no-op implementation
will want to access the buffer. So might as well pass it.
Signed-off-by: Daniel Stone <daniels@collabora.com>
The comment about needing to have destroyed images is somewhat less true
now that we actively avoid doing so.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Fixes: 0b51b02c5e ("gl-renderer: Don't re-import dmabufs")
Add some logging helper functions along with two log scopes for debug
and extremely verbose debugging information.
Also add tangentially related logging for the synchronize event, so
the debug stream isn't empty right now. The vast majority of verbose
usage will come later.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
There are currently compatibility issues between FreeRDP's implementation
of the RemoteFX codec and Microsoft's implementation.
Perhaps this will be fixed in the future and this option can go away,
but for now it's necessary to have a way to disable the codec if the
windows client is going to be connecting to a weston server.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This allows easily co-developing a Wayland protocol and Weston.
Example setup:
ln -s subprojects/wayland-protocols /path/to/wayland-protocols
meson configure build/ --force-fallback-for=wayland-protocols
Signed-off-by: Simon Ser <contact@emersion.fr>
With commit 'screen-share: Add option to start screen sharing on weston
star' an section option has been added to start screen-sharing by
default on all outputs. This has the side-effect of attempting to start
screen-share on the same (RDP) output performing the screen-share. That
happens due to re-loading the screen-share module once more.
This patch recommends users to use --no-config option or alternatively,
use a different configuration file to avoid that.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Found while running with b_sanitize=undefined, which yields:
runtime error: shift exponent 909199186 is too large for 32-bit type 'int'
Converts the shm_formats to a boolean and checks for the correct pixel
format it directly, instead of trying to gather them all in an array and
then later on to do the check.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Trivial fix to clean itself on compositor tear-down. While at it
properly free the command string.
Fixes#298
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
With commit e825fe38, we no longer display the pointer if no movement is
detected, which will cause screen share to fail to start if that is the
case. There could be also legitimate cases where there's no pointer, so
let's allow screen share to function in those cases as well. Makes uses
of previous helper methods to find a proper output to share in case we
don't have an pointer.
Re-uses the shell utils functions.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Seems that we're still missing layer clean-ups, with the touch
calibrator being one of them. Call the appropriate function when
shutting down the compositor instance.
Fixes: #603
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Toy toolkit apps already do this since commit 807cd2e589
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
Input method process may also be a XIM server which requires the correct
DISPLAY to be set after xwayland start up. This helps input method to
work for both wayland and Xwayland.
Signed-off-by: Weng Xuetian <wengxt@gmail.com>
Server generated key repeats are ignored - and they don't generate
matching release events. We early return to avoid generating events
for them.
We also need to push the idle inhibition counting after that early
return to prevent breaking idle inhibition with unmatched events.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
We want to wait for Xwayland to be ready before issuing it blocking
requests, but relying on USR1 is a bit unsafe:
- we can't ascertain the signal originated from Xwayland
- if weston is started as PID1 (e.g. in its own container), then
Xwayland will not send SIGUSR1 and X11 connections will be stuck
forever: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1312
Creating a pipe and using -displayfd, even if we don't care about the
display value itself, is safe and works for all cases
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
These added lines are comments (do not affect output) that make it
easier to browse this file and find the section headings.
Removal of the preceding empty lines on two of the section headers
remove a blank line from PDF output. The blank line was kind of nice,
but presumably .SH should add any necessary blanks before a new section
heading. Now all the section headings have the same vertical space to
the preceding content in PDF.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Try to reduce the cargo-culted directives that do not seem to have any
effect on the output.
I verified that are no changes by doing before this patch
$ man -Tpdf man/weston.ini.5 > ref.pdf
and then after this patch
$ ninja && man -Tpdf man/weston.ini.5 > test.pdf
and looking at
$ diffpdf ref.pdf test.pdf
I used PDF as the format for comparisons, because it can express much
more typesetting features than plain text.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Without this, the earliest signal the ivi controller receives for a new surface
is configure_desktop_changed signal. And this is not emitted until the surface
has the first buffer attached.
By emitting the signal during surface creation, the controller is able to set
the initial width and height for the first configure.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
The pixel format was hardcoded to PIXMAN_a8r8g8b8. All other
renderer->read_pixels() calls in weston use dynamic format selection via
the compositor->read_format instead.
The problem was spotted on the ARM devices with Mali-400 GPU. The visual
effect of the problem was black screen on the remote display, when using
screen-share with the VNC or RDP backends. Related wayland-devel thread:
https://lists.freedesktop.org/archives/wayland-devel/2020-September/041624.html
Signed-off-by: Maciej Pijanowski <maciej.pijanowski@3mdeb.com>
Refactor some of rdp.c into a header file.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
We've already allocated the listener by the time we hit this failure,
so we must exit through the path that frees it.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
By some dark magic this accidentally doesn't crash because there are
zeroes in the right places at startup, but black_surface_get_label()
very much expects surface_private to be a view.
Let's hand craft a new bespoke label function for this use.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
When we launch clients they currently stay in the same session as
weston, and share its controlling terminal. This means hitting ctrl-c
in weston's controlling terminal will send SIGINT to the clients as
well. It also means SIGHUP will be propagated to our launched clients
if weston's controlling terminal is closed.
While generally not harmful, this behaviour is not beneficial, and
is present by default and not by design.
Problems arise when launching weston in a debugger, as a ctrl-c sent to
the debugger will be propagated not only to the debugger, but all the child
processes sharing weston's session. This results in weston-desktop-shell
being killed by the ctrl-c that was intended to stop weston for debugging.
If weston-desktop-shell is killed within 30 second of startup, it will
result weston performing a clean shutdown. This clean shutdown can
make debugging a little too surprising.
Ostensibly, clients launched via weston_client_launch will be wayland
clients that terminate cleanly on their own if weston is killed, so
there should be no need for them to remain in weston's session to
catch ctrl-c from its controlling terminal. Nor should they need a
controlling terminal for their general operation.
Use setsid() to move them to their own session, devoid of controlling
terminal, to make using a debugger a little less confusing in some
cases.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
signalfd interacts badly with gdb's signal trapping - when hitting
ctrl-c in a debugger attached to weston, weston will receive the
signal. This results in weston exiting cleanly when the intent
was to use gdb to interfere with its operation.
Trapping SIGINT was introduced in commit 50dc6989 which ensured we
would call wl_display_terminate() on SIGINT or SIGTERM to clean
up our socket.
Killing weston with SIGINT is quite common for several developers,
so it's important to preserve this clean shutdown behaviour, so
we can't naively stop trapping SIGINT entirely.
Instead, use the sigaction() function to trap SIGINT, and have
the SIGINT handler send weston SIGUSR2 (SIGUSR1 is already
used by xwayland). SIGUSR2 can be trapped in the proper wayland
way via wl_event_loop_add_signal(). This way we can properly
break our event loop and clean up on SIGINT, but we can also
have gdb intercept SIGINT.
There are other ways around this, but I'm hoping this one allows
people to continue using ctrl-c to stop weston, and doesn't
require additional project specific gdb knowledge.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
We've been trapping SIGQUIT for a "clean shutdown" since commit 3cad436a
However, sources such as:
http://www.gnu.org/software/libc/manual/html_node/Termination-Signals.html
indicate we probably shouldn't be trapping it at all, as the intent of
SIGQUIT is to leave a core file and debug artifacts from the run.
We should perform the minimal amount of clean up to ensure the system isn't
left in an unusable state - but these days that's performed by other
software such as logind.
We can safely stop trapping SIGQUIT entirely.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
We can use it just once to define a string instead of having preprocessor
conditionals sprinkled about the code.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
In 913d7c15f7 stricter error checking was
introduced to the strtol call, which broke reading backlight values.
Since every sysfs backlight file ends with a newline.
As noted in a comment in the previous MR to prevent damaged pointers
after calling asprintf, replace every asprintf call with str_printf.
Previous-MR: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/543
Signed-off-by: Sören Meier <soerenmeier@livgood.ch>
The repaint_data is entirely backend specific. Moreover, it is only used by the
drm backend, while other backends ignore the repaint data.
There will always be only one repaint active, thus, there is no need to pass the
repaint data from the outside.
The repaint_data breaks with the multi-backend series, which calls repaint begin
for all backends to get the repaint_data. The repaint_data of the last backend
will then be passed to all other backend. At the moment, this works, because the
drm backend is the only backend that implements the begin_repaint call.
Another option would be to track the repaint data per backend in the compositor,
but actually, it the backend needs to track state across the calls, it's its own
responsibility.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The pending_state is already stored in the backend and can be directly retrieved
from there.
This avoids involving the compositor in passing state between the repaint
phases for a single backend.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Use the common helper provided by the shell-utils helper dependency,
rather than rolling our own.
This commit currently introduces no functional change to
fullscreen-shell, as the 'curtain' provided by shell-utils behaves
identically to the previous solid-color surface created by
fullscreen-shell, given the parameters provided to
weston_curtain_create().
However, now that a common weston_curtain implementation is being used
rather than an open-coded variant, future changes to the implementation
of weston_curtain will result in changes to this code called by
fullscreen-shell, although it is intended that these will not result in
any user-visible behavioural changes.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Unlike desktop-shell and kiosk-shell, the fullscreen-shell does not link
with the common shell-utils helpers. This is largely because
fullscreen-shell is largely in 'maintenance mode', seeing little more
than occasional bug fixes or changes required to accommodate new
interfaces.
This commit adds a dependency from fullscreen-shell to use the
shell-utils helper, in order to allow fullscreen-shell to use the new
weston_curtain infrastructure, rather than continuing to open-code the
common pattern of creating a surface and view consisting only of a solid
colour for the background of fullscreen surfaces which do not wholly
cover the output.
In doing this, the 'surface_subsurfaces_boundingbox()' function is
removed, as this has been duplicated between the fullscreen-shell and
the common helper 'library'.
There is no functional change within this commit, as the two functions
were identical, other than a change to the comment which identifies a
known bug within this helper.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Use the helper we have for these, rather than open-coding.
This commit is not believed to result in any functional changes.
Signed-off-by: Daniel Stone <daniels@collabora.com>
shell-utils contains a number of helpers which are currently in use by
both desktop-shell and kiosk-shell. In order to extend this use to
fullscreen-shell as well (which can benefit from reusing the
weston_curtain infrastructure to be able to create solid-colour views
which may or may not be opaque, as well as one function within
fullscreen-shell which was copied wholesale to shell-utils), we need to
create a separate Meson dependency object, and avoid the existing
pattern of including the source from shared/ within the source list for
each shell.
This requires creating a new top-level directory for these shared helper
functions which are required by each shell, but are not part of
libweston in and of itself.
shell-utils depends on libweston-desktop; libweston-desktop depends on
libweston; libweston depends on shared.
Thus it is not possible to expose a dependency object from the shared/
directory which declares a dependency on the libweston-desktop
dependency, as Meson processes directories in order and resolves
variable references as they are parsed.
In order to break this deadlock, this commit creates a new top-level
directory called 'shell-utils' containing only this file, which can be
parsed by Meson after libweston-desktop (making the libweston-desktop
Meson dependency variable available to the build file to declare a
dependency on that), but before the shells (making the new Meson
depenendency object available to each shell which wishes to use it).
This commit contains no functional changes to any observable code.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Use the common infrastructure we have, rather than open-coding again.
In changing to the common weston_curtain infrastructure which was
introduced in a previous commit, there are two small functional
derivations.
After adding the view to a layer, we explicitly call
weston_view_geometry_dirty(). This is believed to have no functional
impact, as weston_views have their geometry-dirty flag set when they are
created.
weston_surface_damage() is also called to ensure that the surface is
scheduled for a repaint. As there is currently no good common mechanic
in the common code to ensure that output repaint will occur, due to the
damage propagating outwards from the weston_surface, we are forced to
call this to predictably ensure that the output will be repainted after
we have made this change to the scene graph which should result in the
user observing the new content being repainted.
It is possible that these changes are not strictly required in order for
the correct behaviour to occur. However, it is felt that explicitly
adding these calls is perhaps the least fragile way to ensure that the
behaviour continues to be correct and breakage is not observed,
especially with both view mapping and surface damage identified as areas
for future work which could be beneficial to Weston. If it is felt that
these calls can be removed, then this is certainly possible at a later
stage.
Lastly, there are many users within desktop-shell of the common pattern
of creating a weston_curtain and inserting it into the scene graph to be
painted. These users have been an area of both theoretical concern and
real-life observed fragility and breakage recently. By making each user
follow a common and predictable pattern of usage, each user is no longer
its own special case. This should make it easier to make the
desktop-shell codebase more robust, not only simplifying the codebase,
but improving observed behaviour.
In doing this, it is hoped that future structural and interface changes
become easier to make, allowing us to improve some of the more difficult
corners of the libweston API.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This will allow us to create a solid weston_buffer as well, since we
need to store that separately.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Just as we do for fullscreen backgrounds, reuse the curtain infrastructure
for focus animations.
This introduces a small functional change, in that the surface's output
is no longer directly assigned. Instead, we call
weston_view_set_output() ourselves. As setting the weston_view's
position (done from the common helper function of weston_curtain_create
which has been introduced in previous commits) will call
weston_view_set_position(), the view's geometry will be dirtied as a
result.
When the geometry of a weston_view is dirty, it is marked to be updated,
which will occur whilst traversing the view list during output repaint.
This occurs for every view which is currently assigned to a layer; when
building the view list, any view reachable through the view list whose
geometry is dirty will have its position recalculated and an output
assigned. Doing so results in the surface's current output being
updated.
It is believed that there is no functional impact from the
weston_surface not having a primary output assigned between creation and
output repaint being called.
Signed-off-by: Daniel Stone <daniels@collabora.com>
desktop-shell's focus surfaces want to reuse this, but they don't want
to capture the input, instead allowing it to fall through.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Rationalise it a little bit so we don't need pre-declarations of static
functions, and the order of creation more closely matches the others,
including making the same calls to explicitly set the output.
Doing this makes the behaviour match the other users of the same code
pattern. In making them the same, we make desktop-shell code a little
less magically divergent where people might wonder what the correct
pattern is to use. After we have moved all users to a uniform pattern,
later commits are then able to migrate these users to common helper
code, which reduces code duplication, improves code clarity as it is no
longer necessary to wonder about the exact semantics of every
special-case user of this common pattern, and makes it easier to make
interface changes which improve and clarify the patterns which are
prevalent throughout the desktop-shell code.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Dirtying the geometry only sets a flag on the view saying that the
geometry is dirty, so we don't need to do it twice back-to-back.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Opaque regions are in surface co-ordinate space, not global co-ordinate
space, so the region should be anchored to (0,0).
Signed-off-by: Daniel Stone <daniels@collabora.com>
Not all solid-colour views want to be opaque: sometimes we use them with
non-opaque alpha values in order to shade views underneath them.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Given that we have a struct for argument params, we might as well use it
rather than have them split between the struct and native params. For
consistency between the implementations, this also includes a shift from
float to int positioning for the base offset within the compositor's
global co-ordinate space.
Signed-off-by: Daniel Stone <daniels@collabora.com>
The name implied that it was a surface in and of itself, rather than
parameters used by a helper to create a surface and view.
Rename it now that we have weston_curtain as a name, and clean up
initialisers.
Signed-off-by: Daniel Stone <daniels@collabora.com>
create_solid_color_surface actually returns a weston_view that it
creates internally. Since weston_solid_color_view is long and dull,
rename it to weston_curtain.
Signed-off-by: Daniel Stone <daniels@collabora.com>
desktop_shell_removed() won't get called when we shut down, so
explicitly destroy the fullscreen black view.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We could overflow a local buffer if there were more than ten million
concurrently active displays within the current user's session. This
seems vanishingly unlikely, and harmless, but does at least squash a
compiler warning emitted by gcc 12+.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Fbdev backend was deprecated in the Weston 10.0.0 release with
6338dbd581. Before that, I suggested
already in 2019 to remove it, but it was too soon then. Now it seems the
final voices asking for fbdev to be kept have been satisfied, see the
linked issue.
Fbdev-backend uses a kernel graphics UAPI (fbdev) which is sub-par for a
Wayland compositor: you cannot do GPU accelerated graphics in any
reasonable way, no hotplug support, multi-output support is tedious, and
so on. Most importantly, Linux has deprecated fbdev a long time ago due
to the UAPI fitting modern systems and use cases very poorly, but cannot
get rid of it if any users remain. Let's do here what we can to reduce
fbdev usage.
I am doing color management related additions to libweston which require
adding checks to every backend. One backend less is less churn to write
and review.
Libweston major version has already been bumped to 11, so the next
release will be Weston 11, without fbdev. enum weston_compositor_backend
entries change their numerical values.
Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/581
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Tablet shell was removed in 873b515aee in
2013. Time to remove the hopefully last reference to it.
We also gained kiosk shell in the mean time, so mention that instead.
Yes, it's a bit of a lie, because we also have ivi-shell and
fullscreen-shell, but I heard they might be on their way out, so I
didn't add them here.
Would be nice to add kiosk-shell in the SHELLS section too, but in this
patch I am only concerned about dropping the tablet shell reference.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Looks like at least from 2016 onwards the gbm-format option has also
been recognized in an output section.
Time to document that.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Since 62a9436417 the gbm-format option has
recognized all pixel formats listed in libweston/pixel-formats.c.
Clarify what pixel formats can be used.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This option is used only with the DRM-backend. Options in weston.ini(5)
should be either generic or for backends that do not have their own man
page yet.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This option is used only with the DRM-backend. Options in weston.ini(5)
should be either generic or for backends that do not have their own man
page yet.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We have a string helper which wraps asprintf(). Uses that one because it
clears out the destination string, but also it won't return the number
of bytes unlinke asprintf().
Fixes warnings like:
warning: ignoring return value of ‘asprintf’ declared with attribute
‘warn_unused_result’.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
weston_config_section_get_double() was not covered with tests before.
This patch follows the testing style already present in the file.
Cannot use ZUC_ASSERT_EQ() here, because that would convert the values
to integers before comparison. Luckily, simple strict equality
comparison works here, because we are testing conversion to float, not
the results of lossy calculations.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This was the only file in Weston using WL_EXPORT on its own line. Fix
the style to follow everything else.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Using `pixel_format_get_info()` can result in formats being
reported as `UNKNOWN` when used on compositors other than Weston.
As `weston-simple-dmabuf-feedback` somewhat succeeds `wayland-info`
as tool for `zwp_linux_dmabuf_v1` debugging from version 4 on, copy
the approach from the later for these cases.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
These are supported by some other compositors already.
Add them to the list so `weston-simple-dmabuf-feedback`
reports them correctly.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
The built-in backend of libseat requires users to enable a logging
level in order for libseat to start writing out log messages. For that
to happen we split out the info and error log level messages into the
compositor's log scope, while debug level messages go into a dedicated
scope.
With that, this patch brings in a new scope, called libseat-debug, which
users need to explicity create a subscription for it as to retrieve/have
access to debug message coming out of libseat. Note that by default we
have a subscription for the log-scope so any errors/info from libseat
would be displayed to the user.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Everywhere else where use this trick, we also have 'used' in the
attributes, except here. Make this consistent.
Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/517
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Otherwise, the client will assume that dragging is in progress and remains in
that state forever.
This can happen when weston processes the mouse up event just before the
start_drag() arrives.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
When using the libseat launcher, there is one more detail to take care:
stop libseat from managing the VT.
A normal user does not have permissions to manage a VT, so launching
would just fail. In this use case you also do not want to be managing
the VT, because your normal desktop is already owning the seat
associated with the VT.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Give a little more details about how running Weston via ssh or serial
terminal is best done, now that launcher-direct and weston-launch are
gone.
Hopefully the removal of launcher-direct also makes less people run
Weston as root, when seatd is the privileged process. Running 'weston'
as root might still work through libseat's builtin backend without
seatd.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Now that launcher-direct and weston-launch are gone, libseat takes their
place.
Enable libseat support by default to give users a hint in case they miss
either of those.
People who used to get launcher-logind when libseat support was disabled
will now be using logind through libseat. This should not cause any
regressions, and if it does, we want to hear about them, because the
separate logind-launcher is also planned to be deprecated in the future.
Disabling logind-launcher by default is left for when it actually gets
deprecated.
In case someone does not have libseat available but do have logind
running, they can just disable libseat support.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This doesn't work with any of the launchers we've kept. Remove the option
and all the bits that handle it.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Moving forward we're going to be supporting libseat and logind as our
only launchers. We're doing this to reduce our maintenance burden,
and security impact.
Libseat supports all our existing use cases, and seatd can replace
weston-launch so we no longer have to carry a setuid-root program.
This patch removes weston-launch, and launcher-direct, leaving only
libseat and logind.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Fix a typo. No CM functional change, just redirect LCMS error
into created cmsContext which output into weston log.
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
This is probably the simplest case to demonstrate how to use
WESTON_EXPORT_FOR_TESTS.
Previously, vertex-clip test re-built vertex-clipping.c for itself. Now
it directly links in gl-renderer.so instead as that is where
vexter-clipping.c gets built into for actual use. This probably will not
work for any installed program, but luckily tests are never installed,
so Meson makes sure the DSO is found.
Unfortunately we cannot remove the definition of dep_vertex_clipping
yet, because clients/cliptest.c needs it.
This makes vertex-clip test depend on GL-renderer, but that is where the
code is really used.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This is a new function exporting macro that intends to make writing unit
tests in the Weston test suite easier.
A test needs to access a private function to be able to verify its
behavior. Previously we have used things like putting such functions in
a separate .c file and then building that file into the corresponding
test. That is a bit awkward and can lead to proliferation of arbitrary
.c files for no good reason. It may also require pre-processor magic,
and sometimes copying chunks of code causing a risk of deviating the
code being tested from the code actually used.
This patch proposes another approach: a private export from a DSO.
Except, private exports do not really exist, and this is just a normal
export with a specific C macro, and omitting the function from public
headers.
Once exported, a test program can link the DSO during build, be that a
shared library or even a plugin, use the private header declaring the
function, and simply call the function in the test.
The declaration of WESTON_EXPORT_FOR_TESTS is in shared/helpers.h so
that it is available to all components equally while still not being in
a public header. Other places that were considered:
- include/libweston/libweston.h is a public header, but external users
should not know about the macro.
- libweston/libweston-private.h is too private and not available to all
components, particularly color-lcms plugin.
- libweston/backend.h is not appropriate for color-lcms plugin either.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Although weston_compositor_run_key_binding() is called when the current
keyboard grab is default_grab or input_method_grab, swallowing the key
event is processed only on default_grab. As a result key events that
should be swallowed are sent to the input method unexpectedly.
For example, when a user press `Super + s` on weston-editor to take a
screen shot, `s` will be unexpectedly entered to the text area.
I confirmed such behaviour with weston-simple-im and fcitx5-5.0.10.
It doesn't occur with weston-keyboard because it doesn't install
keyboard grab.
Signed-off-by: Takuro Ashie <ashie@clear-code.com>
Iterate over rgb[] array instead of repeating the code for .r, .g and
.b.
Also in process_pipeline_comparison() f_max_err variable is dropped
since it was not used much.
This should make the code easier to read.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Individual struct fields are inconvenient to index into, yet most
operations on a color just repeat the same for each of RGB channel.
Being able to index into RGB avoids repeating the same code for each
channel.
Alpha channel is left as separate, since it is almost never handled the
same as RGB.
The union keeps the old .r, .g and .b addressing working. The static
asserts ensure the aliasing is correct.
For demonstration, two simple functions in color_util.c are converted.
Unfortunately initializers need to be corrected everywhere. Field .a is
not explicitly initialized because it is unused in these cases.
This change should make code easier to read.
This change requires gnu99 or c11 standard. gnu99 is already the default
in top-level meson.build.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Given that the test-helper code relies on the screenshooter protocol,
make sure it's available for us to build, and the dependency ensures we
build in order.
Fixes: #588
Signed-off-by: Daniel Stone <daniels@collabora.com>
In certain situations these clients crash a lot due to the low
buffer limit. Four buffers is also what EGL allows without blocking
and what is arguably the upper limit of what a compositor should
demand.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
1. Use fixture_setup to set the generated by LCMS output profile based on
given chromaticities and white points. The following list of well known
chromaticities:
- sRGB
- adobe RGB
- bt2020
and white point is D65. Use INTENT_ABSOLUTE_COLORIMETRIC to avoid BPC.
Input profile is always sRGB and it is used internally by Weston as
stock profile.
2. Use these hardcoded matrixes as part of pipeline 1DLUT->3x3->1DLUT.
The diagnostic code to retrieve the transform matrix is availble into
test in the comments. The conversion matrixes generated for the
following cases:
- sRGB to sRGB (unity)
- sRGB to adobeRGB
- sRGB to BT2020
3. Compare GPU shaders(gl texture3D) vs manual pipeline calculation
Use different max tolerable error per transform.
There are comments how number of points in 3DLUT is related to tolerance.
Tolerance depends more on the 1D LUT used for the inv EOTF than
the tested 3D LUT size: 9x9x9, 17x17x17, 33x33x33, 127x127x127.
4. Enable build matrix-shaper test if color-management-lcms is enabled.
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Use 3D LUT for color mapping.
For category CMLCMS_CATEGORY_INPUT_TO_BLEND use transform which has
3 profiles: input, output and light linearizing transfer function.
For category CMLCMS_CATEGORY_INPUT_TO_OUTPUT use input and output profiles +VCGT.
For category CMLCMS_CATEGORY_BLEND_TO_OUTPUT use output inverse EOTF + VCGT.
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Graeme sketched a linearization method there:
https://lists.freedesktop.org/archives/wayland-devel/2019-March/040171.html
Sebastian prototyped there:
https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/14/commits
Thanks to Pekka for great simplifications in implementation, like the xyz_dot_prod()
Quote: "should help untangle lots of the multiplications and summations by saying
we are computing dot products, etc".
The approach was validated using matrix-shaper and cLUT type of profiles.
If profile is matrix-shaper type then an optimization is applied.
The extracted EOTF is inverted and concatenated with VCGT, if it is availible.
Introduce function cmlcms_reasonable_1D_points which would be shared between
linearization method and number of points in 1DLUT for the transform.
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Co-authored-by: Sebastian Wick <sebastian@sebastianwick.net>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Add to search parameter cmlcms_category, input and output profiles,
and render intent for output which would be used for both profiles.
Add common function setup_search_param for every category.
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
The stock profile would be used when client or output
do not provide any profile or unaware of color management.
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
1. The cmlcms_category is used to identify the purpose of transform:
- CMLCMS_CATEGORY_INPUT_TO_BLEND
- CMLCMS_CATEGORY_BLEND_TO_OUTPUT
- CMLCMS_CATEGORY_INPUT_TO_OUTPUT
2. Added following fields to cmlcms_color_profile:
- output_eotf - If the profile does support being an output profile and it
is used as an output then this field represents a light linearizing
transfer function and it can not be null. The field is null only if
the profile is not usable as an output profile. The field is set when
cmlcms_color_profile is created.
- vcgt - VCGT tag cached from output profile, it could be null if not exist
- output_inv_eotf_vcgt - if the profile does support being an output profile and it
is used as an output then this field represents a concatenation of inverse
EOTF + VCGT, if the tag exists and it can not be null.
3. Added field cmsHTRANSFORM to cmlcms_color_transform.
It is used to store LCMS optimized pipeline.
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
The following GL extensions provide support for shaders CM:
-GL_OES_texture_float_linear makes GL_RGB32F linear filterable.
-GL ES 3.0 provides Texture3D support in GL API.
-GL_OES_texture_3D provides sampler3D support in ESSL 1.00.
If abovesaid is supported then renderer sets flag WESTON_CAP_COLOR_OPS
which means that all fields in struct weston_color_transform are
supported, for example, 1DLUT and 3DLUT.
Use GL_OES_texture_3D to implement 3DLUT function which
uses trilinear interpolation for pixel processing or bypass as is.
Quote from https://nick-shaw.github.io/cinematiccolor/luts-and-transforms.html
"3D LUTs have long been embraced by color scientists and are one of
the tools commonly used for gamut mapping. In fact, 3D LUTs are used
within ICC profiles to model the complex device behaviors necessary
for accurate color image reproduction".
Quote from https://developer.nvidia.com/gpugems/gpugems2/part-iii-high-quality-rendering/
chapter-24-using-lookup-tables-accelerate-color
is about interpolation: "By generating intermediate results based
on a weighted average of the eight corners of the bounding cube,
this algorithm is typically sufficient for color processing,
and it is implemented in graphics hardware".
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Introduce shader color mapping identity and 3D LUT.
Shader requirements struct uses union for color mapping
to prepare the place for 3x3 matrix.
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Introduce 3D LUT definition as part of Weston
color transform struct. A 3D LUT is a LUT containing
entries for each possible RGB triplets.
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Add VGEM to the Linux image that runs in the CI. There are tests that we
plan to add in the future that need this.
This brings a complication, as we already have VKMS in the image. The
order in which DRM devices are loaded is not always the same, so the
node they receive is non-deterministic. Until now we were sure that VKMS
(the virtual device we use to run the DRM-backend tests in the CI) would
be in "/dev/dri/card0", but now we can't be sure. To deal with this
problem we find the node of each device using a one-liner shell script.
This commit also updates the documentation section that describes
specificities of DRM-backend tests in our test suite.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
Make it slightly easier to disambiguate clients when we log the protocol
stream from the server side.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Name the seat created by the screen share plugin "screen-share" instead of
"default", to make it easier to recognize. No functional change intended.
Signed-off-by: Marek Vasut <marex@denx.de>
Test different scenarios where child subsurfaces of unmapped
subsurfaces would get mapped. This test will fail in various
ways without the commit
"libweston/compositor: Do not map subsurfaces without buffer"
Also try to test potential regressions of that patch.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
We can end in `subsurface_committed()` in different scenarios
without the surface having an attached buffer. While setting
the mapped state to `true` in that case doesn't matter for
that (sub)surface itself, it triggers its own child subsurfaces
to get mapped when they shouldn't.
Closes https://gitlab.freedesktop.org/wayland/weston/-/issues/426
Signed-off-by: Robert Mader <robert.mader@collabora.com>
With commit 'compositor: Remove desktop zoom' weston_output_zoom was removed
from weston_output thus needing a libweston major bump.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This isn't about opening new windows but the start-up animation
that desktop-shell does when it is being brought on/started.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Users should rely on wayland-info from wayland-utils [1] instead.
We've been printing a deprecation since 85382d394a ("clients:
deprecate weston-info"), so users should be aware already.
[1]: https://gitlab.freedesktop.org/wayland/wayland-utils/
Signed-off-by: Simon Ser <contact@emersion.fr>
Many years ago (2014) systemd-logind was brought into libsystemd.
We've supported old versions of systemd-logind ever since.
Let's remove support for old versions of systemd-logind before the
merge for a tiny code simplification.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Zoom is a neat trick, but in its current form it's very hard to test
and maintain.
It also causes output damage to scale outside of the output's boundaries,
which leads to an extra clipping step that's only necessary when zoom
is enabled.
Remove it to simplify desktop-shell and compositor.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Changing `wl_surface_damage()` to `wl_surface_damage_buffer()`
should not have an effect on the existing tests.
The new test will fail without the commit
"libweston/compositor: Cache buffer damage for synced subsurfaces"
Signed-off-by: Robert Mader <robert.mader@collabora.com>
The spec states:
> Because buffer transformation changes and damage requests may be
> interleaved in the protocol stream, it is impossible to determine
> the actual mapping between surface and buffer damage until
> wl_surface.commit time. Therefore, compositors wishing to take both
> kinds of damage into account will have to accumulate damage from the
> two requests separately and only transform from one to the other after
> receiving the wl_surface.commit.
For subsurfaces in sync mode, arguably the same is the case until the
cached state gets applied eventually. Thus, in order to keep complexity
to a sane level, just accumulate buffer damage and convert it only
when the cached state gets applied.
This mirrors how other compositors like Mutter implement cached damage
and what the spec arguably should demand.
Closes https://gitlab.freedesktop.org/wayland/weston/-/issues/446
Signed-off-by: Robert Mader <robert.mader@collabora.com>
In multiple output cases, finding the succesor from the inactive layer
might result in picking the wrong view when there are multiple views
being stacked in the inactive layer. This adds two additional checks to
favor views on the same output as the one being destroyed/removed.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This adds an additional check to make sure the current focus surface
is on the same output as the surface that is going to be activated.
This is necessary in order to avoid placing the currently focused one in
the inactive layer, which shouldn't happen in situations where the new
surface is going to be placed on a different output than the currently
focused one.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Some applications would set-up the app_id after the initial commit
(without a buffer) which is too late to correctly assign the application
to the corresponding output set-up in the configuration file.
This patch fixes that by checking one more time, after a buffer has been
attached, if indeed there's an output with an app_id set.
Fixes: #469
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
These formats are useful because they are often easier to produce
on CPU than half-float formats, and abgr16161616 has both >= 10bpc
color channels and adequate alpha, unlike abgr2101010.
The 16-bpc textures created from buffers with these formats require
the GL_EXT_texture_norm16 extension.
As WL_SHM_FORMAT_ABGR16161616 was introduced in libwayland 1.20,
update Weston's build requirements and CI.
The formats also needed to be registered in the pixel format table,
and defined in a fallback path if recent libdrm is not available.
Signed-off-by: Manuel Stoeckl <code@mstoeckl.com>
Specifically log if there were no suitable planes for us to use, or if
we tried to place it on a plane but were told no by the kernel.
Signed-off-by: Daniel Stone <daniels@collabora.com>
There's no real reason for these to be separate now that the eligibility
checks have been moved up so we don't call them unless it makes sense.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We just copy the SHM buffer straight into a separately-allocated GBM BO,
so no need to take a reference on the buffer itself or keep it from
being released.
All drm_output_try_view_on_plane really does at this point is to call
the prepare_*_view function for the requisite plane type, and take a ref
on the weston_buffer from the client. Given that we don't need to keep
the client buffer alive, we can short-circuit
drm_output_try_view_on_plane, and instead just call
drm_output_prepare_cursor_view directly when we have a cursor plane.
This also makes it easier to just remove drm_output_try_view_on_plane in
following patches when we merge the overlay/scanout plane path into one.
Doing so gives us two clearly-separated paths: one for copying a SHM
client buffer into a cursor, and another for directly scanning out
client content.
Signed-off-by: Daniel Stone <daniels@collabora.com>
At some point this got hobbled, such that NO_PLANES and
NO_PLANES_ACCEPTED became the same thing, so we can just check if the
returned plane_state is NULL or not.
Signed-off-by: Daniel Stone <daniels@collabora.com>
For views which cover the entire output, we always attempt to place them
on the primary plane, to avoid a situation where we place a fullscreen
view into an overlay plane and then have to disable the primary plane,
which doesn't always work.
Move this check earlier, so we don't consider overlay planes to be
candidates for fullscreen views. This check should be changed in future
to only filter for opaque views, but that's for another time.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We shouldn't get down into trying to place a view on a cursor plane if
these checks are not met, so change them to asserts rather than early
returns.
Signed-off-by: Daniel Stone <daniels@collabora.com>
When we introduced support for variable zpos, we did so by filtering the
list of acceptable planes and then creating a separate zpos-ordered
list. Now that the planes are already zpos-sorted in the backend list,
and we have more early filtering, we can replace this with a single
plane-list walk.
Signed-off-by: Daniel Stone <daniels@collabora.com>
For better or worse, cursor planes can only be used by uploaded SHM
buffers right now, so ignore them when we're calculating the acceptable
plane mask for client dmabufs.
Signed-off-by: Daniel Stone <daniels@collabora.com>
If we're in renderer-only mode, we can only use the renderer and the
cursor plane. Don't even try to import client buffers as it makes no
sense.
Signed-off-by: Daniel Stone <daniels@collabora.com>
If we have a SHM buffer, it can only go into a cursor plane - and only
then if it's of the right format.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Each output is hardcoded to the use of a single 'special' (primary or
cursor) plane; make sure we don't try to steal them from other outputs
which might not be happy to discover that we've taken it off them.
Signed-off-by: Daniel Stone <daniels@collabora.com>
GBM is how we import all our client content into DRM FBs, so don't try
anything other than renderer-only without it.
Signed-off-by: Daniel Stone <daniels@collabora.com>
It's possible to write this with a few less twisty special cases. Tested
manually with a randomly-distributed input tree as well as manually
trying to hit special cases around first/last entries.
Signed-off-by: Daniel Stone <daniels@collabora.com>
@ -141,7 +143,7 @@ if get_option('color-management-colord')
foreachdepname:['glib-2.0','gobject-2.0']
foreachdepname:['glib-2.0','gobject-2.0']
dep=dependency(depname,required:false)
dep=dependency(depname,required:false)
ifnotdep.found()
ifnotdep.found()
error('cms-colord requires \'@0@\' which was not found. If you rather not build this, set \'-Dcolor-management-colord=false\'.'.format(depname))
error('cms-colord requires \'@0@\' which was not found. If you rather not build this, set \'-Ddeprecated-color-management-colord=false\'.'.format(depname))
endif
endif
plugin_colord_deps+=dep
plugin_colord_deps+=dep
endforeach
endforeach
@ -156,6 +158,8 @@ if get_option('color-management-colord')
Nikita Tokarchuk
Marius Vlad
Liu, Kai1
Michael Tretter
Sergio Gómez
Michael Olbrich
Alexandros Frantzis
Leandro Ribeiro
Derek Foreman
Paul Kocialkowski
vanfanel
Simon Ser
Pekka Paalanen
Erik Kurzinger
Hideyuki Nagase
Thomas Petazzoni
Jonas Ådahl
Daniel Stone
Joshua Watt
Michel Dänzer
Ivan Nikolaenko
Robert Mader
Philipp Zabel
Luigi Santivetti
Vitaly Prosyak
Erik Faye-Lund
Brenton DeGeer
Weng Xuetian
Dominique Martinet
Maciej Pijanowski
shierote
James Hilliard
Sören Meier
nerdopolis
James Le Cuirot
Takuro Ashie
Marek Vasut
Veeresh Kadasani
Manuel Stoeckl
