Add output config #1503

emersion · 2017-12-06T11:33:10Z

Merge new output config params
Init new output config params
wlr_output_layout_add

Test plan:

Configure each property (resolution, refresh rate, position, scale, transform)
Test hotplugging
Test IPC

acrisci · 2017-12-06T17:56:38Z

sway/config/output.c

+		}
+	}
+	if (!best) {
+		sway_log(L_ERROR, "Configured mode for %s not available", output->name);


If no mode is ever set, the drm backend won't ever start up and (I think) the user loses the ability to switch VT so this is a pretty hard crash for a single monitor setup.

The obvious solution is to just set the default mode, but that's a silent error if you don't know to check the logs.

This is an excellent use case for system notifications but we don't have those yet.

This is already done here: https://github.com/swaywm/sway/blob/wlroots/sway/desktop/output.c#L111

acrisci · 2017-12-06T17:57:11Z

sway/config/output.c

+
+void merge_output_config(struct output_config *dst, struct output_config *src) {
+	if (src->name) {
+		if (dst->name) {


No need for this guard, just free.

acrisci · 2017-12-06T18:20:25Z

sway/tree/container.c

@@ -23,6 +25,30 @@ void swayc_descendants_of_type(swayc_t *root, enum swayc_types type,
 	}
 }

+static void update_root_geometry() {


You can get output layout extents with wlr_output_layout_get_box(layout, NULL).

acrisci · 2017-12-06T18:35:41Z

sway/commands/output.c

+	if (output->name) {
+		// Try to find the output container and apply configuration now. If
+		// this is during startup then there will be no container and config
+		// will be applied during normal "new output" event from wlc.


event from what?

Don't just copy+paste more than a few lines at a time, @emersion - make sure you read everything you import.

acrisci · 2017-12-06T18:40:35Z

sway/commands/output.c

+	}
+	const char *name = argv[0];
+
+	struct output_config *output = calloc(1, sizeof(struct output_config));


Would be simpler to just have a create func here rather than exposing output_config_defaults().

acrisci · 2017-12-06T18:42:41Z

sway/tree/container.c

+	if (output->children->length > 0) {
+		// TODO save workspaces when there are no outputs.
+		// TODO also check if there will ever be no outputs except for exiting
+		// program


I think if there are no outputs, sway should exit.

acrisci · 2017-12-06T18:44:38Z

sway/tree/container.c

@@ -139,6 +167,34 @@ static void free_swayc(swayc_t *cont) {
 	free(cont);
 }

+swayc_t *destroy_output(swayc_t *output) {


Do we need to explicitly take it out of the output layout?

acrisci · 2017-12-06T18:45:38Z

Code looks ok, will test it in a few hours. Can you provide an example config snippet?

emersion · 2017-12-09T14:53:57Z

Example config:

output WL-1 pos 10,10 transform 90 scale 2

Try changing these at runtime.

acrisci · 2017-12-11T09:19:14Z

sway/config/output.c

@@ -14,13 +14,18 @@ int output_name_cmp(const void *item, const void *data) {
 	return strcmp(output->name, name);
 }

-void output_config_defaults(struct output_config *oc) {
+struct output_config *new_output_config() {


We'll need to standardize our create function names.

acrisci · 2017-12-11T09:27:07Z

include/sway/config.h

@@ -6,6 +6,7 @@
 #include <libinput.h>
 #include <stdint.h>
 #include <string.h>
+#include <wayland-server.h>


acrisci · 2017-12-11T09:33:56Z

sway/commands/output.c

+
+	struct output_config *output = new_output_config();
+	if (!output) {
+		return cmd_results_new(CMD_FAILURE, "output", "Unable to allocate output config");


If we're out of memory, this is probably going to fail too.

acrisci · 2017-12-11T09:46:33Z

Instead of this update_root_geometry() madness, why not listen for the change event of the output layout and update the root geometry then?

acrisci · 2017-12-11T09:49:07Z

sway/config/output.c

+	output->x = output_layout_box->x;
+	output->y = output_layout_box->y;
+	output->width = output_layout_box->width;
+	output->height = output_layout_box->height;


The output container could listen to the output layout change event and automatically keep these in sync.

acrisci · 2017-12-11T09:53:52Z

sway/config/output.c

+		}
+	}
+
+	int output_i;


Not used yet, but will be when we'll implement background (see commented code below).

acrisci · 2017-12-11T10:01:16Z

output_add_notify() needs to handle the case where the new output is disabled.

emersion · 2017-12-12T20:15:09Z

output_add_notify() needs to handle the case where the new output is disabled.

This is already the case, but this is handled in apply_output_config.

acrisci · 2017-12-13T12:58:25Z

sway/desktop/output.c

+	arrange_windows(soutput->swayc, -1, -1);
+}
+
+static void output_transform_notify(struct wl_listener *listener, void *data) {


Have you considered not listening to scale/transform/resolution but only output layout changes for determining when to rearrange the windows?

ddevault · 2017-12-13T23:16:36Z

Initial feedback

Xwayland crashes on changes
I would prefer to replace refresh_rate/resolution with a single mode command: WIDTHxHEIGHT@RATE, and have res just be an alias for mode

emersion · 2017-12-14T00:20:21Z

Xwayland crashes on changes

I don't believe this is due to invalid code. I added some code in rootston that changes the output scale on click and Xwayland doesn't crash. I think it's related to having no seat. Plus, the segfault happens in CloseInput.

ddevault · 2017-12-14T00:42:34Z

sway/commands/output.c

+	}
+	output->name = strdup(name);
+
+	// TODO: atoi doesn't handle invalid numbers


Can we resolve this now?

ddevault · 2017-12-14T00:47:00Z

sway/desktop/output.c


-	if (wl_list_length(&wlr_output->modes) > 0) {
-		struct wlr_output_mode *mode = NULL;
-		mode = wl_container_of((&wlr_output->modes)->prev, mode, link);


ddevault · 2017-12-14T00:50:08Z

sway/tree/container.c

+			}
+			sort_workspaces(root_container.children->items[p]);
+			// TODO WLR: is this needed anymore?
+			//update_visibility(root_container.children->items[p]);


It's not, pull it out

ddevault · 2017-12-14T00:50:51Z

sway/tree/container.c

+	}
+
+	if (output->children->length > 0) {
+		// TODO save workspaces when there are no outputs.


Note to self, put these in the wlroots ticket

ddevault · 2017-12-14T00:52:27Z

LGTM 👍

ddevault · 2017-12-14T01:36:09Z

Thanks!

emersion changed the title ~~Add output config~~ [WIP] Add output config Dec 6, 2017

Add output config

aaae590

emersion force-pushed the output-config branch from 5b7531c to aaae590 Compare December 6, 2017 11:36

Init, merge output config params, use wlr_output_layout

68ae989

emersion changed the title ~~[WIP] Add output config~~ Add output config Dec 6, 2017

ddevault requested review from acrisci and ddevault December 6, 2017 17:06

acrisci reviewed Dec 6, 2017

View reviewed changes

emersion added 2 commits December 6, 2017 19:11

Merge branch 'wlroots' into output-config

c25ad5e

Simplify free calls, use wlr_output_set_scale

7c5d8c5

acrisci reviewed Dec 6, 2017

View reviewed changes

Use wlr_output_layout_get_box

41dd291

acrisci reviewed Dec 6, 2017

View reviewed changes

emersion added 2 commits December 6, 2017 19:45

Add new_output_config, update root container size on output hotplug

8764dc2

Remove output from layout

4a14aa9

emersion force-pushed the output-config branch from 616d49e to 4a14aa9 Compare December 9, 2017 14:49

acrisci reviewed Dec 11, 2017

View reviewed changes

Use custom modes when output has no mode

475a013

emersion added 3 commits December 12, 2017 19:40

Add scale and transform events to sway_output

f3d880b

Listen to output layout change

c7abb77

Update output container box in event handler

d293c42

acrisci reviewed Dec 13, 2017

View reviewed changes

emersion added 2 commits December 13, 2017 15:52

Update output containers on output layout change

a4619e9

Handle output remove

ec2fd6e

emersion added 2 commits December 14, 2017 00:45

Replace refresh_rate and position by mode in output command

4d389f8

Fail if unknown output subcommand

9254c5a

emersion force-pushed the output-config branch from 05cdce0 to 9254c5a Compare December 13, 2017 23:50

ddevault reviewed Dec 14, 2017

View reviewed changes

Use strtol instead of atoi in output command

cba592b

ddevault merged commit 1aab9ae into swaywm:wlroots Dec 14, 2017

emersion deleted the output-config branch July 23, 2018 15:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add output config #1503

Add output config #1503

emersion commented Dec 6, 2017 •

edited by ddevault

Loading

acrisci Dec 6, 2017

emersion Dec 6, 2017

acrisci Dec 6, 2017

acrisci Dec 6, 2017

acrisci Dec 6, 2017

ddevault Dec 6, 2017

acrisci Dec 6, 2017

acrisci Dec 6, 2017

acrisci Dec 6, 2017

acrisci commented Dec 6, 2017

emersion commented Dec 9, 2017

acrisci Dec 11, 2017

acrisci Dec 11, 2017

acrisci Dec 11, 2017

acrisci commented Dec 11, 2017

acrisci Dec 11, 2017

acrisci Dec 11, 2017

emersion Dec 12, 2017 •

edited

Loading

acrisci commented Dec 11, 2017

emersion commented Dec 12, 2017

acrisci Dec 13, 2017

ddevault commented Dec 13, 2017

emersion commented Dec 14, 2017

ddevault Dec 14, 2017

ddevault Dec 14, 2017

ddevault Dec 14, 2017

ddevault Dec 14, 2017

ddevault commented Dec 14, 2017

ddevault commented Dec 14, 2017

Add output config #1503

Add output config #1503

Conversation

emersion commented Dec 6, 2017 • edited by ddevault Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acrisci commented Dec 6, 2017

emersion commented Dec 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acrisci commented Dec 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emersion Dec 12, 2017 • edited Loading

Choose a reason for hiding this comment

acrisci commented Dec 11, 2017

emersion commented Dec 12, 2017

Choose a reason for hiding this comment

ddevault commented Dec 13, 2017

emersion commented Dec 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddevault commented Dec 14, 2017

ddevault commented Dec 14, 2017

emersion commented Dec 6, 2017 •

edited by ddevault

Loading

emersion Dec 12, 2017 •

edited

Loading