[PATCH v2 3/4] winex11.drv: Retrieve virtual and primary monitor rectangles from SetupAPI.

[Zhiyi Zhang]

So that a process can still get the correct virtual and primary monitor
rectangles if display devices are reinitialized in another process.
Note that we can't use EnumDisplayMonitor() and GetMonitorInfo() here
because they are not loaded when loading winex11.drv.

Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com>
---
 dlls/winex11.drv/display.c | 162 +++++++++++++++++++++++++------------
 1 file changed, 109 insertions(+), 53 deletions(-)

diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c
index 6c4097a790..5b0b062ff6 100644
--- a/dlls/winex11.drv/display.c
+++ b/dlls/winex11.drv/display.c
@@ -104,33 +104,134 @@ static const WCHAR monitor_hardware_idW[] = {
     'D','e','f','a','u','l','t','_','M','o','n','i','t','o','r',0,0};
 
 static struct x11drv_display_device_handler handler;
+
+/* Cached screen information, protected by screen_section */
 static RECT virtual_screen_rect;
 static RECT primary_monitor_rect;
+static FILETIME last_query_screen_time;
+static CRITICAL_SECTION screen_section;
+static CRITICAL_SECTION_DEBUG screen_critsect_debug =
+{
+    0, 0, &screen_section,
+    {&screen_critsect_debug.ProcessLocksList, &screen_critsect_debug.ProcessLocksList},
+     0, 0, {(DWORD_PTR)(__FILE__ ": screen_section")}
+};
+static CRITICAL_SECTION screen_section = {&screen_critsect_debug, -1, 0, 0, 0, 0};
+
+static HANDLE get_display_device_init_mutex(void)
+{
+    static const WCHAR init_mutexW[] = {'d','i','s','p','l','a','y','_','d','e','v','i','c','e','_','i','n','i','t',0};
+    HANDLE mutex = CreateMutexW(NULL, FALSE, init_mutexW);
+
+    WaitForSingleObject(mutex, INFINITE);
+    return mutex;
+}
+
+static void release_display_device_init_mutex(HANDLE mutex)
+{
+    ReleaseMutex(mutex);
+    CloseHandle(mutex);
+}
+
+/* Update screen rect cache from SetupAPI if it's outdated, return FALSE on failure and TRUE on success */
+static BOOL update_screen_cache(void)
+{
+    RECT virtual_rect = {0}, primary_rect = {0}, monitor_rect;
+    SP_DEVINFO_DATA device_data = {sizeof(device_data)};
+    HDEVINFO devinfo = INVALID_HANDLE_VALUE;
+    FILETIME filetime;
+    HKEY video_key;
+    HANDLE mutex;
+    DWORD i = 0;
+    INT result;
+    DWORD type;
+    LSTATUS lr;
+    BOOL ret = FALSE;
+
+    if (RegOpenKeyW(HKEY_LOCAL_MACHINE, video_keyW, &video_key))
+        return FALSE;
+    lr = RegQueryInfoKeyW(video_key, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, &filetime);
+    RegCloseKey(video_key);
+    if (lr)
+        return FALSE;
+
+    EnterCriticalSection(&screen_section);
+    result = CompareFileTime(&filetime, &last_query_screen_time);
+    LeaveCriticalSection(&screen_section);
+    if (result < 1)
+        return TRUE;
+
+    mutex = get_display_device_init_mutex();
+
+    devinfo = SetupDiGetClassDevsW(&GUID_DEVCLASS_MONITOR, displayW, NULL, DIGCF_PRESENT);
+    if (devinfo == INVALID_HANDLE_VALUE)
+        goto fail;
+
+    while (SetupDiEnumDeviceInfo(devinfo, i++, &device_data))
+    {
+        if (!SetupDiGetDevicePropertyW(devinfo, &device_data, &WINE_DEVPROPKEY_MONITOR_RCMONITOR, &type,
+                                       (BYTE *)&monitor_rect, sizeof(monitor_rect), NULL, 0))
+            goto fail;
+
+        UnionRect(&virtual_rect, &virtual_rect, &monitor_rect);
+        if (i == 1)
+            primary_rect = monitor_rect;
+    }
+
+    EnterCriticalSection(&screen_section);
+    virtual_screen_rect = virtual_rect;
+    primary_monitor_rect = primary_rect;
+    last_query_screen_time = filetime;
+    LeaveCriticalSection(&screen_section);
+    ret = TRUE;
+fail:
+    SetupDiDestroyDeviceInfoList(devinfo);
+    release_display_device_init_mutex(mutex);
+    if (!ret)
+        ERR("Update screen cache failed!\n");
+    return ret;
+}
 
 POINT virtual_screen_to_root(INT x, INT y)
 {
+    RECT virtual = get_virtual_screen_rect();
     POINT pt;
-    pt.x = x - virtual_screen_rect.left;
-    pt.y = y - virtual_screen_rect.top;
+
+    pt.x = x - virtual.left;
+    pt.y = y - virtual.top;
     return pt;
 }
 
 POINT root_to_virtual_screen(INT x, INT y)
 {
+    RECT virtual = get_virtual_screen_rect();
     POINT pt;
-    pt.x = x + virtual_screen_rect.left;
-    pt.y = y + virtual_screen_rect.top;
+
+    pt.x = x + virtual.left;
+    pt.y = y + virtual.top;
     return pt;
 }
 
 RECT get_virtual_screen_rect(void)
 {
-    return virtual_screen_rect;
+    RECT virtual;
+
+    update_screen_cache();
+    EnterCriticalSection(&screen_section);
+    virtual = virtual_screen_rect;
+    LeaveCriticalSection(&screen_section);
+    return virtual;
 }
 
 RECT get_primary_monitor_rect(void)
 {
-    return primary_monitor_rect;
+    RECT primary;
+
+    update_screen_cache();
+    EnterCriticalSection(&screen_section);
+    primary = primary_monitor_rect;
+    LeaveCriticalSection(&screen_section);
+    return primary;
 }
 
 void X11DRV_DisplayDevices_SetHandler(const struct x11drv_display_device_handler *new_handler)
