In particular, modified public creator and destructor function names,
added a display destroy listener, safely extract user data from
resources, send correct time (in usecs) in rootston, etc.
This is a common interface that can be used for all primary selection
protocols, as discussed in [1]. A new function wlr_seat_set_primary_selection
is added to set the primary selection for all protocols.
The seat now owns again the source, and resets the selection to NULL when
destroyed.
[1]: https://github.com/swaywm/wlroots/issues/1367#issuecomment-442403454
This commits completely refactors wlr_gtk_primary_selection. The goal is to
remove gtk-primary-selection state from the seat and better handle inert
resources where it makes sense.
wlr_seat_client.primary_selection_devices has been removed and replaced by
wlr_gtk_primary_selection_device. This allows us to make offers inert when the
current selection is replaced.
wlr_seat_set_primary_selection has been removed because it relied on wlr_seat
instead of wlr_gtk_primary_selection_device_manager. A new function,
wlr_gtk_primary_selection_device_manager_set_selection (candidate for the
longest function name in wlroots) has been added. It doesn't take a serial
anymore as serial checking only makes sense for set_selection requests coming
from Wayland clients (serial checking is now done in the Wayland interface
implementation).
Since wlr_gtk_primary_selection_device_manager is now required to set the
selection, a new function wlr_xwayland_set_gtk_primary_selection_device_manager
(candidate number two for longest function name) has been added.
Devices are now made inert when the seat goes away.
Future work includes removing the last primary selection bits from the seat,
mainly wlr_seat.primary_selection_source and wlr_seat.events.primary_selection,
replacing those with new fields in wlr_gtk_primary_selection_device. Or maybe
we could keep those in the seat and replace them with a re-usable interface
(for future zwp_primary_selection_v1 support). We need to think how we'll sync
these three protocols (GTK, X11 and wayland-protocols).
See https://github.com/swaywm/wlroots/issues/1388
wlr_subsurface_from_wlr_surface can return NULL if the wl_surface is still
alive and if the wl_subsurface has been destroyed. Make sure we check for NULL.
Fixes https://github.com/swaywm/sway/issues/3195
This commit makes it possible for a single client to have multiple data devices
for the same seat. This fixes issues with Firefox.
This mainly removes wlr_data_source.offer. We make sure we create one data
offer per device. We now make the offer inert when the source is destroyed.
Fixes the second half of https://github.com/swaywm/wlroots/issues/1041
The read format is dependent on the output, so we first need to make it
current. This fixes a race condition in wlr-screencopy-v1 where a dmabuf
client would cause EGL_NO_SURFACE to be bound at the time when
screencopy needs to query for the preferred format, causing GL errors.
When a client was creating multiple data devices for the same seat, we were
only creating one resource. This is a protocol error.
Instead, create one offer per data device.
This commit also makes offers inert when their source is destroyed.
Fixes part of https://github.com/swaywm/wlroots/issues/1041
Supersedes https://github.com/swaywm/wlroots/pull/1113
ie. don't destroy surface->toplevel on xdg_toplevel destroy. Instead do this on
xdg_surface destroy.
This allows compositors to add toplevel listeners when the surface appears and
remove them when the surface is destroyed.
We were assuming GL_BGRA_EXT was always supported.
We now check that it's supported for rendering. We fail if it isn't because
this format is specified as "always supported" by the Wayland protocol.
We also check if it's supported for reading pixels. A new preferred_read_format
function returns the preferred format that can be used to read pixels. This is
used by the screencopy protocol.
There was a missing copy_drm_surface_mgpu call in drm_connector_schedule_frame
so we asked for a pageflip with an unknown BO, resulting in ENOENT.
Additionally, this commit makes schedule_frame return a bool indicating
failures. This allows schedule_frame_handle_idle_timer to only set
frame_pending to true if a frame has been successfully scheduled. Thus, if a
pageflip fails, rendering won't be blocked forever anymore.
In case a pageflip is already pending, true is returned because a frame has
already been scheduled and will be sent sometime soon.
shm_open is a POSIX function creating an in-memory file. Using it simplifies
the code and removes the dependency on XDG_RUNTIME_DIR. The only downside is
that we need to generate a random name for the shm file.
Prior to this commit, setting up a zero timeout resulted in a timer that never
expires, which isn't particularly useful.
This commit allows setting up timers that expire immediately, which is useful
to immediately enter idle state.
wlr_cursor: make sure the output doesn't have a cursor before
creating a new one
wlr_output_layout: don't emit the "add" event when the output is
already in the layout
The prevoius implementation would always raise an error in the following sequence:
-> enter
-> leave
<- enable
The text-input type is not equipped to manage the validity of clents' requests, which should be handled in the compositor, as rootston does.
If a client uses an older version of the dmabuf protocol, use the
`formats` event instead of `modifiers` (since that didn't exist in older
versions).
With a bit of necessary guessing, support dmabuf importing even when
EGL_EXT_image_dma_buf_import_modifiers isn't present instead of
failing up front.
This prevents the idle event to be activated on a destroyed
output.
This also makes the backend responsible for free-ing modes, as it
is the one allocating them and adding them to the list. Note that
the DRM backend (the only one using modes) already frees them.
Since the source doesn't always come from a client, this field
doesn't make sense. It is replaced by a new "finalized" field in
wlr_client_data_source. This is used to make sure set_actions is
not sent after start_drag has been sent.
A check in data_offer_choose_action has been removed: if an offer
has been sent then start_drag has been called, no need to check.
I also wanted to add a check for wl_data_source.offer, but it
turns out (1) this isn't in the spec (2) it breaks GTK+.
This is some preliminary work for Firefox on Wayland compatibility.
This desynchronizes our rendering loop with the vblank cycle.
In case a compositor doesn't swap buffers but schedules a frame,
emitting a frame event immediately enters a busy-loop.
Instead, ask the backend to send a frame when appropriate. On
Wayland we can just register a frame callback on our surface. On
DRM we can do a no-op pageflip.
Fixes#617Fixesswaywm/sway#2748
When we send an output frame event, we should not assume that the
compositor is going to call wlr_output_swap_buffers in response to it.
If the compositor does not swap the buffers, the idle event source still
exists and is executed every time the Wayland event loop becomes idle,
which means we send frame events repeatedly until the buffers are
swapped.
This moves the removal of the idle event source out of
wlr_output_swap_buffers and into wlr_output_send_frame, where it is
guaranteed to be removed.
This calculates and returns the effective damage of the surface in
surface coordinates, including the client damage (in buffer
coordinates), and damage induced by resize or move events.
* Rename the constraint_create signal to new_constraint for
consistency
* Move the constraint_destroy signal to the constraint itself
* Use rotate_child_position instead of duplicating logic
* Fix inert constraint resource handling
* Style fixes
I do not think the conversion is specifically defined, but on my system and SirCmpwn's
the floats are rounded instead of floored, which is incorrect in this case, since
for a range from 0 to 256, any value greater or equal to 0 and less than 256 is valid.
I.e. [0;256[, or 0 <= x < 256, but if x is e.g. -0.1, then it will be rounded to 0, which
is invalid. The correct behavior would be to floor to -1.
To prevent wl_keyboard keymap being written to by clients, use a unique
file descriptor for each wl_keyboard resource.
Reference: weston, commit 76829fc4eaea329d2a525c3978271e13bd76c078
sx, sy used to store the buffer offset of the drag surface which was
then be added (by rootston) to the drag icon position.
Buffer offsets are handled already in surface_intersect_output
(output.c) so they were added twice for dnd surfaces.
A few pedantic changes and unused variables (1-4), and genuine bugs (5,
6).
The reports with the corresponding files and lines numbers are as
follows.
1. backend/libinput/tablet_pad.c@31,44,57
"Allocator sizeof operand mismatch"
"Result of 'calloc' is converted to a pointer of type 'unsigned int',
which is incompatible with sizeof operand type 'int'"
2. types/tablet_v2/wlr_tablet_v2_pad.c@371
"Allocator sizeof operand mismatch"
"Result of 'calloc' is converted to a pointer of type 'uint32_t', which
is incompatible with sizeof operand type 'int'"
3. types/wlr_cursor.c@335
"Dead initialization"
"Value stored to 'dx'/'dy' during its initialization is never read"
4. rootston/xdg_shell.c@510
"Dead initialization"
"Value stored to 'desktop' during its initialization is never read"
5. types/tablet_v2/wlr_tablet_v2_pad.c@475
"Dereference of null pointer"
"Access to field 'strips' results in a dereference of a null pointer
(loaded from field 'current_client')"
The boolean logic was incorrect (c.f. the check in the following
function).
6. examples/idle.c@163,174,182
"Uninitialized argument value"
"1st function call argument is an uninitialized value"
If close_timeout != 0, but simulate_activity_timeout >= close_timeout,
the program would segfault at pthread_cancel(t1).
After destroying a keyboard input device, seat's listeners could still be pointing to destroyed wlr_input_device signals. This patch makes sure the references are removed while the input device is being destroyed.
Implement the tablet-v2 tablet tool's implicit grab semantics for
buttons and tip.
This avoids losing focus (to other [sub]surfaces) when a button is held,
or the tip is down.
This should help when the device is used close to a surface's border and
would otherwise have to be very precise.
Implement the basic logic for tablet-v2 tablet_pad's grabs. And plug in
the default grab.
Features like "holding" the focus should be implemented via grabs, like
they are for pointer and keyboard.
There were a few issues after rebase, that the merge algorithm didn't
throw at my face:
wlr_output did a check on the actual role, not a string anymore, so that
had to go to allow tablet-v2 to set cursor surfaces.
A few L_DEBUG/L_ERRORs were still around
There was a user-after-free in tablet-group free()ing, probably after
insufficient testing from a previous feedback pass
The previous naming was based on the input-device capability names from
libinput.
With code that uses the libinput_tablet_tool and mapping into tablet-v2,
this is confusing, so the name is changed to follow the names used in
the protocol.
To begin with, no-op updates are unnecessary, so this patch is an
improvement on its own.
Then, this fixes hotplugging issues with xwayland. xwayland waits
for both wl_output and xdg_output to send a "done" event. However,
it doesn't handle well desynchronized "done" updates: if xdg-output
sends "done" twice, the second one will wait for the next wl_output
"done" event. This is an issue when the first is a no-op and the
second is a real update: the second isn't applied. I've considered
patching xwayland instead, but it seems pretty complicated.
wl_resource_for_each_safe isn't safe to use here because it accesses
the list's head memory one last time at the end of the loop. Work
around this by breaking out early.
==19880==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d0000e6368 at pc 0x7fab68619de2 bp 0x7ffd5c91cee0 sp 0x7ffd5c91ced0
READ of size 8 at 0x60d0000e6368 thread T0
#0 0x7fab68619de1 in wlr_seat_destroy ../types/seat/wlr_seat.c:179
#1 0x7fab68619fb9 in handle_display_destroy ../types/seat/wlr_seat.c:196
#2 0x7fab688e4f8f in wl_priv_signal_emit src/wayland-server.c:2024
#3 0x7fab688e56ca in wl_display_destroy src/wayland-server.c:1092
#4 0x40c11e in server_fini ../sway/server.c:138
#5 0x40b1a8 in main ../sway/main.c:438
#6 0x7fab67b5e18a in __libc_start_main ../csu/libc-start.c:308
#7 0x409359 in _start (/opt/wayland/bin/sway+0x409359)
0x60d0000e6368 is located 24 bytes inside of 144-byte region [0x60d0000e6350,0x60d0000e63e0)
freed by thread T0 here:
#0 0x7fab6a7d6880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x7fab68619805 in seat_client_handle_resource_destroy ../types/seat/wlr_seat.c:97
#2 0x7fab688e5025 in destroy_resource src/wayland-server.c:688
previously allocated by thread T0 here:
#0 0x7fab6a7d6e50 in calloc (/lib64/libasan.so.5+0xeee50)
#1 0x7fab686198df in seat_handle_bind ../types/seat/wlr_seat.c:127
#2 0x7fab6530503d in ffi_call_unix64 (/lib64/libffi.so.6+0x603d)
It is common to want to iterate an xdg-surface's popups separately from
the toplevel and subsurfaces. For example, popups are typically rendered
on top of most other surfaces.
wlr_xdg_surface_for_each_surface continues to iterate both surfaces and
popups to maintain backwards compatibility.
- Rename handlers to <type>_handle_resource_destroy and
<type>_handle_destroy to be coherent
- Make sure we never destroy wl_resources when we shouldn't
Updates #999
There was no way to tell wlr_idle to stop processing input events
and rearm timers all the time, such an API is required to have
some form of idle inhibitor.
popups have a link in parent's surface->popups list and needs
to be freed before:
==6902==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120001a0300 at pc 0x7fc1447acb50 bp 0x7fffd396e680 sp 0x7fffd396e670
WRITE of size 8 at 0x6120001a0300 thread T0
#0 0x7fc1447acb4f in wl_list_remove ../util/signal.c:55
#1 0x7fc14477d206 in destroy_xdg_popup_v6 ../types/xdg_shell_v6/wlr_xdg_popup_v6.c:162
#2 0x7fc1447816e0 in destroy_xdg_surface_v6 ../types/xdg_shell_v6/wlr_xdg_surface_v6.c:108
#3 0x7fc144a1c025 in destroy_resource src/wayland-server.c:688
#4 0x7fc144a1c091 in wl_resource_destroy src/wayland-server.c:705
#5 0x7fc14477fd6f in xdg_client_v6_handle_resource_destroy ../types/xdg_shell_v6/wlr_xdg_shell_v6.c:72
#6 0x7fc144a1c025 in destroy_resource src/wayland-server.c:688
#7 0x7fc144a20851 (/lib64/libwayland-server.so.0+0xc851)
#8 0x7fc144a20d92 (/lib64/libwayland-server.so.0+0xcd92)
#9 0x7fc144a1c140 in wl_client_destroy src/wayland-server.c:847
#10 0x7fc144a1c21c in destroy_client_with_error src/wayland-server.c:307
#11 0x7fc144a1c21c in wl_client_connection_data src/wayland-server.c:330
#12 0x7fc144a1df01 in wl_event_loop_dispatch src/event-loop.c:641
#13 0x7fc144a1c601 in wl_display_run src/wayland-server.c:1260
#14 0x40a2f4 in main ../sway/main.c:433
#15 0x7fc143ef718a in __libc_start_main ../csu/libc-start.c:308
#16 0x40b749 in _start (/opt/wayland/bin/sway+0x40b749)
0x6120001a0300 is located 64 bytes inside of 264-byte region [0x6120001a02c0,0x6120001a03c8)
freed by thread T0 here:
#0 0x7fc14690d880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x7fc1447acce8 in wlr_signal_emit_safe ../util/signal.c:29
#2 0x7fc1447a3cac in surface_handle_resource_destroy ../types/wlr_surface.c:576
#3 0x7fc144a1c025 in destroy_resource src/wayland-server.c:688
previously allocated by thread T0 here:
#0 0x7fc14690de50 in calloc (/lib64/libasan.so.5+0xeee50)
#1 0x7fc144781d38 in create_xdg_surface_v6 ../types/xdg_shell_v6/wlr_xdg_surface_v6.c:415
#2 0x7fc14147503d in ffi_call_unix64 (/lib64/libffi.so.6+0x603d)
Alternative would be to have popups listen to the parent's surface
destroy event and remove themselves from the list at this point OR on
their own destroy, whichever happens first, but that seems more
complicated for little benefit.
seat->primary_election_source_destroy points to the source that just got
freed by the cancel.
==7843==ERROR: AddressSanitizer: heap-use-after-free on address 0x60b0004269b0 at pc 0x7fb95bf4ccd0 bp 0x7ffd75013940 s
p 0x7ffd75013930
WRITE of size 8 at 0x60b0004269b0 thread T0
#0 0x7fb95bf4cccf in wl_list_remove ../util/signal.c:55
#1 0x7fb95bf3f4c6 in wlr_seat_set_primary_selection ../types/wlr_primary_selection.c:238
#2 0x7fb95becb1a7 in xwm_handle_selection_event ../xwayland/selection/selection.c:124
#3 0x7fb95bed2e5d in x11_event_handler ../xwayland/xwm.c:1139
#4 0x7fb95c1bdf01 in wl_event_loop_dispatch src/event-loop.c:641
#5 0x7fb95c1bc601 in wl_display_run src/wayland-server.c:1260
#6 0x40a2f4 in main ../sway/main.c:433
#7 0x7fb95b69718a in __libc_start_main (/lib64/libc.so.6+0x2318a)
#8 0x40b749 in _start (/opt/wayland/bin/sway+0x40b749)
0x60b0004269b0 is located 64 bytes inside of 112-byte region [0x60b000426970,0x60b0004269e0)
freed by thread T0 here:
#0 0x7fb95e0ad880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x7fb95bf3f49e in wlr_seat_set_primary_selection ../types/wlr_primary_selection.c:236
#2 0x7fb95becb1a7 in xwm_handle_selection_event ../xwayland/selection/selection.c:124
#3 0x7fb95bed2e5d in x11_event_handler ../xwayland/xwm.c:1139
#4 0x7fb95c1bdf01 in wl_event_loop_dispatch src/event-loop.c:641
previously allocated by thread T0 here:
#0 0x7fb95e0ade50 in calloc (/lib64/libasan.so.5+0xeee50)
#1 0x7fb95bec7ad6 in xwm_selection_get_targets ../xwayland/selection/incoming.c:355
#2 0x7fb95bec7ad6 in xwm_handle_selection_notify ../xwayland/selection/incoming.c:402
#3 0x7fb95becb1a7 in xwm_handle_selection_event ../xwayland/selection/selection.c:124
#4 0x7fb95bed2e5d in x11_event_handler ../xwayland/xwm.c:1139
#5 0x7fb95c1bdf01 in wl_event_loop_dispatch src/event-loop.c:641
SUMMARY: AddressSanitizer: heap-use-after-free ../util/signal.c:55 in wl_list_remove
Shadow bytes around the buggy address:
0x0c168007cce0: fd fd fd fa fa fa fa fa fa fa fa fa fd fd fd fd
0x0c168007ccf0: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa
0x0c168007cd00: fa fa fd fd fd fd fd fd fd fd fd fd fd fd fd fa
0x0c168007cd10: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c168007cd20: fd fd fd fd fd fa fa fa fa fa fa fa fa fa fd fd
=>0x0c168007cd30: fd fd fd fd fd fd[fd]fd fd fd fd fd fa fa fa fa
0x0c168007cd40: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
0x0c168007cd50: fd fa fa fa fa fa fa fa fa fa fd fd fd fd fd fd
0x0c168007cd60: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
0x0c168007cd70: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa
0x0c168007cd80: fa fa fa fa fa fa fd fd fd fd fd fd fd fd fd fd
Prior to this commit, we re-uploaded the buffer even if a new one
wasn't attached. After uploading, we send wl_buffer.release. So,
this sequence of requests resulted in a double release:
surface.attach(buffer, 0, 0)
surface.commit()
<- buffer.release()
surface.commit()
<- buffer.release()
If the layer surface has been closed by the compositor, using
layer_surface_close(), then the unmap event is emitted. However, when
the layer surface is later destroyed by the client, the compositor used
to get a second unmap, which is fixed with this commit.
After some discussions on #wayland, it seems that as soon as you
hold a reference to a DMA-BUF (via EGLImage for instance), the
underlying memory won't get free'd. The client is allowed to
re-use the DMA-BUF and upload something else to it though.
This lets clients bind to a seat multiple times by re-using the existing
wlr_seat_client whenever a duplicate request happens.
Previously, an independant wlr_seat_client would be created and only
events from one would be processed.
Fixes#1023.
The printf conversion specifiers in a call to wl_resource_post_error
do not specify the type correctly on armhf:
../types/wlr_linux_dmabuf.c: In function 'params_add':
../types/wlr_linux_dmabuf.c:104:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'uint64_t {aka long long unsigned int}' [-Werror=format=]
"sent modifier %lu for plane %u, expected modifier %lu like other planes",
~~^
%llu
To fix this, we use standard printf conversion specifier macros from
inttypes.h.
The anchor and gravity bitfields in xdg-shell-unstable-v6 have
been changed to a plain enum whose values cannot be used as a
bitfield in xdg-shell. While it makes input validation easier, it
also makes positioner operations a pain in the ass.
This changes the `wlr_output_impl.set_cursor` function to take a
`wlr_texture` instead of a byte buffer. This simplifies the
DRM and Wayland backends since they were creating textures from
the byte buffer anyway.
With this commit, performance should be improved when moving the
cursor since outputs don't need to be re-rendered anymore.
All public resource creators now take a new ID for the resource
and an optional list where the resource link is added. When the
resource is destroyed it is its own responsibility to remove
itself from the list. This removes the need for the caller to add
a destroy listener.
This commit fixes a few segfaults with resources not removed from
the list when destroyed.
wlr_keyboard manages the xkb-common state of the compositor.
It used to update the state, update the modifiers, then notify the
compositor.
When [Shift_L] was pressed and released, this resulted in an event chain:
Modifiers: Shift
Key: Shift_L (Pressed)
Modifiers:
Key: Shift_L (Release)
The xkb-docs state that the state should be updated *after* the key was
handled [1], to prevent the new state from influencing the actual key
generated.
To achieve this, the event to the compositor is emitted, *before*
wlroots handles the xkb and internal keyboard state.
With this patch applied, the emitted events ill be:
Modifiers:
Key: Shift_L (Pressed)
Modifiers: Shift
Key: Shift_L (Release)
[1] https://xkbcommon.org/doc/current/group__state.html#gac554aa20743a621692c1a744a05e06ce
When a client attaches a wl_drm or a linux_dmabuf buffer, we only
update it if the size is different from the one of the old buffer.
This means that if the client attaches a new, updated buffer with
the same size as the old buffer, the texture won't get updated.
This commit changes this behavior and re-creates the texture if
the client attaches a new buffer, without requiring the size to be
different.
The motivation for this is:
- `get_popup` and `get_toplevel` allocate role-specific resources.
- On the first non-null commit, the surface gets mapped.
- On a null commit, the surface gets unmapped. It can be mapped
again with a non-null commit.
- When the role object (xdg-toplevel or xdg-popup) is
destroyed, the surface is unmapped and role-specific resources
are destroyed. The client can call `get_popup` or `get_toplevel`
again on that surface.
- When the xdg-surface object is destroyed, the surface is
unmapped, role-specific resources are destroyed and the surface
itself is destroyed.
- Textures are now immutable (apart from those created from raw
pixels), no more invalid textures
- Move all wl_drm stuff in wlr_renderer
- Most of wlr_texture fields are now private
- Remove some duplicated DMA-BUF code in the DRM backend
- Add more assertions
- Stride is now always given as bytes rather than pixels
- Drop wl_shm functions
Fun fact: this patch has been written 10,000 meters up in the air.
Some clients create an xdg_surface, then create an xdg_toplevel,
but don't map it and destroy it right after. The xdg_surface ends
up in a state where it isn't mapped but role-specific resources
have been allocated. xdg_surface_unmap needs to free these
resources without emitting the unmap signal.
