Rework output connection to always go through on_output_config_changed()

This has the following benefits:
1. connector_connected() is now more closely mirroring
   connector_disconnected() in that it merely lights up the connector,
   and doesn't check if the connector should be off from the config.
2. We can use more complex on/off logic that depends on multiple
   connectors. For example, this commit adds logic to only disable the
   laptop panel on lid close if there are other connected outputs.

We don't want to disable the laptop panel on lid close if it's the only
connected output because it causes screen lockers to create their
surface from scratch on normal laptop unsuspend, which is undesirable
and also confuses some screen lockers.

2 files changed, 60 insertions, 46 deletions

diff --git a/src/backend/tty.rs b/src/backend/tty.rs
index fd0f04a2..b5991d3f 100644
--- a/src/backend/tty.rs
+++ b/src/backend/tty.rs
@@ -61,7 +61,7 @@ use crate::niri::{Niri, RedrawState, State};
 use crate::render_helpers::debug::draw_damage;
 use crate::render_helpers::renderer::AsGlesRenderer;
 use crate::render_helpers::{resources, shaders, RenderTarget};
-use crate::utils::{get_monotonic_time, logical_output};
+use crate::utils::{get_monotonic_time, is_laptop_panel, logical_output};
 
 const SUPPORTED_COLOR_FORMATS: &[Fourcc] = &[Fourcc::Argb8888, Fourcc::Abgr8888];
 
@@ -636,22 +636,31 @@ impl Tty {
                     connector,
                     crtc: Some(crtc),
                 } => {
-                    if let Err(err) = self.connector_connected(niri, node, connector, crtc) {
-                        warn!("error connecting connector: {err:?}");
-                    }
+                    let connector_name = format_connector_name(&connector);
+                    let output_name =
+                        make_output_name(&device.drm, connector.handle(), connector_name, false);
+                    debug!(
+                        "new connector: {} \"{}\"",
+                        &output_name.connector,
+                        output_name.format_make_model_serial(),
+                    );
+
+                    // Assign an id to this crtc.
+                    device.output_ids.insert(crtc, OutputId::next());
                 }
                 DrmScanEvent::Disconnected {
                     crtc: Some(crtc), ..
                 } => {
-                    self.connector_disconnected(niri, node, crtc);
                     removed.push(crtc);
                 }
                 _ => (),
             }
         }
 
-        // FIXME: this is better done in connector_disconnected(), but currently we call that to
-        // turn off outputs temporarily, too. So we can't do this there.
+        for crtc in &removed {
+            self.connector_disconnected(niri, node, *crtc);
+        }
+
         let Some(device) = self.devices.get_mut(&node) else {
             error!("device disappeared");
             return;
@@ -663,7 +672,12 @@ impl Tty {
             }
         }
 
