Update to latest version of linux_dmabuf protocol #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We can now just rely on the common code for this.
Unless we're dealing with a multi-GPU setup and the backend being initialized is secondary, we don't need a renderer nor an allocator. Stop initializing these.
These are bools but should be pointers.
We no longer require wlr_output_impl.{attach,rollback}_render to
be populated.
No backend uses these anymore.
This enum will get dropped in the next commit.
This is now unconditionally set to WLR_OUTPUT_STATE_BUFFER_SCANOUT.
On little-endian, we can enable pixel formats which don't use gl_type = GL_UNSIGNED_BYTE. See [1]. [1]: https://afrantzis.com/pixel-format-guide/
This change introduces a new function to check whether the renderer has the needed GL extensions to read a given pixel format.
The half-float formats depend on GL_OES_texture_half_float_linear, not just the GL_OES_texture_half_float extension, because the latter does not include support for linear magni/minification filters. The new 2101010 and 16161616F formats are only available on little- endian builds, since their gl_types are larger than a byte and thus endianness dependent.
According to the viewport protocol, upon wp_viewport::destroy():
> The associated wl_surface's crop and scale state is removed.
> The change is applied on the next wl_surface.commit.
Therefore, wp_viewport_destroy(viewport) should remove all viewport state.
Currently, wlroots does not remove the crop and scale state. Instead, a
client must do:
wl_fixed_t clear = wl_fixed_from_int(-1);
wp_viewport_set_source(viewport, clear, clear, clear, clear);
wp_viewport_set_destination(viewport, -1, -1);
wp_viewport_destroy(viewport);
This commit adds the necessary logic into viewport_destroy and makes
wlroots comply with the protocol.
The kernel orders the mode list from highest to lowest. Preserve this ordering in the wlr_output.modes list.
These will be added to Pixman in the next commit.
Add a bunch of new formats for Pixman: a few missing 32-bit ones, some 16-bit and 32-bit formats as well. Mostly based on a Weston patch [1]. [1]: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/664
When enabling an output, skip the empty buffer allocation if the backend accepts modesets without a buffer. This fixes mode-setting with the noop backend.
When testing a modeset, make sure the caller has also provided a buffer. This allows df0e75b ("output: try skipping buffer allocation if the backend allows it") to work as expected with the DRM backend. Closes: swaywm#3086
The protocol allows compositors to not send any keymap to Wayland clients. Handle a keymap-less keyboard correctly by sending WL_KEYBOARD_KEYMAP_FORMAT_NO_KEYMAP instead of erroring out in the mmap call.
wl_fixed_t is a 32-bit data type, but our doubles are 64-bit. This meant that two doubles that would map to the same wl_fixed_t could compare unequal, and send a duplicate motion event. Refs swaywm/sway#4632.
Add a very basic smoke test which uses VKMS to fire up the DRM backend.
Some listeners weren't removed and caused a use-after-free with e.g. vkms when used as a secondary GPU.
This allows callers to use wlr_output_test to check whether a mode can be enabled, for instance. Closes: swaywm#2250
Closes #3324.
Removing an input device requires unlinking it from the list of all headless input devices. For that implement a destroy function.
We were send a protocol error if INTERLACED or BOTTOM_FIRST was set. This is incorrect for the zwp_linux_dmabuf_params.create code-path because this kills the client without allowing it to gracefully handle the error. We should only send a protocol error if the client provides a bit not listed in the protocol definition.
They are never used in practice, which makes all of our flag handling effectively dead code. Also, APIs such as KMS don't provide a good way to deal with the flags. Let's just fail the DMA-BUF import when clients provide flags.
A wlroots user can easily get confused and think that `cap` refers to wlroots buffer capabilities, not array capacity.
|
We use tabs for indentation. Throughout your patches, spaces are used. |
emersion
reviewed
Nov 19, 2021
types/wlr_linux_dmabuf_v1.c
Outdated
| struct wl_resource *resource) { | ||
| struct wl_array dev_array = { | ||
| .size = sizeof(feedback->main_device), | ||
| .data = (void *)&feedback->main_device, | ||
| }; | ||
| zwp_linux_dmabuf_feedback_v1_send_main_device(resource, &dev_array); | ||
|
|
||
| zwp_linux_dmabuf_feedback_v1_send_format_table(resource, | ||
| linux_dmabuf->format_table.fd, |
There was a problem hiding this comment.
This sends the FD in read-write mode. This means clients can write to the format table, and confuse other clients.
emersion
reviewed
Nov 19, 2021
types/wlr_linux_dmabuf_v1.c
Outdated
| @@ -751,6 +740,104 @@ static void handle_renderer_destroy(struct wl_listener *listener, void *data) { | |||
| linux_dmabuf_v1_destroy(linux_dmabuf); | |||
| } | |||
|
|
|||
| /* from the gamma example */ | |||
| static int create_anonymous_file(off_t size) { | |||
There was a problem hiding this comment.
We have allocate_shm_file_pair as a common helper
emersion
reviewed
Nov 19, 2021
types/wlr_linux_dmabuf_v1.c
Outdated
| return fd; | ||
| } | ||
|
|
||
| static bool wlr_dmabuf_format_table_init(struct wlr_linux_dmabuf_v1 *linux_dmabuf, |
There was a problem hiding this comment.
Internal wlroots functions shouldn't have a wlr_ prefix, this is only used for public functions.
emersion
reviewed
Nov 19, 2021
types/wlr_linux_dmabuf_v1.c
Outdated
|
|
||
| wl_array_init(&linux_dmabuf->format_table.indices); | ||
| if (!wl_array_add(&linux_dmabuf->format_table.indices, len * sizeof(uint16_t))) { | ||
| wlr_log(WLR_ERROR, "failed to mmap()\n"); |
There was a problem hiding this comment.
This error message doesn't match the if
|
Ideally this work would update the original commits, so that the resulting final patchset history is clean (see CONTRIBUTING). |
emersion
reviewed
Nov 19, 2021
| /* This will be mmapped from the fd above */ | ||
| struct wlr_dmabuf_format_table_entry *data; | ||
| /* The default indices list. datatype is uint16_t */ | ||
| struct wl_array indices; |
There was a problem hiding this comment.
Hm, I think it would be better to remove this, and just use wlr_linux_dmabuf_feedback_v1_tranche.indices?
emersion
reviewed
Nov 19, 2021
types/wlr_linux_dmabuf_v1.c
Outdated
| int fd, idx = 0, len = 0; | ||
| struct wlr_dmabuf_format_table_entry *data; | ||
| size_t size; | ||
| uint16_t *index; |
There was a problem hiding this comment.
Style: declare variables as needed, instead of doing it at the top of the function body
emersion
reviewed
Nov 19, 2021
types/wlr_linux_dmabuf_v1.c
Outdated
|
|
||
| /* Now calculate the indices (in order, nothing fancy) */ | ||
| idx = 0; | ||
| wl_array_for_each(index, &linux_dmabuf->format_table.indices) { |
There was a problem hiding this comment.
Nit: rename to index_ptr to make it clear it's a pointer.
This allows compositors to initialize wl_shm without initializing other globals like linux-dmabuf.
Implement a basic version of linux-dmabuf-unstable-v1 version 4. Only default hints are implemented.
This data structure will allow compositors to define their own custom hints.
This was changed during the latest protocol updates.
|
Thanks! Updated to address feedback, but to do so I had to rebase everything (including your dmabuf-hints branch) onto master. This was needed to get the definition of |
|
Hm, this approach doesn't let compositors set per-surface format preferences. |
|
Thanks for working on this, but I decided to take another approach to better fit the new protocol design (see upstream wlroots MR). Sorry for the lost time! |
amshafer
pushed a commit
to amshafer/wlroots
that referenced
this pull request
Jul 28, 2022
I am running a custom compiled version of chromium with a patch to get it up and running on sway git at the moment, and in that development build I compiled there is a bug where the browser will crash if you try to open a file select dialog. When this crash happens, chromium will not close, but instead will remain open and impossible to close unless you send a SIGKILL signal to the process. However, sway will crash to tty when you send the SIGKILL. I have a hunch that when chromium is opening the file select dialog it is creating some sort of a xdg toplevel surface. But it freezes before it fully initializes the surface. When the SIGKILL signal is given, sway/wlroots will try to free the xdg_toplevel surface but because it hasn't fully initialized due to the frozen window, it segfaults. Don't be fooled by the assert, the assert is not firing, the surface pointer is indeed NULL here. * thread emersion#1, name = 'sway', stop reason = signal SIGSEGV: invalid address (fault address: 0x28) frame #0: 0x00007ffff78b9041 libwlroots.so.11`wlr_xdg_toplevel_set_parent(surface=0x0000000000000000, parent=0x0000000000000000) at wlr_xdg_toplevel.c:159:37 156 157 void wlr_xdg_toplevel_set_parent(struct wlr_xdg_surface *surface, 158 struct wlr_xdg_surface *parent) { -> 159 assert(surface->role == WLR_XDG_SURFACE_ROLE_TOPLEVEL); 160 assert(!parent || parent->role == WLR_XDG_SURFACE_ROLE_TOPLEVEL); 161 162 if (surface->toplevel->parent) { (lldb) up error: sway {0x0003442a}: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x67) attribute, but range extraction failed (invalid range list offset 0x67), please file a bug and attach the file at the start of this error message error: sway {0x0003442a}: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x67) attribute, but range extraction failed (invalid range list offset 0x67), please file a bug and attach the file at the start of this error message frame emersion#1: 0x00007ffff78e176e libwlroots.so.11`destroy_imported(imported=0x000055555626d570) at wlr_xdg_foreign_v1.c:154:3 151 wl_list_for_each_safe(child, child_tmp, &imported->children, link) { 152 struct wlr_xdg_surface *xdg_child = 153 wlr_xdg_surface_from_wlr_surface(child->surface); -> 154 wlr_xdg_toplevel_set_parent(xdg_child, NULL); 155 } 156 157 wl_list_remove(&imported->exported_destroyed.link); (lldb) up frame emersion#2: 0x00007ffff78e1b9d libwlroots.so.11`xdg_imported_handle_resource_destroy(resource=0x00005555562555a0) at wlr_xdg_foreign_v1.c:280:2 277 struct wl_resource *resource) { 278 struct wlr_xdg_imported_v1 *imported = xdg_imported_from_resource(resource); 279 if (!imported) { -> 280 return; 281 } 282 283 destroy_imported(imported); (lldb) up frame swaywm#3: 0x00007ffff794989a libwayland-server.so.0`___lldb_unnamed_symbol211 + 154 libwayland-server.so.0`___lldb_unnamed_symbol211: -> 0x7ffff794989a <+154>: andl $0x1, %r13d 0x7ffff794989e <+158>: je 0x7ffff79498b0 ; <+176> 0x7ffff79498a0 <+160>: addq $0x8, %rsp 0x7ffff79498a4 <+164>: movl $0x1, %eax (lldb) up frame swaywm#4: 0x00007ffff794fec0 libwayland-server.so.0`___lldb_unnamed_symbol290 + 64 libwayland-server.so.0`___lldb_unnamed_symbol290: -> 0x7ffff794fec0 <+64>: cmpl $0x1, %eax 0x7ffff794fec3 <+67>: jne 0x7ffff794fed3 ; <+83> 0x7ffff794fec5 <+69>: addq $0x8, %rbx 0x7ffff794fec9 <+73>: cmpq %rbx, %r13 (lldb) up frame swaywm#5: 0x00007ffff79503e0 libwayland-server.so.0`___lldb_unnamed_symbol300 + 32 libwayland-server.so.0`___lldb_unnamed_symbol300: -> 0x7ffff79503e0 <+32>: cmpl $0x1, %eax 0x7ffff79503e3 <+35>: je 0x7ffff79503f0 ; <+48> 0x7ffff79503e5 <+37>: popq %rbx 0x7ffff79503e6 <+38>: popq %r12 (lldb) up frame swaywm#6: 0x00007ffff794a30e libwayland-server.so.0`wl_client_destroy + 126 libwayland-server.so.0`wl_client_destroy: -> 0x7ffff794a30e <+126>: movq %r12, %rdi 0x7ffff794a311 <+129>: callq 0x7ffff7950150 ; ___lldb_unnamed_symbol293 0x7ffff794a317 <+135>: movq 0x8(%rbp), %rdi 0x7ffff794a31b <+139>: callq *0xdc77(%rip) (lldb) up frame swaywm#7: 0x00007ffff794a3f7 libwayland-server.so.0`___lldb_unnamed_symbol214 + 119 libwayland-server.so.0`___lldb_unnamed_symbol214: -> 0x7ffff794a3f7 <+119>: movq 0x28(%rsp), %rax 0x7ffff794a3fc <+124>: subq %fs:0x28, %rax 0x7ffff794a405 <+133>: jne 0x7ffff794a727 ; <+935> 0x7ffff794a40b <+139>: addq $0x38, %rsp (lldb) up frame swaywm#8: 0x00007ffff794d1ca libwayland-server.so.0`wl_event_loop_dispatch + 202 libwayland-server.so.0`wl_event_loop_dispatch: -> 0x7ffff794d1ca <+202>: addq $0xc, %r15 0x7ffff794d1ce <+206>: cmpq %r15, %rbp 0x7ffff794d1d1 <+209>: jne 0x7ffff794d1b8 ; <+184> 0x7ffff794d1d3 <+211>: movq 0x8(%rsp), %rcx (lldb) up frame swaywm#9: 0x00007ffff794ad37 libwayland-server.so.0`wl_display_run + 39 libwayland-server.so.0`wl_display_run: -> 0x7ffff794ad37 <+39>: movl 0x8(%rbx), %eax 0x7ffff794ad3a <+42>: testl %eax, %eax 0x7ffff794ad3c <+44>: jne 0x7ffff794ad20 ; <+16> 0x7ffff794ad3e <+46>: popq %rbx (lldb) up frame swaywm#10: 0x000055555557689a sway`server_run(server=0x00005555555f26c0) at server.c:307:2 304 wlr_backend_destroy(server->backend); 305 return false; 306 } -> 307 308 return true; 309 } 310 (lldb) up frame swaywm#11: 0x0000555555575a93 sway`main(argc=3, argv=0x00007fffffffe978) at main.c:431:2 428 swaynag_show(&config->swaynag_config_errors); 429 } 430 -> 431 server_run(&server); 432 433 shutdown: 434 sway_log(SWAY_INFO, "Shutting down sway");
amshafer
pushed a commit
to amshafer/wlroots
that referenced
this pull request
Jul 28, 2022
Running with WLR_BACKENDS=headless, there is no keyboard device.
Avoid crashes like so:
../tinywl/tinywl.c:136:2: runtime error: member access within null pointer of type 'struct wlr_keyboard'
../tinywl/tinywl.c:136:2: runtime error: member access within null pointer of type 'struct wlr_keyboard'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==331107==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000120 (pc 0x556ed03e4e99 bp 0x7ffce834bc10 sp 0x7ffce834bbb0 T0)
==331107==The signal is caused by a READ memory access.
==331107==Hint: address points to the zero page.
#0 0x556ed03e4e99 in focus_view ../tinywl/tinywl.c:136
emersion#1 0x556ed03eb3be in xdg_toplevel_map ../tinywl/tinywl.c:603
emersion#2 0x7f75d6f768db in wlr_signal_emit_safe ../util/signal.c:29
swaywm#3 0x7f75d6e9cac7 in xdg_surface_role_commit ../types/xdg_shell/wlr_xdg_surface.c:315
swaywm#4 0x7f75d6eb6944 in surface_commit_state ../types/wlr_compositor.c:466
swaywm#5 0x7f75d6eb7b02 in surface_handle_commit ../types/wlr_compositor.c:523
swaywm#6 0x7f75d5714d49 (/usr/lib/libffi.so.8+0x6d49)
swaywm#7 0x7f75d5714266 (/usr/lib/libffi.so.8+0x6266)
swaywm#8 0x7f75d68cb322 (/usr/lib/libwayland-server.so.0+0xd322)
swaywm#9 0x7f75d68c65cb (/usr/lib/libwayland-server.so.0+0x85cb)
swaywm#10 0x7f75d68c91c9 in wl_event_loop_dispatch (/usr/lib/libwayland-server.so.0+0xb1c9)
swaywm#11 0x7f75d68c6d36 in wl_display_run (/usr/lib/libwayland-server.so.0+0x8d36)
swaywm#12 0x556ed03eef55 in main ../tinywl/tinywl.c:905
swaywm#13 0x7f75d5d2330f in __libc_start_call_main (/usr/lib/libc.so.6+0x2d30f)
swaywm#14 0x7f75d5d233c0 in __libc_start_main@GLIBC_2.2.5 (/usr/lib/libc.so.6+0x2d3c0)
swaywm#15 0x556ed03e46e4 in _start (/home/simon/src/wlroots/build/tinywl/tinywl+0x136e4)
amshafer
pushed a commit
to amshafer/wlroots
that referenced
this pull request
Mar 9, 2023
Stack trace:
#0 0x00007f17081f5b99 in wl_list_insert (list=list@entry=0x2d8, elm=elm@entry=0x7ffe7f7e85d0)
at ../wayland-1.21.0/src/wayland-util.c:48
emersion#1 0x00007f17081f5f2e in wl_signal_emit_mutable (signal=signal@entry=0x2d8, data=data@entry=0x7ffe7f7e8660)
at ../wayland-1.21.0/src/wayland-server.c:2167
emersion#2 0x00007f170815a971 in handle_switch_toggle (wlr_switch=0x2a0, event=0x55d5ba13dc00)
at ../backend/libinput/switch.c:50
swaywm#3 handle_libinput_event (event=0x55d5ba13dc00, backend=0x55d5b975d740) at ../backend/libinput/events.c:234
swaywm#4 handle_libinput_readable (fd=<optimized out>, mask=<optimized out>, _backend=<optimized out>)
at ../backend/libinput/backend.c:58
swaywm#5 handle_libinput_readable (fd=fd@entry=34, mask=mask@entry=1, _backend=_backend@entry=0x55d5b975d740)
at ../backend/libinput/backend.c:48
swaywm#6 0x00007f170815c110 in backend_start (wlr_backend=0x55d5b975d740) at ../backend/libinput/backend.c:109
swaywm#7 0x00007f1708160996 in multi_backend_start (wlr_backend=0x55d5b97583d0) at ../backend/multi/backend.c:32
amshafer
pushed a commit
to amshafer/wlroots
that referenced
this pull request
Mar 9, 2023
In output_ensure_buffer() we create a swapchain and attach an empty
buffer to the output if necessary. We do that during the first commit.
This is fine when the first commit enables the output, however this breaks
when the first commit disables the output. A commit which disables an
output and has a buffer attached is invalid (see output_basic_test()), and
makes the DRM backend crash:
00:00:00.780 [wlr] [backend/drm/drm.c:622] connector eDP-1: Turning off
../subprojects/wlroots/backend/drm/drm.c:652:44: runtime error: member access within null pointer of type 'struct wlr_drm_crtc'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==2524==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f22e894afc1 bp 0x7ffe1d57c550 sp 0x7ffe1d57c420 T0)
==2524==The signal is caused by a READ memory access.
==2524==Hint: address points to the zero page.
#0 0x7f22e894afc1 in drm_connector_commit_state ../subprojects/wlroots/backend/drm/drm.c:652
emersion#1 0x7f22e894b1f5 in drm_connector_commit ../subprojects/wlroots/backend/drm/drm.c:674
emersion#2 0x7f22e89e8da9 in wlr_output_commit_state ../subprojects/wlroots/types/output/output.c:756
swaywm#3 0x555ab325624d in apply_output_config ../sway/config/output.c:517
swaywm#4 0x555ab31a1aa1 in handle_new_output ../sway/desktop/output.c:974
swaywm#5 0x7f22e9272f6d in wl_signal_emit_mutable (/usr/lib/libwayland-server.so.0+0x9f6d)
swaywm#6 0x7f22e899b012 in new_output_reemit ../subprojects/wlroots/backend/multi/backend.c:161
swaywm#7 0x7f22e9272f6d in wl_signal_emit_mutable (/usr/lib/libwayland-server.so.0+0x9f6d)
swaywm#8 0x7f22e895a153 in scan_drm_connectors ../subprojects/wlroots/backend/drm/drm.c:1488
swaywm#9 0x7f22e893c2e4 in backend_start ../subprojects/wlroots/backend/drm/backend.c:24
swaywm#10 0x7f22e892ed00 in wlr_backend_start ../subprojects/wlroots/backend/backend.c:56
swaywm#11 0x7f22e8999b83 in multi_backend_start ../subprojects/wlroots/backend/multi/backend.c:31
swaywm#12 0x7f22e892ed00 in wlr_backend_start ../subprojects/wlroots/backend/backend.c:56
swaywm#13 0x555ab317d5cc in server_start ../sway/server.c:316
swaywm#14 0x555ab317748d in main ../sway/main.c:400
swaywm#15 0x7f22e783c28f (/usr/lib/libc.so.6+0x2328f)
swaywm#16 0x7f22e783c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349)
swaywm#17 0x555ab3134c84 in _start (/home/simon/src/sway/build/sway/sway+0x377c84)
Fixes: 3be6658 ("output: allocate swapchain on first commit")
Closes: swaywm/sway#7373
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This handles the naming change from dmabuf hints to dmabuf feedback. It also implements handling the format table event.
I did basic testing with weston-simple-dmabuf-feedback, but probably needs more thorough testing eventually.