We will soon support custom swapchains. In order to track output damage
we should instead use the damage_ring which will hold all the buffers
we are currently tracking anyway across an arbitrary amount of swapchains.
We would fail to call scene_node_update() which would then call output
events for us. We need to make sure to update the node when we first map
a buffer, as the comment explained.
In [1] we discovered a bug where wlr_drm_connector_state.primary_fb
would not be set up correctly because drm_connector_alloc_crtc() was
called after drm_connector_state_init(). This is tricky to discover,
so instead assert() that we have a usable CRTC by the time
drm_connector_state_init() is called.
[1]: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4569
I assume that I'm not the only one who has changed/will change their
email over the years of wlroots development, so this seems like a
reasonable thing to have.
When pressing the keybinding to shut down the compositor, the following
use-after-free is triggered:
==1165966==ERROR: AddressSanitizer: heap-use-after-free on address 0x51800000ade0 at pc 0x7fa6728b4531 bp 0x7ffe540a6aa0 sp 0x7ffe540a6a90
READ of size 8 at 0x51800000ade0 thread T0
#0 0x7fa6728b4530 in wlr_seat_set_keyboard ../types/seat/wlr_seat_keyboard.c:124
#1 0x58a83fa7fd4e in keyboard_handle_key ../tinywl/tinywl.c:228
#2 0x7fa673a1901d in wl_signal_emit_mutable (/usr/lib/libwayland-server.so.0+0xa01d) (BuildId: d943a6a6069d1b5293dad7c842d26ce407ebdd19)
#3 0x7fa67295b4be in wlr_keyboard_notify_key ../types/wlr_keyboard.c:102
#4 0x7fa67295c791 in wlr_keyboard_finish ../types/wlr_keyboard.c:165
#5 0x7fa672848cb1 in destroy_wl_seat ../backend/wayland/seat.c:293
#6 0x7fa672833dca in backend_destroy ../backend/wayland/backend.c:493
#7 0x7fa6727b49e8 in wlr_backend_destroy ../backend/backend.c:67
#8 0x7fa67282d334 in multi_backend_destroy ../backend/multi/backend.c:59
#9 0x7fa67282da5a in handle_event_loop_destroy ../backend/multi/backend.c:110
#10 0x7fa673a18b98 in wl_event_loop_destroy (/usr/lib/libwayland-server.so.0+0x9b98) (BuildId: d943a6a6069d1b5293dad7c842d26ce407ebdd19)
#11 0x7fa673a1b43c in wl_display_destroy (/usr/lib/libwayland-server.so.0+0xc43c) (BuildId: d943a6a6069d1b5293dad7c842d26ce407ebdd19)
#12 0x58a83fa8ada1 in main ../tinywl/tinywl.c:1068
#13 0x7fa672043ccf (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
#14 0x7fa672043d89 in __libc_start_main (/usr/lib/libc.so.6+0x25d89) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
#15 0x58a83fa7e7c4 in _start (/home/simon/src/wlroots/build/tinywl/tinywl+0x167c4) (BuildId: 1febf2a5a18bda0f6b67377a132484061875e248)
0x51800000ade0 is located 352 bytes inside of 880-byte region [0x51800000ac80,0x51800000aff0)
freed by thread T0 here:
#0 0x7fa6732dfdb2 in __interceptor_free /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:52
#1 0x7fa6728c6a1e in wlr_seat_destroy ../types/seat/wlr_seat.c:245
#2 0x7fa6728c6a7a in handle_display_destroy ../types/seat/wlr_seat.c:251
#3 0x7fa673a1b3c6 in wl_display_destroy (/usr/lib/libwayland-server.so.0+0xc3c6) (BuildId: d943a6a6069d1b5293dad7c842d26ce407ebdd19)
previously allocated by thread T0 here:
#0 0x7fa6732e0cc1 in __interceptor_calloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77
#1 0x7fa6728c6a9d in wlr_seat_create ../types/seat/wlr_seat.c:255
#2 0x58a83fa8a8d3 in main ../tinywl/tinywl.c:1024
#3 0x7fa672043ccf (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
This happens because the wlr_seat is destroyed before the
wlr_keyboard. Destroying the wlr_keyboard has the side effect of
implicitly releasing keys currently held down.
Explicitly destroying the wlr_backend before the wl_display fixes
this.
Suggested-by: Isaac Freund <ifreund@ifreund.xyz>
Use the same logic for cursor FBs as we currently use for primary
FBs. This also fixes the same bug as [1] but in a different, more
robust way.
The new logic integrates better with atomic and will be required
anyways in the future when set_cursor will be superseded by a better
API.
[1]: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4577
With the following sequence of events, the cursor FB fields could
end up being all set to NULL while the cursor is enabled:
1. set_cursor is called, conn->cursor_pending_fb is set to a FB
pointer.
2. The output is committed with a buffer. crtc->cursor->queued_fb
is set to the FB pointer, conn->cursor_pending_fb is reset to
NULL. A page-flip event is expected in the future.
3. The output is committed with a modeset before the page-flip
event is triggered. crtc->cursor->queued_fb is reset to NULL.
At this point all of crtc->cursor->current_fb,
crtc->cursor->queued_fb and conn->cursor_pending_fb are NULL which
is a bogus state when the cursor plane is enabled.
To avoid this issue, avoid overwriting crtc->cursor->queued_fb
with a NULL pointer on commit. The cursor logic still isn't great,
but let's keep a rework of that for a separate patch.
Closes: https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3734
Only print the list of connectors once, with both the old and new
status. Use CRTC object IDs instead of CRTC indices. Make it obvious
when a connector keeps the same CRTC.
If a connector has no current/queued buffer, but has a pending
buffer in the commit, we need to process that pending buffer before
checking pending.primary_fb.
When turning off a CRTC, we don't need a buffer.
It doesn't matter whether this is a modeset or not: we always need
a buffer even for regular page-flips as long as a connector is
active.
Fixes: 374daeb256 ("backend/drm: Ensure a primary fb is available when configuring an output")
We can never hit the case where we try to light up an output without
a buffer. output_ensure_buffer() will catch this for now, and when that's
removed, output_basic_test() will catch this case.
drm_connect_state_init() will set primary_fd to null if no CRTC is active
for the connector and can crash later if the code expects a CRTC (like
when lighting up an output).
Saving the old context and immediately making our own context
current is a common pattern. Let's make it easier to do.
No functional change, just refactoring.
For XWayland surfaces that start maximized, it's best to send an initial
Configure event to set the size of the surface before mapping it. This
reduces visual glitches since the application sees the correct maximized
size when performing its initial layout and drawing.
wlroots surfaces emit their first "map" event after the XWayland window
has already been mapped and the first frame has been drawn & committed.
This is too late to send the initial Configure event.
So, add a new "map_request" event which is emitted immediately before
telling XWayland to map the window. Compositors can connect to this
event to send the initial Configure event to an XWayland app based on
its requested maximized (or fullscreen) state.
Compositors should not place anything visually on the screen at this
point but rather wait until the "map" event as before.
scan-build is a little confused, thinking xwm->xres value could change
during the execution of xwayland_surface_create so client_id_cookie
could end up used uninitialized.
The struct is just an unsigned int, so no harm in initializing it to get
it off the list.
In query_modifier_support, the calloc for either or both of render_mods
and texture_mods may fail, in which case both are freed for convenience.
However, if one is non-NULL, vulkan_format_props_finish will try to free
it again.
NULL them to avoid double-free.
If drm_format_set_extend fails, we need to make sure each wlr_drm_format
is cleaned up together with the formats array. Finish the set to take
care of it.
