Sway: output toggle?

Created on 9 May 2019  路  11Comments  路  Source: swaywm/sway

I'd like to implement a swaymsg output eDP-1 toggle, is this welcomed? If so, is the following location the place to put add a output_toggle() function? https://github.com/swaywm/sway/blob/75e7bd24ccb9731065bb7f8313aa53ba11ddc420/sway/tree/output.c#L254-L257

enhancement

All 11 comments

The feature sounds fine to me.

To implement it, you'll need to:

  • Create the file sway/commands/output/toggle.c with the command handler output_cmd_toggle. You can use enable.c or disable.c in the same directory for reference
  • Add the command handler to the list in sway/commands/output.c
  • Add the file to sway/meson.build
  • Add the function declaration to include/sway/commands.h

An output can be identified by the name (connector for DRM outputs) or the identifier (make, model, and serial) so it may be beneficial to use find_output_config to access a copy of the merged output config that is applied to the output (the returned config should not be modified and needs to be freed after use). You'll need to use a modified version of output_by_name_or_id that uses root->all_outputs (enabled and disabled outputs) instead of root->outputs (enabled outputs) to obtain a matching struct sway_output. If the output cannot be found or find_output_config returns NULL, just fallback to disable since outputs are enabled by default.

Hi. I tried the most naive flipping approach (https://github.com/swaywm/sway/compare/master...Moelf:master) and find out that the disable leg works but not the enable one, I would assume this is because of the output not being found? Should I use the said functions in the toggle.c(as a if clause) or somewhere in output.c?

I also would like to learn the best practice for testing this as I'm launching a sway instance and run commands myself which feels wrong?

find out that the disable leg works but not the enable one

Ya, sorry about that, I spaced out for a second and misled you in my previous comment. config->handler_context->output_config->enable should not be read as it will most likely _(1)_ be -1 since config->handler_context->output_config is a delta. You'll have to add more to get it working as a naive approach will not work.

As an updated fallback procedure, I think the following would be best:

  • If a struct sway_output is not found, the output is disconnected/non-existent, so just return new cmd_results_new(CMD_FAILURE, "Cannot toggle disconnected output")
  • If a struct sway_output is found, but find_output_config returns NULL, there is no output config for the output so it is using the defaults. Since outputs default to enabled, disable the output.

_(1) Output subcommands can be chained such as output * enable bg $wallpaper fill res 1920x1080 so if you have a mix of enable, disable, or toggle, it may not actually be -1_

Should I use the said functions in the toggle.c?

Yes.

I also would like to learn the best practice for testing this as I'm launching a sway instance and run commands myself which feels wrong?

That is correct. I usually test in a second vt, but you can also run sway within sway as a nested session. If you use the latter, the create_output command may be useful and I would recommend adding the following to your config (choosing whatever keys you prefer):

mode passthrough bindsym Pause mode default
bindsym $mod+Pause mode passthrough

Just press the binding to go into passthrough mode after starting the nested session, use the bindings you normally would, and then use the mode binding to go back to the default mode after exiting the nested session

Thanks for the detailed explanation, I think I made some progress by finding out how to use the two functions to search by name and then get the corresponding sway_output. However, it seems the output_disable does not work at this point? Also, enable needs a config right? My current toggle.c would crush upon calling.

https://github.com/Moelf/sway/blob/088e5522338d324717230f9166d77bd40ed4da94/sway/commands/output/toggle.c#L10-L19

You'll need to use a modified version of output_by_name_or_id that uses root->all_outputs (enabled and disabled outputs) instead of root->outputs (enabled outputs) to obtain a matching struct sway_output.

Also, don't call output_enable or output_disable. Just set config->handler_context->output_config->enable. The main output command handler will make the necessary call to apply the output config.

Looks like root->all_outputs is not a list_t but a wl_list, I can't find the constructor and it doesn't seem to have length anyways?

struct output_config *config;
wl_list_for_each(config, &root->all_outputs, link) {
    // config is set to an element in the list
}

Edit: Ignore this comment. See below

It complains that output_config has no member named link.

This is much more convoluted than I initially thought. (I thought there's a helper function telling me if an output is enabled or not and I can just do flipping).

I shall just use a bash script for the moment, I will look more into this when I have more time.

Whoops. Apparently my brain isn't working. It should be

struct sway_output *output;
wl_list_for_each(output, &root->all_outputs, link) {
 // output is set
}

Hell, this actually works now! https://github.com/swaywm/sway/compare/master...Moelf:master

Please advice on how to make this better and can handle exceptions? (I will update the man pages later)

Please advice on how to make this better and can handle exceptions?

Both s_output and oc in your patch can be NULL. I think the fallbacks I listed above are fine:

  • If s_output is NULL, return cmd_results_new(CMD_FAILURE, "Cannot toggle disconnected output")
  • If oc is NULL, treat it as a disable since the output does not have an output config and is enabled by default

Also, this needs to handle the wildcard output config (*). If you want to implement this, you would need to:

  • Iterate over all outputs

    • Creating a new output config based on the name or id (I'm not really sure which would be better) of the output

    • Setting the new state for that output

    • Calling store_output_config

  • Setting config->handler_context.output_config->enabled to -1 (so when the wildcard config is stored, it does not overwrite the other output configs)

If you don't feel like implementing support for the wildcard toggling, just return something like cmd_results_new(CMD_FAILURE, "Cannot apply toggle to all outputs"). We don't currently support wildcard relative transforms which requires basically the same logic so I don't see wildcard toggling as a blocker for toggling.

Finally, please review the style guide before submitting the PR

Was this page helpful?
0 / 5 - 0 ratings

Related issues

WhyNotHugo picture WhyNotHugo  路  3Comments

johanhelsing picture johanhelsing  路  3Comments

mcmfb picture mcmfb  路  3Comments

Alphare picture Alphare  路  3Comments

StephenBrown2 picture StephenBrown2  路  4Comments