After disabling a connector, we need to cleanup the connector to
teardown the surfaces and unlock the FBs.
Move this logic into drm_connector_apply_commit() so that it's
applied when drm_commit() is called from somewhere else than
drm_connector_commit_state().
There is some duplicated logic between these two functions.
The commit codepath was calling the test function before doing the
real commit, so this also saves an unnecessary test-only commit
when performing a real commit.
This centralizes logic common for both the atomic and libliftoff
backends. Additionally, a struct will make it easier to implement
multi-connector commits (since it can be stored in an array).
This allows us to remove the renderer destroy listener. The
listener was buggy: compositors can't destroy surface resources on
their own.
The wlr_compositor will always outlive the wlr_surface, so no need
for a destroy listener.
If we hit this case, we effectively failed to render something, this might
be because a texture failed to upload or the texture is momentarily
unavailable after a GPU reset. If we fail to render, we have to continue
to track damage for the next frame in hopes that the texture becomes
available then.
An alternative approach would be to fail the commit completely if we
find this case, but in the case of gpu resets, clients may not commit
a new buffer for a while, and a frozen display does not help.
This fixes damage tracking issues after a gpu reset.
The spec for GL_EXT_disjoint_timer_query says
> The GetInteger64vEXT command is required only if OpenGL ES 3.0 or later
> is not supported.
Some GLES 3.2 implementations like the proprietary mali driver on the
rk3566 based OrangePi advertise GL_EXT_disjoint_timer_query but lack
glGetInteger64vEXT. Use glGetInteger64v instead.
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
Simon Ser
Isaac Freund
He Yong
Alexander Orzechowski
Guido Günther
Vaxry