[PATCH v5 0/6] MR2712: winewayland.drv: part 3: Handle dynamic wl_output events
This MR introduces the driver mechanisms to handle dynamic events from the Wayland compositor, using wl_output events as the guiding use case (i.e., we want to update the win32u display settings when the host settings change). In this design we create a dedicated thread to read and dispatch Wayland events received from the compositor. If a Wayland event handler wants some code to be run in the context of a particular HWND's thread, it can add an internal event to a custom queue we have for each (GUI enabled) thread. The ProcessEvents driver callback processes internal events from that queue. In order to wake up waiting threads we use a pipe to notify about new internal events, with the read end acting as the thread's host queue fd. This is similar to how winemac.drv works. We use the aforementioned mechanisms to queue win32u display device updates to the desktop window thread. Since there are many pieces that need to fall into place, this MR gradually reaches the final design: 1. We first introduce the dedicated read/dispatch thread and handle events (and also display device updates if in the desktop process) in that thread. 2. We ensure access to Wayland output information is thread-safe and consistent (since in step 3 we will need to access it from a different thread). 3. We finally introduce per-thread internal event queues and, if we are in the desktop process, queue the display device update to the desktop window thread internal event queue. Note that the main portion of the wl_output event code is still handled in the dedicated read/dispatch thread. -- v5: winewayland.drv: Update desktop window size on display changes. winewayland.drv: Update display devices from the desktop window thread. https://gitlab.winehq.org/wine/wine/-/merge_requests/2712
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> There is currently no benefit to dynamic allocation, and static allocation allows us to simplify some aspects of initialization. --- dlls/winewayland.drv/display.c | 2 +- dlls/winewayland.drv/wayland.c | 41 ++++++++++++--------------- dlls/winewayland.drv/wayland_output.c | 8 +++--- dlls/winewayland.drv/waylanddrv.h | 4 +-- 4 files changed, 25 insertions(+), 30 deletions(-) diff --git a/dlls/winewayland.drv/display.c b/dlls/winewayland.drv/display.c index 5234a1f03aa..d822a785898 100644 --- a/dlls/winewayland.drv/display.c +++ b/dlls/winewayland.drv/display.c @@ -290,7 +290,7 @@ BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manage wl_array_init(&output_info_array); - wl_list_for_each(output, &process_wayland->output_list, link) + wl_list_for_each(output, &process_wayland.output_list, link) { if (!output->current_mode) continue; output_info = wl_array_add(&output_info_array, sizeof(*output_info)); diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 22a7a8cc63f..dab23a555a2 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -32,8 +32,10 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); -struct wl_display *process_wl_display = NULL; -struct wayland *process_wayland = NULL; +struct wayland process_wayland = +{ + .output_list = {&process_wayland.output_list, &process_wayland.output_list} +}; /********************************************************************** * Registry handling @@ -54,12 +56,12 @@ static void registry_handle_global(void *data, struct wl_registry *registry, { struct wayland_output *output; - process_wayland->zxdg_output_manager_v1 = + process_wayland.zxdg_output_manager_v1 = wl_registry_bind(registry, id, &zxdg_output_manager_v1_interface, version < 3 ? version : 3); /* Add zxdg_output_v1 to existing outputs. */ - wl_list_for_each(output, &process_wayland->output_list, link) + wl_list_for_each(output, &process_wayland.output_list, link) wayland_output_use_xdg_extension(output); } } @@ -71,7 +73,7 @@ static void registry_handle_global_remove(void *data, struct wl_registry *regist TRACE("id=%u\n", id); - wl_list_for_each_safe(output, tmp, &process_wayland->output_list, link) + wl_list_for_each_safe(output, tmp, &process_wayland.output_list, link) { if (output->global_id == id) { @@ -97,48 +99,41 @@ BOOL wayland_process_init(void) { struct wl_display *wl_display_wrapper; - process_wl_display = wl_display_connect(NULL); - if (!process_wl_display) + process_wayland.wl_display = wl_display_connect(NULL); + if (!process_wayland.wl_display) return FALSE; - process_wayland = calloc(1, sizeof(*process_wayland)); - if (!process_wayland) - return FALSE; + TRACE("wl_display=%p\n", process_wayland.wl_display); - TRACE("process_wayland=%p wl_display=%p\n", process_wayland, process_wl_display); - - if (!(process_wayland->wl_event_queue = wl_display_create_queue(process_wl_display))) + if (!(process_wayland.wl_event_queue = wl_display_create_queue(process_wayland.wl_display))) { ERR("Failed to create event queue\n"); return FALSE; } - if (!(wl_display_wrapper = wl_proxy_create_wrapper(process_wl_display))) + if (!(wl_display_wrapper = wl_proxy_create_wrapper(process_wayland.wl_display))) { ERR("Failed to create proxy wrapper for wl_display\n"); return FALSE; } wl_proxy_set_queue((struct wl_proxy *) wl_display_wrapper, - process_wayland->wl_event_queue); + process_wayland.wl_event_queue); - process_wayland->wl_registry = wl_display_get_registry(wl_display_wrapper); + process_wayland.wl_registry = wl_display_get_registry(wl_display_wrapper); wl_proxy_wrapper_destroy(wl_display_wrapper); - if (!process_wayland->wl_registry) + if (!process_wayland.wl_registry) { ERR("Failed to get to wayland registry\n"); return FALSE; } - wl_list_init(&process_wayland->output_list); - /* Populate registry */ - wl_registry_add_listener(process_wayland->wl_registry, ®istry_listener, - process_wayland); + wl_registry_add_listener(process_wayland.wl_registry, ®istry_listener, NULL); /* We need two roundtrips. One to get and bind globals, one to handle all * initial events produced from registering the globals. */ - wl_display_roundtrip_queue(process_wl_display, process_wayland->wl_event_queue); - wl_display_roundtrip_queue(process_wl_display, process_wayland->wl_event_queue); + wl_display_roundtrip_queue(process_wayland.wl_display, process_wayland.wl_event_queue); + wl_display_roundtrip_queue(process_wayland.wl_display, process_wayland.wl_event_queue); wayland_init_display_devices(); diff --git a/dlls/winewayland.drv/wayland_output.c b/dlls/winewayland.drv/wayland_output.c index 5cd6e74f24a..5c93e16f3e1 100644 --- a/dlls/winewayland.drv/wayland_output.c +++ b/dlls/winewayland.drv/wayland_output.c @@ -230,7 +230,7 @@ BOOL wayland_output_create(uint32_t id, uint32_t version) goto err; } - output->wl_output = wl_registry_bind(process_wayland->wl_registry, id, + output->wl_output = wl_registry_bind(process_wayland.wl_registry, id, &wl_output_interface, version < 2 ? version : 2); output->global_id = id; @@ -252,10 +252,10 @@ BOOL wayland_output_create(uint32_t id, uint32_t version) goto err; } - if (process_wayland->zxdg_output_manager_v1) + if (process_wayland.zxdg_output_manager_v1) wayland_output_use_xdg_extension(output); - wl_list_insert(process_wayland->output_list.prev, &output->link); + wl_list_insert(process_wayland.output_list.prev, &output->link); return TRUE; @@ -293,7 +293,7 @@ void wayland_output_destroy(struct wayland_output *output) void wayland_output_use_xdg_extension(struct wayland_output *output) { output->zxdg_output_v1 = - zxdg_output_manager_v1_get_xdg_output(process_wayland->zxdg_output_manager_v1, + zxdg_output_manager_v1_get_xdg_output(process_wayland.zxdg_output_manager_v1, output->wl_output); zxdg_output_v1_add_listener(output->zxdg_output_v1, &zxdg_output_v1_listener, output); diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index e1ea66f3b33..d5cbb47bf9e 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -39,8 +39,7 @@ * Globals */ -extern struct wl_display *process_wl_display DECLSPEC_HIDDEN; -extern struct wayland *process_wayland DECLSPEC_HIDDEN; +extern struct wayland process_wayland DECLSPEC_HIDDEN; /********************************************************************** * Definitions for wayland types @@ -48,6 +47,7 @@ extern struct wayland *process_wayland DECLSPEC_HIDDEN; struct wayland { + struct wl_display *wl_display; struct wl_event_queue *wl_event_queue; struct wl_registry *wl_registry; struct zxdg_output_manager_v1 *zxdg_output_manager_v1; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2712
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Create a dedicated thread from which we read and dispatch Wayland events (beyond the initial ones). --- dlls/winewayland.drv/dllmain.c | 20 ++++++++++++++++++++ dlls/winewayland.drv/unixlib.h | 1 + dlls/winewayland.drv/waylanddrv_main.c | 12 ++++++++++++ 3 files changed, 33 insertions(+) diff --git a/dlls/winewayland.drv/dllmain.c b/dlls/winewayland.drv/dllmain.c index 89d981a4314..d040620957b 100644 --- a/dlls/winewayland.drv/dllmain.c +++ b/dlls/winewayland.drv/dllmain.c @@ -20,8 +20,25 @@ #include "waylanddrv_dll.h" +#include "wine/debug.h" + +WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); + +static DWORD WINAPI wayland_read_events_thread(void *arg) +{ + WAYLANDDRV_UNIX_CALL(read_events, NULL); + /* This thread terminates only if an unrecoverable error occurred + * during event reading (e.g., the connection to the Wayland + * compositor is broken). */ + ERR("Failed to read events from the compositor, terminating process\n"); + TerminateProcess(GetCurrentProcess(), 1); + return 0; +} + BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason, void *reserved) { + DWORD tid; + if (reason != DLL_PROCESS_ATTACH) return TRUE; DisableThreadLibraryCalls(instance); @@ -30,5 +47,8 @@ BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason, void *reserved) if (WAYLANDDRV_UNIX_CALL(init, NULL)) return FALSE; + /* Read wayland events from a dedicated thread. */ + CloseHandle(CreateThread(NULL, 0, wayland_read_events_thread, NULL, 0, &tid)); + return TRUE; } diff --git a/dlls/winewayland.drv/unixlib.h b/dlls/winewayland.drv/unixlib.h index 427837523e3..dc3bfdf8893 100644 --- a/dlls/winewayland.drv/unixlib.h +++ b/dlls/winewayland.drv/unixlib.h @@ -26,6 +26,7 @@ enum waylanddrv_unix_func { waylanddrv_unix_func_init, + waylanddrv_unix_func_read_events, waylanddrv_unix_func_count, }; diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index a9297edc500..a578db76c66 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -49,9 +49,20 @@ err: return STATUS_UNSUCCESSFUL; } +static NTSTATUS waylanddrv_unix_read_events(void *arg) +{ + while (wl_display_dispatch_queue(process_wayland.wl_display, + process_wayland.wl_event_queue) != -1) + continue; + /* This function only returns on a fatal error, e.g., if our connection + * to the Wayland server is lost. */ + return STATUS_UNSUCCESSFUL; +} + const unixlib_entry_t __wine_unix_call_funcs[] = { waylanddrv_unix_init, + waylanddrv_unix_read_events, }; C_ASSERT(ARRAYSIZE(__wine_unix_call_funcs) == waylanddrv_unix_func_count); @@ -61,6 +72,7 @@ C_ASSERT(ARRAYSIZE(__wine_unix_call_funcs) == waylanddrv_unix_func_count); const unixlib_entry_t __wine_unix_call_wow64_funcs[] = { waylanddrv_unix_init, + waylanddrv_unix_read_events, }; C_ASSERT(ARRAYSIZE(__wine_unix_call_wow64_funcs) == waylanddrv_unix_func_count); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2712
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Handle Wayland output events sent after process initialization, and update the win32u display devices when handling these events in the desktop window process. --- dlls/winewayland.drv/display.c | 14 +++++++++++--- dlls/winewayland.drv/wayland.c | 4 +++- dlls/winewayland.drv/wayland_output.c | 24 ++++++++++++++++++++++++ dlls/winewayland.drv/waylanddrv.h | 3 ++- 4 files changed, 40 insertions(+), 5 deletions(-) diff --git a/dlls/winewayland.drv/display.c b/dlls/winewayland.drv/display.c index d822a785898..f734c1714fa 100644 --- a/dlls/winewayland.drv/display.c +++ b/dlls/winewayland.drv/display.c @@ -34,9 +34,15 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); -void wayland_init_display_devices(void) +static BOOL force_display_devices_refresh; + +void wayland_init_display_devices(BOOL force) { UINT32 num_path, num_mode; + + TRACE("force=%d\n", force); + + if (force) force_display_devices_refresh = TRUE; /* Trigger refresh in win32u */ NtUserGetDisplayConfigBufferSizes(QDC_ONLY_ACTIVE_PATHS, &num_path, &num_mode); } @@ -284,9 +290,11 @@ BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manage struct wl_array output_info_array; struct output_info *output_info; - if (!force) return TRUE; + if (!force && !force_display_devices_refresh) return TRUE; - TRACE("force=%d\n", force); + TRACE("force=%d force_refresh=%d\n", force, force_display_devices_refresh); + + force_display_devices_refresh = FALSE; wl_array_init(&output_info_array); diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index dab23a555a2..38c53e280c5 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -135,7 +135,9 @@ BOOL wayland_process_init(void) wl_display_roundtrip_queue(process_wayland.wl_display, process_wayland.wl_event_queue); wl_display_roundtrip_queue(process_wayland.wl_display, process_wayland.wl_event_queue); - wayland_init_display_devices(); + wayland_init_display_devices(FALSE); + + process_wayland.initialized = TRUE; return TRUE; } diff --git a/dlls/winewayland.drv/wayland_output.c b/dlls/winewayland.drv/wayland_output.c index 5c93e16f3e1..8293f43b56f 100644 --- a/dlls/winewayland.drv/wayland_output.c +++ b/dlls/winewayland.drv/wayland_output.c @@ -97,6 +97,26 @@ static void wayland_output_add_mode(struct wayland_output *output, if (current) output->current_mode = mode; } +static void maybe_init_display_devices(void) +{ + DWORD desktop_pid = 0; + HWND desktop_hwnd; + + /* Right after process init we initialize all the display devices, so there + * is no need to react to each individual event at that time. This check + * also helps us avoid calling NtUserGetDesktopWindow() (see below) at + * process init time, since it may not be safe. */ + if (!process_wayland.initialized) return; + + desktop_hwnd = NtUserGetDesktopWindow(); + NtUserGetWindowThread(desktop_hwnd, &desktop_pid); + + /* We only update the display devices from the desktop process. */ + if (GetCurrentProcessId() != desktop_pid) return; + + wayland_init_display_devices(TRUE); +} + static void wayland_output_done(struct wayland_output *output) { struct wayland_output_mode *mode; @@ -111,6 +131,8 @@ static void wayland_output_done(struct wayland_output *output) mode->width, mode->height, mode->refresh, output->current_mode == mode ? "*" : ""); } + + maybe_init_display_devices(); } static void output_handle_geometry(void *data, struct wl_output *wl_output, @@ -283,6 +305,8 @@ void wayland_output_destroy(struct wayland_output *output) wl_output_destroy(output->wl_output); free(output->name); free(output); + + maybe_init_display_devices(); } /********************************************************************** diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index d5cbb47bf9e..95d115113b3 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -47,6 +47,7 @@ extern struct wayland process_wayland DECLSPEC_HIDDEN; struct wayland { + BOOL initialized; struct wl_display *wl_display; struct wl_event_queue *wl_event_queue; struct wl_registry *wl_registry; @@ -80,7 +81,7 @@ struct wayland_output */ BOOL wayland_process_init(void) DECLSPEC_HIDDEN; -void wayland_init_display_devices(void) DECLSPEC_HIDDEN; +void wayland_init_display_devices(BOOL force) DECLSPEC_HIDDEN; /********************************************************************** * Wayland output -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2712
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Store all incoming Wayland output state as pending and make it current in a thread-safe way when the "done" event arrives. This enables other threads, with proper locking, to safely read consistent and complete Wayland output information. --- dlls/winewayland.drv/Makefile.in | 2 +- dlls/winewayland.drv/display.c | 10 ++- dlls/winewayland.drv/wayland.c | 5 +- dlls/winewayland.drv/wayland_output.c | 115 +++++++++++++++++++------- dlls/winewayland.drv/waylanddrv.h | 18 +++- 5 files changed, 112 insertions(+), 38 deletions(-) diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index 614ce8fe6e1..3f9c052f2b0 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -1,7 +1,7 @@ MODULE = winewayland.drv UNIXLIB = winewayland.so UNIX_CFLAGS = $(WAYLAND_CLIENT_CFLAGS) -UNIX_LIBS = -lwin32u $(WAYLAND_CLIENT_LIBS) +UNIX_LIBS = -lwin32u $(WAYLAND_CLIENT_LIBS) $(PTHREAD_LIBS) SOURCES = \ display.c \ diff --git a/dlls/winewayland.drv/display.c b/dlls/winewayland.drv/display.c index f734c1714fa..ebe151ffab0 100644 --- a/dlls/winewayland.drv/display.c +++ b/dlls/winewayland.drv/display.c @@ -50,7 +50,7 @@ void wayland_init_display_devices(BOOL force) struct output_info { int x, y; - struct wayland_output *output; + struct wayland_output_state *output; }; static int output_info_cmp_primary_x_y(const void *va, const void *vb) @@ -298,11 +298,13 @@ BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manage wl_array_init(&output_info_array); + pthread_mutex_lock(&process_wayland.output_mutex); + wl_list_for_each(output, &process_wayland.output_list, link) { - if (!output->current_mode) continue; + if (!output->current.current_mode) continue; output_info = wl_array_add(&output_info_array, sizeof(*output_info)); - if (output_info) output_info->output = output; + if (output_info) output_info->output = &output->current; else ERR("Failed to allocate space for output_info\n"); } @@ -321,5 +323,7 @@ BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manage wl_array_release(&output_info_array); + pthread_mutex_unlock(&process_wayland.output_mutex); + return TRUE; } diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 38c53e280c5..34aa8bec04e 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -34,7 +34,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); struct wayland process_wayland = { - .output_list = {&process_wayland.output_list, &process_wayland.output_list} + .output_list = {&process_wayland.output_list, &process_wayland.output_list}, + .output_mutex = PTHREAD_MUTEX_INITIALIZER }; /********************************************************************** @@ -77,7 +78,7 @@ static void registry_handle_global_remove(void *data, struct wl_registry *regist { if (output->global_id == id) { - TRACE("removing output->name=%s\n", output->name); + TRACE("removing output->name=%s\n", output->current.name); wayland_output_destroy(output); return; } diff --git a/dlls/winewayland.drv/wayland_output.c b/dlls/winewayland.drv/wayland_output.c index 8293f43b56f..5ea04340347 100644 --- a/dlls/winewayland.drv/wayland_output.c +++ b/dlls/winewayland.drv/wayland_output.c @@ -35,6 +35,11 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); static const int32_t default_refresh = 60000; static uint32_t next_output_id = 0; +#define WAYLAND_OUTPUT_CHANGED_MODES 0x01 +#define WAYLAND_OUTPUT_CHANGED_NAME 0x02 +#define WAYLAND_OUTPUT_CHANGED_LOGICAL_XY 0x04 +#define WAYLAND_OUTPUT_CHANGED_LOGICAL_WH 0x08 + /********************************************************************** * Output handling */ @@ -62,9 +67,9 @@ static int wayland_output_mode_cmp_rb(const void *key, return 0; } -static void wayland_output_add_mode(struct wayland_output *output, - int32_t width, int32_t height, - int32_t refresh, BOOL current) +static void wayland_output_state_add_mode(struct wayland_output_state *state, + int32_t width, int32_t height, + int32_t refresh, BOOL current) { struct rb_entry *mode_entry; struct wayland_output_mode *mode; @@ -75,7 +80,7 @@ static void wayland_output_add_mode(struct wayland_output *output, .refresh = refresh, }; - mode_entry = rb_get(&output->modes, &key); + mode_entry = rb_get(&state->modes, &key); if (mode_entry) { mode = RB_ENTRY_VALUE(mode_entry, struct wayland_output_mode, entry); @@ -91,10 +96,10 @@ static void wayland_output_add_mode(struct wayland_output *output, mode->width = width; mode->height = height; mode->refresh = refresh; - rb_put(&output->modes, mode, &mode->entry); + rb_put(&state->modes, mode, &mode->entry); } - if (current) output->current_mode = mode; + if (current) state->current_mode = mode; } static void maybe_init_display_devices(void) @@ -117,19 +122,65 @@ static void maybe_init_display_devices(void) wayland_init_display_devices(TRUE); } +static void wayland_output_mode_free_rb(struct rb_entry *entry, void *ctx) +{ + free(RB_ENTRY_VALUE(entry, struct wayland_output_mode, entry)); +} + static void wayland_output_done(struct wayland_output *output) { struct wayland_output_mode *mode; + /* Update current state from pending state. */ + pthread_mutex_lock(&process_wayland.output_mutex); + + if (output->pending_flags & WAYLAND_OUTPUT_CHANGED_MODES) + { + RB_FOR_EACH_ENTRY(mode, &output->pending.modes, struct wayland_output_mode, entry) + { + wayland_output_state_add_mode(&output->current, + mode->width, mode->height, mode->refresh, + mode == output->pending.current_mode); + } + rb_destroy(&output->pending.modes, wayland_output_mode_free_rb, NULL); + rb_init(&output->pending.modes, wayland_output_mode_cmp_rb); + } + + if (output->pending_flags & WAYLAND_OUTPUT_CHANGED_NAME) + { + free(output->current.name); + output->current.name = output->pending.name; + output->pending.name = NULL; + } + + if (output->pending_flags & WAYLAND_OUTPUT_CHANGED_LOGICAL_XY) + { + output->current.logical_x = output->pending.logical_x; + output->current.logical_y = output->pending.logical_y; + } + + if (output->pending_flags & WAYLAND_OUTPUT_CHANGED_LOGICAL_WH) + { + output->current.logical_w = output->pending.logical_w; + output->current.logical_h = output->pending.logical_h; + } + + if (wl_list_empty(&output->link)) + wl_list_insert(process_wayland.output_list.prev, &output->link); + + output->pending_flags = 0; + + pthread_mutex_unlock(&process_wayland.output_mutex); + TRACE("name=%s logical=%d,%d+%dx%d\n", - output->name, output->logical_x, output->logical_y, - output->logical_w, output->logical_h); + output->current.name, output->current.logical_x, output->current.logical_y, + output->current.logical_w, output->current.logical_h); - RB_FOR_EACH_ENTRY(mode, &output->modes, struct wayland_output_mode, entry) + RB_FOR_EACH_ENTRY(mode, &output->current.modes, struct wayland_output_mode, entry) { TRACE("mode %dx%d @ %d %s\n", mode->width, mode->height, mode->refresh, - output->current_mode == mode ? "*" : ""); + output->current.current_mode == mode ? "*" : ""); } maybe_init_display_devices(); @@ -153,8 +204,10 @@ static void output_handle_mode(void *data, struct wl_output *wl_output, /* Windows apps don't expect a zero refresh rate, so use a default value. */ if (refresh == 0) refresh = default_refresh; - wayland_output_add_mode(output, width, height, refresh, - (flags & WL_OUTPUT_MODE_CURRENT)); + wayland_output_state_add_mode(&output->pending, width, height, refresh, + (flags & WL_OUTPUT_MODE_CURRENT)); + + output->pending_flags |= WAYLAND_OUTPUT_CHANGED_MODES; } static void output_handle_done(void *data, struct wl_output *wl_output) @@ -187,8 +240,9 @@ static void zxdg_output_v1_handle_logical_position(void *data, { struct wayland_output *output = data; TRACE("logical_x=%d logical_y=%d\n", x, y); - output->logical_x = x; - output->logical_y = y; + output->pending.logical_x = x; + output->pending.logical_y = y; + output->pending_flags |= WAYLAND_OUTPUT_CHANGED_LOGICAL_XY; } static void zxdg_output_v1_handle_logical_size(void *data, @@ -198,8 +252,9 @@ static void zxdg_output_v1_handle_logical_size(void *data, { struct wayland_output *output = data; TRACE("logical_w=%d logical_h=%d\n", width, height); - output->logical_w = width; - output->logical_h = height; + output->pending.logical_w = width; + output->pending.logical_h = height; + output->pending_flags |= WAYLAND_OUTPUT_CHANGED_LOGICAL_WH; } static void zxdg_output_v1_handle_done(void *data, @@ -218,8 +273,9 @@ static void zxdg_output_v1_handle_name(void *data, { struct wayland_output *output = data; - free(output->name); - output->name = strdup(name); + free(output->pending.name); + output->pending.name = strdup(name); + output->pending_flags |= WAYLAND_OUTPUT_CHANGED_NAME; } static void zxdg_output_v1_handle_description(void *data, @@ -259,14 +315,15 @@ BOOL wayland_output_create(uint32_t id, uint32_t version) wl_output_add_listener(output->wl_output, &output_listener, output); wl_list_init(&output->link); - rb_init(&output->modes, wayland_output_mode_cmp_rb); + rb_init(&output->pending.modes, wayland_output_mode_cmp_rb); + rb_init(&output->current.modes, wayland_output_mode_cmp_rb); /* Have a fallback while we don't have compositor given name. */ name_len = snprintf(NULL, 0, "WaylandOutput%d", next_output_id); - output->name = malloc(name_len + 1); - if (output->name) + output->current.name = malloc(name_len + 1); + if (output->current.name) { - snprintf(output->name, name_len + 1, "WaylandOutput%d", next_output_id++); + snprintf(output->current.name, name_len + 1, "WaylandOutput%d", next_output_id++); } else { @@ -277,8 +334,6 @@ BOOL wayland_output_create(uint32_t id, uint32_t version) if (process_wayland.zxdg_output_manager_v1) wayland_output_use_xdg_extension(output); - wl_list_insert(process_wayland.output_list.prev, &output->link); - return TRUE; err: @@ -286,9 +341,10 @@ err: return FALSE; } -static void wayland_output_mode_free_rb(struct rb_entry *entry, void *ctx) +static void wayland_output_state_deinit(struct wayland_output_state *state) { - free(RB_ENTRY_VALUE(entry, struct wayland_output_mode, entry)); + rb_destroy(&state->modes, wayland_output_mode_free_rb, NULL); + free(state->name); } /********************************************************************** @@ -298,12 +354,15 @@ static void wayland_output_mode_free_rb(struct rb_entry *entry, void *ctx) */ void wayland_output_destroy(struct wayland_output *output) { - rb_destroy(&output->modes, wayland_output_mode_free_rb, NULL); + pthread_mutex_lock(&process_wayland.output_mutex); wl_list_remove(&output->link); + pthread_mutex_unlock(&process_wayland.output_mutex); + + wayland_output_state_deinit(&output->pending); + wayland_output_state_deinit(&output->current); if (output->zxdg_output_v1) zxdg_output_v1_destroy(output->zxdg_output_v1); wl_output_destroy(output->wl_output); - free(output->name); free(output); maybe_init_display_devices(); diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 95d115113b3..600fcfce355 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -25,6 +25,7 @@ # error You must include config.h to use this header #endif +#include <pthread.h> #include <wayland-client.h> #include "xdg-output-unstable-v1-client-protocol.h" @@ -53,6 +54,8 @@ struct wayland struct wl_registry *wl_registry; struct zxdg_output_manager_v1 *zxdg_output_manager_v1; struct wl_list output_list; + /* Protects the output_list and the wayland_output.current states. */ + pthread_mutex_t output_mutex; }; struct wayland_output_mode @@ -63,17 +66,24 @@ struct wayland_output_mode int32_t refresh; }; -struct wayland_output +struct wayland_output_state { - struct wl_list link; - struct wl_output *wl_output; - struct zxdg_output_v1 *zxdg_output_v1; struct rb_tree modes; struct wayland_output_mode *current_mode; char *name; int logical_x, logical_y; int logical_w, logical_h; +}; + +struct wayland_output +{ + struct wl_list link; + struct wl_output *wl_output; + struct zxdg_output_v1 *zxdg_output_v1; uint32_t global_id; + unsigned int pending_flags; + struct wayland_output_state pending; + struct wayland_output_state current; }; /********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2712
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Use a driver internal window message to dispatch updates to display devices from the desktop window thread. --- dlls/winewayland.drv/Makefile.in | 1 + dlls/winewayland.drv/wayland_output.c | 2 +- dlls/winewayland.drv/waylanddrv.h | 6 ++++ dlls/winewayland.drv/waylanddrv_main.c | 1 + dlls/winewayland.drv/window.c | 47 ++++++++++++++++++++++++++ 5 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 dlls/winewayland.drv/window.c diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index 3f9c052f2b0..454a7e5a4b4 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -10,4 +10,5 @@ SOURCES = \ wayland.c \ wayland_output.c \ waylanddrv_main.c \ + window.c \ xdg-output-unstable-v1.xml diff --git a/dlls/winewayland.drv/wayland_output.c b/dlls/winewayland.drv/wayland_output.c index 5ea04340347..4b2cb0505be 100644 --- a/dlls/winewayland.drv/wayland_output.c +++ b/dlls/winewayland.drv/wayland_output.c @@ -119,7 +119,7 @@ static void maybe_init_display_devices(void) /* We only update the display devices from the desktop process. */ if (GetCurrentProcessId() != desktop_pid) return; - wayland_init_display_devices(TRUE); + NtUserPostMessage(desktop_hwnd, WM_WAYLAND_INIT_DISPLAY_DEVICES, 0, 0); } static void wayland_output_mode_free_rb(struct rb_entry *entry, void *ctx) diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 600fcfce355..ecd144950c2 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -46,6 +46,11 @@ extern struct wayland process_wayland DECLSPEC_HIDDEN; * Definitions for wayland types */ +enum wayland_window_message +{ + WM_WAYLAND_INIT_DISPLAY_DEVICES = 0x80001000 +}; + struct wayland { BOOL initialized; @@ -107,5 +112,6 @@ void wayland_output_use_xdg_extension(struct wayland_output *output) DECLSPEC_HI BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manager, BOOL force, void *param) DECLSPEC_HIDDEN; +LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) DECLSPEC_HIDDEN; #endif /* __WINE_WAYLANDDRV_H */ diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index a578db76c66..1d9877f4518 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -32,6 +32,7 @@ static const struct user_driver_funcs waylanddrv_funcs = { .pUpdateDisplayDevices = WAYLAND_UpdateDisplayDevices, + .pWindowMessage = WAYLAND_WindowMessage }; static NTSTATUS waylanddrv_unix_init(void *arg) diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c new file mode 100644 index 00000000000..946ad6801a9 --- /dev/null +++ b/dlls/winewayland.drv/window.c @@ -0,0 +1,47 @@ +/* + * Wayland window handling + * + * Copyright 2020 Alexandros Frantzis for Collabora Ltd + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "config.h" + +#include "waylanddrv.h" + +#include "wine/debug.h" + +WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); + +/********************************************************************** + * WAYLAND_WindowMessage + */ +LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) +{ + switch (msg) + { + case WM_WAYLAND_INIT_DISPLAY_DEVICES: + wayland_init_display_devices(TRUE); + return 0; + default: + FIXME("got window msg %x hwnd %p wp %lx lp %lx\n", msg, hwnd, (long)wp, lp); + return 0; + } +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2712
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Update the desktop window size to match the current virtual screen rect. --- dlls/winewayland.drv/waylanddrv.h | 1 + dlls/winewayland.drv/waylanddrv_main.c | 1 + dlls/winewayland.drv/window.c | 26 ++++++++++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index ecd144950c2..57125e011cb 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -110,6 +110,7 @@ void wayland_output_use_xdg_extension(struct wayland_output *output) DECLSPEC_HI * USER driver functions */ +LRESULT WAYLAND_DesktopWindowProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) DECLSPEC_HIDDEN; BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manager, BOOL force, void *param) DECLSPEC_HIDDEN; LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) DECLSPEC_HIDDEN; diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index 1d9877f4518..2a7bed9d1d8 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -31,6 +31,7 @@ static const struct user_driver_funcs waylanddrv_funcs = { + .pDesktopWindowProc = WAYLAND_DesktopWindowProc, .pUpdateDisplayDevices = WAYLAND_UpdateDisplayDevices, .pWindowMessage = WAYLAND_WindowMessage }; diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 946ad6801a9..ea5ac22a8a2 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -30,6 +30,16 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); +static void wayland_resize_desktop(void) +{ + RECT virtual_rect = NtUserGetVirtualScreenRect(); + NtUserSetWindowPos(NtUserGetDesktopWindow(), 0, + virtual_rect.left, virtual_rect.top, + virtual_rect.right - virtual_rect.left, + virtual_rect.bottom - virtual_rect.top, + SWP_NOZORDER | SWP_NOACTIVATE | SWP_DEFERERASE); +} + /********************************************************************** * WAYLAND_WindowMessage */ @@ -39,9 +49,25 @@ LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) { case WM_WAYLAND_INIT_DISPLAY_DEVICES: wayland_init_display_devices(TRUE); + wayland_resize_desktop(); return 0; default: FIXME("got window msg %x hwnd %p wp %lx lp %lx\n", msg, hwnd, (long)wp, lp); return 0; } } + +/********************************************************************** + * WAYLAND_DesktopWindowProc + */ +LRESULT WAYLAND_DesktopWindowProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) +{ + switch (msg) + { + case WM_DISPLAYCHANGE: + wayland_resize_desktop(); + break; + } + + return NtUserMessageCall(hwnd, msg, wp, lp, 0, NtUserDefWindowProc, FALSE); +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2712
On Tue May 23 14:15:06 2023 +0000, Alexandros Frantzis wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/2712/diffs?diff_id=48222&start_sha=a0669e06347fef91e93e0e58019d23906b03afa9#0529b4a040cd83447fe520f50525b3459c88f5d5_128_122) TL;DR: Changed to `NtUserPostMessage`
This was an intentional change, since this new approach makes the sync method straightforward and seems to me to be more "correct", as it removes any, even transient, disagreement between Wayland output and Wine monitor state in the desktop window process.
From a practical perspective I think both sync and async processing of this message would work OK with the proper care. With async we need to ensure we properly handle any transient output/monitor state disagreement. With sync we need to avoid potential deadlocks between the dispatch thread and the (desktop) window thread.
So, after all that, I think that I now have changed my mind again :) I'd rather deal with the potential of state disagreement (which is somewhat inherent anyway, and which I can manage internally) than deadlocks (however unlikely), so I have switched `NtUserPostMessage`. Thanks for raising this @rbernon! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2712#note_33532
On Tue May 23 14:15:07 2023 +0000, Alexandros Frantzis wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/2712/diffs?diff_id=48222&start_sha=a0669e06347fef91e93e0e58019d23906b03afa9#0529b4a040cd83447fe520f50525b3459c88f5d5_109_105) Thanks, not needed after the `NtUserPostMessage` change.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2712#note_33533
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2712
On Tue May 23 14:15:36 2023 +0000, Alexandros Frantzis wrote:
TL;DR: Changed to `NtUserPostMessage` This was an intentional change, since this new approach makes the sync method straightforward and seems to me to be more "correct", as it removes any, even transient, disagreement between Wayland output and Wine monitor state in the desktop window process. From a practical perspective I think both sync and async processing of this message would work OK with the proper care. With async we need to ensure we properly handle any transient output/monitor state disagreement. With sync we need to avoid potential deadlocks between the dispatch thread and the (desktop) window thread. So, after all that, I think that I now have changed my mind again :) I'd rather deal with the potential of state disagreement (which is somewhat inherent anyway, and which I can manage internally) than deadlocks (however unlikely), so I have switched `NtUserPostMessage`. Thanks for raising this @rbernon! BTW, this changes the effective "mask" of the operation from `QS_SENDMESSAGE` to `QS_POSTMESSAGE`, but I don't think this makes any practical difference in this case (esp. given that this is only relevant to the desktop window thread)? Is there some rule or rationale about which one to prefer for display change events, I see winex11 uses `QS_SENDMESSAGE` (the default case in `filter_event` for xrandr events, if I am not misunderstanding the logic), but winemac `QS_POSTMESSAGE`? If we want `QS_SENDMESSAGE` I could switch to `send_notify_message`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2712#note_33539
participants (3)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Rémi Bernon