From 6e1b11bae4ca09cd20c896552aafc8b454092dba Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Fri, 11 Aug 2017 10:18:45 -0700 Subject: [PATCH] Fixed bug 3714 - Windows: SDL_WINDOW_FULLSCREEN_DESKTOP broken on 3 monitor setup w/ DPI scaling Eric Wasylishen 2017-07-26 18:42:58 UTC I set up an (admittedly exotic) 3-monitor setup, and when I enter fullscreen-desktop on the middle display (#2), the SDL window is off center. (covers half of monitor #2 and most of monitor #3). The displays are arranged from left to right: Display #1 (main): 2880x1800, 200% scaling Display #2: 1920x1200, 150% scaling Display #3: 1920x1080, 100% scaling SDL display bounds: INFO: Bounds: 1440x900 at 0,0 INFO: Bounds: 1281x801 at 1921,0 (these are incorrect) INFO: Bounds: 1920x1080 at 4800,0 Correct bounds reported by calling EnumDisplayMonitors and printing the LPRECT param of the callback: 1440x900 at (0, 0) 1280x800 at (2880, 0) 1920x1080 at (4800, 0) It seems like you need 3 displays to reproduce this, and the left two need DPI scaling, and the 3rd display needs to have a different scale factor than the others. Related: https://bugzilla.libsdl.org/show_bug.cgi?id=3709 SDL: current hg (11235:6a587b9e0ec8) Windows 10, Version 10.0.15063 Build 15063 Tested with testdraw2 and testgl2, and pressing alt+enter to enter fullscreen desktop. This patch reworks SDL_windowsmodes.c to use EnumDisplayMonitors instead of EnumDisplayDevices, so we always have an HMONITOR for each SDL display. With access to an HMONITOR, we can get the monitor bounds in virtual screen coordinates the proper way, by calling GetMonitorInfo. (whereas the original code was doing some calculations - e.g. "data->DeviceMode.dmPosition.x * data->ScaleX" - to try to get virtual screen coordinates. These worked in simple cases, but failed in more complex cases like this bug) The one potential problem with my patch is, the ChangeDisplaySettingsEx docs say that you're supposed to get the display name from EnumDisplayDevices, but I'm getting the display name from GetMonitorInfo now. --- src/video/windows/SDL_windowsmodes.c | 294 +++++++++++---------------- src/video/windows/SDL_windowsmodes.h | 6 +- 2 files changed, 125 insertions(+), 175 deletions(-) diff --git a/src/video/windows/SDL_windowsmodes.c b/src/video/windows/SDL_windowsmodes.c index b227678218f70..3340afe86fcb3 100644 --- a/src/video/windows/SDL_windowsmodes.c +++ b/src/video/windows/SDL_windowsmodes.c @@ -24,51 +24,14 @@ #include "SDL_windowsvideo.h" #include "../../../include/SDL_assert.h" +#include "../../../include/SDL_log.h" /* Windows CE compatibility */ #ifndef CDS_FULLSCREEN #define CDS_FULLSCREEN 0 #endif -typedef struct _WIN_GetMonitorDPIData { - SDL_VideoData *vid_data; - SDL_DisplayMode *mode; - SDL_DisplayModeData *mode_data; -} WIN_GetMonitorDPIData; - -static BOOL CALLBACK -WIN_GetMonitorDPI(HMONITOR hMonitor, - HDC hdcMonitor, - LPRECT lprcMonitor, - LPARAM dwData) -{ - WIN_GetMonitorDPIData *data = (WIN_GetMonitorDPIData*) dwData; - UINT hdpi, vdpi; - - if (data->vid_data->GetDpiForMonitor(hMonitor, MDT_EFFECTIVE_DPI, &hdpi, &vdpi) == S_OK && - hdpi > 0 && - vdpi > 0) { - float hsize, vsize; - - data->mode_data->HorzDPI = (float)hdpi; - data->mode_data->VertDPI = (float)vdpi; - - // Figure out the monitor size and compute the diagonal DPI. - hsize = data->mode->w / data->mode_data->HorzDPI; - vsize = data->mode->h / data->mode_data->VertDPI; - - data->mode_data->DiagDPI = SDL_ComputeDiagonalDPI( data->mode->w, - data->mode->h, - hsize, - vsize ); - - // We can only handle one DPI per display mode so end the enumeration. - return FALSE; - } - - // We didn't get DPI information so keep going. - return TRUE; -} +/* #define DEBUG_MODES */ static void WIN_UpdateDisplayMode(_THIS, LPCTSTR deviceName, DWORD index, SDL_DisplayMode * mode) @@ -89,39 +52,8 @@ WIN_UpdateDisplayMode(_THIS, LPCTSTR deviceName, DWORD index, SDL_DisplayMode * int logical_width = GetDeviceCaps( hdc, HORZRES ); int logical_height = GetDeviceCaps( hdc, VERTRES ); - data->ScaleX = (float)logical_width / data->DeviceMode.dmPelsWidth; - data->ScaleY = (float)logical_height / data->DeviceMode.dmPelsHeight; mode->w = logical_width; mode->h = logical_height; - - // WIN_GetMonitorDPI needs mode->w and mode->h - // so only call after those are set. - if (vid_data->GetDpiForMonitor) { - WIN_GetMonitorDPIData dpi_data; - RECT monitor_rect; - - dpi_data.vid_data = vid_data; - dpi_data.mode = mode; - dpi_data.mode_data = data; - monitor_rect.left = data->DeviceMode.dmPosition.x; - monitor_rect.top = data->DeviceMode.dmPosition.y; - monitor_rect.right = monitor_rect.left + 1; - monitor_rect.bottom = monitor_rect.top + 1; - EnumDisplayMonitors(NULL, &monitor_rect, WIN_GetMonitorDPI, (LPARAM)&dpi_data); - } else { - // We don't have the Windows 8.1 routine so just - // get system DPI. - data->HorzDPI = (float)GetDeviceCaps( hdc, LOGPIXELSX ); - data->VertDPI = (float)GetDeviceCaps( hdc, LOGPIXELSY ); - if (data->HorzDPI == data->VertDPI) { - data->DiagDPI = data->HorzDPI; - } else { - data->DiagDPI = SDL_ComputeDiagonalDPI( mode->w, - mode->h, - (float)GetDeviceCaps( hdc, HORZSIZE ) / 25.4f, - (float)GetDeviceCaps( hdc, VERTSIZE ) / 25.4f ); - } - } SDL_zero(bmi_data); bmi = (LPBITMAPINFO) bmi_data; @@ -199,13 +131,6 @@ WIN_GetDisplayMode(_THIS, LPCTSTR deviceName, DWORD index, SDL_DisplayMode * mod mode->driverdata = data; data->DeviceMode = devmode; - /* Default basic information */ - data->ScaleX = 1.0f; - data->ScaleY = 1.0f; - data->DiagDPI = 0.0f; - data->HorzDPI = 0.0f; - data->VertDPI = 0.0f; - mode->format = SDL_PIXELFORMAT_UNKNOWN; mode->w = data->DeviceMode.dmPelsWidth; mode->h = data->DeviceMode.dmPelsHeight; @@ -217,7 +142,7 @@ WIN_GetDisplayMode(_THIS, LPCTSTR deviceName, DWORD index, SDL_DisplayMode * mod } static SDL_bool -WIN_AddDisplay(_THIS, LPTSTR DeviceName) +WIN_AddDisplay(_THIS, HMONITOR hMonitor, const MONITORINFOEX *info) { SDL_VideoDisplay display; SDL_DisplayData *displaydata; @@ -225,9 +150,10 @@ WIN_AddDisplay(_THIS, LPTSTR DeviceName) DISPLAY_DEVICE device; #ifdef DEBUG_MODES - printf("Display: %s\n", WIN_StringToUTF8(DeviceName)); + SDL_Log("Display: %s\n", WIN_StringToUTF8(info->szDevice)); #endif - if (!WIN_GetDisplayMode(_this, DeviceName, ENUM_CURRENT_SETTINGS, &mode)) { + + if (!WIN_GetDisplayMode(_this, info->szDevice, ENUM_CURRENT_SETTINGS, &mode)) { return SDL_FALSE; } @@ -235,12 +161,13 @@ WIN_AddDisplay(_THIS, LPTSTR DeviceName) if (!displaydata) { return SDL_FALSE; } - SDL_memcpy(displaydata->DeviceName, DeviceName, + SDL_memcpy(displaydata->DeviceName, info->szDevice, sizeof(displaydata->DeviceName)); + displaydata->MonitorHandle = hMonitor; SDL_zero(display); device.cb = sizeof(device); - if (EnumDisplayDevices(DeviceName, 0, &device, 0)) { + if (EnumDisplayDevices(info->szDevice, 0, &device, 0)) { display.name = WIN_StringToUTF8(device.DeviceString); } display.desktop_mode = mode; @@ -251,63 +178,53 @@ WIN_AddDisplay(_THIS, LPTSTR DeviceName) return SDL_TRUE; } -int -WIN_InitModes(_THIS) +typedef struct _WIN_AddDisplaysData { + SDL_VideoDevice *video_device; + SDL_bool want_primary; +} WIN_AddDisplaysData; + +static BOOL CALLBACK +WIN_AddDisplaysCallback(HMONITOR hMonitor, + HDC hdcMonitor, + LPRECT lprcMonitor, + LPARAM dwData) { - int pass; - DWORD i, j, count; - DISPLAY_DEVICE device; + WIN_AddDisplaysData *data = (WIN_AddDisplaysData*)dwData; + MONITORINFOEX info; - device.cb = sizeof(device); + SDL_zero(info); + info.cbSize = sizeof(info); - /* Get the primary display in the first pass */ - for (pass = 0; pass < 2; ++pass) { - for (i = 0; ; ++i) { - TCHAR DeviceName[32]; + if (GetMonitorInfo(hMonitor, (LPMONITORINFO)&info) != 0) { + const SDL_bool is_primary = ((info.dwFlags & MONITORINFOF_PRIMARY) == MONITORINFOF_PRIMARY); - if (!EnumDisplayDevices(NULL, i, &device, 0)) { - break; - } - if (!(device.StateFlags & DISPLAY_DEVICE_ATTACHED_TO_DESKTOP)) { - continue; - } - if (pass == 0) { - if (!(device.StateFlags & DISPLAY_DEVICE_PRIMARY_DEVICE)) { - continue; - } - } else { - if (device.StateFlags & DISPLAY_DEVICE_PRIMARY_DEVICE) { - continue; - } - } - SDL_memcpy(DeviceName, device.DeviceName, sizeof(DeviceName)); -#ifdef DEBUG_MODES - printf("Device: %s\n", WIN_StringToUTF8(DeviceName)); -#endif - count = 0; - for (j = 0; ; ++j) { - if (!EnumDisplayDevices(DeviceName, j, &device, 0)) { - break; - } - if (!(device.StateFlags & DISPLAY_DEVICE_ATTACHED_TO_DESKTOP)) { - continue; - } - if (pass == 0) { - if (!(device.StateFlags & DISPLAY_DEVICE_PRIMARY_DEVICE)) { - continue; - } - } else { - if (device.StateFlags & DISPLAY_DEVICE_PRIMARY_DEVICE) { - continue; - } - } - count += WIN_AddDisplay(_this, device.DeviceName); - } - if (count == 0) { - WIN_AddDisplay(_this, DeviceName); - } + if (is_primary == data->want_primary) { + WIN_AddDisplay(data->video_device, hMonitor, &info); } } + + // continue enumeration + return TRUE; +} + +static void +WIN_AddDisplays(_THIS) +{ + WIN_AddDisplaysData callback_data; + callback_data.video_device = _this; + + callback_data.want_primary = SDL_TRUE; + EnumDisplayMonitors(NULL, NULL, WIN_AddDisplaysCallback, (LPARAM)&callback_data); + + callback_data.want_primary = SDL_FALSE; + EnumDisplayMonitors(NULL, NULL, WIN_AddDisplaysCallback, (LPARAM)&callback_data); +} + +int +WIN_InitModes(_THIS) +{ + WIN_AddDisplays(_this); + if (_this->num_displays == 0) { return SDL_SetError("No displays available"); } @@ -317,67 +234,104 @@ WIN_InitModes(_THIS) int WIN_GetDisplayBounds(_THIS, SDL_VideoDisplay * display, SDL_Rect * rect) { - SDL_DisplayModeData *data = (SDL_DisplayModeData *) display->current_mode.driverdata; + const SDL_DisplayData *data = (const SDL_DisplayData *)display->driverdata; + MONITORINFO minfo; + BOOL rc; - rect->x = (int)SDL_ceil(data->DeviceMode.dmPosition.x * data->ScaleX); - rect->y = (int)SDL_ceil(data->DeviceMode.dmPosition.y * data->ScaleY); - rect->w = (int)SDL_ceil(data->DeviceMode.dmPelsWidth * data->ScaleX); - rect->h = (int)SDL_ceil(data->DeviceMode.dmPelsHeight * data->ScaleY); + SDL_zero(minfo); + minfo.cbSize = sizeof(MONITORINFO); + rc = GetMonitorInfo(data->MonitorHandle, &minfo); + + if (!rc) { + return SDL_SetError("Couldn't find monitor data"); + } + + rect->x = minfo.rcMonitor.left; + rect->y = minfo.rcMonitor.top; + rect->w = minfo.rcMonitor.right - minfo.rcMonitor.left; + rect->h = minfo.rcMonitor.bottom - minfo.rcMonitor.top; return 0; } int -WIN_GetDisplayDPI(_THIS, SDL_VideoDisplay * display, float * ddpi, float * hdpi, float * vdpi) +WIN_GetDisplayDPI(_THIS, SDL_VideoDisplay * display, float * ddpi_out, float * hdpi_out, float * vdpi_out) { - SDL_DisplayModeData *data = (SDL_DisplayModeData *) display->current_mode.driverdata; + const SDL_DisplayData *displaydata = (SDL_DisplayData *)display->driverdata; + const SDL_VideoData *videodata = (SDL_VideoData *)display->device->driverdata; + float hdpi = 0, vdpi = 0, ddpi = 0; + + if (videodata->GetDpiForMonitor) { + UINT hdpi_uint, vdpi_uint; + // Windows 8.1+ codepath + if (videodata->GetDpiForMonitor(displaydata->MonitorHandle, MDT_EFFECTIVE_DPI, &hdpi_uint, &vdpi_uint) == S_OK) { + // GetDpiForMonitor docs promise to return the same hdpi/vdpi + hdpi = (float)hdpi_uint; + vdpi = (float)hdpi_uint; + ddpi = (float)hdpi_uint; + } else { + return SDL_SetError("GetDpiForMonitor failed"); + } + } else { + // Window 8.0 and below: same DPI for all monitors. + HDC hdc; + int hdpi_int, vdpi_int, hpoints, vpoints, hpix, vpix; + float hinches, vinches; + + hdc = GetDC(NULL); + if (hdc == NULL) { + return SDL_SetError("GetDC failed"); + } + hdpi_int = GetDeviceCaps(hdc, LOGPIXELSX); + vdpi_int = GetDeviceCaps(hdc, LOGPIXELSY); + ReleaseDC(NULL, hdc); - if (ddpi) { - *ddpi = data->DiagDPI; + hpoints = GetSystemMetrics(SM_CXVIRTUALSCREEN); + vpoints = GetSystemMetrics(SM_CYVIRTUALSCREEN); + + hpix = MulDiv(hpoints, hdpi_int, 96); + vpix = MulDiv(vpoints, vdpi_int, 96); + + hinches = (float)hpoints / 96.0f; + vinches = (float)vpoints / 96.0f; + + hdpi = (float)hdpi_int; + vdpi = (float)vdpi_int; + ddpi = SDL_ComputeDiagonalDPI(hpix, vpix, hinches, vinches); } - if (hdpi) { - *hdpi = data->HorzDPI; + + if (ddpi_out) { + *ddpi_out = ddpi; } - if (vdpi) { - *vdpi = data->VertDPI; + if (hdpi_out) { + *hdpi_out = hdpi; + } + if (vdpi_out) { + *vdpi_out = vdpi; } - return data->DiagDPI != 0.0f ? 0 : SDL_SetError("Couldn't get DPI"); + return ddpi != 0.0f ? 0 : SDL_SetError("Couldn't get DPI"); } int WIN_GetDisplayUsableBounds(_THIS, SDL_VideoDisplay * display, SDL_Rect * rect) { - const SDL_DisplayModeData *data = (const SDL_DisplayModeData *) display->current_mode.driverdata; - const DEVMODE *pDevMode = &data->DeviceMode; - POINT pt = { - /* !!! FIXME: no scale, right? */ - (LONG) (pDevMode->dmPosition.x + (pDevMode->dmPelsWidth / 2)), - (LONG) (pDevMode->dmPosition.y + (pDevMode->dmPelsHeight / 2)) - }; - HMONITOR hmon = MonitorFromPoint(pt, MONITOR_DEFAULTTONULL); + const SDL_DisplayData *data = (const SDL_DisplayData *)display->driverdata; MONITORINFO minfo; - const RECT *work; - BOOL rc = FALSE; - - SDL_assert(hmon != NULL); + BOOL rc; - if (hmon != NULL) { - SDL_zero(minfo); - minfo.cbSize = sizeof (MONITORINFO); - rc = GetMonitorInfo(hmon, &minfo); - SDL_assert(rc); - } + SDL_zero(minfo); + minfo.cbSize = sizeof(MONITORINFO); + rc = GetMonitorInfo(data->MonitorHandle, &minfo); if (!rc) { return SDL_SetError("Couldn't find monitor data"); } - work = &minfo.rcWork; - rect->x = (int)SDL_ceil(work->left * data->ScaleX); - rect->y = (int)SDL_ceil(work->top * data->ScaleY); - rect->w = (int)SDL_ceil((work->right - work->left) * data->ScaleX); - rect->h = (int)SDL_ceil((work->bottom - work->top) * data->ScaleY); + rect->x = minfo.rcWork.left; + rect->y = minfo.rcWork.top; + rect->w = minfo.rcWork.right - minfo.rcWork.left; + rect->h = minfo.rcWork.bottom - minfo.rcWork.top; return 0; } diff --git a/src/video/windows/SDL_windowsmodes.h b/src/video/windows/SDL_windowsmodes.h index 14d0f80dcd5f0..94894d8d8ccc4 100644 --- a/src/video/windows/SDL_windowsmodes.h +++ b/src/video/windows/SDL_windowsmodes.h @@ -26,16 +26,12 @@ typedef struct { TCHAR DeviceName[32]; + HMONITOR MonitorHandle; } SDL_DisplayData; typedef struct { DEVMODE DeviceMode; - float ScaleX; - float ScaleY; - float DiagDPI; - float HorzDPI; - float VertDPI; } SDL_DisplayModeData; extern int WIN_InitModes(_THIS);