Add output enable/disable signal and change the current ones

[anriha]

This PR does two things.

Part 1: Changes to Output signals

It changes output signals for the api. Currently there are OutputConnect and OutputDisconnect. But these didn't really do what they should. Connect was only fired when output was connected for the first time, so there was no way to get signal when output was activated. And disconnect fired every time the monitor was switched off in any way, not just when it was actually disconnected. This fixes this behavior and adds two new signals OutputEnable and OutputDisable.

Part 2: Fix Silent Output Reactivation on Hotplug

This commit resolves an issue where an output could be re-enabled by the backend without firing the appropriate signals.

Motivation:
I was observing an issue where one of my monitors would power back on a few seconds after being disabled by an idle manager. This behavior was also present in other compositors like river and dwl, suggesting a backend or driver-level quirk that the compositor should be resilient to. Strangely enough it didn't happen in sway.

My initial attempt to fix this in my config by listening for an "enable" signal failed, as no signal was being fired when the monitor reawakened.

Root Cause:
Investigation revealed that this reactivation scenario bypasses the standard set_output_enabled and set_output_powered code paths. The backend handles the underlying hotplug event by calling change_output_state directly to apply a mode. This action physically powered on the monitor but did not update the compositor's logical state or fire the OutputEnable signal, leading to a desynchronization.

Solution:
To resolve this, the following precondition has been added to the top of change_output_state:

if mode.is_some() {
    self.set_output_enabled(output, true);
}

This change makes sure that setting a mode on an output is an explicit act of enabling it. By ensuring set_output_enabled(true) is called first, we guarantee that all code paths that physically activate a monitor are funneled through the correct signaling logic. This keeps the compositor's state consistent with the hardware and resolves the bug.

[Ottatop]

Connect was only fired when output was connected for the first time, so there was no way to get signal when output was activated.

This is due to the fact that it's mainly used to set up each output. It only running once is to prevent redundant setup. I guess connect isn't the best name for it and it would be better to have a setup signal and an actual connect signal. Or maybe just de-dup setup client-side somehow, not sure yet.

And disconnect fired every time the monitor was switched off in any way, not just when it was actually disconnected.

Otoh, disconnect means disconnect. It's likely that your monitor completely powers off its ports or stops advertising itself when turned off, in which case it will always look like you've unplugged it to any compositor.

What idle command are you running? You might be powering the output off which leads to a disconnect, in which case adding signals for enable/disable aren't what you want. To summarize, the states of an output are:

• Connected/disconnected: Whether the monitor is plugged in or not, or in some cases off or on
• Enabled/disabled: Whether the monitor is mapped to the global space and usable
  • If it is disabled then Pinnacle will ignore the monitor (i.e. no tags, can't move the pointer into it, etc.), but it's still connected.
• Powered/unpowered: Whether Pinnacle actively pushes frames to the monitor.
  • If the monitor is enabled and unpowered then it will still exist on the compositor (your tags are still there, you can move windows into and out of the output), but you just won't be able to see anything.

Instead of enable/disable signals, I think we should instead fix OutputSignal::connect so that it fires correctly on all connections (though since you've already done some work here I guess I could merge these signals after some fixes). However this would require some changes to output state persistence.

[Ottatop]

Powered != enable/disable, so this should be removed.

[Ottatop]

This should be in the if should_signal block above so it doesn't get triggered again if you enable an already enabled output.

[Ottatop]

This should be in the if block above for the same reason above.

[Ottatop]

Suggested change

	if let Some(_mode) = mode {
	if mode.is_some() {

[Ottatop]

I'm not a fan of this being here. If the output was explicitly disabled I don't think I'd want a mode change to re-enable it; I'd expect it to stay disabled.

[anriha]

What idle command are you running? You might be powering the output off which leads to a disconnect, in which case adding signals for enable/disable aren't what you want. To summarize, the states of an output are:

I am running this:

    tokio::spawn(async move {
        while let Some(action) = rx.recv().await {
            let mut state = state_for_receiver.lock().unwrap();
            match action {
                IdleAction::Idle => {
                    log::info!("Detected idle");
                    state.compositor.is_idle = true;

                    for output in pinnacle_api::output::get_all_enabled() {
                        output.set_powered(false);
                    }
                }
                IdleAction::Resume => {
                    log::info!("Detected activity");
                    state.compositor.is_idle = false;

                    for output in pinnacle_api::output::get_all() {
                        output.set_powered(true);
                    }
                }
            }
        }
    });

Which just listens to idle state from other part of my config. My issues and the reason I did this in the first place is just a single one of my monitors. When I power it off like this after a few seconds, it tries to turn on again. Honestly not sure what causes it, quirk of firmware or something else, not sure. But it only happens for this monitor and only when it is connected via HDMI. That is why I needed this signals, so now I can have this:

    output::connect_signal(OutputSignal::Enable(Box::new(move |output| {
        log::info!("[pinnacle] New output enable: {}", output.name());
        let state = state_for_new_output.lock().unwrap();
        if state.compositor.is_idle {
            log::info!("[pinnacle] System is idle, disabling newly connected output.");
            output.set_powered(false);
        }
    })));

Which with this PR fixes things. Which is also the reason for this line:

        if let Some(_mode) = mode {
            self.set_output_enabled(output, true);

Without it, the monitor activated, but no signal was fired. (Even including the enable/disable ones). Honestly not sure why that happens, but the monitor visibly powered off and on again and didn't fire any signals. Just so you understand where I am coming from with this PR.

[anriha]

Instead of enable/disable signals, I think we should instead fix OutputSignal::connect so that it fires correctly on all connections (though since you've already done some work here I guess I could merge these signals after some fixes). However this would require some changes to output state persistence.

This does make sense. I was mainly assuming this enable/disable signal is what you intended because of this comment;

// TODO: Create a new output_disable/enable signal and trigger it 
// instead of connect and disconnect

But, having connect fire every time, we need it to would make sense, too, especially if there was a setup command or something like that.

[Ottatop]

Without it, the monitor activated, but no signal was fired.

The only thing that causes set_output_enabled currently is zwlr_output_configuration_v1::{enable_head,disable_head}. Enabling the output when the mode is set is not the ideal solution here.

[anriha]

Looking into it. I think what must be happening is this. When my monitor turns on again, it goes here:
https://github.com/pinnacle-comp/pinnacle/blob/main/src/backend/udev.rs#L1068
Which maps everything and basically turns it on (starts showing stuff). But pinnacle still thinks that it is not enabled. So it starts mapping output that is not enabled. So actually, just changing it so that pinnacle doesn't map anything on an output that is disabled would work here.

[anriha]

But you are right that

        if let Some(_mode) = mode {
            self.set_output_enabled(output, true);

Shouldn't be there. I will debug things more to really find out what is going here.

[Ottatop]

When my monitor turns on again, it goes here:
https://github.com/pinnacle-comp/pinnacle/blob/main/src/backend/udev.rs#L1068
Which maps everything and basically turns it on (starts showing stuff). But pinnacle still thinks that it is not enabled.

It's not that Pinnacle thinks it's disabled (again, because only wlr-output-management currently sets that state); rather, when a monitor is disconnected and reconnected, it shows up as a completely new output to the compositor. Currently the only state that's saved between these reconnects is the location, tags, and scale, so Pinnacle is "forgetting" that the output is unpowered. I suppose you could extend ConnectorSavedState to include that information and restore it under that line and that should work.

[anriha]

Ok, this now this starts to make sense. When this happens, I don't get connect signals because that one is emitted only when there is no saved state for the output. Pinnacle restores the state as from its point of view it is just reconnected display. Output enabled is never set because that one would have to come from the output-management protocol. Which in this case isn't being used. So yeah, it does sound like the best way would be for pinnacle to save powered and probably enabled state in ConnectorSavedState. That should solve my main issues.
At that point, there shouldn't be a case where monitors turn on, except when you explicitly power it on again.

Btw thanks for the patience, I really have to wrap my head around how all the protocols interact.