pinnacle-comp
Add output enable/disable signal and change the current ones
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.
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.
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.
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.
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.
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.
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.
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.