The invariant is clearly documented in code comments, but the code
failed to ensure it in all cases. Fix it.
There is one very specific protocol sequence triggered by a development
version of the Wine Wayland driver when Chrome (win64 app) is switched
from window to fullscreen and then back by pressing F11 key. The switch
back triggered
weston: ../libweston/color.c:217: weston_paint_node_ensure_color_transform: Assertion 'it->surf_xform_valid == false' failed
For some reason, that specific protocol sequence causes
weston_compositor_build_view_list() to create a transient second view
for a sub-surface. In the Chrome traces, I have seen that happen twice
per run. The first time it works, the old view gets immediately
destroyed. The second time (during un-fullscreening) a new transient
view is create and then it fails the invariant check.
The fix is in weston_paint_node_create() which is supposed to ensure the
invariant. However, it went through the (new) view's paint node list,
which will not contain paint nodes from other views. In hindsight this
is an obvious bug, but perhaps all views having exactly one associated
surface each somehow confused the author. Since the invariant is about
surface+output, go through the surface's paint node list instead. That
list contains all the relevant paint nodes by definition.
Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/568
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Give a role and a label for the test desktop shell background surface.
This makes it easier reading scenegraph dumps and other surface related
debug messages in tests when you don't have to guess what this
mysterious "PID 0, surface ID 0" surface is.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
There is no weston_surface_set_transform_parent(), it is called
weston_view_set_transform_parent() now since
a7af70436b .
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Adds appid for all clients using the toolkit, flower, fullscreen, image,
resizor, scaler, smoke, stacking, subsurfaces, terminal,
touch-calibrator, transformed, etc.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Commit 0e4f097d broke opaque regions, and since then weston will waste
time rendering occluded areas.
I think this is because we're taking the intersection of the opaque
and scissor regions even when the scissor region isn't enabled.
An easy test is to turn on triangle fan debugging with the gl renderer,
then run weston-simple-damage and move another opaque application such as
weston-terminal over it.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
With the recent changes, weston could be started as user service. This
adds some examples and howtos one might do that. Includes some simple
systemd unit files.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Leak found running drm-smoke-test with ASan.
Do not forget to destroy the SHM buffer by the end of the test.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
This fixes a leak found running drm-smoke-test with ASan. Do not forget
to call prog_args_fini() when:
- skipping DRM-backend tests because we don't have
WESTON_TEST_SUITE_DRM_DEVICE set.
- we fail to acquire the lock needed to run DRM-backend tests.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
Leak found running drm-smoke-test with ASan.
Do not forget to free the launcher before returning when we can't open
the launcher fd. Also, just set 'out = launcher' after all error paths,
otherwise we give the caller a stale pointer.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
Leak found running drm-smoke-test with ASan.
Do not forget to free string specific_device in load_drm_backend().
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
We need to remove our listener link before we free the structure
it's inside, or the signal list walk will try to dereference it.
Remove the improbable NULL check at the same time.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
The code currently checked the return value of weston_config_section_get_bool()
which is incorrect. The return value of weston_config_section_get_bool() is
zero whether the config option is present or not, and it is non-zero in case
the config option is not present.
The code must check whether the config option is either true or false,
or in case the option is not present then default to false. Adjust the
code accordingly.
Reviewed-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Marek Vasut <marex@denx.de>
Since commit 992ee045f1 we create a new surface when we update the cursor
image. This broke animated cursors by discarding any existing frame
callback used for timing, and moving the setup for frame callbacks to
after the commit on the pointer surface.
To fix this we need another surface commit for the frame callbacks, but
this alone is not enough to fix the regression, as a lingering kludge
intended to fix problems when reusing the pointer surface is no longer
working as intended.
Since we no longer re-use the same surface, we can delete the old surface
on pointer exit, along with any callbacks set on it. Then a frame callback
will be recreated naturally. This lets us remove the now broken kludge
from the past and restore animated cursor functionality.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
We need to unlink this before freeing it since it's being called from
weston_signal_emit_mutable.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
weston_touch_destroy(), which is called from weston_seat_release(),
asserts that all its touch devices have been destroyed. The Wayland
backend currently destroys the touch devices ... immediately after
calling weston_seat_release().
Invert the ordering so that touch devices are destroyed first and we
don't trip over the assert.
Signed-off-by: Daniel Stone <daniels@collabora.com>
When EGL initialization fails (failure to create a GLES3 or GLES2
context) we will end up calling gbm_device_destroy() twice, once
in init_egl() and once in the drm_backend_create() error path.
Given that we should also take care of properly destroying the gbm
device when we don't have any inputs for instance, mark the gbm device
as NULL to avoid calling gbm_device_destroy() once more when destroying
the DRM-backend.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
As mesa includes gbm_bo_get_fd_for_plane() from 21.1.0 version onwards,
build the dma-buf feedback client only after that. This should provide
some sanity for package maintainers, as this would need pulling a
rather newer mesa version to build it (which might not be available).
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
When setting the window as opaque or fullscreen (which creates an opaque
region) make it so we don't have any alpha pixels set.
This was mistakenly dropped from a previous patch series update to
simple-egl.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Nothing special, but avoids hitting the warning about
terminating/finishing a layer with views on it.
Fixes#509.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This properly takes care of removing any of the views being added into
different layers upon shell destruction. Aggregates the shell_surface
destruction into one place to make things much more clearer. Keep the
animation part only the weston_desktop_surface destruction rather than
doing it at shutdown.
Fixes#509.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Replaces potential corruption signal emit call sites with the more safer
weston_signal_emit_mutable().
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This simulates an output removal which should trigger a crash when
the compositor is shutdown abruptly by having a view with a listener installed
on its output_destroy signal.
This patch assumes that weston_compositor_remove_output() has already
been amended to use the more safer version for triggering signal
emission.
As both shells use this construct it should catch any potential signal
corruption.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This uses the more safer version of signal emission to avoid a potential
crash when the output is destroyed that will follow a surface/view
destruction for which it has a listener attached (to the output_destroy
signal).
Fixes: #734
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
This avoids crashes due to removal of notification listeners from within
invocations of other listener callbacks in the same signal emission.
Fixes: #415
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Wayland signals (wl_signal) do not reliably handle changes to the
notification list during signal emission. Such scenarios occasionally
come up and can be difficult to investigate and debug.
This commit introduces the weston_signal_emit_mutable() function which can be
used in place of wl_signal_emit() and safely implements the following
behavior regarding notification list changes:
1. Listeners deleted during a signal emission and which have not already been
notified at the time of deletion are not notified by that emission.
2. Listeners added during signal emission are ignored by that emission.
The implementation of weston_signal_emit_mutable() is copied from the
wlr_signal_emit_safe() function of the wlroots project.
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
One of the best things about a real test framework is that they handle
timeouts for you.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reported-by: Simon Ser <contact@emersion.fr>
This was previously introduced with commit '.gitlab.ci: Enable debug for
libsteat and for the DRM backend' in order to figure out another CI
issue we were seeing.
Unfortunatelly, not keeping the return value after the tests ran it
would silently make the entire CI succeed when it should actually fail.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This avoids some unwarranted errors about conditional jumps or
invalid access from ASan as malloc doesn't set the memory to zero.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
According to xdg-shell spec, if the surface doesn't cover the whole
output we should center it and install a border fill covering the rest
of the output.
While we center out the surface we never got around installing the
border fill. This patch re-uses the activation of a surface to control
this bit as well, by making use of an new layer to place the
surface while not being active.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Walking the format/modifier list to try to find out if our FB is
compatible with the plane is surprisingly expensive. Since the plane's
capabilities are static over the lifetime of the KMS device, cache the
set of planes for which the FB is theoretically
format/modifier-compatible when it's created, and use that to do an
early cull of the set of acceptable planes.
Signed-off-by: Daniel Stone <daniels@collabora.com>
When we first create a drm_fb from a weston_buffer, cache it and keep it
alive as long as the buffer lives. This allows us to reuse the gbm_bo
and kernel-side DRM framebuffer, rather than constantly creating and
destroying them at every repaint. The overhead of doing so (e.g. MMU
updates) can be significant on some platforms.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Currently each drm_fb takes a reference on a client buffer it wraps.
This prevents us from being able to reuse a drm_fb in multiple places
(e.g. two views of the same client buffer) simultaneously, or even back
to back.
Move the buffer reference to the plane state, as preparation for
allowing drm_fb to be cached inside the weston_buffer.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Currently we take a reference on the underlying client buffer every time
we materialise a drm_fb from a view, and release it when the drm_fb is
destroyed. This means that we need to create and destroy a drm_fb every
time we want to use it, which is pathologically unperformant on some
platforms.
To start working towards being able to cache drm_fb, only take the
reference when we apply it to a plane state.
Signed-off-by: Daniel Stone <daniels@collabora.com>
No sense walking the plane list every frame if we can't use it because
it's neither a SHM buffer nor a client buffer we can directly import as
a framebuffer.
Signed-off-by: Daniel Stone <daniels@collabora.com>
If we still have a pending idle timer when the compositor is being
destroyed, make sure to free it first.
Signed-off-by: Daniel Stone <daniels@collabora.com>
I do not know what happens if multiple jobs declare
artifacts:reports:cobertura. Maybe the last job to finish wins? Who
cares, Gitlab can only show the results from one cobertura report in the
MR diff view.
Recently, the MR diffs have the coverage annotations, but there was a
time when they seemed to be missing. I don't know why.
Let's make sure the cobertura report Gitlab will use is from the "main"
build only: 64-bit Debian with full features and gcc.
I do not move the after_script, because maybe someone could have a
reason to look at arch-specific coverage reports, so I want to leave the
HTML there in the artifacts.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
drm-smoke-test can't run at the same time as anything else which touches
DRM devices. This includes any test which would use the GL renderer or
GL/GBM on the client side, since they will open DRM devices to probe
them at init time.
Signed-off-by: Daniel Stone <daniels@collabora.com>
As the drm-smoke test randomly reports having the connector disabled,
and with it libseat reports setMaster errors, this enables DRM backend
debug messages for the kernel, and for libseat in an attempt to
track down the issue, whenever it might happen again.
These are pretty harmless, in terms of data being generated as we only
have a single DRM test using VKMS, and the libseat debug message aren't
that verbose, so we're safe keeping them for the time being.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
EGLNativeWindowType can be a lot of different things, including a
pointer which an XID is not. Explicitly cast it through uintptr_t so we
don't throw build warnings either way.
Signed-off-by: Daniel Stone <daniels@collabora.com>
GStreamer can accept up to four planes for a buffer; gcc isn't _quite_
smart enough to figure out that only the first plane of offset/stride
will be accessed and so throws a warning that the params are too small.
Fix that by making them arrays.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Simple client to test the dma-buf feedback implementation. This does not
replace the need to implement a dma-buf feedback test that can be run in
the CI. But as we still don't know exactly how to do this, this client
can be helpful to run tests manually.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
In commit "libweston: add initial dma-buf feedback implementation" we've
added initial support to dma-buf feedback, but it was not using the
feedback from DRM-backend to add useful per-surface feedback.
In this patch we add this. The scanout tranche of the per-surface
feedback is based on the union of formats/modifiers of the primary and
overlay planes available. These are intersected with the
formats/modifiers supported by the renderer device.
Also, it's important to mention that the scene can change a lot and we
can't predict much. So this patch also adds hysteresis to the dma-buf
feedback. We wait a few seconds to be sure that we reached stability
before adding or removing the scanout tranche from dma-buf feedback and
resending them. This help us to avoid spamming clients and leading to
unnecessary buffer reallocations on their end.
Here's an example of what we want to avoid:
1. We detect that a view was not placed in a plane only because its
format is not supported by the plane, so we add the scanout tranche
to the feedback and send the events.
2. A few milliseconds after, the view gets occluded. So now the view
can't be placed in a plane anymore. We need to remove the scanout
tranche and resend the feedback with formats/modifiers optimal for
the renderer device. The client will then reallocate its buffers.
3. A few milliseconds after, the view that was causing the occlusion
gets minimized. So we got back to the first situation, in which the
format of the view is not compatible with the plane. Then we need to
add a scanout tranche and resend the feedback...
This patch is based on previous work of Scott Anderson (@ascent).
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
Signed-off-by: Scott Anderson <scott.anderson@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Pekka Paalanen
Marius Vlad
Derek Foreman
Leandro Ribeiro
Marek Vasut
Daniel Stone
Simon Ser
Alexandros Frantzis