-        self.refresh_ipc_outputs(niri);
+        // This will connect any new connectors if needed, and apply other changes, such as
+        // connecting back the internal laptop monitor once it becomes the only monitor left.
+        //
+        // It will also call refresh_ipc_outputs(), which will catch the disconnected connectors
+        // above.
+        self.on_output_config_changed(niri);
     }
 
     fn device_removed(&mut self, device_id: dev_t, niri: &mut Niri) {
@@ -772,10 +786,6 @@ impl Tty {
             return Ok(());
         }
 
-        // This should be unique per CRTC connection, however currently we can call
-        // connector_connected() multiple times for turning the output off and on.
-        device.output_ids.entry(crtc).or_insert_with(OutputId::next);
-
         let config = self
             .config
             .borrow()
@@ -784,15 +794,6 @@ impl Tty {
             .cloned()
             .unwrap_or_default();
 
-        if config.off {
-            debug!("output is disabled in the config");
-            return Ok(());
-        }
-        if !niri.should_enable_laptop_panel(&output_name.connector) {
-            debug!("output is disabled because it is a laptop panel and the lid is closed");
-            return Ok(());
-        }
-
         for m in connector.modes() {
             trace!("{m:?}");
         }
@@ -1724,6 +1725,24 @@ impl Tty {
         }
         self.update_output_config_on_resume = false;
 
+        // Figure out if we should disable laptop panels.
+        let mut disable_laptop_panels = false;
+        if niri.is_lid_closed {
+            let config = self.config.borrow();
+            if !config.debug.keep_laptop_panel_on_when_lid_is_closed {
+                // Check if any external monitor is connected.
+                'outer: for device in self.devices.values() {
+                    for (connector, _crtc) in device.drm_scanner.crtcs() {
+                        if !is_laptop_panel(&format_connector_name(connector)) {
+                            disable_laptop_panels = true;
+                            break 'outer;
+                        }
+                    }
+                }
+            }
+        }
+        let should_disable = |connector: &str| disable_laptop_panels && is_laptop_panel(connector);
+
         let mut to_disconnect = vec![];
         let mut to_connect = vec![];
 
@@ -1736,7 +1755,7 @@ impl Tty {
                     .find(&surface.name)
                     .cloned()
                     .unwrap_or_default();
-                if config.off || !niri.should_enable_laptop_panel(&surface.name.connector) {
+                if config.off || should_disable(&surface.name.connector) {
                     to_disconnect.push((node, crtc));
                     continue;
                 }
@@ -1832,7 +1851,11 @@ impl Tty {
                 }
 
                 // Check if already enabled.
-                if device.surfaces.contains_key(&crtc) {
+                if device.surfaces.contains_key(&crtc)
+                    || device
+                        .non_desktop_connectors
+                        .contains(&(connector.handle(), crtc))
+                {
                     continue;
                 }
 
@@ -1851,8 +1874,8 @@ impl Tty {
                     .cloned()
                     .unwrap_or_default();
 
-                if !config.off && niri.should_enable_laptop_panel(&output_name.connector) {
-                    to_connect.push((node, connector.clone(), crtc));
+                if !(config.off || should_disable(&output_name.connector)) {
+                    to_connect.push((node, connector.clone(), crtc, output_name));
                 }
             }
         }
@@ -1861,7 +1884,11 @@ impl Tty {
             self.connector_disconnected(niri, node, crtc);
         }
 
-        for (node, connector, crtc) in to_connect {
+        // Sort by output name to get more predictable first focused output at initial compositor
+        // startup, when multiple connectors appear at once.
+        to_connect.sort_unstable_by(|a, b| a.3.compare(&b.3));
+
+        for (node, connector, crtc, _name) in to_connect {
             if let Err(err) = self.connector_connected(niri, node, connector, crtc) {
                 warn!("error connecting connector: {err:?}");
             }
@@ -1896,7 +1923,11 @@ impl Tty {
                 }
 
                 // Check if already enabled.
-                if device.surfaces.contains_key(&crtc) {
+                if device.surfaces.contains_key(&crtc)
+                    || device
+                        .non_desktop_connectors
+                        .contains(&(connector.handle(), crtc))
+                {
                     continue;
                 }
 
diff --git a/src/niri.rs b/src/niri.rs
index 50cf444f..79f99858 100644
--- a/src/niri.rs
+++ b/src/niri.rs
@@ -143,9 +143,8 @@ use crate::ui::screenshot_ui::{OutputScreenshot, ScreenshotUi, ScreenshotUiRende
 use crate::utils::scale::{closest_representable_scale, guess_monitor_scale};
 use crate::utils::spawning::CHILD_ENV;
 use crate::utils::{
-    center, center_f64, get_monotonic_time, ipc_transform_to_smithay, is_laptop_panel,
-    logical_output, make_screenshot_path, output_matches_name, output_size, send_scale_transform,
-    write_png_rgba8,
+    center, center_f64, get_monotonic_time, ipc_transform_to_smithay, logical_output,
+    make_screenshot_path, output_matches_name, output_size, send_scale_transform, write_png_rgba8,
 };
 use crate::window::{InitialConfigureState, Mapped, ResolvedWindowRules, Unmapped, WindowRef};
 use crate::{animation, niri_render_elements};
@@ -4811,22 +4810,6 @@ impl Niri {
             .unwrap();
         self.pointer_inactivity_timer = Some(token);
     }
-
-    pub fn should_enable_laptop_panel(&self, connector: &str) -> bool {
-        // Make sure the output config is reloaded when any of the conditions in this function
-        // change.
-
-        let config = self.config.borrow();
-
-        // We reload the output config when this flag changes.
-        if config.debug.keep_laptop_panel_on_when_lid_is_closed {
-            return true;
-        }
-
-        // We reload the output config when the lid switch is toggled, and the connector name is
-        // static.
-        !(self.is_lid_closed && is_laptop_panel(connector))
-    }
 }
 
 pub struct ClientState {