[core,settings] Fix MONITOR_DEF settings

* Make FreeRDP_MonitorLocalShiftX and FreeRDP_MonitorLocalShiftY signed
* Add function freerdp_settings_set_monitor_def_array_sorted to proplery
  align the MONITOR_DEF array according to RDP requirements

Author: akallabeth

include/freerdp/settings.h

@@ -752,6 +752,27 @@ extern "C"
 	WINPR_ATTR_MALLOC(free, 1)
 	FREERDP_API char* freerdp_settings_get_config_path(void);
 
+	/** @brief Sort monitor array according to:
+	 *  1. First monitor is at x/y 0/0 and is the primary monitor
+	 *  2. The primary monitor must be at 0/0, if not set
+	 * FreeRDP_MonitorLocalShiftX/FreeRDP_MonitorLocalShiftY
+	 *
+	 *  The FreeRDP_MonitorLocalShiftX/FreeRDP_MonitorLocalShiftY is required to map the local
+	 * monitors / mouse / touch coordinates to the remote ones.
+	 *
+	 *  @param settings The settings to set the monitors for
+	 *  @param monitors The unsorted monitors array
+	 *  @param count The number of monitors in the unsorted array
+	 *
+	 *  @return \b TRUE if the configuration is valid (or could be corrected to a valid one), \b
+	 * FALSE otherwise.
+	 *
+	 *  @version since 3.11.0
+	 */
+	FREERDP_API BOOL freerdp_settings_set_monitor_def_array_sorted(rdpSettings* settings,
+	                                                               const rdpMonitor* monitors,
+	                                                               size_t count);
+
 #ifdef __cplusplus
 }
 #endif

include/freerdp/settings_types_private.h

@@ -160,8 +160,8 @@ struct rdp_settings
 	SETTINGS_DEPRECATED(ALIGN64 BOOL ListMonitors);            /*    392 */
 	SETTINGS_DEPRECATED(ALIGN64 UINT32* MonitorIds);           /*    393 */
 	SETTINGS_DEPRECATED(ALIGN64 UINT32 NumMonitorIds);         /*    394 */
-	SETTINGS_DEPRECATED(ALIGN64 UINT32 MonitorLocalShiftX);    /*395 */
-	SETTINGS_DEPRECATED(ALIGN64 UINT32 MonitorLocalShiftY);    /*    396 */
+	SETTINGS_DEPRECATED(ALIGN64 INT32 MonitorLocalShiftX);     /*395 */
+	SETTINGS_DEPRECATED(ALIGN64 INT32 MonitorLocalShiftY);     /*    396 */
 	SETTINGS_DEPRECATED(ALIGN64 BOOL HasMonitorAttributes);    /*    397 */
 	SETTINGS_DEPRECATED(ALIGN64 UINT32 MonitorFlags);          /* 398 */
 	SETTINGS_DEPRECATED(ALIGN64 UINT32 MonitorAttributeFlags); /* 399 */

libfreerdp/common/settings.c

@@ -2257,3 +2257,114 @@ BOOL freerdp_settings_are_valid(const rdpSettings* settings)
 {
 	return settings != NULL;
 }
