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>
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>
When we as the WM tell the X server to map a window, it gets mapped. We
can start drawing into it immediately. There is no reason to wait for
any other events before drawing the decorations.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
This comes via Wayland, WL_SURFACE_ID comes via X11. They race. Nice to
get both printed.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Having it in a separate function makes it more clear what it is, and
allows it to be called from elsewhere.
This really is the set_pending_state() alternative for override-redirect
windows, because OR windows do not get a frame window created. Also OR
windows will never hit the normal set_pending_state() because
weston_wm_window_schedule_repaint() special-cased windows without a
frame window and returned early without scheduling.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Move the region fini just above the region init. They are a pair and
belong togeether. Split a long line.
Reads better this way. No functional changes.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Split the function into two:
- weston_wm_window_draw_decoration() that only draws the decorations
with Cairo, and
- weston_wm_window_set_pending_state() which sets up the surface state
to be latches into use on the next commit from Xwayland.
The new weston_wm_window_do_repaint() is the equivalent of the old
weston_wm_window_draw_decorations(), everything still happens the same
way as it was. Just some debug messages have been reworded.
weston_wm_window_read_properties() is moved into
weston_wm_window_do_repaint() because it is not strictly a part of
drawing decorations. The same with resetting repaint_source.
draw_decorations does not need the child position nor xwayland
interface. Also some convenience variables have been eliminated.
set_pending_state code has been un-indented by one level, so the change
is best viewed with whitespace changes ignored.
This patch makes the code more readable, and prepares for calling the
draw_decorations and set_pending_state from different places.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
X11 applications expect -geometry command line option to work for
setting the initial window position, but currently this does not work.
During map, detect X11 windows that set an explicit position. This works
by heuristics: if window position is not 0,0 then it is explicitly
positioned. Legacy fullscreen windows are also at 0,0 but these are
detected earlier.
Explicitly store the window position at map request time to detect
client-positioned windows, and use it as the suggested initial position.
weston_wm_window::x and y have been overwritten due to reparenting when
we eventually need the initial position.
This patch requires that the new set_toplevel_with_position() hook is
implemented in the shell.
Note that this patch is about positioning xwayland toplevels, not
override-redirect windows which are already handled.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Reviewed-by: Daniel Stone <daniels@collabora.com>
To reproduce the problem:
- start weston (x11 backend worked, glamor in Xwayland makes no
difference)
- start xterm
- very very slowly move the pointer in the xterm decorations onto or
away from a button
- the moment the decorations are updated, they will appear incomplete,
e.g. completely without buttons and title text
- if you cause just one more pointer motion event, the decorations will
update to completely drawn appearance
Another way to reproduce the problem is to have an xterm and change its
window title. This is easy if you use a shell prompt that updates the
terminal window title. When the title updates, decorations will be
half-drawn until something happens in XWM.
The fix: flush.
Apparently the drawing commands did not get flushed to the X server
until any other X11 action pushed them through.
xcb_flush() is the real fix here. cairo_surface_flush() is added just
for good measure, because documentation indicates it would be better
used, however it was not strictly necessary to fix the problem in my
experiments.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Tested-by: Emmanuel Gil Peyrot <emmanuel.peyrot@collabora.com>
Reviewed-by: Emmanuel Gil Peyrot <emmanuel.peyrot@collabora.com>
Obviously unused. Looks like weston_wm_window_activate() is doing that
job.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Changing the opaque region has no immediate effect, therefore there is
no need to mark the view geometry dirty.
The view geometry will be invalidated automatically by the next commit
from Xwayland, in weston_surface_commit_state(). The dirtying did not
apply pending state.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Use wm_log_continue() to avoid printing the timestamp in the middle of a
message.
Print the name of the property that got deleted.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
For every event we handle and that delivers the override-redirect flag,
print it to debug log.
Add a comment to one code path explaining when it gets hit, because it
is unobvious. It also serves as a reminder that we do not handle changes
to the OR flag after Window creation.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Move the calls to set_title() and set_pid() out of
weston_wm_window_read_properties() and into the three callers, each
slightly different.
xserver_map_shell_surface(): already calls these functions after
creating the shell surface, so no need to add calls.
weston_wm_handle_map_request(): can be called only on unmapped (in X11)
Windows, so no need to add calls.
weston_wm_window_draw_decoration(): window->shsurf and window->surface
are either both set or both NULL, so the check for window->shsurf is
removed when moving the set_title() and set_pid() calls under a
window->surface check.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
The only thing using the frame title is frame_repaint(). Move the call
to frame_set_title() from weston_wm_window_read_properties() into
weston_wm_window_draw_decoration() where the only call to
frame_repaint() is.
Do not check for window->name == NULL, because frame_set_title() handles
NULL just fine. Also, once window->name becomes set, it cannot become
NULL again unless strndup() fails. The name string can be reset to
the empty string in any case.
This change is prompted by future refactoring where at
weston_wm_window_read_properties() time the frame might not have been
created yet.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
The props array contained offsets to struct members. It is convenient
for writing static const arrays as you only store a constant offset and
compute the pointer later. However, the array was not static to begin
with, the atoms are not build time constants. We can as well just store
the pointer directly in the array.
Entries that did not use the offset had bogus offsets, producing
pointers to arbitrary fields. They are changed to have a NULL pointer.
If the code unintentionally used the pointer, it will now explode rather
than corrupt memory.
Also explain the use of the #defined constants and #undef them when they
get out of scope. This clearly documents that they are just a convenient
hack to avoid lots of special cases in the function.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
The legacy fullscreen state needs to be detected at MapRequest time,
because that is when the X11 client has alredy set up the initial window
state.
Doing it at xserver_map_shell_surface() meant that it would be done as a
response to Xwayland creating the wl_surface and XWM receiving the
WL_SURFACE_ID ClientMessage, whichever came later. At that point the X11
client might still be setting things up in theory, though in practice
most of the X11 communication has already happened when
xserver_map_shell_surface() gets called.
The real reason for this is to clean up xserver_map_shell_surface() from
everything that would affect drawing the decorations. This patch is one
part of that clean-up.
The weston_output_weak_ref logic is not put into compositor.h, because
there are no other users for it at this time. We need to protect against
the output going away.
A side-effect of this patch is that saved_width and saved_height will
now get overwritten also for legacy fullscreen windows. Previously, they
were left to zero as far as I could tell.
NOTE: This stops override-redirect legacy fullscreen windows from being
detected as fullscreen. MapRequest processing does not happen for OR
windows. These windows get detected as type XWAYLAND instead.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Add WM debug prints on map, decoration drawing and geometry setting.
These help see the sequence and timing of operations, when debugging
Xwayland window management glitches.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Helps debugging initial placement problems.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Helps debugging X11 window positioning issues.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Starting an xterm with no input device led to a crash
because weston_wm_pick_seat() was returning garbage and
weston_wm_selection_init() was trying to use the garbage.
Signed-off-by: Tom Hochstein <tom.hochstein@nxp.com>
Reviewed-by: Giulio Camuffo <giuliocamuffo@gmail.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
compositor.h already helpfully defines WL_HIDE_DEPRECATED for us, so we
don't get warnings about wl_buffer (in particular) being deprecated when
we have wayland-server headers defining it as deprecated, and then
wayland-client headers using the type.
Move it to be before all our other includes, so we actually make use of
it.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Yong Bakos <ybakos@humanoriented.com>
Tested-by: Yong Bakos <ybakos@humanoriented.com>
This silences two warnings:
clients/window.c:2450:20: warning: implicit conversion from enumeration
type 'enum wl_pointer_button_state' to different enumeration type 'enum
frame_button_state' [-Wenum-conversion]
button, state);
^~~~~
clients/window.c:2453:15: warning: implicit conversion from enumeration
type 'enum wl_pointer_button_state' to different enumeration type 'enum
frame_button_state' [-Wenum-conversion]
button, state);
^~~~~
Warning produced by Clang 3.8.
Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Reviewed-by: Giulio Camuffo <giuliocamuffo@gmail.com>
The xwm used to automatically send to Xwayland the position of X windows
when that changed, using the x,y of the primary view of the surface.
This works fine for the desktop shell but less so for others.
This patch adds a 'send_position' vfunc to the weston_shell_client that
the shell will call when it wants to let Xwayland know what the position
of a window is.
The logic used by the desktop-shell for that is exactly the same the xwm
used to have.
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: David Fort <contact@hardening-consulting.com>
Patch updated to remove dead lines as suggested by Daniel Stone
Signed-off-by: Chris Michael <cp.michael@samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
The xwm gets a primary view for a X window using the get_primary_view
vfunc of the shell_interface struct. Storing it is dangerous though
because it doesn't listen for its destruction so it may end up using the
old stored view pointer after that view was freed, or after the primary
view for that window was changed to another one.
Fetch the primary view just before using it every time and try to not
abuse this 'primary view' concept which may map badly to some shells:
iterate over all the views instead when it makes sense.
Tested-by: Benoit Gschwind <gschwind@gnu-log.net>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
xwayland source is checked, so it dispatches twice on any event.
If the other turn has no events to dispatch, we flush the connection
redundantly
v2. do not flood logs with 'unhandled event' messages
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
- opening braces are on the same line as the if statement
- opening braces are not on the same line as the function name
- space between for/while/if and opening parenthesis
Signed-off-by: Dawid Gajownik <gajownik@gmail.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
When we get a focus in event from an X window which is not the one
we last set as the active window, reset the focus.
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
An earlier patch made surface_resize() and surface_move() take pointers
instead of seats, this updates the weston_shell_interface resize and move to
match.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Keyboards and pointers aren't freed when devices are removed, so we should
really be testing keyboard_device_count and pointer_device_count in most
cases, not the actual pointers. Otherwise we end up with different
behaviour after removing a device than we had before it was inserted.
This commit renames the touch/keyboard/pointer pointers and adds helper
functions to get them that hide this complexity and return NULL when
*_device_count is 0.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
To help reduce code duplication and also 'kitchen-sink' includes
the ARRAY_LENGTH macro was moved to a stand-alone file and
referenced from the sources consuming it. Other macros will be
added in subsequent passes.
Signed-off-by: Jon A. Cruz <jonc@osg.samsung.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
Scott Moreau
Emmanuel Gil Peyrot
Ian Ray
Louis-Francis Ratté-Boulianne
Ilia Bozhinov
Ilia Bozhinov
Olivier Fourdan
Pekka Paalanen
Abdur Rehman
Tom Hochstein
Daniel Stone
Quentin Glidic
Jussi Kukkonen
Jonas Ådahl
Giulio Camuffo
Chris Michael
Benoit Gschwind
Marek Chalupa
Dawid Gajownik
Derek Foreman
Jon Cruz
Bryce Harrington