Add output ID tracking

felixgruenbauer · Jul 5, 2024 · d2087a2 · d2087a2
1 parent c681198
commit d2087a2
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 18 deletions.
diff --git a/src/backend/mod.rs b/src/backend/mod.rs
@@ -9,6 +9,7 @@ use smithay::reexports::wayland_server::protocol::wl_surface::WlSurface;
 
 use crate::input::CompositorMod;
 use crate::niri::Niri;
+use crate::utils::id::IdCounter;
 
 pub mod tty;
 pub use tty::Tty;
@@ -31,7 +32,22 @@ pub enum RenderResult {
     Skipped,
 }
 
-pub type IpcOutputMap = HashMap<String, niri_ipc::Output>;
+pub type IpcOutputMap = HashMap<OutputId, niri_ipc::Output>;
+
+static OUTPUT_ID_COUNTER: IdCounter = IdCounter::new();
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct OutputId(u32);
+
+impl OutputId {
+    fn next() -> OutputId {
+        OutputId(OUTPUT_ID_COUNTER.next())
+    }
+
+    pub fn get(self) -> u32 {
+        self.0
+    }
+}
 
 impl Backend {
     pub fn init(&mut self, niri: &mut Niri) {

diff --git a/src/backend/tty.rs b/src/backend/tty.rs
@@ -57,6 +57,7 @@ use wayland_protocols::wp::linux_dmabuf::zv1::server::zwp_linux_dmabuf_feedback_
 use wayland_protocols::wp::presentation_time::server::wp_presentation_feedback;
 
 use super::{IpcOutputMap, RenderResult};
+use crate::backend::OutputId;
 use crate::frame_clock::FrameClock;
 use crate::niri::{Niri, RedrawState, State};
 use crate::render_helpers::debug::draw_damage;
@@ -118,6 +119,7 @@ pub struct OutputDevice {
     render_node: DrmNode,
     drm_scanner: DrmScanner,
     surfaces: HashMap<crtc::Handle, Surface>,
+    output_ids: HashMap<crtc::Handle, OutputId>,
     // SAFETY: drop after all the objects used with them are dropped.
     // See https://github.com/Smithay/smithay/issues/1102.
     drm: DrmDevice,
@@ -575,6 +577,7 @@ impl Tty {
             gbm,
             drm_scanner: DrmScanner::new(),
             surfaces: HashMap::new(),
+            output_ids: HashMap::new(),
             drm_lease_state,
             active_leases: Vec::new(),
             non_desktop_connectors: HashSet::new(),
@@ -599,6 +602,7 @@ impl Tty {
             return;
         };
 
+        let mut removed = Vec::new();
         for event in device.drm_scanner.scan_connectors(&device.drm) {
             match event {
                 DrmScanEvent::Connected {
@@ -611,11 +615,27 @@ impl Tty {
                 }
                 DrmScanEvent::Disconnected {
                     crtc: Some(crtc), ..
-                } => self.connector_disconnected(niri, node, 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.
+        let Some(device) = self.devices.get_mut(&node) else {
+            error!("device disappeared");
+            return;
+        };
+
+        for crtc in removed {
+            if device.output_ids.remove(&crtc).is_none() {
+                error!("output ID missing for disconnected crtc: {crtc:?}");
+            }
+        }
+
         self.refresh_ipc_outputs(niri);
     }
 
@@ -724,6 +744,10 @@ 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()
@@ -1464,7 +1488,7 @@ impl Tty {
 
         for (node, device) in &self.devices {
             for (connector, crtc) in device.drm_scanner.crtcs() {
-                let connector_name = format!(
+                let name = format!(
                     "{}-{}",
                     connector.interface().as_str(),
                     connector.interface_id(),
@@ -1527,7 +1551,7 @@ impl Tty {
                     .map(logical_output);
 
                 let ipc_output = niri_ipc::Output {
-                    name: connector_name.clone(),
+                    name,
                     make,
                     model,
                     physical_size,
@@ -1538,7 +1562,11 @@ impl Tty {
                     logical,
                 };
 
-                ipc_outputs.insert(connector_name, ipc_output);
+                let id = device.output_ids.get(&crtc).copied().unwrap_or_else(|| {
+                    error!("output ID missing for crtc: {crtc:?}");
+                    OutputId::next()
+                });
+                ipc_outputs.insert(id, ipc_output);
             }
         }
 

diff --git a/src/backend/winit.rs b/src/backend/winit.rs
@@ -17,7 +17,7 @@ use smithay::reexports::wayland_protocols::wp::presentation_time::server::wp_pre
 use smithay::reexports::winit::dpi::LogicalSize;
 use smithay::reexports::winit::window::Window;
 
-use super::{IpcOutputMap, RenderResult};
+use super::{IpcOutputMap, OutputId, RenderResult};
 use crate::niri::{Niri, RedrawState, State};
 use crate::render_helpers::debug::draw_damage;
 use crate::render_helpers::{resources, shaders, RenderTarget};
@@ -61,7 +61,7 @@ impl Winit {
 
         let physical_properties = output.physical_properties();
         let ipc_outputs = Arc::new(Mutex::new(HashMap::from([(
-            "winit".to_owned(),
+            OutputId::next(),
             niri_ipc::Output {
                 name: output.name(),
                 make: physical_properties.make,
@@ -98,7 +98,7 @@ impl Winit {
 
                     {
                         let mut ipc_outputs = winit.ipc_outputs.lock().unwrap();
-                        let output = ipc_outputs.get_mut("winit").unwrap();
+                        let output = ipc_outputs.values_mut().next().unwrap();
                         let mode = &mut output.modes[0];
                         mode.width = size.w.clamp(0, u16::MAX as i32) as u16;
                         mode.height = size.h.clamp(0, u16::MAX as i32) as u16;

diff --git a/src/dbus/mutter_display_config.rs b/src/dbus/mutter_display_config.rs
@@ -57,11 +57,12 @@ impl DisplayConfig {
             .ipc_outputs
             .lock()
             .unwrap()
-            .iter()
+            .values()
             // Take only enabled outputs.
-            .filter(|(_, output)| output.current_mode.is_some() && output.logical.is_some())
-            .map(|(c, output)| {
+            .filter(|output| output.current_mode.is_some() && output.logical.is_some())
+            .map(|output| {
                 // Loosely matches the check in Mutter.
+                let c = &output.name;
                 let is_laptop_panel = matches!(c.get(..4), Some("eDP-" | "LVDS" | "DSI-"));
 
                 // FIXME: use proper serial when we have libdisplay-info.

diff --git a/src/dbus/mutter_screen_cast.rs b/src/dbus/mutter_screen_cast.rs
@@ -191,7 +191,11 @@ impl Session {
     ) -> fdo::Result<OwnedObjectPath> {
         debug!(connector, ?properties, "record_monitor");
 
-        let Some(output) = self.ipc_outputs.lock().unwrap().get(connector).cloned() else {
+        let output = {
+            let ipc_outputs = self.ipc_outputs.lock().unwrap();
+            ipc_outputs.values().find(|o| o.name == connector).cloned()
+        };
+        let Some(output) = output else {
             return Err(fdo::Error::Failed("no such monitor".to_owned()));
         };
 

diff --git a/src/ipc/server.rs b/src/ipc/server.rs
@@ -143,7 +143,8 @@ async fn process(ctx: &ClientCtx, request: Request) -> Reply {
         Request::Version => Response::Version(version()),
         Request::Outputs => {
             let ipc_outputs = ctx.ipc_outputs.lock().unwrap().clone();
-            Response::Outputs(ipc_outputs)
+            let outputs = ipc_outputs.values().cloned().map(|o| (o.name.clone(), o));
+            Response::Outputs(outputs.collect())
         }
         Request::FocusedWindow => {
             let window = ctx.ipc_focused_window.lock().unwrap().clone();
@@ -183,8 +184,8 @@ async fn process(ctx: &ClientCtx, request: Request) -> Reply {
         Request::Output { output, action } => {
             let ipc_outputs = ctx.ipc_outputs.lock().unwrap();
             let found = ipc_outputs
-                .keys()
-                .any(|name| name.eq_ignore_ascii_case(&output));
+                .values()
+                .any(|o| o.name.eq_ignore_ascii_case(&output));
             let response = if found {
                 OutputConfigChanged::Applied
             } else {
@@ -223,7 +224,8 @@ async fn process(ctx: &ClientCtx, request: Request) -> Reply {
                         .ipc_outputs()
                         .lock()
                         .unwrap()
-                        .get(&active_output)
+                        .values()
+                        .find(|o| o.name == active_output)
                         .cloned()
                 });
 

diff --git a/src/niri.rs b/src/niri.rs
@@ -1166,12 +1166,12 @@ impl State {
 
         let _span = tracy_client::span!("State::refresh_ipc_outputs");
 
-        for (name, ipc_output) in self.backend.ipc_outputs().lock().unwrap().iter_mut() {
+        for ipc_output in self.backend.ipc_outputs().lock().unwrap().values_mut() {
             let logical = self
                 .niri
                 .global_space
                 .outputs()
-                .find(|output| output.name() == *name)
+                .find(|output| output.name() == ipc_output.name)
                 .map(logical_output);
             ipc_output.logical = logical;
         }