refactor(output config): move to config crate

[wash2]

this allows the greeter to easily sync to the user config.

[Drakulix]

Why does the greeter need to parse and understand the output config? Due to the somewhat fragile nature, this only being read/written by cosmic-comp is very intentional.

[wash2]

Why does the greeter need to parse and understand the output config? Due to the somewhat fragile nature, this only being read/written by cosmic-comp is very intentional.

I'm aware that this is very intentionally internal state, but I'm just not sure what the alternative is. The greeter daemon is currently what reads the user config and then the greeter applies it. Cosmic-comp also reads its config on start and doesn't watch the outputs.ron for changes as far as I can tell, so the greeter can't just copy the file. It parses the config if it can, then uses cosmic-randr to apply the config, checking the difference between the active config and user config.

[Drakulix]

I see, cosmic-greeter parses it from the user and applies it via cosmic-randr again in pop-os/cosmic-greeter#284.

I dislike this a lot, but I don't have a much better idea. However I would like to make the following changes:

• Please lets not make cosmic-greeter directly depend on the outputs.ron-layout. Instead I think cosmic-comp-config should have a method to convert OutputConfig into a type from cosmic-randr e.g. cosmic_randr_shell::Output. That way, when I need to change the format, I can just adjust that method in cosmic-comp-config and update the cosmic-greeter dependency. This has a much lower chance of me forgetting to do so and thus breaking the feature. We kinda need the same for OutputInfo given we want to introduce EDID-hashes as identifiers in the future, but maybe something like impl PartialEq<cosmic_randr_shell::Output> for OutputInfo would be enough here to not depend on the contents directly.
• Second applying all outputs and config options one-by-one isn't really a good idea here, because we have absolutely no idea, if the "in-between"-configurations are valid. E.g. we could try to mirror a previously disabled output. Or enable a higher resolution / refresh rate mode, which we don't have the bandwidth for before we reduced the resolution of another output or disabled it.
  • At the very least this means that we can't just silently drop the cosmic-randr error continuing to execute our futures. This needs to be one task, that aborts once cosmic-randr hits an error. And what it actually should do is revert the previous changes in that case...
  • Mid- to Long-Term this is an issue of cosmic-randr, which didn't really come up in cosmic-settings as you edit settings one-by-one (even though I would vastly prefer, if you could line up multiple changes and then atomically commit them like wdisplays). But with this the issue becomes really apparent and it absolutely doesn't have to be that way, because the protocol is explicitly designed around atomically submitting a whole configuration for all outputs including reverting if something goes wrong for you. So IMO we should really address the API of cosmic-randr here to make it possible to construct such a request programmatically (instead of calling into the binary for everything) and submit it in one go. (Preferably also from cosmic-settings.)

[wash2]

Please lets not make cosmic-greeter directly depend on the outputs.ron-layout. Instead I think cosmic-comp-config should have a method to convert OutputConfig into a type from cosmic-randr e.g. cosmic_randr_shell::Output.

Ok, that sounds good to me.

At the very least this means that we can't just silently drop the cosmic-randr error continuing to execute our futures. This needs to be one task, that aborts once cosmic-randr hits an error. And what it actually should do is revert the previous changes in that case...

Sure, I'll refactor the application of the changes a bit to try batching the changes, and revert on failure if possible. Long term, I agree that cosmic-randr could use some changes.

[mmstick]

Perhaps cosmic-randr could take a KDL-serialized input from stdin for performing multiple operations atomically.

[wash2]

The latest changes seem to work now, but there seems to be a bug related to surface positioning. My guess is that the greeter needs to reposition the menu subsurface after being reconfigured. Alternatively, it could wait for output reconfiguration to be completed before creating the menus to begin with.

[Drakulix]

This look good to me now (except of course fixing cosmic-comp-config's default features and the pending merge for cosmic-randr-shell).

Very cool how this now mostly simplified cosmic-comps code instead of just introducing a bunch of wrapper types. 👍

[Drakulix]

The latest changes seem to work now, but there seems to be a bug related to surface positioning. My guess is that the greeter needs to reposition the menu subsurface after being reconfigured. Alternatively, it could wait for output reconfiguration to be completed before creating the menus to begin with.

So is the output configuration only synced once or whenever a new user is selected as well?
In that case we would need to fix this via repositioning anyway, no?

[wash2]

In that case we would need to fix this via repositioning anyway, no?

Ya that's a good point. I keep forgetting that the configuration needs to change per user. I may need to add a message to cosmic-greeter after a different user is selected to re-do the sync too, then once completed, reposition.