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.

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

river-options: remove protocol #274

Merged
merged 4 commits into from
Apr 26, 2021
Merged

river-options: remove protocol #274

merged 4 commits into from
Apr 26, 2021

Conversation

ifreund
Copy link
Member

@ifreund ifreund commented Apr 25, 2021

This protocol involves far too much accidental complexity. The original
motivating use-case was to provide a convenient way to send arbitrary
data to layout clients at runtime in order to avoid layout clients
needing to implement their own IPC and do this over a side-channel.
Instead of implementing a quite complex but still rigid options protocol
and storing this state in the compositor, instead simply add requests
and events to the layout protocol to support this use case.

Consider the status quo event sequence:

  1. send get_option_handle request (riverctl)
  2. roundtrip waiting for first event (riverctl)
  3. send set_foo_value request (riverctl)
  4. receive set_foo_value request (river)
  5. send foo_value event to all current handles (river)
  6. receive foo_value event (rivertile)
  7. send parameters_changed request (rivertile)
  8. receive parameters_changed request (river)
  9. send layout_demand (river)

And compare with the event sequence after the proposed change:

  1. send set_foo request (riverctl)
  2. receive set_foo request (river)
  3. send set_foo event (river)
  4. send layout_demand (river)

This requires much less back and forth between the server and clients
and is clearly much simpler.

Note: river-options is currently used for 2 core river features, setting
the active layout and setting the output title. These features are
planned to be moved to the

@ifreund
Copy link
Member Author

ifreund commented Apr 25, 2021

I'm considering removing the set_foo requests from the layout manager and making them a normal river command instead. They would the become part of the revised river-control protocol which would allow them to participate in whatever solution I come up with for atomic batches of commands.

@ifreund ifreund force-pushed the layout-v2 branch 2 times, most recently from 3988aaa to 42fcbba Compare April 25, 2021 19:29
@ifreund
Copy link
Member Author

ifreund commented Apr 26, 2021

My current plan for rivertile is to only expose main_location, main_count, and main_factor as runtime configurable options, view_padding and outer_padding will be set exclusively through command line arguments.

@ifreund
Copy link
Member Author

ifreund commented Apr 26, 2021

I'm considering removing the set_foo requests from the layout manager and making them a normal river command instead. They would the become part of the revised river-control protocol which would allow them to participate in whatever solution I come up with for atomic batches of commands.

Just did this.

Additionally, I've realized that we need a convenient way to apply deltas to layout values for interactive usage. To this end, I've added mod_int_value and mod_fixed_value events. Since applying a delta to a string does not make sense, there is no mod_string_value event. Similarly since applying negative modifiers is useful and expected, a mod_uint_value taking a uint argument would be rather useless. Instead, let's just keep things simple and use ints for everything. This leaves us with the following new events on the river-layout object:

  • set_int_value
  • mod_int_value
  • set_fixed_value
  • mod_fixed_value
  • set_string_value

- Replace the layout option with new default-layout and output-layout
commands.
- Remove the ability to get/set the output title entirely.
This protocol involves far too much accidental complexity. The original
motivating use-case was to provide a convenient way to send arbitrary
data to layout clients at runtime in order to avoid layout clients
needing to implement their own IPC and do this over a side-channel.
Instead of implementing a quite complex but still rigid options protocol
and storing this state in the compositor, instead we will simply add
events to the layout protocol to support this use case.

Consider the status quo event sequence:

1. send get_option_handle request (riverctl)
2. roundtrip waiting for first event (riverctl)
3. send set_foo_value request (riverctl)
4. receive set_foo_value request (river)
5. send foo_value event to all current handles (river)
6. receive foo_value event (rivertile)
7. send parameters_changed request (rivertile)
8. receive parameters_changed request (river)
9. send layout_demand (river)

And compare with the event sequence after the proposed change:

1. send set_foo_value request (riverctl)
2. receive set_foo_value request (river)
3. send set_foo_value event (river)
4. send layout_demand (river)

This requires *much* less back and forth between the server and clients
and is clearly much simpler.
This implements the changes to the river-layout protocol proposed
in the previous commit removing river-options.
Add support for command line arguments to set default values for the
various options of rivertile, bringing us back to rough feature parity
with before the commit removing the river-options protocol.
@ifreund ifreund marked this pull request as ready for review April 26, 2021 22:04
@ifreund ifreund merged commit 07fd1b8 into master Apr 26, 2021
@ifreund ifreund deleted the layout-v2 branch April 26, 2021 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant