From 49f16894963149ffa4701b80bc80399408ee113c Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 2 Dec 2024 19:33:04 +0100 Subject: [PATCH 1/4] [core,settings] verbose output on monitor layout failure --- libfreerdp/core/settings.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/libfreerdp/core/settings.c b/libfreerdp/core/settings.c index 68c680470..fbe1576d9 100644 --- a/libfreerdp/core/settings.c +++ b/libfreerdp/core/settings.c @@ -64,6 +64,14 @@ struct bounds_t INT32 height; }; +static const char* bounds2str(const struct bounds_t* bounds, char* buffer, size_t len) +{ + WINPR_ASSERT(bounds); + (void)_snprintf(buffer, len, "{%dx%d-%dx%d}", bounds->x, bounds->y, bounds->x + bounds->width, + bounds->y + bounds->height); + return buffer; +} + static struct bounds_t union_rect(const struct bounds_t* a, const struct bounds_t* b) { WINPR_ASSERT(a); @@ -335,7 +343,7 @@ void freerdp_settings_print_warnings(const rdpSettings* settings) } } -static BOOL monitor_operlaps(const rdpSettings* settings, UINT32 start, UINT32 count, +static BOOL monitor_operlaps(const rdpSettings* settings, UINT32 orig, UINT32 start, UINT32 count, const rdpMonitor* compare) { const struct bounds_t rect1 = { @@ -351,6 +359,12 @@ static BOOL monitor_operlaps(const rdpSettings* settings, UINT32 start, UINT32 c if (intersect_rects(&rect1, &rect2)) { + char buffer1[32] = { 0 }; + char buffer2[32] = { 0 }; + + WLog_ERR(TAG, "Monitor %" PRIu32 " and %" PRIu32 " are overlapping:", orig, x); + WLog_ERR(TAG, "%s overlapps with %s", bounds2str(&rect1, buffer1, sizeof(buffer1)), + bounds2str(&rect2, buffer2, sizeof(buffer2))); WLog_ERR( TAG, "Mulitimonitor mode requested, but local layout has gaps or overlapping areas!"); @@ -487,8 +501,12 @@ static BOOL find_path_exists_with_dijkstra(UINT32** graph, size_t count, UINT32 { if (!visited[y]) { - if (mindistance + cost[nextnode][y] < distance[y]) + UINT32 dist = mindistance + cost[nextnode][y]; + if (dist < distance[y]) + { + distance[y] = dist; parent[y] = nextnode; + } } } pos++; @@ -548,7 +566,7 @@ static BOOL freerdp_settings_client_monitors_overlap(const rdpSettings* settings { const rdpMonitor* monitor = freerdp_settings_get_pointer_array(settings, FreeRDP_MonitorDefArray, x); - if (monitor_operlaps(settings, x + 1, count, monitor)) + if (monitor_operlaps(settings, x, x + 1, count, monitor)) return TRUE; } return FALSE; From def00a32bec41307c4b7a421eaf5a22c72879a19 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 2 Dec 2024 19:33:32 +0100 Subject: [PATCH 2/4] [core,settings] add testcases --- libfreerdp/core/test/TestSettings.c | 74 +++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/libfreerdp/core/test/TestSettings.c b/libfreerdp/core/test/TestSettings.c index 5dc9b2467..1f102b7b2 100644 --- a/libfreerdp/core/test/TestSettings.c +++ b/libfreerdp/core/test/TestSettings.c @@ -1269,6 +1269,78 @@ static BOOL test_validity_check(void) .deviceScaleFactor = 100 } } }; + const rdpMonitor multi_monitor_valid_2[] = { + { .x = 0, + .y = 0, + .width = 1920, + .height = 1080, + .is_primary = TRUE, + .orig_screen = 0, + .attributes = { .physicalWidth = 100, + .physicalHeight = 100, + .orientation = ORIENTATION_PREFERENCE_LANDSCAPE, + .desktopScaleFactor = 100, + .deviceScaleFactor = 100 } }, + { .x = 3840, + .y = 0, + .width = 1920, + .height = 1080, + .is_primary = FALSE, + .orig_screen = 0, + .attributes = { .physicalWidth = 100, + .physicalHeight = 100, + .orientation = ORIENTATION_PREFERENCE_LANDSCAPE, + .desktopScaleFactor = 100, + .deviceScaleFactor = 100 } }, + { .x = 1920, + .y = 0, + .width = 1920, + .height = 1080, + .is_primary = FALSE, + .orig_screen = 0, + .attributes = { .physicalWidth = 100, + .physicalHeight = 100, + .orientation = ORIENTATION_PREFERENCE_LANDSCAPE, + .desktopScaleFactor = 100, + .deviceScaleFactor = 100 } } + }; + + const rdpMonitor multi_monitor_valid_3[] = { + { .x = 1920, + .y = 0, + .width = 1920, + .height = 1080, + .is_primary = TRUE, + .orig_screen = 0, + .attributes = { .physicalWidth = 100, + .physicalHeight = 100, + .orientation = ORIENTATION_PREFERENCE_LANDSCAPE, + .desktopScaleFactor = 100, + .deviceScaleFactor = 100 } }, + { .x = 3840, + .y = 0, + .width = 1920, + .height = 1080, + .is_primary = FALSE, + .orig_screen = 0, + .attributes = { .physicalWidth = 100, + .physicalHeight = 100, + .orientation = ORIENTATION_PREFERENCE_LANDSCAPE, + .desktopScaleFactor = 100, + .deviceScaleFactor = 100 } }, + { .x = 0, + .y = 0, + .width = 1920, + .height = 1080, + .is_primary = FALSE, + .orig_screen = 0, + .attributes = { .physicalWidth = 100, + .physicalHeight = 100, + .orientation = ORIENTATION_PREFERENCE_LANDSCAPE, + .desktopScaleFactor = 100, + .deviceScaleFactor = 100 } } + }; + const struct validity_test_case tests[] = { { TRUE, ARRAYSIZE(single_monitor_valid), single_monitor_valid }, { FALSE, ARRAYSIZE(single_monitor_invalid_1), single_monitor_invalid_1 }, @@ -1280,6 +1352,8 @@ static BOOL test_validity_check(void) { FALSE, ARRAYSIZE(multi_monitor_invalid_2), multi_monitor_invalid_2 }, { FALSE, ARRAYSIZE(multi_monitor_invalid_3), multi_monitor_invalid_3 }, { FALSE, ARRAYSIZE(multi_monitor_invalid_4), multi_monitor_invalid_4 }, + { TRUE, ARRAYSIZE(multi_monitor_valid_2), multi_monitor_valid_2 }, + { TRUE, ARRAYSIZE(multi_monitor_valid_3), multi_monitor_valid_3 } }; rc = TRUE; From d48cd481e7529ac59939c4f60fec83a6767e7334 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 3 Dec 2024 12:53:31 +0100 Subject: [PATCH 3/4] [core,settings] dump monitor configuration to log --- libfreerdp/core/settings.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/libfreerdp/core/settings.c b/libfreerdp/core/settings.c index fbe1576d9..17e4a4580 100644 --- a/libfreerdp/core/settings.c +++ b/libfreerdp/core/settings.c @@ -559,6 +559,33 @@ fail: return rc; } +static void log_monitor(UINT32 idx, const rdpMonitor* monitor, wLog* log, DWORD level) +{ + WINPR_ASSERT(monitor); + + WLog_Print(log, level, + "[%" PRIu32 "] [%s] {%dx%d-%dx%d} [%" PRIu32 "] {%" PRIu32 "x%" PRIu32 + ", orientation: %" PRIu32 ", desktopScale: %" PRIu32 ", deviceScale: %" PRIu32 "}", + idx, monitor->is_primary ? "primary" : " ", monitor->x, monitor->y, + monitor->width, monitor->height, monitor->orig_screen, + monitor->attributes.physicalWidth, monitor->attributes.physicalHeight, + monitor->attributes.orientation, monitor->attributes.desktopScaleFactor, + monitor->attributes.deviceScaleFactor); +} + +static void log_monitor_configuration(const rdpSettings* settings, wLog* log, DWORD level) +{ + const UINT32 count = freerdp_settings_get_uint32(settings, FreeRDP_MonitorCount); + WLog_Print(log, level, "[BEGIN] MonitorDefArray[%" PRIu32 "]", count); + for (UINT32 x = 0; x < count; x++) + { + const rdpMonitor* monitor = + freerdp_settings_get_pointer_array(settings, FreeRDP_MonitorDefArray, x); + log_monitor(x, monitor, log, level); + } + WLog_Print(log, level, "[END] MonitorDefArray[%" PRIu32 "]", count); +} + static BOOL freerdp_settings_client_monitors_overlap(const rdpSettings* settings) { const UINT32 count = freerdp_settings_get_uint32(settings, FreeRDP_MonitorCount); @@ -652,6 +679,7 @@ static BOOL freerdp_settings_client_monitors_check_primary_and_origin(const rdpS BOOL freerdp_settings_check_client_after_preconnect(const rdpSettings* settings) { + log_monitor_configuration(settings, WLog_Get(TAG), WLOG_DEBUG); if (freerdp_settings_client_monitors_overlap(settings)) return FALSE; if (freerdp_settings_client_monitors_have_gaps(settings)) From 646cacca0e81c95ed72f97d52dbbc22b4fc3ec8f Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 3 Dec 2024 13:05:51 +0100 Subject: [PATCH 4/4] [core,settings] add instructions on fail Add bug reporting instructions for failing monitor configurations --- libfreerdp/core/settings.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/libfreerdp/core/settings.c b/libfreerdp/core/settings.c index 17e4a4580..f963d37f7 100644 --- a/libfreerdp/core/settings.c +++ b/libfreerdp/core/settings.c @@ -679,14 +679,30 @@ static BOOL freerdp_settings_client_monitors_check_primary_and_origin(const rdpS BOOL freerdp_settings_check_client_after_preconnect(const rdpSettings* settings) { - log_monitor_configuration(settings, WLog_Get(TAG), WLOG_DEBUG); + wLog* log = WLog_Get(TAG); + BOOL rc = TRUE; + log_monitor_configuration(settings, log, WLOG_DEBUG); if (freerdp_settings_client_monitors_overlap(settings)) - return FALSE; + rc = FALSE; if (freerdp_settings_client_monitors_have_gaps(settings)) - return FALSE; + rc = FALSE; if (!freerdp_settings_client_monitors_check_primary_and_origin(settings)) - return FALSE; - return TRUE; + rc = FALSE; + if (!rc) + { + DWORD level = WLOG_ERROR; + WLog_Print(log, level, "Invalid or unsupported monitor configuration detected"); + WLog_Print(log, level, "Check if the configuration is valid."); + WLog_Print(log, level, + "If you suspect a bug create a new issue at " + "https://github.com/FreeRDP/FreeRDP/issues/new"); + WLog_Print( + log, level, + "Provide at least the following log lines detailing your monitor configuration:"); + log_monitor_configuration(settings, log, level); + } + + return rc; } BOOL freerdp_settings_set_default_order_support(rdpSettings* settings)