nixos/xserver: Changed xrandrHeads to support corresponding monitor s… #13916

CMCDragonkai · 2016-03-14T13:10:52Z

This is a breaking change (actually it is no longer breaking) to services.xserver.xrandrHeads, however it is a necessary change. This allows xrandrHeads to support per-monitor section configuation in Xorg.conf. There's currently no other way to set the primary monitor or per monitor section configuration when using the xrandrHeads option.

Requested by: #7463

What it fixes:

When you multiple monitors setup. By default, the first monitor detected by xrandr is set to the primary monitor. However this may not be the monitor you need to be set as primary. In fact this monitor set to primary may in fact be disconnected. Which is exactly what happened to me. This has affected 3 programs for me:

XMonad - gets confused with Super + {w,e,r}
SDDM - puts the login screen on the wrong monitor, and does not currently duplicate the login screen on all monitors
XMobar - puts the xmobar on the wrong monitor, as it only puts the taskbar on the primary monitor

This PR fixes all of the above 3 problems (Xmonad is no longer confused, SDDM puts the login screen on my primary monitor (laptop), Xmobar is working. It is simple to use. And simple to change. Previously you would do something like:

services.xserver.xrandrHeads = [ "DP-0" "HDMI-0" ];

Now you can do:

services.xserver.xrandrHeads = {
    DP-0 =  ''
        Option "Primary" "true"
    '';
    HDMI-0 = "";
};

The reason why I did this, is to allow other per-monitor configuration options when using xrandrHeads (such as per-monitor DPI in the future). Note that the services.xserver.monitorSection setting does not work in conjunction with xrandrHeads.

I have tested this in current 15.09-small and it works.

mention-bot · 2016-03-14T13:10:55Z

By analyzing the blame information on this pull request, we identified @aszlig, @edolstra and @nbp to be potential reviewers

aszlig · 2016-03-14T14:23:55Z

The problem here is that the order of keys is alphabetical, so we can't simply write something like xrandrHeads = [ screen3 screen1 screen2 ] and get it aligned from left to right.

aszlig · 2016-03-14T14:35:21Z

I'd make it of type listOf (either str (submodule { options.connector = ...; options.primary ...; options.extraConfig ...; })), so you still can just write

{ xrandrHeads = [ "HDMI-0" "DVI-0" ]; }

as before but can also mix it with something like

{ xrandrHeads = [
    "HDMI-0"
    { display = "DVI-0";
      primary = true;
      extraConfig = "stuff that goes into the monitor section";
    }
  ];
}

and if there is no primary option given the first connector in the list is assigned to be the primary head.

CMCDragonkai · 2016-03-14T14:55:43Z

Good idea. Could you write out the full paths to the new functions you
suggested. I'll have it changed to your style soon.
On 15/03/2016 1:35 AM, "aszlig" notifications@github.com wrote:

I'd make it of type listOf (either str (submodule { options.connector =
...; options.primary ...; options.extraConfig ...; })), so you still can
just write

{ xrandrHeads = [ "HDMI-0" "DVI-0" ]; }

as before but can also mix it with something like

{ xrandrHeads = [
"HDMI-0"
{ display = "DVI-0";
primary = true;
extraConfig = "stuff that goes into the monitor section";
}
];
}

and if there is no primary option given the first connector in the list is
assigned to be the primary head.

—
Reply to this email directly or view it on GitHub
#13916 (comment).

aszlig · 2016-03-14T14:57:18Z

@CMCDragonkai: You mean the type information?

CMCDragonkai · 2016-03-14T14:58:25Z

The either, str, and submodule.
On 15/03/2016 1:35 AM, "aszlig" notifications@github.com wrote:

I'd make it of type listOf (either str (submodule { options.connector =
...; options.primary ...; options.extraConfig ...; })), so you still can
just write

{ xrandrHeads = [ "HDMI-0" "DVI-0" ]; }

as before but can also mix it with something like

{ xrandrHeads = [
"HDMI-0"
{ display = "DVI-0";
primary = true;
extraConfig = "stuff that goes into the monitor section";
}
];
}