@@ -321,9 +422,6 @@ 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;
-    UnionRect(&virtual_screen_rect, &virtual_screen_rect, &monitor->rc_monitor);
-    if (video_index == 0)
-        primary_monitor_rect = monitor->rc_monitor;
     /* RcWork */
     if (!SetupDiSetDevicePropertyW(devinfo, &device_data, &WINE_DEVPROPKEY_MONITOR_RCWORK, DEVPROP_TYPE_BINARY,
                                    (const BYTE *)&monitor->rc_work, sizeof(monitor->rc_work), 0))
@@ -348,9 +446,6 @@ static void prepare_devices(HKEY video_hkey)
     HDEVINFO devinfo;
     DWORD i = 0;
 
-    SetRectEmpty(&virtual_screen_rect);
-    SetRectEmpty(&primary_monitor_rect);
-
     /* Remove all monitors */
     devinfo = SetupDiGetClassDevsW(&GUID_DEVCLASS_MONITOR, displayW, NULL, 0);
     while (SetupDiEnumDeviceInfo(devinfo, i++, &device_data))
@@ -399,39 +494,8 @@ static void cleanup_devices(void)
     SetupDiDestroyDeviceInfoList(devinfo);
 }
 
-/* Initialize virtual screen rect and primary monitor rect for current process */
-static void init_screen_rects(void)
-{
-    SP_DEVINFO_DATA device_data = {sizeof(device_data)};
-    HDEVINFO devinfo;
-    DWORD type;
-    DWORD i = 0;
-    RECT rect;
-
-    /* Already initialized */
-    if (!IsRectEmpty(&virtual_screen_rect))
-        return;
-
-    devinfo = SetupDiGetClassDevsW(&GUID_DEVCLASS_MONITOR, displayW, NULL, 0);
-    while (SetupDiEnumDeviceInfo(devinfo, i++, &device_data))
-    {
-        if (!SetupDiGetDevicePropertyW(devinfo, &device_data, &WINE_DEVPROPKEY_MONITOR_RCMONITOR, &type, (BYTE *)&rect,
-                                       sizeof(rect), NULL, 0))
-            ERR("Failed to get monitor size property\n");
-
-        UnionRect(&virtual_screen_rect, &virtual_screen_rect, &rect);
-        if (i == 1)
-            primary_monitor_rect = rect;
-    }
-    SetupDiDestroyDeviceInfoList(devinfo);
-
-    TRACE("virtual screen rect:%s primary monitor rect:%s\n", wine_dbgstr_rect(&virtual_screen_rect),
-          wine_dbgstr_rect(&primary_monitor_rect));
-}
-
 void X11DRV_DisplayDevices_Init(BOOL force)
 {
-    static const WCHAR init_mutexW[] = {'d','i','s','p','l','a','y','_','d','e','v','i','c','e','_','i','n','i','t',0};
     HANDLE mutex;
     struct x11drv_gpu *gpus = NULL;
     struct x11drv_adapter *adapters = NULL;
@@ -445,8 +509,7 @@ void X11DRV_DisplayDevices_Init(BOOL force)
     WCHAR guidW[40];
     WCHAR driverW[1024];
 
-    mutex = CreateMutexW(NULL, FALSE, init_mutexW);
-    WaitForSingleObject(mutex, INFINITE);
+    mutex = get_display_device_init_mutex();
 
     if (RegCreateKeyExW(HKEY_LOCAL_MACHINE, video_keyW, 0, NULL, REG_OPTION_VOLATILE, KEY_ALL_ACCESS, NULL, &video_hkey,
                         &disposition))
@@ -457,10 +520,7 @@ void X11DRV_DisplayDevices_Init(BOOL force)
 
     /* Avoid unnecessary reinit */
     if (!force && disposition != REG_CREATED_NEW_KEY)
-    {
-        init_screen_rects();
         goto done;
-    }
 
     TRACE("via %s\n", wine_dbgstr_a(handler.name));
 
@@ -511,16 +571,12 @@ void X11DRV_DisplayDevices_Init(BOOL force)
         adapters = NULL;
     }
 
-    TRACE("virtual screen rect:%s primary monitor rect:%s\n", wine_dbgstr_rect(&virtual_screen_rect),
-          wine_dbgstr_rect(&primary_monitor_rect));
-
 done:
     cleanup_devices();
     SetupDiDestroyDeviceInfoList(monitor_devinfo);
     SetupDiDestroyDeviceInfoList(gpu_devinfo);
     RegCloseKey(video_hkey);
-    ReleaseMutex(mutex);
-    CloseHandle(mutex);
+    release_display_device_init_mutex(mutex);
     if (gpus)
         handler.free_gpus(gpus);
     if (adapters)
-- 
2.23.0

[Alexandre Julliard]

Zhiyi Zhang <zzhang(a)codeweavers.com> writes:
> +/* Update screen rect cache from SetupAPI if it's outdated, return FALSE on failure and TRUE on success */
> +static BOOL update_screen_cache(void)
> +{
> +    RECT virtual_rect = {0}, primary_rect = {0}, monitor_rect;
> +    SP_DEVINFO_DATA device_data = {sizeof(device_data)};
> +    HDEVINFO devinfo = INVALID_HANDLE_VALUE;
> +    FILETIME filetime;
> +    HKEY video_key;
> +    HANDLE mutex;
> +    DWORD i = 0;
> +    INT result;
> +    DWORD type;
> +    LSTATUS lr;
> +    BOOL ret = FALSE;
> +
> +    if (RegOpenKeyW(HKEY_LOCAL_MACHINE, video_keyW, &video_key))
> +        return FALSE;
> +    lr = RegQueryInfoKeyW(video_key, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, &filetime);
> +    RegCloseKey(video_key);
> +    if (lr)
> +        return FALSE;
> +
> +    EnterCriticalSection(&screen_section);
> +    result = CompareFileTime(&filetime, &last_query_screen_time);
> +    LeaveCriticalSection(&screen_section);
> +    if (result < 1)
> +        return TRUE;

That looks pretty inefficient as caching mechanism. Hopefully we can
handle that without adding multiple wineserver calls.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Zhiyi Zhang]

On 11/1/19 2:33 AM, Alexandre Julliard wrote:
> Zhiyi Zhang <zzhang(a)codeweavers.com> writes:
> > +/* Update screen rect cache from SetupAPI if it's outdated, return FALSE on failure and TRUE on success */
> > +static BOOL update_screen_cache(void)
> > +{
> > +    RECT virtual_rect = {0}, primary_rect = {0}, monitor_rect;
> > +    SP_DEVINFO_DATA device_data = {sizeof(device_data)};
> > +    HDEVINFO devinfo = INVALID_HANDLE_VALUE;
> > +    FILETIME filetime;
> > +    HKEY video_key;
> > +    HANDLE mutex;
> > +    DWORD i = 0;
> > +    INT result;
> > +    DWORD type;
> > +    LSTATUS lr;
> > +    BOOL ret = FALSE;
> > +
> > +    if (RegOpenKeyW(HKEY_LOCAL_MACHINE, video_keyW, &video_key))
> > +        return FALSE;
> > +    lr = RegQueryInfoKeyW(video_key, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, &filetime);
> > +    RegCloseKey(video_key);
> > +    if (lr)
> > +        return FALSE;
> > +
> > +    EnterCriticalSection(&screen_section);
> > +    result = CompareFileTime(&filetime, &last_query_screen_time);
> > +    LeaveCriticalSection(&screen_section);
> > +    if (result < 1)
> > +        return TRUE;
> > That looks pretty inefficient as caching mechanism. Hopefully we can
> > handle that without adding multiple wineserver calls.

Ok. I sent a new version that only use one wineserver call, at the expense of
keeping a registry key open. Those two rectangles are not used that frequently,
so I was thinking it might be better not to leak a registry key handle.

Thanks,
Zhiyi