[PATCH 0/6] MR561: winex11.drv: Use DEVMODEW instead of x11drv_display_setting.
In preparation for moving display placement to win32u, for https://gitlab.winehq.org/wine/wine/-/merge_requests/551. The idea for win32u to enumerate and work with full modes, with driver extra data, to do the display placement logic and validation, and pass the modes array for all adapters at once to the driver to apply them. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/561
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/display.c | 45 +++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 174411c7915..db1628b2c85 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -37,7 +37,6 @@ struct x11drv_display_setting ULONG_PTR id; BOOL placed; RECT new_rect; - RECT desired_rect; DEVMODEW desired_mode; }; @@ -519,11 +518,6 @@ static LONG get_display_settings(struct x11drv_display_setting **new_displays, displays[display_idx].desired_mode = current_mode; } - SetRect(&displays[display_idx].desired_rect, - displays[display_idx].desired_mode.dmPosition.x, - displays[display_idx].desired_mode.dmPosition.y, - displays[display_idx].desired_mode.dmPosition.x + displays[display_idx].desired_mode.dmPelsWidth, - displays[display_idx].desired_mode.dmPosition.y + displays[display_idx].desired_mode.dmPelsHeight); lstrcpyW(displays[display_idx].desired_mode.dmDeviceName, display_device.DeviceName); } @@ -541,6 +535,11 @@ static INT offset_length(POINT offset) return offset.x * offset.x + offset.y * offset.y; } +static void set_rect_from_devmode(RECT *rect, const DEVMODEW *mode) +{ + SetRect(rect, mode->dmPosition.x, mode->dmPosition.y, mode->dmPosition.x + mode->dmPelsWidth, mode->dmPosition.y + mode->dmPelsHeight); +} + /* Check if a rect overlaps with placed display rects */ static BOOL overlap_placed_displays(const RECT *rect, const struct x11drv_display_setting *displays, INT display_count) { @@ -562,11 +561,13 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, POINT points[8], left_top, offset, min_offset = {0, 0}; INT display_idx, point_idx, point_count, vertex_idx; BOOL has_placed = FALSE, first = TRUE; + RECT desired_rect, rect; INT width, height; - RECT rect; + + set_rect_from_devmode(&desired_rect, &displays[placing_idx].desired_mode); /* If the display to be placed is detached, no offset is needed to place it */ - if (IsRectEmpty(&displays[placing_idx].desired_rect)) + if (IsRectEmpty(&desired_rect)) return min_offset; /* If there is no placed and attached display, place this display as it is */ @@ -584,8 +585,8 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, /* Try to place this display with each of its four vertices at every vertex of the placed * displays and see which combination has the minimum offset length */ - width = displays[placing_idx].desired_rect.right - displays[placing_idx].desired_rect.left; - height = displays[placing_idx].desired_rect.bottom - displays[placing_idx].desired_rect.top; + width = desired_rect.right - desired_rect.left; + height = desired_rect.bottom - desired_rect.top; for (display_idx = 0; display_idx < display_count; ++display_idx) { @@ -604,21 +605,21 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, point_count = 4; /* Intersected points when moving the display to be placed horizontally */ - if (displays[placing_idx].desired_rect.bottom >= displays[display_idx].new_rect.top && - displays[placing_idx].desired_rect.top <= displays[display_idx].new_rect.bottom) + if (desired_rect.bottom >= displays[display_idx].new_rect.top && + desired_rect.top <= displays[display_idx].new_rect.bottom) { points[point_count].x = displays[display_idx].new_rect.left; - points[point_count++].y = displays[placing_idx].desired_rect.top; + points[point_count++].y = desired_rect.top; points[point_count].x = displays[display_idx].new_rect.right; - points[point_count++].y = displays[placing_idx].desired_rect.top; + points[point_count++].y = desired_rect.top; } /* Intersected points when moving the display to be placed vertically */ - if (displays[placing_idx].desired_rect.left <= displays[display_idx].new_rect.right && - displays[placing_idx].desired_rect.right >= displays[display_idx].new_rect.left) + if (desired_rect.left <= displays[display_idx].new_rect.right && + desired_rect.right >= displays[display_idx].new_rect.left) { - points[point_count].x = displays[placing_idx].desired_rect.left; + points[point_count].x = desired_rect.left; points[point_count++].y = displays[display_idx].new_rect.top; - points[point_count].x = displays[placing_idx].desired_rect.left; + points[point_count].x = desired_rect.left; points[point_count++].y = displays[display_idx].new_rect.bottom; } @@ -651,9 +652,9 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, break; } - offset.x = left_top.x - displays[placing_idx].desired_rect.left; - offset.y = left_top.y - displays[placing_idx].desired_rect.top; - rect = displays[placing_idx].desired_rect; + offset.x = left_top.x - desired_rect.left; + offset.y = left_top.y - desired_rect.top; + rect = desired_rect; OffsetRect(&rect, offset.x, offset.y); if (!overlap_placed_displays(&rect, displays, display_count)) { @@ -702,7 +703,7 @@ static void place_all_displays(struct x11drv_display_setting *displays, INT disp if (placing_idx == -1) break; - displays[placing_idx].new_rect = displays[placing_idx].desired_rect; + set_rect_from_devmode(&displays[placing_idx].new_rect, &displays[placing_idx].desired_mode); OffsetRect(&displays[placing_idx].new_rect, min_offset.x, min_offset.y); displays[placing_idx].placed = TRUE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/561
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/display.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index db1628b2c85..3319697e51f 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -35,7 +35,6 @@ static struct x11drv_settings_handler settings_handler; struct x11drv_display_setting { ULONG_PTR id; - BOOL placed; RECT new_rect; DEVMODEW desired_mode; }; @@ -548,7 +547,7 @@ static BOOL overlap_placed_displays(const RECT *rect, const struct x11drv_displa for (display_idx = 0; display_idx < display_count; ++display_idx) { - if (displays[display_idx].placed && + if ((displays[display_idx].desired_mode.dmFields & DM_POSITION) && intersect_rect(&intersect, &displays[display_idx].new_rect, rect)) return TRUE; } @@ -573,7 +572,7 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, /* If there is no placed and attached display, place this display as it is */ for (display_idx = 0; display_idx < display_count; ++display_idx) { - if (displays[display_idx].placed && !IsRectEmpty(&displays[display_idx].new_rect)) + if ((displays[display_idx].desired_mode.dmFields & DM_POSITION) && !IsRectEmpty(&displays[display_idx].new_rect)) { has_placed = TRUE; break; @@ -590,7 +589,7 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, for (display_idx = 0; display_idx < display_count; ++display_idx) { - if (!displays[display_idx].placed || IsRectEmpty(&displays[display_idx].new_rect)) + if (!(displays[display_idx].desired_mode.dmFields & DM_POSITION) || IsRectEmpty(&displays[display_idx].new_rect)) continue; /* Get four vertices of the placed display rectangle */ @@ -681,6 +680,9 @@ static void place_all_displays(struct x11drv_display_setting *displays, INT disp INT placing_idx, display_idx; POINT min_offset, offset; + for (display_idx = 0; display_idx < display_count; ++display_idx) + displays[display_idx].desired_mode.dmFields &= ~DM_POSITION; + /* Place all displays with no extra space between them and no overlapping */ while (1) { @@ -688,7 +690,7 @@ static void place_all_displays(struct x11drv_display_setting *displays, INT disp placing_idx = -1; for (display_idx = 0; display_idx < display_count; ++display_idx) { - if (displays[display_idx].placed) + if (displays[display_idx].desired_mode.dmFields & DM_POSITION) continue; offset = get_placement_offset(displays, display_count, display_idx); @@ -705,7 +707,7 @@ static void place_all_displays(struct x11drv_display_setting *displays, INT disp set_rect_from_devmode(&displays[placing_idx].new_rect, &displays[placing_idx].desired_mode); OffsetRect(&displays[placing_idx].new_rect, min_offset.x, min_offset.y); - displays[placing_idx].placed = TRUE; + displays[placing_idx].desired_mode.dmFields |= DM_POSITION; } for (display_idx = 0; display_idx < display_count; ++display_idx) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/561
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/display.c | 55 ++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 3319697e51f..55962138b50 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -35,7 +35,6 @@ static struct x11drv_settings_handler settings_handler; struct x11drv_display_setting { ULONG_PTR id; - RECT new_rect; DEVMODEW desired_mode; }; @@ -547,8 +546,9 @@ static BOOL overlap_placed_displays(const RECT *rect, const struct x11drv_displa for (display_idx = 0; display_idx < display_count; ++display_idx) { + set_rect_from_devmode(&intersect, &displays[display_idx].desired_mode); if ((displays[display_idx].desired_mode.dmFields & DM_POSITION) && - intersect_rect(&intersect, &displays[display_idx].new_rect, rect)) + intersect_rect(&intersect, &intersect, rect)) return TRUE; } return FALSE; @@ -572,7 +572,8 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, /* If there is no placed and attached display, place this display as it is */ for (display_idx = 0; display_idx < display_count; ++display_idx) { - if ((displays[display_idx].desired_mode.dmFields & DM_POSITION) && !IsRectEmpty(&displays[display_idx].new_rect)) + set_rect_from_devmode(&rect, &displays[display_idx].desired_mode); + if ((displays[display_idx].desired_mode.dmFields & DM_POSITION) && !IsRectEmpty(&rect)) { has_placed = TRUE; break; @@ -589,37 +590,38 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, for (display_idx = 0; display_idx < display_count; ++display_idx) { - if (!(displays[display_idx].desired_mode.dmFields & DM_POSITION) || IsRectEmpty(&displays[display_idx].new_rect)) + set_rect_from_devmode(&rect, &displays[display_idx].desired_mode); + if (!(displays[display_idx].desired_mode.dmFields & DM_POSITION) || IsRectEmpty(&rect)) continue; /* Get four vertices of the placed display rectangle */ - points[0].x = displays[display_idx].new_rect.left; - points[0].y = displays[display_idx].new_rect.top; - points[1].x = displays[display_idx].new_rect.left; - points[1].y = displays[display_idx].new_rect.bottom; - points[2].x = displays[display_idx].new_rect.right; - points[2].y = displays[display_idx].new_rect.top; - points[3].x = displays[display_idx].new_rect.right; - points[3].y = displays[display_idx].new_rect.bottom; + points[0].x = rect.left; + points[0].y = rect.top; + points[1].x = rect.left; + points[1].y = rect.bottom; + points[2].x = rect.right; + points[2].y = rect.top; + points[3].x = rect.right; + points[3].y = rect.bottom; point_count = 4; /* Intersected points when moving the display to be placed horizontally */ - if (desired_rect.bottom >= displays[display_idx].new_rect.top && - desired_rect.top <= displays[display_idx].new_rect.bottom) + if (desired_rect.bottom >= rect.top && + desired_rect.top <= rect.bottom) { - points[point_count].x = displays[display_idx].new_rect.left; + points[point_count].x = rect.left; points[point_count++].y = desired_rect.top; - points[point_count].x = displays[display_idx].new_rect.right; + points[point_count].x = rect.right; points[point_count++].y = desired_rect.top; } /* Intersected points when moving the display to be placed vertically */ - if (desired_rect.left <= displays[display_idx].new_rect.right && - desired_rect.right >= displays[display_idx].new_rect.left) + if (desired_rect.left <= rect.right && + desired_rect.right >= rect.left) { points[point_count].x = desired_rect.left; - points[point_count++].y = displays[display_idx].new_rect.top; + points[point_count++].y = rect.top; points[point_count].x = desired_rect.left; - points[point_count++].y = displays[display_idx].new_rect.bottom; + points[point_count++].y = rect.bottom; } /* Try moving each vertex of the display rectangle to each points */ @@ -705,17 +707,12 @@ static void place_all_displays(struct x11drv_display_setting *displays, INT disp if (placing_idx == -1) break; - set_rect_from_devmode(&displays[placing_idx].new_rect, &displays[placing_idx].desired_mode); - OffsetRect(&displays[placing_idx].new_rect, min_offset.x, min_offset.y); + displays[placing_idx].desired_mode.dmPosition.x += min_offset.x; + displays[placing_idx].desired_mode.dmPosition.y += min_offset.y; displays[placing_idx].desired_mode.dmFields |= DM_POSITION; - } - for (display_idx = 0; display_idx < display_count; ++display_idx) - { - displays[display_idx].desired_mode.dmPosition.x = displays[display_idx].new_rect.left; - displays[display_idx].desired_mode.dmPosition.y = displays[display_idx].new_rect.top; - left_most = min(left_most, displays[display_idx].new_rect.left); - top_most = min(top_most, displays[display_idx].new_rect.top); + left_most = min(left_most, displays[placing_idx].desired_mode.dmPosition.x); + top_most = min(top_most, displays[placing_idx].desired_mode.dmPosition.y); } /* Convert virtual screen coordinates to root coordinates */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/561
From: R��mi Bernon <rbernon(a)codeweavers.com> Keeping the id in the dmDriverExtra bytes. --- dlls/winex11.drv/display.c | 106 ++++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 43 deletions(-) diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 55962138b50..444a9639633 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -34,8 +34,8 @@ static struct x11drv_settings_handler settings_handler; struct x11drv_display_setting { - ULONG_PTR id; - DEVMODEW desired_mode; + DEVMODEW mode; + ULONG_PTR extra; }; struct x11drv_display_depth @@ -472,10 +472,13 @@ static LONG get_display_settings(struct x11drv_display_setting **new_displays, for (display_idx = 0; display_idx < display_count; ++display_idx) { + DEVMODEW *mode = &displays[display_idx].mode; + ULONG_PTR *id = (ULONG_PTR *)(mode + 1); + if (NtUserEnumDisplayDevices( NULL, display_idx, &display_device, 0 )) goto done; - if (!settings_handler.get_id(display_device.DeviceName, &displays[display_idx].id)) + if (!settings_handler.get_id(display_device.DeviceName, id)) { ret = DISP_CHANGE_BADPARAM; goto done; @@ -489,12 +492,11 @@ static LONG get_display_settings(struct x11drv_display_setting **new_displays, registry_mode.dmSize = sizeof(registry_mode); if (!NtUserEnumDisplaySettings( &device_name, ENUM_REGISTRY_SETTINGS, ®istry_mode, 0 )) goto done; - - displays[display_idx].desired_mode = registry_mode; + *mode = registry_mode; } else if (!wcsicmp(dev_name, display_device.DeviceName)) { - displays[display_idx].desired_mode = *dev_mode; + *mode = *dev_mode; if (!(dev_mode->dmFields & DM_POSITION)) { memset(¤t_mode, 0, sizeof(current_mode)); @@ -502,8 +504,8 @@ static LONG get_display_settings(struct x11drv_display_setting **new_displays, if (!NtUserEnumDisplaySettings( &device_name, ENUM_CURRENT_SETTINGS, ¤t_mode, 0 )) goto done; - displays[display_idx].desired_mode.dmFields |= DM_POSITION; - displays[display_idx].desired_mode.dmPosition = current_mode.dmPosition; + mode->dmFields |= DM_POSITION; + mode->dmPosition = current_mode.dmPosition; } } else @@ -512,11 +514,12 @@ static LONG get_display_settings(struct x11drv_display_setting **new_displays, current_mode.dmSize = sizeof(current_mode); if (!NtUserEnumDisplaySettings( &device_name, ENUM_CURRENT_SETTINGS, ¤t_mode, 0 )) goto done; - - displays[display_idx].desired_mode = current_mode; + *mode = current_mode; } - lstrcpyW(displays[display_idx].desired_mode.dmDeviceName, display_device.DeviceName); + mode->dmSize = sizeof(DEVMODEW); + lstrcpyW(mode->dmDeviceName, display_device.DeviceName); + mode->dmDriverExtra = sizeof(ULONG_PTR); } *new_displays = displays; @@ -546,16 +549,16 @@ static BOOL overlap_placed_displays(const RECT *rect, const struct x11drv_displa for (display_idx = 0; display_idx < display_count; ++display_idx) { - set_rect_from_devmode(&intersect, &displays[display_idx].desired_mode); - if ((displays[display_idx].desired_mode.dmFields & DM_POSITION) && - intersect_rect(&intersect, &intersect, rect)) + const DEVMODEW *mode = &displays[display_idx].mode; + set_rect_from_devmode(&intersect, mode); + if ((mode->dmFields & DM_POSITION) && intersect_rect(&intersect, &intersect, rect)) return TRUE; } return FALSE; } /* Get the offset with minimum length to place a display next to the placed displays with no spacing and overlaps */ -static POINT get_placement_offset(const struct x11drv_display_setting *displays, INT display_count, INT placing_idx) +static POINT get_placement_offset(const struct x11drv_display_setting *displays, INT display_count, const DEVMODEW *placing) { POINT points[8], left_top, offset, min_offset = {0, 0}; INT display_idx, point_idx, point_count, vertex_idx; @@ -563,7 +566,7 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, RECT desired_rect, rect; INT width, height; - set_rect_from_devmode(&desired_rect, &displays[placing_idx].desired_mode); + set_rect_from_devmode(&desired_rect, placing); /* If the display to be placed is detached, no offset is needed to place it */ if (IsRectEmpty(&desired_rect)) @@ -572,8 +575,10 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, /* If there is no placed and attached display, place this display as it is */ for (display_idx = 0; display_idx < display_count; ++display_idx) { - set_rect_from_devmode(&rect, &displays[display_idx].desired_mode); - if ((displays[display_idx].desired_mode.dmFields & DM_POSITION) && !IsRectEmpty(&rect)) + const DEVMODEW *mode = &displays[display_idx].mode; + + set_rect_from_devmode(&rect, mode); + if ((mode->dmFields & DM_POSITION) && !IsRectEmpty(&rect)) { has_placed = TRUE; break; @@ -590,8 +595,10 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, for (display_idx = 0; display_idx < display_count; ++display_idx) { - set_rect_from_devmode(&rect, &displays[display_idx].desired_mode); - if (!(displays[display_idx].desired_mode.dmFields & DM_POSITION) || IsRectEmpty(&rect)) + const DEVMODEW *mode = &displays[display_idx].mode; + + set_rect_from_devmode(&rect, mode); + if (!(mode->dmFields & DM_POSITION) || IsRectEmpty(&rect)) continue; /* Get four vertices of the placed display rectangle */ @@ -679,47 +686,53 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, static void place_all_displays(struct x11drv_display_setting *displays, INT display_count) { INT left_most = INT_MAX, top_most = INT_MAX; - INT placing_idx, display_idx; POINT min_offset, offset; + DEVMODEW *placing; + INT display_idx; for (display_idx = 0; display_idx < display_count; ++display_idx) - displays[display_idx].desired_mode.dmFields &= ~DM_POSITION; + { + DEVMODEW *mode = &displays[display_idx].mode; + mode->dmFields &= ~DM_POSITION; + } /* Place all displays with no extra space between them and no overlapping */ while (1) { /* Place the unplaced display with the minimum offset length first */ - placing_idx = -1; + placing = NULL; for (display_idx = 0; display_idx < display_count; ++display_idx) { - if (displays[display_idx].desired_mode.dmFields & DM_POSITION) + DEVMODEW *mode = &displays[display_idx].mode; + if (mode->dmFields & DM_POSITION) continue; - offset = get_placement_offset(displays, display_count, display_idx); - if (placing_idx == -1 || offset_length(offset) < offset_length(min_offset)) + offset = get_placement_offset(displays, display_count, mode); + if (!placing || offset_length(offset) < offset_length(min_offset)) { min_offset = offset; - placing_idx = display_idx; + placing = mode; } } /* If all displays are placed */ - if (placing_idx == -1) + if (!placing) break; - displays[placing_idx].desired_mode.dmPosition.x += min_offset.x; - displays[placing_idx].desired_mode.dmPosition.y += min_offset.y; - displays[placing_idx].desired_mode.dmFields |= DM_POSITION; + placing->dmPosition.x += min_offset.x; + placing->dmPosition.y += min_offset.y; + placing->dmFields |= DM_POSITION; - left_most = min(left_most, displays[placing_idx].desired_mode.dmPosition.x); - top_most = min(top_most, displays[placing_idx].desired_mode.dmPosition.y); + left_most = min(left_most, placing->dmPosition.x); + top_most = min(top_most, placing->dmPosition.y); } /* Convert virtual screen coordinates to root coordinates */ for (display_idx = 0; display_idx < display_count; ++display_idx) { - displays[display_idx].desired_mode.dmPosition.x -= left_most; - displays[display_idx].desired_mode.dmPosition.y -= top_most; + DEVMODEW *mode = &displays[display_idx].mode; + mode->dmPosition.x -= left_most; + mode->dmPosition.y -= top_most; } } @@ -732,24 +745,27 @@ static LONG apply_display_settings(struct x11drv_display_setting *displays, INT for (display_idx = 0; display_idx < display_count; ++display_idx) { - attached_mode = !is_detached_mode(&displays[display_idx].desired_mode); + DEVMODEW *mode = &displays[display_idx].mode; + ULONG_PTR *id = (ULONG_PTR *)(mode + 1); + + attached_mode = !is_detached_mode(mode); if ((attached_mode && !do_attach) || (!attached_mode && do_attach)) continue; - full_mode = get_full_mode(displays[display_idx].id, &displays[display_idx].desired_mode); + full_mode = get_full_mode(*id, mode); if (!full_mode) return DISP_CHANGE_BADMODE; TRACE("handler:%s changing %s to position:(%d,%d) resolution:%ux%u frequency:%uHz " "depth:%ubits orientation:%#x.\n", settings_handler.name, - wine_dbgstr_w(displays[display_idx].desired_mode.dmDeviceName), + wine_dbgstr_w(mode->dmDeviceName), full_mode->dmPosition.x, full_mode->dmPosition.y, full_mode->dmPelsWidth, full_mode->dmPelsHeight, full_mode->dmDisplayFrequency, full_mode->dmBitsPerPel, full_mode->dmDisplayOrientation); - ret = settings_handler.set_current_mode(displays[display_idx].id, full_mode); + ret = settings_handler.set_current_mode(*id, full_mode); if (attached_mode && ret == DISP_CHANGE_SUCCESSFUL) - set_display_depth(displays[display_idx].id, full_mode->dmBitsPerPel); + set_display_depth(*id, full_mode->dmBitsPerPel); free_full_mode(full_mode); if (ret != DISP_CHANGE_SUCCESSFUL) return ret; @@ -764,7 +780,8 @@ static BOOL all_detached_settings(const struct x11drv_display_setting *displays, for (display_idx = 0; display_idx < display_count; ++display_idx) { - if (!is_detached_mode(&displays[display_idx].desired_mode)) + const DEVMODEW *mode = &displays[display_idx].mode; + if (!is_detached_mode(mode)) return FALSE; } @@ -791,9 +808,12 @@ LONG X11DRV_ChangeDisplaySettingsEx( LPCWSTR devname, LPDEVMODEW devmode, { for (display_idx = 0; display_idx < display_count; ++display_idx) { - if (!wcsicmp(displays[display_idx].desired_mode.dmDeviceName, devname)) + DEVMODEW *mode = &displays[display_idx].mode; + ULONG_PTR *id = (ULONG_PTR *)(mode + 1); + + if (!wcsicmp(mode->dmDeviceName, devname)) { - full_mode = get_full_mode(displays[display_idx].id, &displays[display_idx].desired_mode); + full_mode = get_full_mode(*id, mode); if (!full_mode) { free(displays); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/561
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/display.c | 106 ++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 60 deletions(-) diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 444a9639633..3285a6bb35d 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -32,11 +32,7 @@ static struct x11drv_display_device_handler host_handler; struct x11drv_display_device_handler desktop_handler; static struct x11drv_settings_handler settings_handler; -struct x11drv_display_setting -{ - DEVMODEW mode; - ULONG_PTR extra; -}; +#define NEXT_DEVMODEW(mode) ((DEVMODEW *)((char *)((mode) + 1) + (mode)->dmDriverExtra)) struct x11drv_display_depth { @@ -452,11 +448,15 @@ static void free_full_mode(DEVMODEW *mode) free(mode); } -static LONG get_display_settings(struct x11drv_display_setting **new_displays, - INT *new_display_count, const WCHAR *dev_name, DEVMODEW *dev_mode) +static LONG get_display_settings(DEVMODEW **new_displays, const WCHAR *dev_name, DEVMODEW *dev_mode) { - struct x11drv_display_setting *displays; - DEVMODEW registry_mode, current_mode; + struct display_setting + { + DEVMODEW mode; + ULONG_PTR extra; + }; + + DEVMODEW registry_mode, current_mode, *mode, *displays; INT display_idx, display_count = 0; DISPLAY_DEVICEW display_device; LONG ret = DISP_CHANGE_FAILED; @@ -466,15 +466,16 @@ static LONG get_display_settings(struct x11drv_display_setting **new_displays, for (display_idx = 0; !NtUserEnumDisplayDevices( NULL, display_idx, &display_device, 0 ); ++display_idx) ++display_count; - displays = calloc(display_count, sizeof(*displays)); - if (!displays) - goto done; + /* allocate an extra mode to make iteration easier */ + if (!(displays = calloc(display_count + 1, sizeof(struct display_setting)))) goto done; + mode = displays; - for (display_idx = 0; display_idx < display_count; ++display_idx) + for (display_idx = 0; !NtUserEnumDisplayDevices( NULL, display_idx, &display_device, 0 ); ++display_idx) { - DEVMODEW *mode = &displays[display_idx].mode; ULONG_PTR *id = (ULONG_PTR *)(mode + 1); + if (display_idx >= display_count) + goto done; if (NtUserEnumDisplayDevices( NULL, display_idx, &display_device, 0 )) goto done; @@ -517,13 +518,13 @@ static LONG get_display_settings(struct x11drv_display_setting **new_displays, *mode = current_mode; } - mode->dmSize = sizeof(DEVMODEW); lstrcpyW(mode->dmDeviceName, display_device.DeviceName); - mode->dmDriverExtra = sizeof(ULONG_PTR); + mode->dmSize = sizeof(DEVMODEW); + mode->dmDriverExtra = sizeof(struct display_setting) - sizeof(DEVMODEW); + mode = NEXT_DEVMODEW(mode); } *new_displays = displays; - *new_display_count = display_count; return DISP_CHANGE_SUCCESSFUL; done: @@ -542,14 +543,13 @@ static void set_rect_from_devmode(RECT *rect, const DEVMODEW *mode) } /* Check if a rect overlaps with placed display rects */ -static BOOL overlap_placed_displays(const RECT *rect, const struct x11drv_display_setting *displays, INT display_count) +static BOOL overlap_placed_displays(const RECT *rect, const DEVMODEW *displays) { - INT display_idx; + const DEVMODEW *mode; RECT intersect; - for (display_idx = 0; display_idx < display_count; ++display_idx) + for (mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode)) { - const DEVMODEW *mode = &displays[display_idx].mode; set_rect_from_devmode(&intersect, mode); if ((mode->dmFields & DM_POSITION) && intersect_rect(&intersect, &intersect, rect)) return TRUE; @@ -558,12 +558,13 @@ static BOOL overlap_placed_displays(const RECT *rect, const struct x11drv_displa } /* Get the offset with minimum length to place a display next to the placed displays with no spacing and overlaps */ -static POINT get_placement_offset(const struct x11drv_display_setting *displays, INT display_count, const DEVMODEW *placing) +static POINT get_placement_offset(const DEVMODEW *displays, const DEVMODEW *placing) { POINT points[8], left_top, offset, min_offset = {0, 0}; - INT display_idx, point_idx, point_count, vertex_idx; + INT point_idx, point_count, vertex_idx; BOOL has_placed = FALSE, first = TRUE; RECT desired_rect, rect; + const DEVMODEW *mode; INT width, height; set_rect_from_devmode(&desired_rect, placing); @@ -573,10 +574,8 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, return min_offset; /* If there is no placed and attached display, place this display as it is */ - for (display_idx = 0; display_idx < display_count; ++display_idx) + for (mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode)) { - const DEVMODEW *mode = &displays[display_idx].mode; - set_rect_from_devmode(&rect, mode); if ((mode->dmFields & DM_POSITION) && !IsRectEmpty(&rect)) { @@ -593,10 +592,8 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, width = desired_rect.right - desired_rect.left; height = desired_rect.bottom - desired_rect.top; - for (display_idx = 0; display_idx < display_count; ++display_idx) + for (mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode)) { - const DEVMODEW *mode = &displays[display_idx].mode; - set_rect_from_devmode(&rect, mode); if (!(mode->dmFields & DM_POSITION) || IsRectEmpty(&rect)) continue; @@ -664,7 +661,7 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, offset.y = left_top.y - desired_rect.top; rect = desired_rect; OffsetRect(&rect, offset.x, offset.y); - if (!overlap_placed_displays(&rect, displays, display_count)) + if (!overlap_placed_displays(&rect, displays)) { if (first) { @@ -683,31 +680,26 @@ static POINT get_placement_offset(const struct x11drv_display_setting *displays, return min_offset; } -static void place_all_displays(struct x11drv_display_setting *displays, INT display_count) +static void place_all_displays(DEVMODEW *displays) { INT left_most = INT_MAX, top_most = INT_MAX; POINT min_offset, offset; - DEVMODEW *placing; - INT display_idx; + DEVMODEW *mode, *placing; - for (display_idx = 0; display_idx < display_count; ++display_idx) - { - DEVMODEW *mode = &displays[display_idx].mode; + for (mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode)) mode->dmFields &= ~DM_POSITION; - } /* Place all displays with no extra space between them and no overlapping */ while (1) { /* Place the unplaced display with the minimum offset length first */ placing = NULL; - for (display_idx = 0; display_idx < display_count; ++display_idx) + for (mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode)) { - DEVMODEW *mode = &displays[display_idx].mode; if (mode->dmFields & DM_POSITION) continue; - offset = get_placement_offset(displays, display_count, mode); + offset = get_placement_offset(displays, mode); if (!placing || offset_length(offset) < offset_length(min_offset)) { min_offset = offset; @@ -728,24 +720,22 @@ static void place_all_displays(struct x11drv_display_setting *displays, INT disp } /* Convert virtual screen coordinates to root coordinates */ - for (display_idx = 0; display_idx < display_count; ++display_idx) + for (mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode)) { - DEVMODEW *mode = &displays[display_idx].mode; mode->dmPosition.x -= left_most; mode->dmPosition.y -= top_most; } } -static LONG apply_display_settings(struct x11drv_display_setting *displays, INT display_count, BOOL do_attach) +static LONG apply_display_settings(DEVMODEW *displays, BOOL do_attach) { DEVMODEW *full_mode; BOOL attached_mode; - INT display_idx; + DEVMODEW *mode; LONG ret; - for (display_idx = 0; display_idx < display_count; ++display_idx) + for (mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode)) { - DEVMODEW *mode = &displays[display_idx].mode; ULONG_PTR *id = (ULONG_PTR *)(mode + 1); attached_mode = !is_detached_mode(mode); @@ -774,13 +764,12 @@ static LONG apply_display_settings(struct x11drv_display_setting *displays, INT return DISP_CHANGE_SUCCESSFUL; } -static BOOL all_detached_settings(const struct x11drv_display_setting *displays, INT display_count) +static BOOL all_detached_settings(const DEVMODEW *displays) { - INT display_idx; + const DEVMODEW *mode; - for (display_idx = 0; display_idx < display_count; ++display_idx) + for (mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode)) { - const DEVMODEW *mode = &displays[display_idx].mode; if (!is_detached_mode(mode)) return FALSE; } @@ -795,20 +784,17 @@ static BOOL all_detached_settings(const struct x11drv_display_setting *displays, LONG X11DRV_ChangeDisplaySettingsEx( LPCWSTR devname, LPDEVMODEW devmode, HWND hwnd, DWORD flags, LPVOID lpvoid ) { - struct x11drv_display_setting *displays; - INT display_idx, display_count; - DEVMODEW *full_mode; + DEVMODEW *displays, *mode, *full_mode; LONG ret; - ret = get_display_settings(&displays, &display_count, devname, devmode); + ret = get_display_settings( &displays, devname, devmode ); if (ret != DISP_CHANGE_SUCCESSFUL) return ret; if (flags & CDS_UPDATEREGISTRY && devname && devmode) { - for (display_idx = 0; display_idx < display_count; ++display_idx) + for (mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode)) { - DEVMODEW *mode = &displays[display_idx].mode; ULONG_PTR *id = (ULONG_PTR *)(mode + 1); if (!wcsicmp(mode->dmDeviceName, devname)) @@ -833,19 +819,19 @@ LONG X11DRV_ChangeDisplaySettingsEx( LPCWSTR devname, LPDEVMODEW devmode, return DISP_CHANGE_SUCCESSFUL; } - if (all_detached_settings(displays, display_count)) + if (all_detached_settings( displays )) { WARN("Detaching all displays is not permitted.\n"); free(displays); return DISP_CHANGE_SUCCESSFUL; } - place_all_displays(displays, display_count); + place_all_displays( displays ); /* Detach displays first to free up CRTCs */ - ret = apply_display_settings(displays, display_count, FALSE); + ret = apply_display_settings( displays, FALSE ); if (ret == DISP_CHANGE_SUCCESSFUL) - ret = apply_display_settings(displays, display_count, TRUE); + ret = apply_display_settings( displays, TRUE ); if (ret == DISP_CHANGE_SUCCESSFUL) X11DRV_DisplayDevices_Update(TRUE); free(displays); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/561
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/desktop.c | 2 +- dlls/winex11.drv/display.c | 2 +- dlls/winex11.drv/x11drv.h | 2 +- dlls/winex11.drv/xrandr.c | 4 ++-- dlls/winex11.drv/xvidmode.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c index 3e589f9d940..84457582a7c 100644 --- a/dlls/winex11.drv/desktop.c +++ b/dlls/winex11.drv/desktop.c @@ -218,7 +218,7 @@ static BOOL X11DRV_desktop_get_current_mode( ULONG_PTR id, DEVMODEW *mode ) return TRUE; } -static LONG X11DRV_desktop_set_current_mode( ULONG_PTR id, DEVMODEW *mode ) +static LONG X11DRV_desktop_set_current_mode( ULONG_PTR id, const DEVMODEW *mode ) { if (mode->dmFields & DM_BITSPERPEL && mode->dmBitsPerPel != screen_bpp) WARN("Cannot change screen color depth from %dbits to %dbits!\n", screen_bpp, mode->dmBitsPerPel); diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 3285a6bb35d..9a17305d030 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -144,7 +144,7 @@ static BOOL nores_get_current_mode(ULONG_PTR id, DEVMODEW *mode) return TRUE; } -static LONG nores_set_current_mode(ULONG_PTR id, DEVMODEW *mode) +static LONG nores_set_current_mode(ULONG_PTR id, const DEVMODEW *mode) { WARN("NoRes settings handler, ignoring mode change request.\n"); return DISP_CHANGE_SUCCESSFUL; diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 0fe29d8e322..b8997455324 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -741,7 +741,7 @@ struct x11drv_settings_handler * mode must be a valid mode from get_modes() with optional fields, such as dmPosition set. * * Return DISP_CHANGE_*, same as ChangeDisplaySettingsExW() return values */ - LONG (*set_current_mode)(ULONG_PTR id, DEVMODEW *mode); + LONG (*set_current_mode)(ULONG_PTR id, const DEVMODEW *mode); }; extern void X11DRV_Settings_SetHandler(const struct x11drv_settings_handler *handler) DECLSPEC_HIDDEN; diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c index d3b96e3170b..5b81fb07709 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -282,7 +282,7 @@ static BOOL xrandr10_get_current_mode( ULONG_PTR id, DEVMODEW *mode ) return TRUE; } -static LONG xrandr10_set_current_mode( ULONG_PTR id, DEVMODEW *mode ) +static LONG xrandr10_set_current_mode( ULONG_PTR id, const DEVMODEW *mode ) { XRRScreenConfiguration *screen_config; Rotation rotation; @@ -1544,7 +1544,7 @@ done: return ret; } -static LONG xrandr14_set_current_mode( ULONG_PTR id, DEVMODEW *mode ) +static LONG xrandr14_set_current_mode( ULONG_PTR id, const DEVMODEW *mode ) { unsigned int screen_width, screen_height; RROutput output = (RROutput)id, *outputs; diff --git a/dlls/winex11.drv/xvidmode.c b/dlls/winex11.drv/xvidmode.c index 4ca7bc1ac59..27831cabd3b 100644 --- a/dlls/winex11.drv/xvidmode.c +++ b/dlls/winex11.drv/xvidmode.c @@ -215,7 +215,7 @@ static BOOL xf86vm_get_current_mode(ULONG_PTR id, DEVMODEW *mode) return TRUE; } -static LONG xf86vm_set_current_mode(ULONG_PTR id, DEVMODEW *mode) +static LONG xf86vm_set_current_mode(ULONG_PTR id, const DEVMODEW *mode) { XF86VidModeModeInfo *xf86vm_mode; Bool ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/561
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/display.c:
static void place_all_displays(struct x11drv_display_setting *displays, INT display_count) { INT left_most = INT_MAX, top_most = INT_MAX; - INT placing_idx, display_idx; POINT min_offset, offset; + DEVMODEW *placing; + INT display_idx;
for (display_idx = 0; display_idx < display_count; ++display_idx) - displays[display_idx].desired_mode.dmFields &= ~DM_POSITION; + { + DEVMODEW *mode = &displays[display_idx].mode; + mode->dmFields &= ~DM_POSITION; + } mode is only used once. It seems simpler to use displays[display_idx].mode.dmFields here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/561#note_5397
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/display.c:
for (display_idx = 0; display_idx < display_count; ++display_idx) { - if (!is_detached_mode(&displays[display_idx].desired_mode)) + const DEVMODEW *mode = &displays[display_idx].mode; + if (!is_detached_mode(mode))
Same here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/561#note_5398
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/display.c:
*mode = current_mode; }
- mode->dmSize = sizeof(DEVMODEW); lstrcpyW(mode->dmDeviceName, display_device.DeviceName); - mode->dmDriverExtra = sizeof(ULONG_PTR); + mode->dmSize = sizeof(DEVMODEW);
You can keep dmSize in the same place. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/561#note_5399
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/display.c:
for (display_idx = 0; !NtUserEnumDisplayDevices( NULL, display_idx, &display_device, 0 ); ++display_idx) ++display_count;
- displays = calloc(display_count, sizeof(*displays)); - if (!displays) - goto done; + /* allocate an extra mode to make iteration easier */ + if (!(displays = calloc(display_count + 1, sizeof(struct display_setting)))) goto done; + mode = displays;
- for (display_idx = 0; display_idx < display_count; ++display_idx) + for (display_idx = 0; !NtUserEnumDisplayDevices( NULL, display_idx, &display_device, 0 ); ++display_idx)
Why do you need to change the loop? And even if you have to, NtUserEnumDisplayDevices is called twice for the same display_idx. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/561#note_5400
On Mon Aug 1 02:38:10 2022 +0000, Zhiyi Zhang wrote:
mode is only used once. It seems simpler to use displays[display_idx].mode.dmFields here. I see. This is only temporary. Never mind then.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/561#note_5401
participants (2)
-
Rémi Bernon -
Zhiyi Zhang (＠zhiyi)