[PATCH resend 1/4] winex11.drv: Store monitor information in the wineserver for EnumDisplayMonitors().
Fix a regression that Office 2016/365 has a 640x480 main window. Office 2016/365 hooks NtOpenKeyEx() and prevents access to SetupAPI device properties. After querying monitor information from SetupAPI failed, EnumDisplayMonitors() reports a fallback monitor of size 640x480. As to why store the monitor information in the wineserver, it seems that EnumDisplayMonitors() reports monitors connected to current user logon session. For instance, EnumDisplayMonitors() always report one monitor when called by services. Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/user32/sysparams.c | 60 +++++++++++++++++ dlls/user32/tests/monitor.c | 9 +-- dlls/winex11.drv/display.c | 54 +++++++++++++-- server/Makefile.in | 1 + server/display.c | 131 ++++++++++++++++++++++++++++++++++++ server/protocol.def | 35 ++++++++++ server/user.h | 13 +++- 7 files changed, 288 insertions(+), 15 deletions(-) create mode 100644 server/display.c diff --git a/dlls/user32/sysparams.c b/dlls/user32/sysparams.c index 55d6d7e53a7..22f3eec4e94 100644 --- a/dlls/user32/sysparams.c +++ b/dlls/user32/sysparams.c @@ -46,6 +46,7 @@ #include "win.h" #include "user_private.h" #include "wine/gdi_driver.h" +#include "wine/server.h" #include "wine/asm.h" #include "wine/debug.h" @@ -3869,9 +3870,31 @@ fail: BOOL CDECL nulldrv_GetMonitorInfo( HMONITOR handle, MONITORINFO *info ) { UINT index = (UINT_PTR)handle - 1; + WCHAR adapter_name[CCHDEVICENAME]; TRACE("(%p, %p)\n", handle, info); + SERVER_START_REQ( get_monitor_info ) + { + req->handle = wine_server_user_handle( handle ); + wine_server_set_reply( req, adapter_name, sizeof(adapter_name) ); + if (!wine_server_call( req )) + { + SetRect( &info->rcMonitor, reply->monitor_rect.left, reply->monitor_rect.top, + reply->monitor_rect.right, reply->monitor_rect.bottom ); + SetRect( &info->rcWork, reply->work_rect.left, reply->work_rect.top, + reply->work_rect.right, reply->work_rect.bottom ); + if (!IsRectEmpty( &info->rcMonitor ) && !info->rcMonitor.top && !info->rcMonitor.left) + info->dwFlags = MONITORINFOF_PRIMARY; + else + info->dwFlags = 0; + if (info->cbSize >= sizeof(MONITORINFOEXW)) + lstrcpyW( ((MONITORINFOEXW *)info)->szDevice, adapter_name ); + return TRUE; + } + } + SERVER_END_REQ; + /* Fallback to report one monitor */ if (handle == NULLDRV_DEFAULT_HMONITOR) { @@ -3885,7 +3908,10 @@ BOOL CDECL nulldrv_GetMonitorInfo( HMONITOR handle, MONITORINFO *info ) } if (!update_monitor_cache()) + { + SetLastError( ERROR_INVALID_MONITOR_HANDLE ); return FALSE; + } EnterCriticalSection( &monitors_section ); if (index < monitor_count) @@ -4006,11 +4032,45 @@ static BOOL CALLBACK enum_mon_callback( HMONITOR monitor, HDC hdc, LPRECT rect, BOOL CDECL nulldrv_EnumDisplayMonitors( HDC hdc, RECT *rect, MONITORENUMPROC proc, LPARAM lp ) { + HMONITOR monitor = NULL; RECT monitor_rect; + NTSTATUS status; + HANDLE mutex; DWORD i = 0; TRACE("(%p, %p, %p, 0x%lx)\n", hdc, rect, proc, lp); + mutex = get_display_device_init_mutex(); + while (TRUE) + { + SERVER_START_REQ( enum_monitor ) + { + req->index = i; + if (!(status = wine_server_call( req ))) + { + SetRect( &monitor_rect, reply->monitor_rect.left, reply->monitor_rect.top, + reply->monitor_rect.right, reply->monitor_rect.bottom ); + monitor = wine_server_ptr_handle( reply->handle ); + } + } + SERVER_END_REQ; + + if (status) + break; + + ++i; + release_display_device_init_mutex( mutex ); + + if (!proc( monitor, hdc, &monitor_rect, lp )) + return FALSE; + + mutex = get_display_device_init_mutex(); + } + release_display_device_init_mutex( mutex ); + + if (i) + return TRUE; + if (update_monitor_cache()) { while (TRUE) diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c index 6b233970ace..ac2560b7014 100644 --- a/dlls/user32/tests/monitor.c +++ b/dlls/user32/tests/monitor.c @@ -1525,27 +1525,21 @@ static void test_EnumDisplayMonitors(void) HWINSTA winstation, old_winstation; HDESK desktop, old_desktop; INT count, old_count; - DWORD error; BOOL ret; ret = EnumDisplayMonitors(NULL, NULL, test_EnumDisplayMonitors_normal_cb, 0); ok(ret, "EnumDisplayMonitors failed, error %#x.\n", GetLastError()); - SetLastError(0xdeadbeef); ret = EnumDisplayMonitors(NULL, NULL, test_EnumDisplayMonitors_return_false_cb, 0); - error = GetLastError(); ok(!ret, "EnumDisplayMonitors succeeded.\n"); - ok(error == 0xdeadbeef, "Expected error %#x, got %#x.\n", 0xdeadbeef, error); count = GetSystemMetrics(SM_CMONITORS); SetLastError(0xdeadbeef); ret = EnumDisplayMonitors(NULL, NULL, test_EnumDisplayMonitors_invalid_handle_cb, 0); - error = GetLastError(); if (count >= 2) todo_wine ok(!ret, "EnumDisplayMonitors succeeded.\n"); else - ok(ret, "EnumDisplayMonitors failed.\n"); - ok(error == 0xdeadbeef, "Expected error %#x, got %#x.\n", 0xdeadbeef, error); + ok(ret, "EnumDisplayMonitors failed, error %#x.\n", GetLastError()); /* Test that monitor enumeration is not affected by window stations and desktops */ old_winstation = GetProcessWindowStation(); @@ -1965,7 +1959,6 @@ static BOOL CALLBACK test_handle_proc(HMONITOR full_monitor, HDC hdc, LPRECT rec monitor = (HMONITOR)((ULONG_PTR)full_monitor | ((ULONG_PTR)~0u << 16)); SetLastError(0xdeadbeef); ret = GetMonitorInfoW(monitor, &monitor_info); - todo_wine_if(((ULONG_PTR)full_monitor >> 16) == 0) ok(ret, "GetMonitorInfoW failed, error %#x.\n", GetLastError()); monitor = (HMONITOR)((ULONG_PTR)full_monitor & 0xffff); diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 0f61f9f7b2e..fb12b158c17 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -34,6 +34,7 @@ #define WIN32_NO_STATUS #include "winternl.h" #include "wine/debug.h" +#include "wine/server.h" #include "wine/unicode.h" #include "x11drv.h" @@ -47,7 +48,6 @@ DEFINE_DEVPROPKEY(DEVPROPKEY_MONITOR_OUTPUT_ID, 0xca085853, 0x16ce, 0x48aa, 0xb1 DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_VULKAN_UUID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2); DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_MONITOR_STATEFLAGS, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5b, 2); DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_MONITOR_RCMONITOR, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5b, 3); -DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_MONITOR_RCWORK, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5b, 4); DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_MONITOR_ADAPTERNAME, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5b, 5); static const WCHAR driver_date_dataW[] = {'D','r','i','v','e','r','D','a','t','e','D','a','t','a',0}; @@ -586,6 +586,7 @@ static BOOL X11DRV_InitMonitor(HDEVINFO devinfo, const struct x11drv_monitor *mo { SP_DEVINFO_DATA device_data = {sizeof(SP_DEVINFO_DATA)}; WCHAR bufferW[MAX_PATH]; + DWORD size; HKEY hkey; BOOL ret = FALSE; @@ -624,16 +625,33 @@ static BOOL X11DRV_InitMonitor(HDEVINFO devinfo, const struct x11drv_monitor *mo if (!SetupDiSetDevicePropertyW(devinfo, &device_data, &WINE_DEVPROPKEY_MONITOR_RCMONITOR, DEVPROP_TYPE_BINARY, (const BYTE *)&monitor->rc_monitor, sizeof(monitor->rc_monitor), 0)) goto done; - /* RcWork */ - if (!SetupDiSetDevicePropertyW(devinfo, &device_data, &WINE_DEVPROPKEY_MONITOR_RCWORK, DEVPROP_TYPE_BINARY, - (const BYTE *)&monitor->rc_work, sizeof(monitor->rc_work), 0)) - goto done; /* Adapter name */ sprintfW(bufferW, adapter_name_fmtW, video_index + 1); + size = (strlenW(bufferW) + 1) * sizeof(WCHAR); if (!SetupDiSetDevicePropertyW(devinfo, &device_data, &WINE_DEVPROPKEY_MONITOR_ADAPTERNAME, DEVPROP_TYPE_STRING, - (const BYTE *)bufferW, (strlenW(bufferW) + 1) * sizeof(WCHAR), 0)) + (const BYTE *)bufferW, size, 0)) goto done; + /* EnumDisplayMonitors() doesn't enumerate mirrored replicas and inactive monitors */ + if (monitor_index == 0 && monitor->state_flags & DISPLAY_DEVICE_ACTIVE) + { + SERVER_START_REQ(create_monitor) + { + req->monitor_rect.top = monitor->rc_monitor.top; + req->monitor_rect.left = monitor->rc_monitor.left; + req->monitor_rect.right = monitor->rc_monitor.right; + req->monitor_rect.bottom = monitor->rc_monitor.bottom; + req->work_rect.top = monitor->rc_work.top; + req->work_rect.left = monitor->rc_work.left; + req->work_rect.right = monitor->rc_work.right; + req->work_rect.bottom = monitor->rc_work.bottom; + wine_server_add_data(req, bufferW, size); + if (wine_server_call(req)) + goto done; + } + SERVER_END_REQ; + } + ret = TRUE; done: if (!ret) @@ -645,7 +663,9 @@ static void prepare_devices(HKEY video_hkey) { static const BOOL not_present = FALSE; SP_DEVINFO_DATA device_data = {sizeof(device_data)}; + HMONITOR monitor = NULL; HDEVINFO devinfo; + NTSTATUS status; DWORD i = 0; /* Remove all monitors */ @@ -657,6 +677,28 @@ static void prepare_devices(HKEY video_hkey) } SetupDiDestroyDeviceInfoList(devinfo); + while (TRUE) + { + SERVER_START_REQ(enum_monitor) + { + req->index = 0; + if (!(status = wine_server_call(req))) + monitor = wine_server_ptr_handle(reply->handle); + } + SERVER_END_REQ; + + if (status) + break; + + SERVER_START_REQ(destroy_monitor) + { + req->handle = wine_server_user_handle(monitor); + if (wine_server_call(req)) + ERR("Failed to destroy monitor.\n"); + } + SERVER_END_REQ; + } + /* Clean up old adapter keys for reinitialization */ RegDeleteTreeW(video_hkey, NULL); diff --git a/server/Makefile.in b/server/Makefile.in index 4264e3db108..b1aa85862c1 100644 --- a/server/Makefile.in +++ b/server/Makefile.in @@ -11,6 +11,7 @@ C_SRCS = \ debugger.c \ device.c \ directory.c \ + display.c \ event.c \ fd.c \ file.c \ diff --git a/server/display.c b/server/display.c new file mode 100644 index 00000000000..b04b269f8f3 --- /dev/null +++ b/server/display.c @@ -0,0 +1,131 @@ +/* + * Server-side display device management + * + * Copyright (C) 2021 Zhiyi Zhang for CodeWeavers + * + * 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 + */ +#include "config.h" + +#include <stdarg.h> + +#include "ntstatus.h" +#define WIN32_NO_STATUS +#include "winternl.h" + +#include "request.h" +#include "user.h" + +static struct list monitor_list = LIST_INIT(monitor_list); + +/* retrieve a pointer to a monitor from its handle */ +static struct monitor *get_monitor( user_handle_t handle ) +{ + struct monitor *monitor; + + if (!(monitor = get_user_object( handle, USER_MONITOR ))) + set_win32_error( ERROR_INVALID_MONITOR_HANDLE ); + return monitor; +} + +/* create a monitor */ +static struct monitor *create_monitor( const struct unicode_str *adapter_name, + const rectangle_t *monitor_rect, + const rectangle_t *work_rect) +{ + struct monitor *monitor; + + if (!(monitor = mem_alloc( sizeof(*monitor) ))) + goto failed; + + if (!(monitor->adapter_name = memdup( adapter_name->str, adapter_name->len ))) + goto failed; + monitor->adapter_name_len = adapter_name->len; + + if (!(monitor->handle = alloc_user_handle( monitor, USER_MONITOR ))) + goto failed; + + monitor->monitor_rect = *monitor_rect; + monitor->work_rect = *work_rect; + list_add_tail( &monitor_list, &monitor->entry ); + return monitor; + +failed: + if (monitor) + { + if (monitor->adapter_name) + free( monitor->adapter_name ); + free( monitor ); + } + set_error( STATUS_UNSUCCESSFUL ); + return NULL; +} + +/* create a monitor */ +DECL_HANDLER(create_monitor) +{ + struct unicode_str adapter_name; + struct monitor *monitor; + + adapter_name = get_req_unicode_str(); + if ((monitor = create_monitor( &adapter_name, &req->monitor_rect, &req->work_rect ))) + reply->handle = monitor->handle; +} + +/* get information about a monitor */ +DECL_HANDLER(get_monitor_info) +{ + struct monitor *monitor; + + if (!(monitor = get_monitor( req->handle ))) + return; + + reply->monitor_rect = monitor->monitor_rect; + reply->work_rect = monitor->work_rect; + set_reply_data( monitor->adapter_name, min(monitor->adapter_name_len, get_reply_max_size()) ); + return; +} + +/* enumerate monitors */ +DECL_HANDLER(enum_monitor) +{ + struct monitor *monitor; + unsigned int index = 0; + + LIST_FOR_EACH_ENTRY( monitor, &monitor_list, struct monitor, entry ) + { + if (req->index > index++) + continue; + + reply->handle = monitor->handle; + reply->monitor_rect = monitor->monitor_rect; + return; + } + set_error( STATUS_NO_MORE_ENTRIES ); +} + +/* destroy a monitor */ +DECL_HANDLER(destroy_monitor) +{ + struct monitor *monitor; + + if (!(monitor = get_monitor( req->handle ))) + return; + + free_user_handle( monitor->handle ); + list_remove( &monitor->entry ); + free( monitor->adapter_name ); + free( monitor ); +} diff --git a/server/protocol.def b/server/protocol.def index 7f3b785df51..06bd45f8eec 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2687,6 +2687,41 @@ enum coords_relative #define SET_USER_OBJECT_GET_FULL_NAME 2 +/* Create a monitor */ +(a)REQ(create_monitor) + rectangle_t monitor_rect; /* monitor rectangle */ + rectangle_t work_rect; /* monitor work area rectangle */ + VARARG(adapter,unicode_str); /* adapter name */ +(a)REPLY + user_handle_t handle; /* handle to the monitor */ +(a)END + + +/* Get information about a monitor */ +(a)REQ(get_monitor_info) + user_handle_t handle; /* handle to the monitor */ +(a)REPLY + rectangle_t monitor_rect; /* monitor rectangle */ + rectangle_t work_rect; /* monitor work area rectangle */ + VARARG(adapter,unicode_str); /* adapter name */ +(a)END + + +/* Enumerate monitors */ +(a)REQ(enum_monitor) + unsigned int index; /* current index */ +(a)REPLY + user_handle_t handle; /* handle to the monitor */ + rectangle_t monitor_rect; /* monitor rectangle */ +(a)END + + +/* Destroy a monitor */ +(a)REQ(destroy_monitor) + user_handle_t handle; /* handle to the monitor */ +(a)END + + /* Register a hotkey */ @REQ(register_hotkey) user_handle_t window; /* handle to the window */ diff --git a/server/user.h b/server/user.h index 6267f3e2881..6d36f0239d9 100644 --- a/server/user.h +++ b/server/user.h @@ -36,7 +36,8 @@ enum user_object { USER_WINDOW = 1, USER_HOOK, - USER_CLIENT /* arbitrary client handle */ + USER_CLIENT, /* arbitrary client handle */ + USER_MONITOR }; #define DESKTOP_ATOM ((atom_t)32769) @@ -79,6 +80,16 @@ struct desktop unsigned char keystate[256]; /* asynchronous key state */ }; +struct monitor +{ + user_handle_t handle; /* monitor handle */ + struct list entry; /* entry in global monitor list */ + rectangle_t monitor_rect; /* monitor rectangle */ + rectangle_t work_rect; /* monitor work area rectangle */ + WCHAR *adapter_name; /* adapter name */ + data_size_t adapter_name_len; /* adapter name length */ +}; + /* user handles functions */ extern user_handle_t alloc_user_handle( void *ptr, enum user_object type ); -- 2.27.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=86101 Your paranoid android. === debiant2 (32 bit report) === user32: win.c:3058: Test failed: got error 0 in EnableWindow call win.c:3079: Test succeeded inside todo block: Focus should be on child 00080102, not 00080102 === debiant2 (32 bit Chinese:China report) === user32: win.c:3058: Test failed: got error 0 in EnableWindow call win.c:3079: Test succeeded inside todo block: Focus should be on child 00080102, not 00080102
Zhiyi Zhang <zzhang(a)codeweavers.com> writes:
+ SERVER_START_REQ( get_monitor_info ) + { + req->handle = wine_server_user_handle( handle ); + wine_server_set_reply( req, adapter_name, sizeof(adapter_name) ); + if (!wine_server_call( req )) + { + SetRect( &info->rcMonitor, reply->monitor_rect.left, reply->monitor_rect.top, + reply->monitor_rect.right, reply->monitor_rect.bottom ); + SetRect( &info->rcWork, reply->work_rect.left, reply->work_rect.top, + reply->work_rect.right, reply->work_rect.bottom ); + if (!IsRectEmpty( &info->rcMonitor ) && !info->rcMonitor.top && !info->rcMonitor.left) + info->dwFlags = MONITORINFOF_PRIMARY; + else + info->dwFlags = 0; + if (info->cbSize >= sizeof(MONITORINFOEXW)) + lstrcpyW( ((MONITORINFOEXW *)info)->szDevice, adapter_name ); + return TRUE;
Strings in server requests should not be null-terminated. Also please don't return out of a request block.
+ SERVER_START_REQ(create_monitor) + { + req->monitor_rect.top = monitor->rc_monitor.top; + req->monitor_rect.left = monitor->rc_monitor.left; + req->monitor_rect.right = monitor->rc_monitor.right; + req->monitor_rect.bottom = monitor->rc_monitor.bottom; + req->work_rect.top = monitor->rc_work.top; + req->work_rect.left = monitor->rc_work.left; + req->work_rect.right = monitor->rc_work.right; + req->work_rect.bottom = monitor->rc_work.bottom; + wine_server_add_data(req, bufferW, size); + if (wine_server_call(req)) + goto done;
Same issues here.
+failed: + if (monitor) + { + if (monitor->adapter_name) + free( monitor->adapter_name ); + free( monitor ); + } + set_error( STATUS_UNSUCCESSFUL );
Don't use such generic errors. You should simply return the error already set by the various allocation functions. -- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Marvin -
Zhiyi Zhang