-
Notifications
You must be signed in to change notification settings - Fork 566
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.
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
x11: Add support for get_monitors() #1804
Conversation
Like everything X11, getting the list of monitors is complicated. X11 has a concept of "screen" in the core protocol. This is not really used these days, because it is not possible to move windows between screens, but that is the common fallback. Then came the Xinerama extension. Let's ignore that here... Next up is the RandR extension. It allows to query information about the actually connected hardware and its configuration via a GetScreenResources request. Since asking the hardware about its state is sometimes slow, GetScreenResourcesCurrent was later added, which does not ask the hardware, but only provides the newest information that is available to the X11 server. Next came high resolution displays with resolution so high that they needed to be connected with two cables. These display appear as two CRTCs, which is a problem. We don't want applications to think your display is actually two displays. However, RandR provided way too much information about CRTCs and stuff and it was not easy to just hide this all. Thus, RandR monitors were added and a monitor can consist of more than one CRTC and everything is fine. Thanks to the above, there are lots of special cases here. I only tested the RandR monitor case in this commit. Signed-off-by: Uli Schlachter <[email protected]>
Signed-off-by: Uli Schlachter <[email protected]>
Signed-off-by: Uli Schlachter <[email protected]>
Signed-off-by: Uli Schlachter <[email protected]>
Re-use existing App instance, as suggested by @maan2003 Co-authored-by: Manmeet Maan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I'm going to admit that I only tested it with RandR 1.6. Is there a convenient way to test older versions?
Not really. I never tested it at all. I guess one could hack the code so that it "detects" an older version... If you want, I can remove everything but latest RandR. |
Oh good point, I can try using that to test the other code paths. Maybe @cmyr has an opinion about how much maintenance burden we're willing to take on to support older versions? According to wikipedia, RandR 1.5 was released in 2015, and 1.3 was released in 2009. |
Signed-off-by: Uli Schlachter <[email protected]>
Well, the different code paths do have different results. patchdiff --git a/druid-shell/Cargo.toml b/druid-shell/Cargo.toml
index 5815f9d1..c0db8cba 100755
--- a/druid-shell/Cargo.toml
+++ b/druid-shell/Cargo.toml
@@ -36,6 +36,9 @@ tga = ["piet-common/tga"]
farbfeld = ["piet-common/farbfeld"]
dxt = ["piet-common/dxt"]
hdr = ["piet-common/hdr"]
+randr_15 = []
+randr_13 = []
+randr_12 = []
[dependencies]
# NOTE: When changing the piet or kurbo versions, ensure that
diff --git a/druid-shell/src/platform/x11/screen.rs b/druid-shell/src/platform/x11/screen.rs
index 3607cf22..195ba5e5 100644
--- a/druid-shell/src/platform/x11/screen.rs
+++ b/druid-shell/src/platform/x11/screen.rs
@@ -73,15 +73,20 @@ fn get_monitors_impl(
// Monitor support was added in RandR 1.5
let version = conn.randr_query_version(1, 5)?.reply()?;
match (version.major_version, version.minor_version) {
+ #[cfg(feature="randr_15")]
(major, _) if major >= 2 => get_monitors_randr_monitors(conn, screen),
+ #[cfg(feature="randr_15")]
(1, minor) if minor >= 5 => get_monitors_randr_monitors(conn, screen),
+ #[cfg(feature="randr_13")]
(1, minor) if minor >= 3 => get_monitors_randr_screen_resources_current(conn, screen),
+ #[cfg(feature="randr_12")]
(1, minor) if minor >= 2 => get_monitors_randr_screen_resources(conn, screen),
_ => get_monitors_core(screen),
}
}
fn get_monitors_core(screen: &Screen) -> Result<Vec<Monitor>, ReplyOrIdError> {
+ println!("get_monitors_core");
Ok(vec![monitor(
true,
(0, 0),
@@ -93,6 +98,7 @@ fn get_monitors_randr_monitors(
conn: &impl Connection,
screen: &Screen,
) -> Result<Vec<Monitor>, ReplyOrIdError> {
+ println!("get_monitors_randr_monitors");
let result = conn
.randr_get_monitors(screen.root, true)?
.reply()?
@@ -107,6 +113,7 @@ fn get_monitors_randr_screen_resources_current(
conn: &impl Connection,
screen: &Screen,
) -> Result<Vec<Monitor>, ReplyOrIdError> {
+ println!("get_monitors_randr_screen_resources_current");
let reply = conn
.randr_get_screen_resources_current(screen.root)?
.reply()?;
@@ -117,6 +124,7 @@ fn get_monitors_randr_screen_resources(
conn: &impl Connection,
screen: &Screen,
) -> Result<Vec<Monitor>, ReplyOrIdError> {
+ println!("get_monitors_randr_screen_resources");
let reply = conn.randr_get_screen_resources(screen.root)?.reply()?;
get_monitors_randr_crtcs_timestamp(conn, &reply.crtcs, reply.config_timestamp)
} Output (I removed compiler warnings about unused functions that are a side-effect of my patch):
The truth:
Looks pretty much correct except for those "empty" disconnected outputs. New commit pushed which just ignores CRTCs with width or height equal to zero. New output is:
|
Like everything X11, getting the list of monitors is complicated.
X11 has a concept of "screen" in the core protocol. This is not really
used these days, because it is not possible to move windows between
screens, but that is the common fallback.
Then came the Xinerama extension. Let's ignore that here...
Next up is the RandR extension. It allows to query information about the
actually connected hardware and its configuration via a
GetScreenResources request. Since asking the hardware about its state is
sometimes slow, GetScreenResourcesCurrent was later added, which does
not ask the hardware, but only provides the newest information that is
available to the X11 server.
Next came high resolution displays with resolution so high that they
needed to be connected with two cables. These display appear as two
CRTCs, which is a problem. We don't want applications to think your
display is actually two displays. However, RandR provided way too much
information about CRTCs and stuff and it was not easy to just hide this
all. Thus, RandR monitors were added and a monitor can consist of more
than one CRTC and everything is fine.
Thanks to the above, there are lots of special cases here. I only tested
the RandR monitor case in this commit.
Signed-off-by: Uli Schlachter [email protected]
Edit: All the testing I did: