Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unify monitor processing logic. #1895

Merged
merged 2 commits into from
Mar 29, 2022
Merged

Unify monitor processing logic. #1895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a0db63
Unify monitor processing logic.
Nexarian May 25, 2021
bd9147d
Updating to refactor xrdp_client_info
Nexarian Mar 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions common/ms-rdpbcgr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,24 @@
#define RDPSND_SVC_CHANNEL_NAME "rdpsnd"
#define RDPDR_SVC_CHANNEL_NAME "rdpdr"

/* 2.2.1.3.6 Client Monitor Data - */
/* 2.2.1.3.6 Client Monitor Data */
/* monitorCount (4 bytes): A 32-bit, unsigned integer. The number of display */
/* monitor definitions in the monitorDefArray field (the maximum allowed is 16). */
#define CLIENT_MONITOR_DATA_MAXIMUM_MONITORS 16
#define CLIENT_MONITOR_DATA_MAXIMUM_MONITORS 16

/* 2.2.1.3.6 Client Monitor Data */
/* The maximum width of the virtual desktop resulting from the union of the monitors */
/* contained in the monitorDefArray field MUST NOT exceed 32,766 pixels. Similarly, */
/* the maximum height of the virtual desktop resulting from the union of the monitors */
/* contained in the monitorDefArray field MUST NOT exceed 32,766 pixels. */
/* The minimum permitted size of the virtual desktop is 200 x 200 pixels. */
#define CLIENT_MONITOR_DATA_MINIMUM_VIRTUAL_DESKTOP_WIDTH 0xC8
#define CLIENT_MONITOR_DATA_MINIMUM_VIRTUAL_DESKTOP_HEIGHT 0xC8
#define CLIENT_MONITOR_DATA_MAXIMUM_VIRTUAL_DESKTOP_WIDTH 0x7FFE
#define CLIENT_MONITOR_DATA_MAXIMUM_VIRTUAL_DESKTOP_HEIGHT 0x7FFE

/* 2.2.1.3.6.1 Monitor Definition (TS_MONITOR_DEF) */
#define TS_MONITOR_PRIMARY 0x00000001

/* Options field */
/* NOTE: XR_ prefixed to avoid conflict with FreeRDP */
Expand Down
16 changes: 14 additions & 2 deletions common/ms-rdpedisp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,19 @@
#define MS_RDPEDISP_H

/* Display Control Messages: Display Virtual Channel Extension (2.2.2) */
#define DISPLAYCONTROL_PDU_TYPE_MONITOR_LAYOUT 0x00000002
#define DISPLAYCONTROL_PDU_TYPE_CAPS 0x00000005
#define DISPLAYCONTROL_PDU_TYPE_MONITOR_LAYOUT 0x00000002
#define DISPLAYCONTROL_PDU_TYPE_CAPS 0x00000005

/* Display Control Monitor Layout (2.2.2.2.1) */
#define DISPLAYCONTROL_MONITOR_PRIMARY 0x00000001
#define CLIENT_MONITOR_DATA_MINIMUM_VIRTUAL_MONITOR_WIDTH 0xC8
#define CLIENT_MONITOR_DATA_MINIMUM_VIRTUAL_MONITOR_HEIGHT 0xC8
#define CLIENT_MONITOR_DATA_MAXIMUM_VIRTUAL_MONITOR_WIDTH 0x2000
#define CLIENT_MONITOR_DATA_MAXIMUM_VIRTUAL_MONITOR_HEIGHT 0x2000

#define ORIENTATION_LANDSCAPE 0
#define ORIENTATION_PORTRAIT 90
#define ORIENTATION_LANDSCAPE_FLIPPED 180
#define ORIENTATION_PORTRAIT_FLIPPED 270

#endif /* MS_RDPEDISP_H */
35 changes: 28 additions & 7 deletions common/xrdp_client_info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,29 @@
#if !defined(XRDP_CLIENT_INFO_H)
#define XRDP_CLIENT_INFO_H

/*
* 2.2.1.3.6.1 Monitor Definition (TS_MONITOR_DEF)
* 2.2.1.3.9.1 Monitor Attributes (TS_MONITOR_ATTRIBUTES)
* 2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT
*/
struct monitor_info
{
/* From 2.2.1.3.6.1 Monitor Definition (TS_MONITOR_DEF) */
int left;
int top;
int right;
int bottom;
int is_primary;
int flags;

/* From 2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT */
unsigned int physical_width;
unsigned int physical_height;
unsigned int orientation;
unsigned int desktop_scale_factor;
unsigned int device_scale_factor;

/* Derived setting */
unsigned int is_primary;
};

/* xrdp keyboard overrids */
Expand All @@ -41,6 +57,15 @@ struct xrdp_keyboard_overrides
int layout;
};

struct display_size_description
{
unsigned int monitorCount; /* 2.2.2.2 DISPLAYCONTROL_MONITOR_LAYOUT_PDU: number of monitors detected (max = 16) */
struct monitor_info minfo[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data */
struct monitor_info minfo_wm[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data, non-negative values */
Comment on lines +63 to +64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify what the difference between what these two fields represent?
it seems like minfo is the client side representation of the monitor layout, and minfo_wm is the server side representation of the same monitor layout. Is that correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pardon me for jumping in, but since I'm here and I've used these values I can say that that's essentially correct The _wm values are normalised for a top-left co-ordinate of (0,0), which is useful for some bits of the software, namely the VNC multimon stuff and the login window.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. Does this need further documentation in the code?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I'm happy with the existing comment

unsigned int session_width;
unsigned int session_height;
};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some duplication here - the new struct display_size_description contains fields which are already present in this file further down.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know. I didn't want to change xrdp_client_info since that will incur a revision of the header file and also affect xorgxrdp. I want to make the change to unify these two, but not in this PR... I want to do it for a PR that's focused on unifying these two data structures and leave the ones that change xorgxrdp more focused and linked. Make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's fine- you can raise a PR now and mark it as draft if you like to keep the book-keeping simple.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to actually just do it now. It's not as terrible as I thought.

/**
* Information about the xrdp client
*
Expand All @@ -54,8 +79,6 @@ struct xrdp_client_info
int size; /* bytes for this structure */
int version; /* Should be CLIENT_INFO_CURRENT_VERSION */
int bpp;
int width;
int height;
/* bitmap cache info */
int cache1_entries;
int cache1_size;
Expand Down Expand Up @@ -128,9 +151,7 @@ struct xrdp_client_info

int security_layer; /* 0 = rdp, 1 = tls , 2 = hybrid */
int multimon; /* 0 = deny , 1 = allow */
int monitorCount; /* number of monitors detected (max = 16) */
struct monitor_info minfo[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data */
struct monitor_info minfo_wm[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data, non-negative values */
struct display_size_description display_sizes;

int keyboard_type;
int keyboard_subtype;
Expand Down Expand Up @@ -186,6 +207,6 @@ struct xrdp_client_info
};

/* yyyymmdd of last incompatible change to xrdp_client_info */
#define CLIENT_INFO_CURRENT_VERSION 20210723
#define CLIENT_INFO_CURRENT_VERSION 20220320

#endif
Loading