and if there is no primary option given the first connector in the list is
assigned to be the primary head.

—
Reply to this email directly or view it on GitHub
#13916 (comment).

CMCDragonkai · 2016-03-14T15:00:23Z

Oh they are not functions. Read it wrong. Ok looks easy.
On 15/03/2016 1:57 AM, "aszlig" notifications@github.com wrote:

@CMCDragonkai https://github.com/CMCDragonkai: You mean the type
information?

—
Reply to this email directly or view it on GitHub
#13916 (comment).

CMCDragonkai · 2016-03-15T07:32:05Z

@aszlig

It is done. The configuration is now fully backwards compatible. Along with an automatic setting of the primary option if none is set. Also primary option is unique, so the last setting of primary overrides first setting of primary.

Please review and merge.

Also note I wanted to have optional submodule options, by setting a default for the submodule types, but I tried it, and it didn't work. Even when submodule members have default, when using them to rebuild, nix still complains about missing parameters. Might be something that hasn't been implemented yet though. For now, it's not that bad.

Use it like:

xrandrHeads = [ { output = "DP-0"; primary = true; monitorConfig = ""; } "HDMI-0" ];

CMCDragonkai · 2016-03-15T07:36:00Z

Also changing from imap to fold actually starts indexes at 0 not 1.

CMCDragonkai · 2016-03-15T08:23:42Z

In the future, there should a bit more advanced xrandrHeads configuration that allows other kinds of monitor layouts (like 2x2). It might be best to remove the automatic RightOf option, and allow users to set it themselves. This would mean multihead* names cannot be used, as these would not be visible to the user. Instead some name based on the output name should be used like DP-0 or HDMI-0. This will however become a breaking change again. What do you think?

aszlig · 2016-03-15T17:45:22Z

nixos/modules/services/x11/xserver.nix

+          (submodule {
+            options.output = str;
+            options.primary = bool;
+            options.monitorConfig = lines;


These need to be valid options (-> mkOption) as well, like:

submodule { options.output = mkOption { type = types.str; description = '' The display connector used for this XRandR head. ''; }; }

Did you know it works without having mkOption?
On 16/03/2016 4:45 AM, "aszlig" notifications@github.com wrote:

In nixos/modules/services/x11/xserver.nix
#13916 (comment):

@@ -313,12 +335,27 @@ in

xrandrHeads = mkOption { default = [];

example = [ "HDMI-0" "DVI-0" ];

type = with types; listOf string;

example = [ "HDMI-0" { output = "DVI-0"; primary = true; monitorConfig = ""; } ];

type = with types; listOf (either

str

(submodule {

options.output = str;

options.primary = bool;

options.monitorConfig = lines;

These need to be valid options (-> mkOption) as well, like:

submodule {
options.output = mkOption {
type = types.str;
description = '' The display connector used for this XRandR head. '';
};
}

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/NixOS/nixpkgs/pull/13916/files/47535fdc8c0eca8fd4b26f2601c602655f41a741#r56208116

@aszlig is the only advantage is to have a description field? Does that go into the auto generated documentation? Also the default inside the submodules doesn't work!

Well, the problem here is that it should fail for example if you provide a wrong option, unfortunately the only check that is done here is isAttrs x || isFunction x, so we need to fix that in the module system. Another problem this also has, is that you do the processing of it outside of the submodule, while the submodule has better ways to actually write the configuration using an internal option. This also would have the advantage that not every attribute needs to be specified.

For example your implementation fails if there is no monitorConfig.

Yep, I identified that when leaving out monitorConfig it fails. That doesn't change just because I add mkOption with a default parameter. I did that before, and it still failed. So I was wondering what functional difference is adding mkOption. Perhaps it is a feature that hasn't been implemented yet?

aszlig · 2016-03-16T09:17:04Z

Hm, maybe we should then feed even the simple strings into the submodule, imply a rightOf and do the generation at the submodule level. That way it's also easier to follow while reading the manual and you can do things as { output = "foo"; above = "another output"; }, which then does the right thing and doesn't imply the defaults.

aszlig · 2016-03-16T09:20:02Z

So if you want to have a 2x2 layout:

{
  xrandrHeads = [
    "foo"
    "bar"
    { output = "foo_above"; above = "foo"; }
    { output = "bar_above"; above = "bar"; }
  ];
}

CMCDragonkai · 2016-03-16T13:31:14Z

Do we really want to put all xrandr options into the Nix system, or just have above, right, left, and below?

By the way, there doesn't seem to be a way to making monitorConfig optional. Even with mkOption and default setting.

CMCDragonkai · 2016-03-18T05:32:07Z

Upon further reflecting. I think it's a better idea to not having reified configuration parameters except primary. There's too many possible configurations of monitors, and reifying them into Nix is probably a waste of time. Can we just have primary and monitorConfig? It's not that much of a change to go from automatic left to right, to just writing RightOf, RightOf... etc. And it allows people to have more sophisticated configurations.

I did have an idea of using a 2D matrix (list of list) to have a data structure representing monitor layout, but I realised even that's not completely foolproof due to the fact that not every monitor has the same resolution.

Also if we did reify the parameters, that would result in even greater configuration overhead, since the submodule default parameters have no effect in allowing omission.

aszlig · 2016-03-19T14:04:42Z

No, the goal was not to do include every possible configuration option, but just above, below, leftOf and rightOf.

The reason that the submodule type doesn't work with either is that it doesn't currently support submodules at all, so we need to fix either first (I'm going to give it a shot now).

@edolstra

So far the "either" type only handled "flat" types, so you couldn't do something like: type = either int (submodule { options = ...; }); Not only caused this the submodule's options not being checked but also not show up in the documentation. This was something we stumbled on with #13916. Signed-off-by: aszlig <aszlig@redmoonstudios.org> Cc: @edolstra

aszlig · 2016-03-19T16:29:52Z

@CMCDragonkai: As of 0f0805b, submodules should now be handled properly for type either.

nbp · 2016-03-19T17:15:03Z

As commented on commit 0f0805b , I do not understand why this patch got pushed to master, without requesting a review. Also, as commented on commit 0f0805b, I think this change is incomplete in its current state and thus dangerous.

nbp · 2016-03-19T17:16:03Z

@aszlig I would appreciate if you can revert commit 0f0805b, and submit it in a pull request, which would be blocking this one.

@edolstra

So far the "either" type only handled "flat" types, so you couldn't do something like: type = either int (submodule { options = ...; }); Not only caused this the submodule's options not being checked but also not show up in the documentation. This was something we stumbled on with NixOS#13916. Signed-off-by: aszlig <aszlig@redmoonstudios.org> Cc: @edolstra

aszlig · 2016-03-19T18:26:40Z

@nbp: Alright, pull request is in #14053.

CMCDragonkai · 2016-03-20T11:53:52Z

While that other PR is brewing.

Consider this idea:

xrandrheads = [
    [ "DP-1"   "DP-0"  null                                    ]
    [ "HDMI-1" null    "DVI-0                                  ]
    [ null     "VGA-0" { output = "VGA-1"; primary = "true"; } ]
];

I can write code that deal with the 2D sparse matrix here, and arrange the monitor heads in that way. The only thing is that it would assume monitors are of the same resolution. But that would not be any different if we're only offering leftOf, rightOf, above, and below, because there's no pixel offset option. That being said, to allow more advanced configuration, one can just provide monitorConfig.

Furthermore, given that we are offering those specific options. Don't we also need to say that by default, there's no default rightOf configuration? I would not really understand how to default the monitors now if someone just give [ "DP-1" "DP-2" ]. Or if one of them had rightOf but the other didn't.

…ection configuration in Xorg

CMCDragonkai · 2016-05-10T11:27:05Z

Hey so I need to close this PR, because I accidentally submitted this PR from the master branch of my fork. I'll submit a new PR with the updated code with a proper feature branch.

CMCDragonkai · 2016-05-10T11:30:55Z

#15353

CMCDragonkai mentioned this pull request Mar 14, 2016

xrandrHeads could use an option to designate primary monitor #7463

Closed

nbp added 0.kind: enhancement 9.needs: changelog 8.has: module (update) labels Mar 14, 2016

CMCDragonkai force-pushed the master branch from 00e2478 to 47535fd Compare March 15, 2016 07:30

aszlig reviewed Mar 15, 2016
View reviewed changes

aszlig mentioned this pull request Mar 19, 2016

Handle submodules for type "either" #14053

Closed

CMCDragonkai and others added 2 commits May 10, 2016 21:05

nixos/xserver: Changed xrandrHeads to support corresponding monitor s…

a661160

…ection configuration in Xorg

garcosim: Added tracefilegen and tracefilesim

7b79579

CMCDragonkai force-pushed the master branch from e72ec37 to 7b79579 Compare May 10, 2016 11:22

CMCDragonkai closed this May 10, 2016

CMCDragonkai mentioned this pull request May 10, 2016

nixos/xserver: Changed xrandrHeads to support corresponding monitor section configuration in Xorg #15353

Merged

domenkozar removed the 9.needs: changelog label Sep 30, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/xserver: Changed xrandrHeads to support corresponding monitor s… #13916

nixos/xserver: Changed xrandrHeads to support corresponding monitor s… #13916

CMCDragonkai commented Mar 14, 2016

mention-bot commented Mar 14, 2016

aszlig commented Mar 14, 2016

aszlig commented Mar 14, 2016

CMCDragonkai commented Mar 14, 2016

aszlig commented Mar 14, 2016

CMCDragonkai commented Mar 14, 2016

CMCDragonkai commented Mar 14, 2016

CMCDragonkai commented Mar 15, 2016

CMCDragonkai commented Mar 15, 2016

CMCDragonkai commented Mar 15, 2016

aszlig Mar 15, 2016

CMCDragonkai Mar 16, 2016

CMCDragonkai Mar 16, 2016

aszlig Mar 16, 2016

CMCDragonkai Mar 16, 2016

aszlig commented Mar 16, 2016

aszlig commented Mar 16, 2016

CMCDragonkai commented Mar 16, 2016

CMCDragonkai commented Mar 18, 2016

aszlig commented Mar 19, 2016

aszlig commented Mar 19, 2016

nbp commented Mar 19, 2016

nbp commented Mar 19, 2016

aszlig commented Mar 19, 2016

CMCDragonkai commented Mar 20, 2016

CMCDragonkai commented May 10, 2016 •

edited

CMCDragonkai commented May 10, 2016

nixos/xserver: Changed xrandrHeads to support corresponding monitor s… #13916

nixos/xserver: Changed xrandrHeads to support corresponding monitor s… #13916

Conversation

CMCDragonkai commented Mar 14, 2016

mention-bot commented Mar 14, 2016

aszlig commented Mar 14, 2016

aszlig commented Mar 14, 2016

CMCDragonkai commented Mar 14, 2016

aszlig commented Mar 14, 2016

CMCDragonkai commented Mar 14, 2016

CMCDragonkai commented Mar 14, 2016

CMCDragonkai commented Mar 15, 2016

CMCDragonkai commented Mar 15, 2016

CMCDragonkai commented Mar 15, 2016

aszlig Mar 15, 2016

Choose a reason for hiding this comment

CMCDragonkai Mar 16, 2016

Choose a reason for hiding this comment

CMCDragonkai Mar 16, 2016

Choose a reason for hiding this comment

aszlig Mar 16, 2016

Choose a reason for hiding this comment

CMCDragonkai Mar 16, 2016

Choose a reason for hiding this comment

aszlig commented Mar 16, 2016

aszlig commented Mar 16, 2016

CMCDragonkai commented Mar 16, 2016

CMCDragonkai commented Mar 18, 2016

aszlig commented Mar 19, 2016

aszlig commented Mar 19, 2016

nbp commented Mar 19, 2016

nbp commented Mar 19, 2016

aszlig commented Mar 19, 2016

CMCDragonkai commented Mar 20, 2016

CMCDragonkai commented May 10, 2016 • edited

CMCDragonkai commented May 10, 2016

CMCDragonkai commented May 10, 2016 •

edited