[PATCH v7 0/5] MR2476: winewayland.drv: part 2: Display configuration enhancements
This second MR in the Wayland series adds display configuration related enhancements, including providing more display information (like a proper/consistent name and position) to Wine and supporting GetCurrentDisplaySettings. To get additional information from the Wayland compositor we need to use a protocol from the wayland-protocols collection, so we introduce support in the build system for protocol .xml files. These changes are introduced on their own commit in the series, but we can squash it with the next commit, where the functionality is first used, if preferred. To be able to provide a consistent view of the display settings across all processes, this MR leverages the win32u CURRENT_SETTINGS storage mechanism (which was previously used only in the null driver). Thanks! -- v7: winewayland.drv: Infer and report Windows monitor positions. winewayland.drv: Basic support for Windows monitor positioning. winewayland.drv: Use the output name reported by the compositor. tools: Support building Wayland protocol source files. winewayland.drv: Handle wl_output objects only in the desktop process. https://gitlab.winehq.org/wine/wine/-/merge_requests/2476
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Since non-desktop processes rely on the win32u internal current settings handling to get display information, they don't need to access wl_output objects themselves for now. Signed-off-by: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/display.c | 15 ++++----------- dlls/winewayland.drv/wayland.c | 3 +++ dlls/winewayland.drv/waylanddrv.h | 6 ++++++ dlls/winewayland.drv/waylanddrv_main.c | 25 +++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/dlls/winewayland.drv/display.c b/dlls/winewayland.drv/display.c index bd99441170e..2a5c86e2d8c 100644 --- a/dlls/winewayland.drv/display.c +++ b/dlls/winewayland.drv/display.c @@ -44,16 +44,7 @@ static void wayland_refresh_display_devices(void) void wayland_init_display_devices(void) { - struct ntuser_thread_info *thread_info = NtUserGetThreadInfo(); - DWORD current_pid = GetCurrentProcessId(); - HWND desktop_hwnd = UlongToHandle(thread_info->top_window); - DWORD desktop_pid = 0; - - if (desktop_hwnd) NtUserGetWindowThread(desktop_hwnd, &desktop_pid); - - /* Refresh devices only from the desktop window process. */ - if (!desktop_pid || current_pid == desktop_pid) - wayland_refresh_display_devices(); + wayland_refresh_display_devices(); } static void wayland_add_device_gpu(const struct gdi_device_manager *device_manager, @@ -141,7 +132,9 @@ BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manage struct wayland_output *output; INT output_id = 0; - if (!force && !force_display_devices_refresh) return TRUE; + /* For now, update the display devices only when a compositor side update + * is received in the desktop window process. */ + if (!force_display_devices_refresh) return TRUE; TRACE("force=%d force_refresh=%d\n", force, force_display_devices_refresh); diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 104a091ea80..2f728355438 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -45,6 +45,9 @@ static void registry_handle_global(void *data, struct wl_registry *registry, { TRACE("interface=%s version=%u id=%u\n", interface, version, id); + /* For now only the desktop process needs wl_output objects. */ + if (GetCurrentProcessId() != get_desktop_process_id()) return; + if (strcmp(interface, "wl_output") == 0) { if (!wayland_output_create(id, version)) diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 8458ed17df2..7749a0d1e7d 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -84,6 +84,12 @@ void wayland_init_display_devices(void) DECLSPEC_HIDDEN; BOOL wayland_output_create(uint32_t id, uint32_t version) DECLSPEC_HIDDEN; void wayland_output_destroy(struct wayland_output *output) DECLSPEC_HIDDEN; +/********************************************************************** + * USER32 helpers + */ + +DWORD get_desktop_process_id(void) DECLSPEC_HIDDEN; + /********************************************************************** * USER driver functions */ diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index a9297edc500..299aee6382d 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -27,8 +27,33 @@ #include "ntstatus.h" #define WIN32_NO_STATUS +#include "wine/server.h" + #include "waylanddrv.h" +DWORD get_desktop_process_id(void) +{ + struct ntuser_thread_info *thread_info = NtUserGetThreadInfo(); + HWND desktop_hwnd = UlongToHandle(thread_info->top_window); + DWORD desktop_pid; + + /* The thread information may not have been updated yet, so also query + * the server directly to be certain. */ + if (!desktop_hwnd) + { + SERVER_START_REQ(get_desktop_window) + { + req->force = 0; + if (!wine_server_call(req)) + desktop_hwnd = UlongToHandle(reply->top_window); + } + SERVER_END_REQ; + } + + return NtUserGetWindowThread(desktop_hwnd, &desktop_pid) ? + desktop_pid : GetCurrentProcessId(); +} + static const struct user_driver_funcs waylanddrv_funcs = { .pUpdateDisplayDevices = WAYLAND_UpdateDisplayDevices, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2476
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Wayland protocol descriptions are distributed as source XML files that need to be transformed to C source and header files with a version of the wayland-scanner tool compatible with the used libwayland library. This commit enhances the makedep build tool to support building such Wayland protocol XML files. Components can use the WAYLAND_PROTOCOL_SRCS build variable to add protocol XML files to their build. Signed-off-by: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- tools/makedep.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/tools/makedep.c b/tools/makedep.c index 49a7514c9f0..8f7c7389526 100644 --- a/tools/makedep.c +++ b/tools/makedep.c @@ -100,6 +100,7 @@ struct incl_file #define FLAG_C_IMPLIB 0x040000 /* file is part of an import library */ #define FLAG_C_UNIX 0x080000 /* file is part of a Unix library */ #define FLAG_SFD_FONTS 0x100000 /* sfd file generated bitmap fonts */ +#define FLAG_WAYLAND_PROTO 0x200000 /* generates wayland protocol .c/.h files */ static const struct { @@ -157,6 +158,7 @@ static const char *icotool; static const char *msgfmt; static const char *ln_s; static const char *sed_cmd; +static const char *wayland_scanner; /* per-architecture global variables */ static const char *arch_dirs[MAX_ARCHS]; static const char *arch_pe_dirs[MAX_ARCHS]; @@ -1176,6 +1178,25 @@ static void parse_sfd_file( struct file *source, FILE *file ) } +/******************************************************************* + * parse_xml_file + */ +static void parse_xml_file( struct file *source, FILE *file ) +{ + char *buffer; + + input_line = 0; + while ((buffer = get_line( file ))) + { + if (strstr( buffer, "<protocol name=" )) + { + source->flags |= FLAG_WAYLAND_PROTO; + break; + } + } +} + + static const struct { const char *ext; @@ -1193,7 +1214,8 @@ static const struct { ".idl", parse_idl_file }, { ".rc", parse_rc_file }, { ".in", parse_in_file }, - { ".sfd", parse_sfd_file } + { ".sfd", parse_sfd_file }, + { ".xml", parse_xml_file } }; /******************************************************************* @@ -1428,6 +1450,7 @@ static struct file *open_include_file( const struct makefile *make, struct incl_ if ((file = open_local_generated_file( make, pFile, ".cur", ".svg" ))) return file; if ((file = open_local_generated_file( make, pFile, ".ico", ".svg" ))) return file; } + if ((file = open_local_generated_file( make, pFile, "-client-protocol.h", ".xml" ))) return file; /* check for extra targets */ if (strarray_exists( &make->extra_targets, pFile->name )) @@ -1915,6 +1938,21 @@ static void add_generated_sources( struct makefile *make ) strarray_addall_uniq( &make->extra_imports, get_expanded_file_local_var( make, obj, "IMPORTS" )); } + if (source->file->flags & FLAG_WAYLAND_PROTO) + { + char *code_name = replace_extension ( source->name , ".xml", "-protocol.c" ); + char *header_name = replace_extension ( source->name , ".xml", "-client-protocol.h" ); + + file = add_generated_source( make, code_name, NULL, 0 ); + file->file->flags |= FLAG_C_UNIX; + file->use_msvcrt = 0; + file = add_generated_source( make, header_name, NULL, 0 ); + file->file->flags |= FLAG_C_UNIX; + file->use_msvcrt = 0; + + free( code_name ); + free( header_name ); + } } if (make->testdll) { @@ -3101,6 +3139,16 @@ static void output_source_spec( struct makefile *make, struct incl_file *source, } } +static void output_source_xml( struct makefile *make, struct incl_file *source, const char *obj ) +{ + if ((source->file->flags & FLAG_WAYLAND_PROTO) && wayland_scanner) + { + output( "%s-protocol.c: %s\n", obj_dir_path( make, obj ), source->filename ); + output( "\t%s%s private-code $< $@\n", cmd_prefix( "WAYLAND_SCANNER" ), wayland_scanner ); + output( "%s-client-protocol.h: %s\n", obj_dir_path( make, obj ), source->filename ); + output( "\t%s%s client-header $< $@\n", cmd_prefix( "WAYLAND_SCANNER" ), wayland_scanner); + } +} /******************************************************************* * output_source_one_arch @@ -3240,6 +3288,7 @@ static const struct { "in", output_source_in }, { "x", output_source_x }, { "spec", output_source_spec }, + { "xml", output_source_xml }, { NULL, output_source_default } }; @@ -4054,6 +4103,7 @@ static void output_silent_rules(void) "MSG", "SED", "TEST", + "WAYLAND_SCANNER", "WIDL", "WMC", "WRC" @@ -4153,6 +4203,7 @@ static void load_sources( struct makefile *make ) "IN_SRCS", "PO_SRCS", "MANPAGES", + "WAYLAND_PROTOCOL_SRCS", NULL }; const char **var; @@ -4383,6 +4434,7 @@ int main( int argc, char *argv[] ) msgfmt = get_expanded_make_variable( top_makefile, "MSGFMT" ); sed_cmd = get_expanded_make_variable( top_makefile, "SED_CMD" ); ln_s = get_expanded_make_variable( top_makefile, "LN_S" ); + wayland_scanner = get_expanded_make_variable( top_makefile, "WAYLAND_SCANNER" ); if (root_src_dir && !strcmp( root_src_dir, "." )) root_src_dir = NULL; if (tools_dir && !strcmp( tools_dir, "." )) tools_dir = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2476
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Use the xdg-output-unstable-v1 protocol to get a unique, cross-process consistent name for the outputs. Signed-off-by: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- configure | 50 +++- configure.ac | 4 +- dlls/winewayland.drv/Makefile.in | 3 + dlls/winewayland.drv/wayland.c | 12 + dlls/winewayland.drv/wayland_output.c | 111 ++++++++- dlls/winewayland.drv/waylanddrv.h | 6 +- .../xdg-output-unstable-v1.xml | 220 ++++++++++++++++++ 7 files changed, 392 insertions(+), 14 deletions(-) create mode 100644 dlls/winewayland.drv/xdg-output-unstable-v1.xml diff --git a/configure b/configure index 4a33bf69ff1..3a9f9bbd355 100755 --- a/configure +++ b/configure @@ -702,6 +702,7 @@ INOTIFY_LIBS INOTIFY_CFLAGS PCSCLITE_LIBS PCAP_LIBS +WAYLAND_SCANNER WAYLAND_CLIENT_LIBS WAYLAND_CLIENT_CFLAGS X_EXTRA_LIBS @@ -15665,8 +15666,54 @@ fi CPPFLAGS=$ac_save_CPPFLAGS + # Extract the first word of "wayland-scanner", so it can be a program name with args. +set dummy wayland-scanner; ac_word=$2 +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5 +printf %s "checking for $ac_word... " >&6; } +if test ${ac_cv_path_WAYLAND_SCANNER+y} +then : + printf %s "(cached) " >&6 +else $as_nop + case $WAYLAND_SCANNER in + [\\/]* | ?:[\\/]*) + ac_cv_path_WAYLAND_SCANNER="$WAYLAND_SCANNER" # Let the user override the test with a path. + ;; + *) + as_save_IFS=$IFS; IFS=$PATH_SEPARATOR +for as_dir in $PATH +do + IFS=$as_save_IFS + case $as_dir in #((( + '') as_dir=./ ;; + */) ;; + *) as_dir=$as_dir/ ;; + esac + for ac_exec_ext in '' $ac_executable_extensions; do + if as_fn_executable_p "$as_dir$ac_word$ac_exec_ext"; then + ac_cv_path_WAYLAND_SCANNER="$as_dir$ac_word$ac_exec_ext" + printf "%s\n" "$as_me:${as_lineno-$LINENO}: found $as_dir$ac_word$ac_exec_ext" >&5 + break 2 + fi +done + done +IFS=$as_save_IFS + + test -z "$ac_cv_path_WAYLAND_SCANNER" && ac_cv_path_WAYLAND_SCANNER="`test -n "$PKG_CONFIG" && $PKG_CONFIG --variable=wayland_scanner wayland-scanner`" + ;; +esac +fi +WAYLAND_SCANNER=$ac_cv_path_WAYLAND_SCANNER +if test -n "$WAYLAND_SCANNER"; then + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $WAYLAND_SCANNER" >&5 +printf "%s\n" "$WAYLAND_SCANNER" >&6; } +else + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: no" >&5 +printf "%s\n" "no" >&6; } +fi + + fi -if test -z "$WAYLAND_CLIENT_LIBS" +if test -z "$WAYLAND_CLIENT_LIBS" -o -z "$WAYLAND_SCANNER" then : case "x$with_wayland" in x) as_fn_append wine_notices "|Wayland ${notice_platform}development files not found, the Wayland driver won't be supported." ;; @@ -23178,6 +23225,7 @@ X_LIBS = $X_LIBS X_EXTRA_LIBS = $X_EXTRA_LIBS WAYLAND_CLIENT_CFLAGS = $WAYLAND_CLIENT_CFLAGS WAYLAND_CLIENT_LIBS = $WAYLAND_CLIENT_LIBS +WAYLAND_SCANNER = $WAYLAND_SCANNER PCAP_LIBS = $PCAP_LIBS PCSCLITE_LIBS = $PCSCLITE_LIBS INOTIFY_CFLAGS = $INOTIFY_CFLAGS diff --git a/configure.ac b/configure.ac index c4f34e9e851..05cb25997f6 100644 --- a/configure.ac +++ b/configure.ac @@ -1343,8 +1343,10 @@ then [AC_CHECK_HEADERS([wayland-client.h]) AC_CHECK_LIB(wayland-client,wl_display_connect,[:], [WAYLAND_CLIENT_LIBS=""],[$WAYLAND_CLIENT_LIBS])]) + AC_PATH_PROG(WAYLAND_SCANNER,wayland-scanner, + [`test -n "$PKG_CONFIG" && $PKG_CONFIG --variable=wayland_scanner wayland-scanner`]) fi -WINE_NOTICE_WITH(wayland, [test -z "$WAYLAND_CLIENT_LIBS"], +WINE_NOTICE_WITH(wayland, [test -z "$WAYLAND_CLIENT_LIBS" -o -z "$WAYLAND_SCANNER"], [Wayland ${notice_platform}development files not found, the Wayland driver won't be supported.], [enable_winewayland_drv]) diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index d86b51a519f..326a9d765c8 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -10,4 +10,7 @@ C_SRCS = \ wayland_output.c \ waylanddrv_main.c +WAYLAND_PROTOCOL_SRCS = \ + xdg-output-unstable-v1.xml + RC_SRCS = version.rc diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 2f728355438..2eb58fb7139 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -53,6 +53,18 @@ static void registry_handle_global(void *data, struct wl_registry *registry, if (!wayland_output_create(id, version)) ERR("Failed to create wayland_output for global id=%u\n", id); } + else if (strcmp(interface, "zxdg_output_manager_v1") == 0) + { + struct wayland_output *output; + + 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) + wayland_output_use_xdg_extension(output); + } } static void registry_handle_global_remove(void *data, struct wl_registry *registry, diff --git a/dlls/winewayland.drv/wayland_output.c b/dlls/winewayland.drv/wayland_output.c index be9ea32bd72..8eb24fe97f3 100644 --- a/dlls/winewayland.drv/wayland_output.c +++ b/dlls/winewayland.drv/wayland_output.c @@ -97,6 +97,22 @@ static void wayland_output_add_mode(struct wayland_output *output, if (current) output->current_mode = mode; } +static void wayland_output_done(struct wayland_output *output) +{ + struct wayland_output_mode *mode; + + TRACE("name=%s\n", output->name); + + RB_FOR_EACH_ENTRY(mode, &output->modes, struct wayland_output_mode, entry) + { + TRACE("mode %dx%d @ %d %s\n", + mode->width, mode->height, mode->refresh, + output->current_mode == mode ? "*" : ""); + } + + wayland_init_display_devices(); +} + static void output_handle_geometry(void *data, struct wl_output *wl_output, int32_t x, int32_t y, int32_t physical_width, int32_t physical_height, @@ -122,18 +138,12 @@ static void output_handle_mode(void *data, struct wl_output *wl_output, static void output_handle_done(void *data, struct wl_output *wl_output) { struct wayland_output *output = data; - struct wayland_output_mode *mode; - - TRACE("name=%s\n", output->name); - RB_FOR_EACH_ENTRY(mode, &output->modes, struct wayland_output_mode, entry) + if (!output->zxdg_output_v1 || + zxdg_output_v1_get_version(output->zxdg_output_v1) >= 3) { - TRACE("mode %dx%d @ %d %s\n", - mode->width, mode->height, mode->refresh, - output->current_mode == mode ? "*" : ""); + wayland_output_done(output); } - - wayland_init_display_devices(); } static void output_handle_scale(void *data, struct wl_output *wl_output, @@ -148,6 +158,54 @@ static const struct wl_output_listener output_listener = { output_handle_scale }; +static void zxdg_output_v1_handle_logical_position(void *data, + struct zxdg_output_v1 *zxdg_output_v1, + int32_t x, + int32_t y) +{ +} + +static void zxdg_output_v1_handle_logical_size(void *data, + struct zxdg_output_v1 *zxdg_output_v1, + int32_t width, + int32_t height) +{ +} + +static void zxdg_output_v1_handle_done(void *data, + struct zxdg_output_v1 *zxdg_output_v1) +{ + if (zxdg_output_v1_get_version(zxdg_output_v1) < 3) + { + struct wayland_output *output = data; + wayland_output_done(output); + } +} + +static void zxdg_output_v1_handle_name(void *data, + struct zxdg_output_v1 *zxdg_output_v1, + const char *name) +{ + struct wayland_output *output = data; + + free(output->name); + output->name = strdup(name); +} + +static void zxdg_output_v1_handle_description(void *data, + struct zxdg_output_v1 *zxdg_output_v1, + const char *description) +{ +} + +static const struct zxdg_output_v1_listener zxdg_output_v1_listener = { + zxdg_output_v1_handle_logical_position, + zxdg_output_v1_handle_logical_size, + zxdg_output_v1_handle_done, + zxdg_output_v1_handle_name, + zxdg_output_v1_handle_description, +}; + /********************************************************************** * wayland_output_create * @@ -156,6 +214,7 @@ static const struct wl_output_listener output_listener = { BOOL wayland_output_create(uint32_t id, uint32_t version) { struct wayland_output *output = calloc(1, sizeof(*output)); + int name_len; if (!output) { @@ -172,8 +231,21 @@ BOOL wayland_output_create(uint32_t id, uint32_t version) wl_list_init(&output->link); rb_init(&output->modes, wayland_output_mode_cmp_rb); - snprintf(output->name, sizeof(output->name), "WaylandOutput%d", - next_output_id++); + /* 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) + { + snprintf(output->name, name_len + 1, "WaylandOutput%d", next_output_id++); + } + else + { + ERR("Couldn't allocate space for output name\n"); + goto err; + } + + if (process_wayland->zxdg_output_manager_v1) + wayland_output_use_xdg_extension(output); wl_list_insert(process_wayland->output_list.prev, &output->link); @@ -198,6 +270,23 @@ void wayland_output_destroy(struct wayland_output *output) { rb_destroy(&output->modes, wayland_output_mode_free_rb, NULL); wl_list_remove(&output->link); + if (output->zxdg_output_v1) + zxdg_output_v1_destroy(output->zxdg_output_v1); wl_output_destroy(output->wl_output); + free(output->name); free(output); } + +/********************************************************************** + * wayland_output_use_xdg_extension + * + * Use the zxdg_output_v1 extension to get output information. + */ +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, + 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 7749a0d1e7d..98c31068f2f 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -26,6 +26,7 @@ #endif #include <wayland-client.h> +#include "xdg-output-unstable-v1-client-protocol.h" #include "windef.h" #include "winbase.h" @@ -49,6 +50,7 @@ struct wayland { struct wl_event_queue *wl_event_queue; struct wl_registry *wl_registry; + struct zxdg_output_manager_v1 *zxdg_output_manager_v1; struct wl_list output_list; }; @@ -64,9 +66,10 @@ struct wayland_output { 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[20]; + char *name; uint32_t global_id; }; @@ -83,6 +86,7 @@ void wayland_init_display_devices(void) DECLSPEC_HIDDEN; BOOL wayland_output_create(uint32_t id, uint32_t version) DECLSPEC_HIDDEN; void wayland_output_destroy(struct wayland_output *output) DECLSPEC_HIDDEN; +void wayland_output_use_xdg_extension(struct wayland_output *output) DECLSPEC_HIDDEN; /********************************************************************** * USER32 helpers diff --git a/dlls/winewayland.drv/xdg-output-unstable-v1.xml b/dlls/winewayland.drv/xdg-output-unstable-v1.xml new file mode 100644 index 00000000000..9a5b7900097 --- /dev/null +++ b/dlls/winewayland.drv/xdg-output-unstable-v1.xml @@ -0,0 +1,220 @@ +<?xml version="1.0" encoding="UTF-8"?> +<protocol name="xdg_output_unstable_v1"> + + <copyright> + Copyright © 2017 Red Hat Inc. + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice (including the next + paragraph) shall be included in all copies or substantial portions of the + Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. + </copyright> + + <description summary="Protocol to describe output regions"> + This protocol aims at describing outputs in a way which is more in line + with the concept of an output on desktop oriented systems. + + Some information are more specific to the concept of an output for + a desktop oriented system and may not make sense in other applications, + such as IVI systems for example. + + Typically, the global compositor space on a desktop system is made of + a contiguous or overlapping set of rectangular regions. + + Some of the information provided in this protocol might be identical + to their counterparts already available from wl_output, in which case + the information provided by this protocol should be preferred to their + equivalent in wl_output. The goal is to move the desktop specific + concepts (such as output location within the global compositor space, + the connector name and types, etc.) out of the core wl_output protocol. + + Warning! The protocol described in this file is experimental and + backward incompatible changes may be made. Backward compatible + changes may be added together with the corresponding interface + version bump. + Backward incompatible changes are done by bumping the version + number in the protocol and interface names and resetting the + interface version. Once the protocol is to be declared stable, + the 'z' prefix and the version number in the protocol and + interface names are removed and the interface version number is + reset. + </description> + + <interface name="zxdg_output_manager_v1" version="3"> + <description summary="manage xdg_output objects"> + A global factory interface for xdg_output objects. + </description> + + <request name="destroy" type="destructor"> + <description summary="destroy the xdg_output_manager object"> + Using this request a client can tell the server that it is not + going to use the xdg_output_manager object anymore. + + Any objects already created through this instance are not affected. + </description> + </request> + + <request name="get_xdg_output"> + <description summary="create an xdg output from a wl_output"> + This creates a new xdg_output object for the given wl_output. + </description> + <arg name="id" type="new_id" interface="zxdg_output_v1"/> + <arg name="output" type="object" interface="wl_output"/> + </request> + </interface> + + <interface name="zxdg_output_v1" version="3"> + <description summary="compositor logical output region"> + An xdg_output describes part of the compositor geometry. + + This typically corresponds to a monitor that displays part of the + compositor space. + + For objects version 3 onwards, after all xdg_output properties have been + sent (when the object is created and when properties are updated), a + wl_output.done event is sent. This allows changes to the output + properties to be seen as atomic, even if they happen via multiple events. + </description> + + <request name="destroy" type="destructor"> + <description summary="destroy the xdg_output object"> + Using this request a client can tell the server that it is not + going to use the xdg_output object anymore. + </description> + </request> + + <event name="logical_position"> + <description summary="position of the output within the global compositor space"> + The position event describes the location of the wl_output within + the global compositor space. + + The logical_position event is sent after creating an xdg_output + (see xdg_output_manager.get_xdg_output) and whenever the location + of the output changes within the global compositor space. + </description> + <arg name="x" type="int" + summary="x position within the global compositor space"/> + <arg name="y" type="int" + summary="y position within the global compositor space"/> + </event> + + <event name="logical_size"> + <description summary="size of the output in the global compositor space"> + The logical_size event describes the size of the output in the + global compositor space. + + For example, a surface without any buffer scale, transformation + nor rotation set, with the size matching the logical_size will + have the same size as the corresponding output when displayed. + + Most regular Wayland clients should not pay attention to the + logical size and would rather rely on xdg_shell interfaces. + + Some clients such as Xwayland, however, need this to configure + their surfaces in the global compositor space as the compositor + may apply a different scale from what is advertised by the output + scaling property (to achieve fractional scaling, for example). + + For example, for a wl_output mode 3840×2160 and a scale factor 2: + + - A compositor not scaling the surface buffers will advertise a + logical size of 3840×2160, + + - A compositor automatically scaling the surface buffers will + advertise a logical size of 1920×1080, + + - A compositor using a fractional scale of 1.5 will advertise a + logical size of 2560×1440. + + For example, for a wl_output mode 1920×1080 and a 90 degree rotation, + the compositor will advertise a logical size of 1080x1920. + + The logical_size event is sent after creating an xdg_output + (see xdg_output_manager.get_xdg_output) and whenever the logical + size of the output changes, either as a result of a change in the + applied scale or because of a change in the corresponding output + mode(see wl_output.mode) or transform (see wl_output.transform). + </description> + <arg name="width" type="int" + summary="width in global compositor space"/> + <arg name="height" type="int" + summary="height in global compositor space"/> + </event> + + <event name="done"> + <description summary="all information about the output have been sent"> + This event is sent after all other properties of an xdg_output + have been sent. + + This allows changes to the xdg_output properties to be seen as + atomic, even if they happen via multiple events. + + For objects version 3 onwards, this event is deprecated. Compositors + are not required to send it anymore and must send wl_output.done + instead. + </description> + </event> + + <!-- Version 2 additions --> + + <event name="name" since="2"> + <description summary="name of this output"> + Many compositors will assign names to their outputs, show them to the + user, allow them to be configured by name, etc. The client may wish to + know this name as well to offer the user similar behaviors. + + The naming convention is compositor defined, but limited to + alphanumeric characters and dashes (-). Each name is unique among all + wl_output globals, but if a wl_output global is destroyed the same name + may be reused later. The names will also remain consistent across + sessions with the same hardware and software configuration. + + Examples of names include 'HDMI-A-1', 'WL-1', 'X11-1', etc. However, do + not assume that the name is a reflection of an underlying DRM + connector, X11 connection, etc. + + The name event is sent after creating an xdg_output (see + xdg_output_manager.get_xdg_output). This event is only sent once per + xdg_output, and the name does not change over the lifetime of the + wl_output global. + </description> + <arg name="name" type="string" summary="output name"/> + </event> + + <event name="description" since="2"> + <description summary="human-readable description of this output"> + Many compositors can produce human-readable descriptions of their + outputs. The client may wish to know this description as well, to + communicate the user for various purposes. + + The description is a UTF-8 string with no convention defined for its + contents. Examples might include 'Foocorp 11" Display' or 'Virtual X11 + output via :1'. + + The description event is sent after creating an xdg_output (see + xdg_output_manager.get_xdg_output) and whenever the description + changes. The description is optional, and may not be sent at all. + + For objects of version 2 and lower, this event is only sent once per + xdg_output, and the description does not change over the lifetime of + the wl_output global. + </description> + <arg name="description" type="string" summary="output description"/> + </event> + + </interface> +</protocol> -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2476
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Use the xdg-output-unstable-v1 protocol to get the positions of the Wayland outputs in the compositor logical space, and use this information to arrange the monitors in the Windows virtual screen space. For now we assume that the logical positions match the physical pixel positions (i.e., the outputs are not scaled), a deficiency which will be addressed in an upcoming commit. Signed-off-by: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/display.c | 88 +++++++++++++++++++++++---- dlls/winewayland.drv/wayland_output.c | 7 ++- dlls/winewayland.drv/waylanddrv.h | 1 + 3 files changed, 83 insertions(+), 13 deletions(-) diff --git a/dlls/winewayland.drv/display.c b/dlls/winewayland.drv/display.c index 2a5c86e2d8c..fd87b088ba9 100644 --- a/dlls/winewayland.drv/display.c +++ b/dlls/winewayland.drv/display.c @@ -30,6 +30,8 @@ #include "ntuser.h" +#include <stdlib.h> + WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); static BOOL force_display_devices_refresh; @@ -47,6 +49,46 @@ void wayland_init_display_devices(void) wayland_refresh_display_devices(); } +struct output_info +{ + int x, y; + struct wayland_output *output; +}; + +static int output_info_cmp_primary_x_y(const void *va, const void *vb) +{ + const struct output_info *a = va; + const struct output_info *b = vb; + BOOL a_is_primary = a->x == 0 && a->y == 0; + BOOL b_is_primary = b->x == 0 && b->y == 0; + + if (a_is_primary && !b_is_primary) return -1; + if (!a_is_primary && b_is_primary) return 1; + if (a->x < b->x) return -1; + if (a->x > b->x) return 1; + if (a->y < b->y) return -1; + if (a->y > b->y) return 1; + return 0; +} + +static void output_info_array_arrange_physical_coords(struct wl_array *output_info_array) +{ + struct output_info *info; + size_t num_outputs = output_info_array->size / sizeof(struct output_info); + + /* Set physical pixel coordinates. */ + wl_array_for_each(info, output_info_array) + { + info->x = info->output->logical_x; + info->y = info->output->logical_y; + } + + /* Now that we have our physical pixel coordinates, sort from physical left + * to right, but ensure the primary output is first. */ + qsort(output_info_array->data, num_outputs, sizeof(struct output_info), + output_info_cmp_primary_x_y); +} + static void wayland_add_device_gpu(const struct gdi_device_manager *device_manager, void *param) { @@ -76,13 +118,13 @@ static void wayland_add_device_adapter(const struct gdi_device_manager *device_m } static void wayland_add_device_monitor(const struct gdi_device_manager *device_manager, - void *param, struct wayland_output *output) + void *param, struct output_info *output_info) { struct gdi_monitor monitor = {0}; - SetRect(&monitor.rc_monitor, 0, 0, - output->current_mode->width, - output->current_mode->height); + SetRect(&monitor.rc_monitor, output_info->x, output_info->y, + output_info->x + output_info->output->current_mode->width, + output_info->y + output_info->output->current_mode->height); /* We don't have a direct way to get the work area in Wayland. */ monitor.rc_work = monitor.rc_monitor; @@ -90,7 +132,7 @@ static void wayland_add_device_monitor(const struct gdi_device_manager *device_m monitor.state_flags = DISPLAY_DEVICE_ATTACHED | DISPLAY_DEVICE_ACTIVE; TRACE("name=%s rc_monitor=rc_work=%s state_flags=0x%x\n", - output->name, wine_dbgstr_rect(&monitor.rc_monitor), + output_info->output->name, wine_dbgstr_rect(&monitor.rc_monitor), (UINT)monitor.state_flags); device_manager->add_monitor(&monitor, param); @@ -109,16 +151,22 @@ static void populate_devmode(struct wayland_output_mode *output_mode, DEVMODEW * } static void wayland_add_device_modes(const struct gdi_device_manager *device_manager, - void *param, struct wayland_output *output) + void *param, struct output_info *output_info) { struct wayland_output_mode *output_mode; - RB_FOR_EACH_ENTRY(output_mode, &output->modes, struct wayland_output_mode, entry) + RB_FOR_EACH_ENTRY(output_mode, &output_info->output->modes, + struct wayland_output_mode, entry) { DEVMODEW mode = {.dmSize = sizeof(mode)}; - BOOL mode_is_current = output_mode == output->current_mode; + BOOL mode_is_current = output_mode == output_info->output->current_mode; populate_devmode(output_mode, &mode); - if (mode_is_current) mode.dmFields |= DM_POSITION; + if (mode_is_current) + { + mode.dmFields |= DM_POSITION; + mode.dmPosition.x = output_info->x; + mode.dmPosition.y = output_info->y; + } device_manager->add_mode(&mode, mode_is_current, param); } } @@ -131,6 +179,8 @@ BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manage { struct wayland_output *output; INT output_id = 0; + struct wl_array output_info_array; + struct output_info *output_info; /* For now, update the display devices only when a compositor side update * is received in the desktop window process. */ @@ -140,16 +190,30 @@ BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manage force_display_devices_refresh = FALSE; - wayland_add_device_gpu(device_manager, param); + wl_array_init(&output_info_array); 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)); + if (output_info) output_info->output = output; + else ERR("Failed to allocate space for output_info\n"); + } + + output_info_array_arrange_physical_coords(&output_info_array); + + /* Populate GDI devices. */ + wayland_add_device_gpu(device_manager, param); + + wl_array_for_each(output_info, &output_info_array) + { wayland_add_device_adapter(device_manager, param, output_id); - wayland_add_device_monitor(device_manager, param, output); - wayland_add_device_modes(device_manager, param, output); + wayland_add_device_monitor(device_manager, param, output_info); + wayland_add_device_modes(device_manager, param, output_info); output_id++; } + wl_array_release(&output_info_array); + return TRUE; } diff --git a/dlls/winewayland.drv/wayland_output.c b/dlls/winewayland.drv/wayland_output.c index 8eb24fe97f3..29f5411bba5 100644 --- a/dlls/winewayland.drv/wayland_output.c +++ b/dlls/winewayland.drv/wayland_output.c @@ -101,7 +101,8 @@ static void wayland_output_done(struct wayland_output *output) { struct wayland_output_mode *mode; - TRACE("name=%s\n", output->name); + TRACE("name=%s logical=%d,%d\n", + output->name, output->logical_x, output->logical_y); RB_FOR_EACH_ENTRY(mode, &output->modes, struct wayland_output_mode, entry) { @@ -163,6 +164,10 @@ static void zxdg_output_v1_handle_logical_position(void *data, int32_t x, int32_t y) { + struct wayland_output *output = data; + TRACE("logical_x=%d logical_y=%d\n", x, y); + output->logical_x = x; + output->logical_y = y; } static void zxdg_output_v1_handle_logical_size(void *data, diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 98c31068f2f..7e606f45f78 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -70,6 +70,7 @@ struct wayland_output struct rb_tree modes; struct wayland_output_mode *current_mode; char *name; + int logical_x, logical_y; uint32_t global_id; }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2476
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Use the xdg-output-unstable-v1 protocol to get the size of the Wayland outputs in the compositor logical space, and use this information, along with the logical position and size in physical pixels (i.e., the current mode) to infer the Windows virtual screen coordinates of the monitors. Note that there are multiple possible mappings from Wayland logical coordinates to Windows virtual screen coordinates. We choose one algorithm that works well for the majority of output arrangements, and we can refine in the future as needed. Signed-off-by: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/display.c | 112 +++++++++++++++++++++++++- dlls/winewayland.drv/wayland_output.c | 9 ++- dlls/winewayland.drv/waylanddrv.h | 1 + 3 files changed, 119 insertions(+), 3 deletions(-) diff --git a/dlls/winewayland.drv/display.c b/dlls/winewayland.drv/display.c index fd87b088ba9..aa5ee6dc22c 100644 --- a/dlls/winewayland.drv/display.c +++ b/dlls/winewayland.drv/display.c @@ -71,18 +71,128 @@ static int output_info_cmp_primary_x_y(const void *va, const void *vb) return 0; } +static inline BOOL output_info_overlap(struct output_info *a, struct output_info *b) +{ + return b->x < a->x + a->output->current_mode->width && + b->x + b->output->current_mode->width > a->x && + b->y < a->y + a->output->current_mode->height && + b->y + b->output->current_mode->height > a->y; +} + +/* Map a point to one of the four quadrants of our 2d coordinate space: + * 0: bottom right (x >= 0, y >= 0) + * 1: top right (x >= 0, y < 0) + * 2: bottom left (x < 0, y >= 0) + * 3: top left (x < 0, y < 0) */ +static inline int point_to_quadrant(int x, int y) +{ + return (x < 0) * 2 + (y < 0); +} + +/* Decide which of two outputs to keep stationary in order + * to resolve an overlap. */ +static struct output_info *output_info_get_overlap_anchor(struct output_info *a, + struct output_info *b) +{ + /* Preferences for the direction of growth in each quadrant, with a + * lower value signifying a higher preference. */ + static const int quadrant_prefs[4][4] = + { + {0, 1, 2, 3}, /* quadrant 0 */ + {3, 0, 2, 1}, /* quadrant 1 */ + {2, 3, 0, 1}, /* quadrant 2 */ + {3, 2, 1, 0}, /* quadrant 3 */ + }; + int qa = point_to_quadrant(a->output->logical_x, a->output->logical_y); + int qb = point_to_quadrant(b->output->logical_x, b->output->logical_y); + /* Direction of growth if a is the anchor. */ + int qab = point_to_quadrant(b->output->logical_x - a->output->logical_x, + b->output->logical_y - a->output->logical_y); + /* Direction of growth if b is the anchor. */ + int qba = point_to_quadrant(a->output->logical_x - b->output->logical_x, + a->output->logical_y - b->output->logical_y); + + /* If the two output origins are in different quadrants, use the output + * in the lower valued quadrant as the anchor (so effectively outputs + * grow/move away from quadrant 0). */ + if (qa != qb) return (qa < qb) ? a : b; + + /* If the outputs are in the same quadrant, use the preference for the + * direction of growth in that quadrant to select the anchor. Again the + * intended effect is to grow/move outputs away from the origin. */ + return (quadrant_prefs[qa][qab] < quadrant_prefs[qa][qba]) ? a : b; +} + +static BOOL output_info_array_resolve_overlaps(struct wl_array *output_info_array) +{ + struct output_info *a, *b; + BOOL found_overlap = FALSE; + + wl_array_for_each(a, output_info_array) + { + wl_array_for_each(b, output_info_array) + { + struct output_info *anchor, *move; + BOOL x_use_end, y_use_end; + double rel_x, rel_y; + + /* Break if we reach the same output in the inner loop, so that we + * don't process output pairs twice (since order doesn't matter for + * our algorithm.) */ + if (a == b) break; + + if (!output_info_overlap(a, b)) continue; + found_overlap = TRUE; + + /* Decide which output to move to resolve the overlap. */ + anchor = output_info_get_overlap_anchor(a, b); + move = anchor == a ? b : a; + + /* Move the selected output on the X axis to resolve the overlap, + * while maintaining the same relative positioning of the outputs as + * the one they have in logical space. Use either the start or end + * of the moved output as the point to maintain the relative + * position of, depending on whether the anchor is before or after + * the moved output on the axis. */ + x_use_end = move->output->logical_x < anchor->output->logical_x; + rel_x = (move->output->logical_x - anchor->output->logical_x + + (x_use_end ? move->output->logical_w : 0)) / + (double)anchor->output->logical_w; + move->x = anchor->x + anchor->output->current_mode->width * rel_x - + (x_use_end ? move->output->current_mode->width : 0); + + /* Similarly for the Y axis. */ + y_use_end = move->output->logical_y < anchor->output->logical_y; + rel_y = (move->output->logical_y - anchor->output->logical_y + + (y_use_end ? move->output->logical_h : 0)) / + (double)anchor->output->logical_h; + move->y = anchor->y + anchor->output->current_mode->height * rel_y - + (y_use_end ? move->output->current_mode->height : 0); + } + } + + return found_overlap; +} + static void output_info_array_arrange_physical_coords(struct wl_array *output_info_array) { struct output_info *info; size_t num_outputs = output_info_array->size / sizeof(struct output_info); + int steps = 0; - /* Set physical pixel coordinates. */ + /* Set the initial physical pixel coordinates. */ wl_array_for_each(info, output_info_array) { info->x = info->output->logical_x; info->y = info->output->logical_y; } + /* Try to iteratively resolve overlaps, but be defensive and set an upper + * iteration bound to ensure we avoid infinite loops. */ + while (output_info_array_resolve_overlaps(output_info_array) && + ++steps < num_outputs) + continue; + /* Now that we have our physical pixel coordinates, sort from physical left * to right, but ensure the primary output is first. */ qsort(output_info_array->data, num_outputs, sizeof(struct output_info), diff --git a/dlls/winewayland.drv/wayland_output.c b/dlls/winewayland.drv/wayland_output.c index 29f5411bba5..4d9033d3f73 100644 --- a/dlls/winewayland.drv/wayland_output.c +++ b/dlls/winewayland.drv/wayland_output.c @@ -101,8 +101,9 @@ static void wayland_output_done(struct wayland_output *output) { struct wayland_output_mode *mode; - TRACE("name=%s logical=%d,%d\n", - output->name, output->logical_x, output->logical_y); + TRACE("name=%s logical=%d,%d+%dx%d\n", + output->name, output->logical_x, output->logical_y, + output->logical_w, output->logical_h); RB_FOR_EACH_ENTRY(mode, &output->modes, struct wayland_output_mode, entry) { @@ -175,6 +176,10 @@ static void zxdg_output_v1_handle_logical_size(void *data, int32_t width, int32_t height) { + struct wayland_output *output = data; + TRACE("logical_w=%d logical_h=%d\n", width, height); + output->logical_w = width; + output->logical_h = height; } static void zxdg_output_v1_handle_done(void *data, diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 7e606f45f78..79319e9d9a4 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -71,6 +71,7 @@ struct wayland_output struct wayland_output_mode *current_mode; char *name; int logical_x, logical_y; + int logical_w, logical_h; uint32_t global_id; }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2476
On Mon Apr 3 16:26:40 2023 +0000, Alexandros Frantzis wrote:
I'd suggest moving the add_mode change to a separate MR and splitting it more, keeping driver changes separate and moving code around (for instance in winemac) before doing actual logic changes. https://gitlab.winehq.org/wine/wine/-/merge_requests/2568 I'm also not sure to see why you need to separate registry initialization from current mode. I'd think that you could just have a BOOL current argument, ... Although, 'BOOL current' was my initial approach, I wanted to minimize implicit changes in other drivers, so that drivers that, e.g., don't need to use the default win32u for current settings can just update the registry settings. The other part was that the internal design of drivers makes it more straightforward to get the current mode independently of mode enumeration, hence the separate 'target' rather than a 'current' flag. In any case, these are not insurmountable concerns, so in the separate MR I have switch to the `current` flag approach, since it seems to be preferable. I think all concerns in this thread have been addressed in the other MR. The only open question in my mind is the one stated in https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_28444 (i.e., being able to query current win32u settings from within UpdateDisplayDevices). But that's not going to be needed until the Wayland driver needs to properly support ChangeDisplaySettings, which will happen later in the series, so we can discuss more then.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_29744
On Fri Mar 24 18:20:56 2023 +0000, Alexandros Frantzis wrote:
@julliard Thanks! I will give the registry approach a try (see above), and hopefully it will be enough (but also happy to move in the direction of making more generic code improvements if preferred). After !2568, the driver is now using the internal win32u mechanisms to store an authoritative version of the session display configuration.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_29745
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/display.c:
void wayland_init_display_devices(void) { - struct ntuser_thread_info *thread_info = NtUserGetThreadInfo(); - DWORD current_pid = GetCurrentProcessId(); - HWND desktop_hwnd = UlongToHandle(thread_info->top_window); - DWORD desktop_pid = 0; - - if (desktop_hwnd) NtUserGetWindowThread(desktop_hwnd, &desktop_pid); - - /* Refresh devices only from the desktop window process. */ - if (!desktop_pid || current_pid == desktop_pid) - wayland_refresh_display_devices(); + wayland_refresh_display_devices(); }
Any reason to keep both `wayland_refresh_display_devices` and `wayland_init_display_devices`? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_29754
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland.c:
{ TRACE("interface=%s version=%u id=%u\n", interface, version, id);
+ /* For now only the desktop process needs wl_output objects. */ + if (GetCurrentProcessId() != get_desktop_process_id()) return; I think this should be `GetCurrentThreadId() != NtUserGetWindowThread( NtUserGetDesktopWindow(), NULL )` instead.
However I see that you're running this directly from `wayland_process_init`, which happens very early, and, in the desktop initialization, runs before the desktop window has actually been created. You'll probably get a HWND for it, but NtUserGetWindowThread returns nothing yet. You're working around it in `get_desktop_process_id` by returning `GetCurrentProcessId()` in case things failed, but that looks wrong imho, and instead you should probably try to make this work without having to guess the desktop process id. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_29755
On Wed Apr 12 15:06:10 2023 +0000, Rémi Bernon wrote:
Any reason to keep both `wayland_refresh_display_devices` and `wayland_init_display_devices`? `wayland_init_display_devices` will eventually have more content, but, sure, it should be fine to inline `wayland_refresh_display_devices`. I will updated accordingly.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_29913
... You'll probably get a HWND for it, but NtUserGetWindowThread returns nothing yet.
You're working around it in get_desktop_process_id by returning GetCurrentProcessId() in case things failed, but that looks wrong imho, and instead you should probably try to make this work without having to guess the desktop process id.
I have been looking at the relevant code paths and experimenting with a few alternatives, and it seems that getting an invalid desktop HWND during process startup combined with the process path having the "c:\windows\system32\explorer.exe /desktop..." form, should be a reliable indicator of being in the desktop window process. What do you think? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_29914
On Thu Apr 13 13:29:33 2023 +0000, Alexandros Frantzis wrote:
... You'll probably get a HWND for it, but NtUserGetWindowThread returns nothing yet.
You're working around it in get_desktop_process_id by returning GetCurrentProcessId() in case things failed, but that looks wrong imho, and instead you should probably try to make this work without having to guess the desktop process id. I have been looking at the relevant code paths and experimenting with a few alternatives, and it seems that getting an invalid desktop HWND during process startup combined with the process path having the "c:\windows\system32\explorer.exe /desktop..." form, should be a reliable indicator of being in the desktop window process. What do you think? I'm not completely sure why this if statement is needed.
For me the only reason this if statement should be here would be for security or stability purposes. But after looking at the code I'm not sure what security improvement this if statement introduce... If it's for stability and the code should not be called outside the desktop window thread, adding some logging seems more like welcome here. And if it's neither for security or stability purposes I think it would be a better idea to put the if statement somewhere else, because putting a if statement for a specific window that deep into a core part of `winewayland.drv` feel more like a workaround. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_29917
On Thu Apr 13 14:19:21 2023 +0000, Loïc Rebmeister wrote:
I'm not completely sure why this if statement is needed. For me the only reason this if statement should be here would be for security or stability purposes. But after looking at the code I'm not sure what security improvement this if statement introduce... If it's for stability and the code should not be called outside the desktop window thread, adding some logging seems more like welcome here. And if it's neither for security or stability purposes I think it would be a better idea to put the if statement somewhere else, because putting a if statement for a specific window that deep into a core part of `winewayland.drv` feel more like a workaround. I don't know, this feels brittle. If something relies on being in the desktop window thread, or process, it should not run before the window is fully created, and calling `NtUserGetDesktopWindow()` is the way to make sure that it is.
The desktop startup process is convoluted, and explorer may start multiple times until one of them becomes the desktop process. Maybe this process could be simplified but otherwise I think it's risky to try working around its limitation. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_29918
On Thu Apr 13 14:20:54 2023 +0000, Rémi Bernon wrote:
I don't know, this feels brittle. If something relies on being in the desktop window thread, or process, it should not run before the window is fully created, and calling `NtUserGetDesktopWindow()` is the way to make sure that it is. The desktop startup process is convoluted, and explorer may start multiple times until one of them becomes the desktop process. Maybe this process could be simplified but otherwise I think it's risky to try working around its limitation. Fwiw, I don't know why you need to keep this in the desktop process only but maybe the solution is simply to not enforce that and let any process run this. The display setting cache mechanism should ensure that it's not unnecessarily done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_29922
participants (4)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Loïc Rebmeister
-
Rémi Bernon