The fullscreen state for xwayland surfaces can currently only be
effectively set from the client side. This commit enables
libweston-desktop based shells to properly set the fullscreen state
for xwayland surfaces.
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
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>
Since we want to pass the view to the surface activation listener inside the
constraints code, and the surface is reachable from the view anyway.
The flags field will let us pass the reason for activation to the constraints
code, which will then handle especially the fullscreen case.
Signed-off-by: Sergio Gómez <sergio.g.delreal@gmail.com>
I tried this, and it causes a crash. Leave a note for the future when we
happen to use some other backend with xwayland and find a "leak".
The reason this is a comment and not a Gitlab issue is that you probably
would not go looking for a Gitlab issue saying an idea is a bad one.
This comment is more likely to be found.
It's not really a leak either, it only needs to be fixed if you want a
clean ASan leak report.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
In future code the window link will end up in a state where it may or may
not be on the unpaired_window_list and we'll want to go from that state
to one where it's definitely not on the list.
Initting the list after removal (in these two places) allows us to
unconditionally remove it later.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
The hash table implementation is useful for other modules as well. Move it from
xwayland to the shared code.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
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 ba82af938a
"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>
If we're minimized from maximized or fullscreen state, we want to leave
the saved size alone, so we can restore it if we clear fullscreen or
maximized state later.
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
This fixes a problem that can be seen with Firefox running under xwayland.
- Start it (or another client that remembers its size and maximized state),
- resize it
- maximize it
- exit it
- restart it
- unmaximize it
The size will be changed to a default size and not the previous unmaximized
size.
To fix this, save the unmaximized height and width like we do for the
fullscreen case.
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>
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
It's possible that there are no seats present at startup (especially with
the RDP backend, which creates and removes seats for connections), and
previously we'd just fail to set up XWayland cut and paste properly.
We should set up a listener and find a seat when one becomes available -
but we also need to switch seats if ours is removed.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Co-authored-by: Hideyuki Nagase <hideyukn@microsoft.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Memleak found by ASAN:
Direct leak of 21 byte(s) in 1 object(s) allocated from:
#0 0x7fe7a917fe8f in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.6+0xa9e8f)
#1 0x7fe7a9129874 (/usr/lib/x86_64-linux-gnu/libasan.so.6+0x53874)
#2 0x7fe7a5a23469 in weston_wm_window_read_properties ../xwayland/window-manager.c:574
#3 0x7fe7a5a28d3b in weston_wm_handle_map_request ../xwayland/window-manager.c:1178
#4 0x7fe7a5a31660 in weston_wm_handle_event ../xwayland/window-manager.c:2291
#5 0x7fe7a8c261a1 in wl_event_loop_dispatch ../src/event-loop.c:1027
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Memleak found by ASAN:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7fe7a917fe8f in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.6+0xa9e8f)
#1 0x7fe7a5a40736 in theme_create ../shared/cairo-util.c:419
#2 0x7fe7a5a3363c in weston_wm_create ../xwayland/window-manager.c:2619
#3 0x7fe7a5a2017e in weston_xwayland_xserver_loaded ../xwayland/launcher.c:313
#4 0x7fe7a90b4d14 in handle_sigusr1 ../compositor/xwayland.c:57
#5 0x7fe7a8c2585d in wl_event_source_signal_dispatch ../src/event-loop.c:685
#6 0x7ffcdb04ef6f ([stack]+0x1df6f)
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
There is more state than just the application window width and height that
affects whether calling weston_wm_window_configure() is necessary: everything
that affects the frame window, fullscreen state in particular. Therefore do not
skip the call by just width and height.
If send_configure() happens to be called "unnecessarily", this will now forward
some of those calls to the X11 clients. However, since it uses an idle task, it
should not result in a flood at least. And if send_configure() is spammed,
maybe that should then be fixed in its callers.
This patch should fix the misplacement of a fullscreen X11 window due to the
frame window being incorrectly sized and positioned, and the app window
incorrectly positioned inside the frame window.
The fullscreen window problems were observed in a case where the window does
not hit legacy_fullscreen() but first maps and then sets
_NET_WM_STATE_FULLSCREEN. Additionally the initial window size must match the
output size where it gets fullscreened. In that case the frame window was left
as if not fullscreened.
This practically reverts 3f53d9179b. I'm not sure
what problem that patch was fixing, but I couldn't make any resizing freeze.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Spotted this in debug log:
[xwm-wm-x11] XWM: configure window 4194324: x=32 y=32 width=1920 height=1080 border_width=0 stack_mode=0
[xwm-wm-x11] XWM: configure window 0: width=1984 height=1144
Trying to configure window 0 makes no sense. So do not try.
To avoid patching two different places with the same thing, refactor the code
into a common helper.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
It would lead to use-after-free if there was a pending idle callback to
weston_wm_window_configure() when the weston_wm_window gets destroyed. Make
sure the callback will not fire.
Found by inspection.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This function is called also directly from weston_wm_window_set_toplevel(). If
configure_source is set at that point, simply resetting the pointer will "leak"
the source until it fires and calls this function again.
Let's keep the variable up-to-date by removing the source when called,
dispatched or not. This removes the second call. I only hope it doesn't cause
issues. This is also necessary if we intend to remove the source on window
destruction too.
Found by inspection.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
It looks like commit ad0da4596d introduced a bug
for X11 windows that are initially fullscreen by adding code to the end of
xserver_map_shell_surface() while ignoring the 'return' that this patch
removes. That may have caused some annoying window state issues, but the
problem became more pronounced with 7ace831ca6
when used with an Xwayland version that honours _XWAYLAND_ALLOW_COMMITS.
In the latter case, there is a possiblity the window will never show up, as XWM
forgets to set allow_commits=true. However, the window may sometimes actually
show up due to an oversight in Xwayland: the Present code may be flipping the
window buffers and not checking _XWAYLAND_ALLOW_COMMITS if it is supposed
commit at all.
Since then, f568968f8a added more places where
allow_commits is set to true, masking the window-does-not-show-up issue. Window
pending state likely still remained an issue.
This patch properly fixes the "window never appears" issue by making sure
allow_commit=true is set. At the same time, it ensures the pending state
functions are called at the end of xserver_map_shell_surface(), which may fix
some window state issues like misplaced decorations and/or position of
initially-fullscreen windows. Unfortunately, it certainly does not fix all such
state problems.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The mask argument is uint16_t so declare the variable with the same.
Suggested-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Knowing the kind of decoration drawn will help track down issues with
unexpected decorations.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Print the changes to the debug scope, helping to figure out why Xwayland is or
is not committing.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Add the missing newline to printing a property that is of type cardinal array.
Fixes messed up debug scope output.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
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>
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>
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.]