+
+/* Function to sort rdpMonitor arrays:
+ * 1. first element is primary monitor
+ * 2. all others are sorted by coordinates of x/y
+ */
+static int sort_monitor_fn(const void* pva, const void* pvb)
+{
+	const rdpMonitor* a = pva;
+	const rdpMonitor* b = pvb;
+	WINPR_ASSERT(a);
+	WINPR_ASSERT(b);
+	if (a->is_primary && b->is_primary)
+		return 0;
+	if (a->is_primary)
+		return -1;
+	if (b->is_primary)
+		return 1;
+
+	if (a->x != b->x)
+		return a->x - b->x;
+	if (a->y != b->y)
+		return a->y - b->y;
+	return 0;
+}
+
+BOOL freerdp_settings_set_monitor_def_array_sorted(rdpSettings* settings,
+                                                   const rdpMonitor* monitors, size_t count)
+{
+	WINPR_ASSERT(monitors || (count == 0));
+	if (count == 0)
+	{
+		if (!freerdp_settings_set_int32(settings, FreeRDP_MonitorLocalShiftX, 0))
+			return FALSE;
+		if (!freerdp_settings_set_int32(settings, FreeRDP_MonitorLocalShiftY, 0))
+			return FALSE;
+		if (!freerdp_settings_set_pointer_len(settings, FreeRDP_MonitorDefArray, NULL, 0))
+			return FALSE;
+		return freerdp_settings_set_uint32(settings, FreeRDP_MonitorCount, 0);
+		return TRUE;
+	}
+
+	// Find primary or alternatively the monitor at 0/0
+	const rdpMonitor* primary = NULL;
+	for (size_t x = 0; x < count; x++)
+	{
+		const rdpMonitor* cur = &monitors[x];
+		if (cur->is_primary)
+		{
+			primary = cur;
+			break;
+		}
+	}
+	if (!primary)
+	{
+		for (size_t x = 0; x < count; x++)
+		{
+			const rdpMonitor* cur = &monitors[x];
+			if ((cur->x == 0) && (cur->y == 0))
+			{
+				primary = cur;
+				break;
+			}
+		}
+	}
+
+	if (!primary)
+	{
+		WLog_ERR(TAG, "Could not find primary monitor, aborting");
+		return FALSE;
+	}
+
+	if (!freerdp_settings_set_pointer_len(settings, FreeRDP_MonitorDefArray, NULL, count))
+		return FALSE;
+	rdpMonitor* sorted = freerdp_settings_get_pointer_writable(settings, FreeRDP_MonitorDefArray);
+	WINPR_ASSERT(sorted);
+
+	size_t sortpos = 0;
+
+	/* Set primary. Ensure left/top is at 0/0 and flags contains MONITOR_PRIMARY */
+	sorted[sortpos] = *primary;
+	sorted[sortpos].x = 0;
+	sorted[sortpos].y = 0;
+	sorted[sortpos].is_primary = TRUE;
+	sortpos++;
+
+	/* Set monitor shift to original layout */
+	const INT32 offsetX = primary->x;
+	const INT32 offsetY = primary->y;
+	if (!freerdp_settings_set_int32(settings, FreeRDP_MonitorLocalShiftX, offsetX))
+		return FALSE;
+	if (!freerdp_settings_set_int32(settings, FreeRDP_MonitorLocalShiftY, offsetY))
+		return FALSE;
+
+	for (size_t x = 0; x < count; x++)
+	{
+		const rdpMonitor* cur = &monitors[x];
+		if (cur == primary)
+			continue;
+
+		rdpMonitor m = monitors[x];
+		m.x -= offsetX;
+		m.y -= offsetY;
+		sorted[sortpos++] = m;
+	}
+
+	// Sort remaining monitors by x/y ?
+	qsort(sorted, count, sizeof(rdpMonitor), sort_monitor_fn);
+
+	return freerdp_settings_set_uint32(settings, FreeRDP_MonitorCount,
+	                                   WINPR_ASSERTING_INT_CAST(uint32_t, count));
+}

libfreerdp/common/settings_getters.c

@@ -1812,12 +1812,6 @@ UINT32 freerdp_settings_get_uint32(const rdpSettings* settings, FreeRDP_Settings
 		case FreeRDP_MonitorFlags:
 			return settings->MonitorFlags;
 
-		case FreeRDP_MonitorLocalShiftX:
-			return settings->MonitorLocalShiftX;
-
-		case FreeRDP_MonitorLocalShiftY:
-			return settings->MonitorLocalShiftY;
-
 		case FreeRDP_MultifragMaxRequestSize:
 			return settings->MultifragMaxRequestSize;
 
@@ -2277,14 +2271,6 @@ BOOL freerdp_settings_set_uint32(rdpSettings* settings, FreeRDP_Settings_Keys_UI
 			settings->MonitorFlags = cnv.c;
 			break;
 
-		case FreeRDP_MonitorLocalShiftX:
-			settings->MonitorLocalShiftX = cnv.c;
-			break;
-
-		case FreeRDP_MonitorLocalShiftY:
-			settings->MonitorLocalShiftY = cnv.c;
-			break;
-
 		case FreeRDP_MultifragMaxRequestSize:
 			settings->MultifragMaxRequestSize = cnv.c;
 			break;
@@ -2544,6 +2530,12 @@ INT32 freerdp_settings_get_int32(const rdpSettings* settings, FreeRDP_Settings_K
 
 	switch (id)
 	{
+		case FreeRDP_MonitorLocalShiftX:
+			return settings->MonitorLocalShiftX;
+
+		case FreeRDP_MonitorLocalShiftY:
+			return settings->MonitorLocalShiftY;
+
 		case FreeRDP_XPan:
 			return settings->XPan;
 
@@ -2574,6 +2566,14 @@ BOOL freerdp_settings_set_int32(rdpSettings* settings, FreeRDP_Settings_Keys_Int
 
 	switch (id)
 	{
+		case FreeRDP_MonitorLocalShiftX:
+			settings->MonitorLocalShiftX = cnv.c;
+			break;
+
+		case FreeRDP_MonitorLocalShiftY:
+			settings->MonitorLocalShiftY = cnv.c;
+			break;
+
 		case FreeRDP_XPan:
 			settings->XPan = cnv.c;
 			break;

libfreerdp/common/settings_str.h

@@ -362,8 +362,6 @@ static const struct settings_str_entry settings_map[] = {
 	{ FreeRDP_MonitorCount, FREERDP_SETTINGS_TYPE_UINT32, "FreeRDP_MonitorCount" },
 	{ FreeRDP_MonitorDefArraySize, FREERDP_SETTINGS_TYPE_UINT32, "FreeRDP_MonitorDefArraySize" },
 	{ FreeRDP_MonitorFlags, FREERDP_SETTINGS_TYPE_UINT32, "FreeRDP_MonitorFlags" },
-	{ FreeRDP_MonitorLocalShiftX, FREERDP_SETTINGS_TYPE_UINT32, "FreeRDP_MonitorLocalShiftX" },
-	{ FreeRDP_MonitorLocalShiftY, FREERDP_SETTINGS_TYPE_UINT32, "FreeRDP_MonitorLocalShiftY" },
 	{ FreeRDP_MultifragMaxRequestSize, FREERDP_SETTINGS_TYPE_UINT32,
 	  "FreeRDP_MultifragMaxRequestSize" },
 	{ FreeRDP_MultitransportFlags, FREERDP_SETTINGS_TYPE_UINT32, "FreeRDP_MultitransportFlags" },
@@ -448,6 +446,8 @@ static const struct settings_str_entry settings_map[] = {
 	{ FreeRDP_TlsSecLevel, FREERDP_SETTINGS_TYPE_UINT32, "FreeRDP_TlsSecLevel" },
 	{ FreeRDP_VCChunkSize, FREERDP_SETTINGS_TYPE_UINT32, "FreeRDP_VCChunkSize" },
 	{ FreeRDP_VCFlags, FREERDP_SETTINGS_TYPE_UINT32, "FreeRDP_VCFlags" },
+	{ FreeRDP_MonitorLocalShiftX, FREERDP_SETTINGS_TYPE_INT32, "FreeRDP_MonitorLocalShiftX" },
+	{ FreeRDP_MonitorLocalShiftY, FREERDP_SETTINGS_TYPE_INT32, "FreeRDP_MonitorLocalShiftY" },
 	{ FreeRDP_XPan, FREERDP_SETTINGS_TYPE_INT32, "FreeRDP_XPan" },
 	{ FreeRDP_YPan, FREERDP_SETTINGS_TYPE_INT32, "FreeRDP_YPan" },
 	{ FreeRDP_ParentWindowId, FREERDP_SETTINGS_TYPE_UINT64, "FreeRDP_ParentWindowId" },

libfreerdp/core/settings.c

@@ -606,6 +606,7 @@ static BOOL freerdp_settings_client_monitors_check_primary_and_origin(const rdpS
 	const UINT32 count = freerdp_settings_get_uint32(settings, FreeRDP_MonitorCount);
 	BOOL havePrimary = FALSE;
 	BOOL foundOrigin = FALSE;
+	BOOL primaryIsOrigin = FALSE;
 	BOOL rc = TRUE;
 
 	struct bounds_t bounds = { 0 };
@@ -644,6 +645,7 @@ static BOOL freerdp_settings_client_monitors_check_primary_and_origin(const rdpS
 				rc = FALSE;
 			}
 			foundOrigin = TRUE;
+			primaryIsOrigin = monitor->is_primary != 0;
 		}
 	}
 
@@ -672,6 +674,11 @@ static BOOL freerdp_settings_client_monitors_check_primary_and_origin(const rdpS
 		WLog_ERR(TAG, "Monitor configuration must start at 0/0 for first monitor!");
 		rc = FALSE;
 	}
+	if (!primaryIsOrigin)
+	{
+		WLog_ERR(TAG, "Monitor configuration must start at 0/0 for primary monitor!");
+		rc = FALSE;
+	}
 
 	return rc;
 }
@@ -970,10 +977,10 @@ rdpSettings* freerdp_settings_new(DWORD flags)
 	if (!freerdp_settings_set_pointer_len(settings, FreeRDP_MonitorDefArray, NULL, 32))
 		goto out_fail;
 
-	if (!freerdp_settings_set_uint32(settings, FreeRDP_MonitorLocalShiftX, 0))
+	if (!freerdp_settings_set_int32(settings, FreeRDP_MonitorLocalShiftX, 0))
 		goto out_fail;
 
-	if (!freerdp_settings_set_uint32(settings, FreeRDP_MonitorLocalShiftY, 0))
+	if (!freerdp_settings_set_int32(settings, FreeRDP_MonitorLocalShiftY, 0))
 		goto out_fail;
 
 	if (!freerdp_settings_set_pointer_len(settings, FreeRDP_MonitorIds, NULL, 0))
@@ -1737,35 +1744,20 @@ BOOL freerdp_settings_enforce_monitor_exists(rdpSettings* settings)
 		if (!freerdp_settings_set_uint32(settings, FreeRDP_MonitorCount, 1))
 			return FALSE;
 
-		rdpMonitor* monitor =
-		    freerdp_settings_get_pointer_array_writable(settings, FreeRDP_MonitorDefArray, 0);
-		if (!monitor)
-			return FALSE;
-		monitor->x = 0;
-		monitor->y = 0;
-		WINPR_ASSERT(width <= INT32_MAX);
-		monitor->width = (INT32)width;
-		WINPR_ASSERT(height <= INT32_MAX);
-		monitor->height = (INT32)height;
-		monitor->is_primary = TRUE;
-		monitor->orig_screen = 0;
-		monitor->attributes.physicalWidth = pwidth;
-		monitor->attributes.physicalHeight = pheight;
-		monitor->attributes.orientation = orientation;
-		monitor->attributes.desktopScaleFactor = desktopScaleFactor;
-		monitor->attributes.deviceScaleFactor = deviceScaleFactor;
-	}
-	else if (fullscreen || (multimon && (count == 1)))
-	{
-		/* not all platforms start primary monitor at 0/0, so enforce this to avoid issues with
-		 * fullscreen mode */
-		rdpMonitor* monitor =
-		    freerdp_settings_get_pointer_array_writable(settings, FreeRDP_MonitorDefArray, 0);
-		if (!monitor)
+		rdpMonitor monitor = { 0 };
+		monitor.x = 0;
+		monitor.y = 0;
+		monitor.width = WINPR_ASSERTING_INT_CAST(int32_t, width);
+		monitor.height = WINPR_ASSERTING_INT_CAST(int32_t, height);
+		monitor.is_primary = TRUE;
+		monitor.orig_screen = 0;
+		monitor.attributes.physicalWidth = pwidth;
+		monitor.attributes.physicalHeight = pheight;
+		monitor.attributes.orientation = orientation;
+		monitor.attributes.desktopScaleFactor = desktopScaleFactor;
+		monitor.attributes.deviceScaleFactor = deviceScaleFactor;
+		if (!freerdp_settings_set_monitor_def_array_sorted(settings, &monitor, 1))
 			return FALSE;
-		monitor->x = 0;
-		monitor->y = 0;
-		monitor->is_primary = TRUE;
 	}
 
 	return TRUE;

libfreerdp/core/test/settings_property_lists.h

@@ -281,8 +281,6 @@ static const size_t uint32_list_indices[] = {
 	FreeRDP_MonitorCount,
 	FreeRDP_MonitorDefArraySize,
 	FreeRDP_MonitorFlags,
-	FreeRDP_MonitorLocalShiftX,
-	FreeRDP_MonitorLocalShiftY,
 	FreeRDP_MultifragMaxRequestSize,
 	FreeRDP_MultitransportFlags,
 	FreeRDP_NSCodecColorLossLevel,
@@ -348,6 +346,8 @@ static const size_t uint32_list_indices[] = {
 
 #define have_int32_list_indices
 static const size_t int32_list_indices[] = {
+	FreeRDP_MonitorLocalShiftX,
+	FreeRDP_MonitorLocalShiftY,
 	FreeRDP_XPan,
 	FreeRDP_YPan,
 };