Initially, `_XWAYLAND_ALLOW_COMMITS` was introduced in commit 7ace831ca
to avoid drawing the window content before it's ready to be shown.
But a repaint might also be triggered by the client damages before the
XWM has finished drawing its window decorations and drop shadows, which
previously was not too much of an issue since the XWM could still
finish updating the X11 window after the buffer was submitted.
However, with the addition of multiple window buffers in Xwayland [1]
which are aimed at preventing the X11 clients from updating the buffer
after it's been committed, this is no longer possible.
As a result, the use of multiple window buffers in Xwayland can cause
ugly repainting effects of the decorations if the buffer is submitted
before the XWM has finished painting its decorations.
Use the X11 property `_XWAYLAND_ALLOW_COMMITS` can be used to avoid
this, by controlling when Xwayland should commit changes to the Wayland
surface.
[1] https://gitlab.freedesktop.org/xorg/xserver/merge_requests/316
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
This introduces a new convention of checking through the compositor destroy
listener if the plugin is already initialized. If the plugin is already
initialized, then the plugin entry function succeeds as a no-op. This makes it
safe to load the same plugin multiple times in a running compositor.
Currently module loading functions return failure if a plugin is already
loaded, but that will change in the future. Therefore we need this other method
of ensuring we do not double-initialize a plugin which would lead to list
corruptions the very least.
All plugins are converted to use the new helper, except:
- those that do not have a destroy listener already, and
- hmi-controller which does the same open-coded as the common code pattern
did not fit there.
Plugins should always have a compositor destroy listener registered since they
very least allocate a struct to hold their data. Hence omissions are
highlighted in code.
Backends do not need this because weston_compositor_load_backend() already
protects against double-init. GL-renderer does not export a standard module
init function so cannot be initialized the usual way and therefore is not
vulnerable to double-init.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We have two kinds of libweston users: internal and external. Weston, the
frontend, counts as an external user, and should not have access to libweston
private headers. The shell plugins are external users as well, because we
intend people to be able to write them. Renderers, backends, and some plugins
are internal users who will need access to private headers.
Create two different Meson dependency objects, one for each kind.
This makes it less likely to accidentally use a private header.
Screen-share is a Weston plugin and therefore counts as an external user, but
it needs the backend API to deliver input. Until we are comfortable exposing
public API for that purpose, let it use internal headers.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
As 'new_subscription' can create additional objects, 'destroy_subscription'
will be needed when cleaning up.
As this requires a libweston_major bump (noticed by @pq), bump it up to
8.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Daniel Stone <daniel.stone@collabora.com>
The xwayland plugin (XWM) is a libweston plugin, so it has no business using
Weston things. Luckily its not, this was just a left-over include. Remove it to
reduce confusion.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Define common_inc which includes both public_inc and the project root directory.
The project root directory will allow access to config.h and all the shared/
headers.
Replacing all custom '.', '..', '../..', '../shared' etc. include paths with
common_inc reduces clutter in the target definitions and enforces the common
#include directive style, as e.g. including shared/ headers without the
subdirectory name no longer works.
Unfortunately this does not prevent one from using private libweston headers
with the usual include pattern for public headers.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
When all shared/ headers are included in the same way, we can drop unnecessary
include seach paths from the compiler.
This include style was chosen because it is prevalent in the code base. Doing
anything different would have been a bigger patch.
This also means that we need to keep the project root directory in the include
search path, which means that one could accidentally include private headers
with
#include "libweston/dbus.h"
or even
#include <libweston/dbus.h>
IMO such problem is smaller than the churn caused by any of the alternatives,
and we should be able to catch those in review. We might even be able to catch
those with grep in CI if necessary.
The "bad" include style was found with:
$ for h in shared/*.h; do git grep -F $(basename $h); done | grep -vF '"shared/'
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Turns out dnd.c does not actually need cairo-util.h. This was found when
unifying the include directives of all shared/ headers. Removing this makes one
less place to fix.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
As we transition towards a more generic API for weston loggging
framework rename weston_debug_compositor to weston_log_context to show
the fact that this is not really debug but a logging context.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
The printf() format specifier "%m" is a glibc extension to print
the string returned by strerror(errno). While supported by other
libraries (e.g. uClibc and musl), it is not widely portable.
In Weston code the format string is often passed to a logging
function that calls other syscalls before the conversion of "%m"
takes place. If one of such syscall modifies the value in errno,
the conversion of "%m" will incorrectly report the error string
corresponding to the new value of errno.
Remove all the occurrences of the specifier "%m" in Weston code
by using directly the string returned by strerror(errno).
While there, fix some minor indentation issue.
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
weston.h is a Weston frontend header, while this is a libweston plugin. A
libweston plugin cannot depend on Weston. Luckily the header is not actually
needed.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The main idea is to make libweston users use the form
#include <libweston/libweston.h>
instead of the plain
#include <compositor.h>
which is prone to name conflicts. This is reflected both in the installed
files, and the internal header search paths so that Weston would use the exact
same form as an external project using libweston would.
The public headers are moved under a new top-level directory include/ to make
them clearly stand out as special (public API).
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This crash was happening *during resizing* of an xwayland window that was destroyed.
Discovered by: John Good @archiesix
[@daniels: Moved tests below declarations.]
Use the pattern to avoid identing everything, when everything in the
file is under the one conditional.
Helps readability.
Signed-off-by: Pekka Paalanen <pq@iki.fi>
Meson is a build system, currently implemented in Python, with multiple
output backends, including Ninja and Make. The build file syntax is
clean and easy to read unlike autotools. In practise, configuring and
building with Meson and Ninja has been observed to be much faster than
with autotools. Also cross-building support is excellent.
More information at http://mesonbuild.com
Since moving to Meson requires some changes from users in any case, we
took this opportunity to revamp build options. Most of the build options
still exist, some have changed names or more, and a few have been
dropped. The option to choose the Cairo flavour is not implemented since
for the longest time the Cairo image backend has been the only
recommended one.
This Meson build should be fully functional and it installs everything
an all-enabled autotools build does. Installed pkg-config files have
some minor differences that should be insignificant. Building of some
developer documentation that was never installed with autotools is
missing.
It is expected that the autotools build system will be removed soon
after the next Weston release.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Co-authored-by: Pekka Paalanen <pq@iki.fi>
Signed-off-by: Pekka Paalanen <pq@iki.fi>
Instead of a compile time choice, offer the XWM debugging messages
through the weston-debug protocol and tool on demand. Users will not
need to recompile weston to get XWM debugging, and it won't flood the
weston log.
The debug scope needs to be initialized in launcher.c for it be
available from start, before the first X11 client tries to connect and
initializes XWM.
Signed-off-by: Pekka Paalanen <pq@iki.fi>
pass the wm_debug scope to weston_debug_scope_printf API to append
the scopename to the timestr
Signed-off-by: Maniraj Devadoss <Maniraj.Devadoss@in.bosch.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
This is preparation for using the weston-debug infrastructure for
WM_DEBUG. dump_property() may be called from different debugging
contexts and often needs to be prefixed with more information.
An alternative to this patch would be to pass in the weston_debug_scope
as an argument to dump_property(), but then all callers would need to be
converted to weston-debug infra in a single commit.
Therefore require the callers to provide the FILE* to print to.
Signed-off-by: Pekka Paalanen <pq@iki.fi>
Signed-off-by: Maniraj Devadoss <Maniraj.Devadoss@in.bosch.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Write the output of dump_property() out in one log call. When multiple
processes (weston and Xwayland) are writing to the same file, this will
keep the property dump uninterrupted by Xwayland debug prints.
This is also preparation for more development in the same direction.
Signed-off-by: Pekka Paalanen <pq@iki.fi>
Signed-off-by: Maniraj Devadoss <Maniraj.Devadoss@in.bosch.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Very useful for TRANSIENT_FOR property debugging.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.com>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Add code to dump CARDINAL arrays from properties. Useful for debugging.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.com>
Reviewed-by: Ian Ray <ian.ray@ge.com>
This (partially) reverts commit bef761796c.
This (partially) reverts commit 4d1cd36c9e.
This (partially) reverts commit 44fc1be913.
This (partially) reverts commit 6b58ea8c43.
The new xwm icon code has proven to be leaky and incomplete, and while
we have patches under consideration to fix the rest of its known problems
they still require changes and review cycles. Currently the known
leaks have been squashed, but it still picks wrong sized icons and
does no scaling, which can lead to very strange rendering. At window
close time the wrong sized icon appears above the window during fade out.
This patch reverts the mostly solid bits and keeps the unfinished
bits behind in favor of a simpler revert than removing the whole
thing.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
This reverts commit 332d1892bb.
And re-introduces the bug it was intended to fix, see:
https://lists.freedesktop.org/archives/wayland-devel/2017-December/036402.html
Reverting this because it causes harm to all xwayland clients - the
input region no longer gets adjusted when resizing windows.
start an xterm, resize it larger, you can no longer interact with the
new area of the window (including the server side decor).
Hopefully sort the last leaks introduced in commit 6b58ea8c
The window could be destroyed before it had a frame but after it had an icon
(I could trigger this with firefox), and the window could be assigned an icon
twice before it had a frame (I could trigger this with terminology).
The latter leak was
Reported-by: Scott Moreau <oreaus@gmail.com>
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
A memory leak introduced by 6b58ea8c led to me finding a bigger leak,
which is xwm was calling frame_create() without calling frame_destroy().
This meant that the associated icon_surface was not being destroyed,
leaving the destroy handler for it broken. Here we fix this by calling
frame_destroy() when the window is destroyed and free the reply in
the icon_surface destroy handler.
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
This reverts commit d2cb711d81.
I missed the call to cairo_image_surface_create_for_data() which assumes
the data will remain present until the cairo surface is destroyed. It
seems the existence of data depends on the reply not being freed.
This will need a more involved fix.
Sorry, I noticed this just seconds after I pushed the patch.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Fix memory leak introduced by 6b58ea8c. weston_wm_handle_icon() was
calling xcb_get_property_reply() without freeing the reply.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
The cairo surface used for the icon must be completely given to the
frame as soon as said frame has been created. To prevent both the
window and the frame from sharing ownership of the icon, we set
window->icon_surface back to NULL right after creating or changing the
frame, only keeping it there when no frame has been created yet.
Fixes https://lists.freedesktop.org/archives/wayland-devel/2018-January/036655.html
Reported-by: Derek Foreman <derekf@osg.samsung.com>
Tested-by: Derek Foreman <derekf@osg.samsung.com>
Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
commit e7fff215ad made initializing the
selection_listener conditional, but didn't make its clean-up
conditional at shutdown. Simply initializing the listener's list
link at init time makes this harmless.
To see this, run weston -Bheadless-backend.so and then connect to it
with an X client. When killing weston it will attempt shutdown but
die with a segfault.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
This fetches the _NET_WM_ICON property of the X11 window, and use the
first image found as the frame icon.
This has been tested with various X11 programs, and improves usability
and user-friendliness a bit.
Changes since v1:
- Changed frame_button_create() to use
frame_button_create_from_surface() internally.
- Removed a check that should never have been commited.
Changes since v2:
- Request UINT32_MAX items instead of 2048, to avoid cutting valid
icons.
- Strengthen checks against malformed input.
- Handle XCB_PROPERTY_DELETE to remove the icon.
- Schedule a repaint if the icon changed.
Changes since v3:
- Keep the previous Cairo surface until the new one has been
successfully loaded.
- Use uint32_t for cardinals. Unsigned is the same type except on
16-bit machines, but uint32_t is clearer.
- Declare length as uint32_t too, like in xcb_get_property_reply_t.
Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
The window frame was created with position and size which include
an offset for margins and shadow. Set the input region to ignore
shadow.
[daniels: Fixed type mismatch, removed unused variable.]
Signed-off-by: Ian Ray <ian.ray@ge.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Tested-by: Scott Moreau <oreaus@gmail.com>
If we configure a window with the same size and wait for the
sync alarm to go off, the resizing is gonna block. The event is
only handled is the size actually changed.
Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
A single client message can be used to modify two properties at once.
That's why when processing such messages we have to check both the second
and the third data entry for states that we must handle.
Signed-off-by: Ilia Bozhinov <iliyabo@hotmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
When we receive configure_notify we should update the surface's
position by calling xwayland_api->set_xwayland(). Otherwise some surfaces
like dnd surfaces from xwayland views are "stuck" at one place. When
setting XWAYLAND state though we should always call view_set_position(),
not just the first time we set this state.
Signed-off-by: Ilia Bozhinov <ammen99@gmail.com>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
xwm would not let X clients change the focus behind the compositor's
back, reverting focus where it's supposed to be when this occurs.
However, X11 grab also issue focus in events, on which some clients
rely and reverting focus in this case braks the client logic (e.g.
combobox menu in gtk+ using the X11 backend).
Check if the focus event is actually coming from a grab or ungrab and
do not revert focus in this case, to avoid breaking clients logic.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Tested-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Acked-by: Daniel Stone <daniels@collabora.com>
Acked-by: Quentin Glidic <sardemff7+git@sardemff7.net>
This patch uses the new feature proposed for Xwayland in the patch
series https://patchwork.freedesktop.org/series/16610/ .
When the frame window is created, immediately forbid Xwayland commits on
it. This prevents commits before the decorations have been drawn and the
initial pending state has been set. Commits are enabled right after
drawing and setting.
This ensures that the decorations are fully drawn when a window is
mapped. This also solves the initial commit/pending race, but the race
is on again after mapping.
If Xwayland does not implement the needed support, we are just setting a
window property with no effect.
This patch is the final piece for solving T7622, excluding the
_NET_WM_SYNC_REQUEST handling.
Task: https://phabricator.freedesktop.org/T7622
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Normal windows enter the MapRequest handler, which schedules drawing the
decorations. Then Xwayland realizes the window, which ends with a call
to xserver_map_shell_surface(). The decorations are already drawn, no
need to draw them a second time. However, MapRequest handler could not
set the pending state because the weston_surface did not exist at the
time, because it gets created only when Xwayland realizes the window,
which happens after XWM has forwarded the MapWindow in MapRequest
handler. Therefore set the pending state explicitly at the end.
Scheduling had it done much later anyway.
Now that the pending state is set much earlier, it seems to be more
likely that it gets set before Xwayland's first commit is handled. This
means that -geometry command line option of X11 apps already takes the
geometry (decorations) into account. I do not think it is reliable yet,
though.
There is still the race between Xwayland committing and XWM setting the
pending state assuming the very next commit latches it in appropriately.
The race exists not because of Wayland, but because WL_SURFACE_ID comes
via X11, and could be processed after wl_compositor.create_surface and
wl_surface.commit. That commit/pending race is solved by a following patch.
For override-redirect windows weston_wm_window_schedule_repaint()
reduced into a call to weston_wm_window_set_pending_state_OR(), so we
can just call that directly. It should not matter that the call is moved
to the end of the function.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Acked-by: Daniel Stone <daniels@collabora.com>
To me it was not obvious that this call is necessary, so provide some
rationale.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Link Mauve
Olivier Fourdan
Pekka Paalanen
Marius Vlad
Antonio Borneo
Emmanuel Gil Peyrot
Greg V
Pekka Paalanen
Daniel Stone
Pekka Paalanen
Derek Foreman
Scott Moreau
Ian Ray
Louis-Francis Ratté-Boulianne
Ilia Bozhinov
Ilia Bozhinov