refactor: dynamic output configuration

src/backend/kms/mod.rs

@@ -5,7 +5,7 @@ use crate::state::Fps;
 
 use crate::{
     backend::render,
-    config::{Config, OutputConfig},
+    config::OutputConfig,
     shell::Shell,
     state::{BackendData, Common, State},
 };
@@ -297,58 +297,64 @@ impl State {
 
         let outputs = device.enumerate_surfaces()?.added; // There are no removed outputs on newly added devices
         let mut wl_outputs = Vec::new();
+        let mut w = self.common.shell.global_space().size.w;
         for (crtc, conn) in outputs {
             match device.setup_surface(
-                &drm_node,
                 crtc,
                 conn,
-                self.backend.kms().signaler.clone(),
                 &mut self.common.event_loop_handle,
+                (0, w),
             ) {
                 Ok(output) => {
-                    self.common.shell.map_output(
+                    w += output.user_data().get::<RefCell<OutputConfig>>().unwrap().borrow().mode_size().w;
-                        &output,
-                        &mut self.backend,
-                        &mut self.common.config,
-                    );
                     wl_outputs.push(output);
                 }
                 Err(err) => slog_scope::warn!("Failed to initialize output: {}", err),
             };
         }
+        self.backend.kms().devices.insert(drm_node, device);
+        
         self.common.output_conf.add_heads(wl_outputs.iter());
         self.common
             .output_conf
             .update(&mut *self.common.display.borrow_mut());
+        for output in wl_outputs {
+            if let Err(err) = self.backend.kms().apply_config_for_output(&output, &mut self.common.shell, false) {
+                slog_scope::warn!("Failed to initialize output: {}", err);
+            }
+        }
+        self.common.config.read_outputs(self.common.output_conf.outputs(), &mut self.backend, &mut self.common.shell);
+        self.common.shell.refresh_outputs();
+        self.common.config.write_outputs(self.common.output_conf.outputs());
 
-        self.backend.kms().devices.insert(drm_node, device);
         Ok(())
     }
 
     fn device_changed(&mut self, dev: dev_t) -> Result<()> {
         let drm_node = DrmNode::from_dev_id(dev)?;
-        let signaler = self.backend.kms().signaler.clone();
         let mut outputs_removed = Vec::new();
         let mut outputs_added = Vec::new();
         if let Some(device) = self.backend.kms().devices.get_mut(&drm_node) {
             let changes = device.enumerate_surfaces()?;
+            let mut w = self.common.shell.global_space().size.w;
             for crtc in changes.removed {
                 if let Some(surface) = device.surfaces.get_mut(&crtc) {
                     if let Some(token) = surface.render_timer_token.take() {
                         self.common.event_loop_handle.remove(token);
                     }
+                    w -= surface.output.current_mode().map(|m| m.size.w).unwrap_or(0);
                     outputs_removed.push(surface.output.clone());
                 }
             }
             for (crtc, conn) in changes.added {
                 match device.setup_surface(
-                    &drm_node,
                     crtc,
                     conn,
-                    signaler.clone(),
                     &mut self.common.event_loop_handle,
+                    (0, w),
                 ) {
                     Ok(output) => {
+                        w += output.user_data().get::<RefCell<OutputConfig>>().unwrap().borrow().mode_size().w;
                         outputs_added.push(output);
                     }
                     Err(err) => slog_scope::warn!("Failed to initialize output: {}", err),
@@ -357,20 +363,19 @@ impl State {
         }
 
         self.common.output_conf.remove_heads(outputs_removed.iter());
-        for output in outputs_removed.into_iter() {
-            self.common
-                .shell
-                .unmap_output(&output, &mut self.backend, &mut self.common.config);
-        }
         self.common.output_conf.add_heads(outputs_added.iter());
-        for output in outputs_added.into_iter() {
+        for output in outputs_added {
-            self.common
+            if let Err(err) = self.backend.kms().apply_config_for_output(&output, &mut self.common.shell, false) {
-                .shell
+                slog_scope::warn!("Failed to initialize output: {}", err);
-                .map_output(&output, &mut self.backend, &mut self.common.config)
+            }
         }
         self.common
             .output_conf
             .update(&mut self.common.display.borrow_mut());
+        self.common.config.read_outputs(self.common.output_conf.outputs(), &mut self.backend, &mut self.common.shell);
+        self.common.shell.refresh_outputs();
+        self.common.config.write_outputs(self.common.output_conf.outputs());
+
         Ok(())
     }
 
@@ -392,14 +397,12 @@ impl State {
             }
         }
         self.common.output_conf.remove_heads(outputs_removed.iter());
-        for output in outputs_removed.into_iter() {
-            self.common
-                .shell
-                .unmap_output(&output, &mut self.backend, &mut self.common.config);
-        }
         self.common
             .output_conf
             .update(&mut *self.common.display.borrow_mut());
+        self.common.config.read_outputs(self.common.output_conf.outputs(), &mut self.backend, &mut self.common.shell);
+        self.common.shell.refresh_outputs();
+        self.common.config.write_outputs(self.common.output_conf.outputs());
 
         Ok(())
     }
@@ -440,11 +443,10 @@ impl Device {
 
     fn setup_surface(
         &mut self,
-        drm_node: &DrmNode,
         crtc: crtc::Handle,
         conn: connector::Handle,
-        signaler: Signaler<Signal>,
         loop_handle: &mut LoopHandle<'static, State>,
+        position: (i32, i32),
     ) -> Result<Output> {
         let drm = &mut *self.drm.as_source_mut();
         let crtc_info = drm.get_crtc(crtc)?;
@@ -461,14 +463,6 @@ impl Device {
                 .unwrap_or(conn_info.modes()[0])
         });
         let refresh_rate = drm_helpers::calculate_refresh_rate(mode);
-        let mut surface = drm.create_surface(crtc, mode, &[conn])?;
-        surface.link(signaler);
-
-        let target =
-            GbmBufferedSurface::new(surface, self.allocator.clone(), self.formats.clone(), None)
-                .with_context(|| format!("Failed to initialize Gbm surface for {}", interface))?;
-
-        //let is_nvidia = driver(drm.device_id()).ok().flatten().map(|x| x == "nvidia").unwrap_or(false);
         let output_mode = OutputMode {
             size: (mode.size().0 as i32, mode.size().1 as i32).into(),
             refresh: refresh_rate as i32,
@@ -499,12 +493,13 @@ impl Device {
             // TODO: Readout property for monitor rotation
             Some(wl_output::Transform::Normal),
             None,
-            None,
+            Some(position.into()),
         );
         output.user_data().insert_if_missing(|| {
             RefCell::new(OutputConfig {
                 mode: ((output_mode.size.w, output_mode.size.h), Some(refresh_rate)),
                 vrr,
+                position,
                 ..Default::default()
             })
         });
@@ -529,16 +524,15 @@ impl Device {
                 }
             })
             .unwrap();
-        timer_handle.add_timeout(Duration::ZERO, (*drm_node, crtc));
 
         let data = Surface {
             output: output.clone(),
-            surface: Some(target),
+            surface: None,
             connector: conn,
             vrr,
             refresh_rate,
             last_submit: None,
-            pending: true,
+            pending: false,
             render_timer: timer_handle,
             render_timer_token: Some(timer_token),
             #[cfg(feature = "debug")]
@@ -633,7 +627,6 @@ impl KmsState {
     pub fn apply_config_for_output(
         &mut self,
         output: &Output,
-        config: &mut Config,
         shell: &mut Shell,
         test_only: bool,
     ) -> Result<(), anyhow::Error> {
@@ -657,7 +650,7 @@ impl KmsState {
                 if !test_only {
                     if surface.surface.take().is_some() {
                         // just drop it
-                        shell.disable_output(output, config);
+                        shell.remove_output(output);
                     }
                 }
                 false
@@ -707,7 +700,7 @@ impl KmsState {
                             )
                         })?;
                         surface.surface = Some(target);
-                        shell.enable_output(output, config);
+                        shell.add_output(output);
                         true
                     }
                 } else {

src/backend/winit.rs

@@ -169,10 +169,8 @@ pub fn init_backend(event_loop: &mut EventLoop<State>, state: &mut State) -> Res
                 }
                 Err(winit::WinitError::WindowClosed) => {
                     let output = state.backend.winit().output.clone();
-                    state.common.shell.unmap_output(
+                    state.common.shell.remove_output(
                         &output,
-                        &mut state.backend,
-                        &mut state.common.config,
                     );
                     if let Some(token) = token.take() {
                         event_loop_handle.remove(token);
@@ -197,7 +195,10 @@ pub fn init_backend(event_loop: &mut EventLoop<State>, state: &mut State) -> Res
     state
         .common
         .shell
-        .map_output(&output, &mut state.backend, &mut state.common.config);
+        .add_output(&output);
+    state.common.config.read_outputs(std::iter::once(&output), &mut state.backend, &mut state.common.shell);
+    state.common.shell.refresh_outputs();
+    state.common.config.write_outputs(std::iter::once(&output));
 
     Ok(())
 }
@@ -271,6 +272,7 @@ impl State {
                 self.common
                     .output_conf
                     .update(&mut *self.common.display.borrow_mut());
+                self.common.shell.refresh_outputs();
                 render_ping.ping();
             }
             WinitEvent::Refresh => render_ping.ping(),

src/backend/x11.rs

@@ -262,7 +262,12 @@ pub fn init_backend(event_loop: &mut EventLoop<State>, state: &mut State) -> Res
     state
         .common
         .shell
-        .map_output(&output, &mut state.backend, &mut state.common.config);
+        .add_output(&output);
+    state.common.config.read_outputs(std::iter::once(&output), &mut state.backend, &mut state.common.shell);
+    state.common.shell.refresh_outputs();
+    state.common.config.write_outputs(std::iter::once(&output));
+
+
 
     event_loop
         .handle()
@@ -286,10 +291,8 @@ pub fn init_backend(event_loop: &mut EventLoop<State>, state: &mut State) -> Res
                     .surfaces
                     .retain(|s| s.window.id() != window_id);
                 for output in outputs_removed.into_iter() {
-                    state.common.shell.unmap_output(
+                    state.common.shell.remove_output(
                         &output,
-                        &mut state.backend,
-                        &mut state.common.config,
                     );
                 }
             }
@@ -327,6 +330,7 @@ pub fn init_backend(event_loop: &mut EventLoop<State>, state: &mut State) -> Res
                         .common
                         .output_conf
                         .update(&mut *state.common.display.borrow_mut());
+                    state.common.shell.refresh_outputs();
                     surface.dirty = true;
                     if !surface.pending {
                         surface.render.ping();

src/config.rs

@@ -1,17 +1,22 @@
 // SPDX-License-Identifier: GPL-3.0-only
 
-use crate::shell::layout::FocusDirection;
+use crate::{
+    shell::{
+        Shell,
+        layout::FocusDirection,
+    },
+    state::BackendData,
+};
 use serde::{Deserialize, Serialize};
-use smithay::wayland::seat::Keysym;
 pub use smithay::{
     backend::input::KeyState,
     utils::{Logical, Physical, Point, Size, Transform},
     wayland::{
-        output::Output,
+        output::{Mode, Output},
-        seat::{keysyms as KeySyms, ModifiersState as KeyModifiers},
+        seat::{keysyms as KeySyms, Keysym, ModifiersState as KeyModifiers},
     },
 };
-use std::{collections::HashMap, fs::OpenOptions, path::PathBuf};
+use std::{cell::RefCell, collections::HashMap, fs::OpenOptions, path::PathBuf};
 use xkbcommon::xkb;
 
 pub struct Config {
@@ -89,6 +94,13 @@ impl OutputConfig {
     pub fn mode_refresh(&self) -> u32 {
         self.mode.1.unwrap_or(60_000)
     }
+
+    pub fn output_mode(&self) -> Mode {
+        Mode {
+            size: self.mode_size(),
+            refresh: self.mode_refresh() as i32,
+        }
+    }
 }
 
 #[derive(Serialize, Deserialize)]
@@ -175,6 +187,103 @@ impl Config {
             config: HashMap::new(),
         }
     }
+    
+    pub fn read_outputs<'a>(
+        &mut self,
+        outputs: impl Iterator<Item=impl std::borrow::Borrow<Output>>,
+        backend: &mut BackendData,
+        shell: &mut Shell,
+    ) {
+        let outputs = outputs.map(|x| x.borrow().clone()).collect::<Vec<_>>();
+        let mut infos = outputs
+            .iter()
+            .cloned()
+            .map(Into::<crate::config::OutputInfo>::into)
+            .collect::<Vec<_>>();
+        infos.sort();
+        if let Some(configs) = self.dynamic_conf.outputs().config.get(&infos).cloned() {
+            let mut reset = false;
+            let known_good_configs = outputs
+                .iter()
+                .map(|output| {
+                    output
+                        .user_data()
+                        .get::<RefCell<OutputConfig>>()
+                        .unwrap()
+                        .borrow()
+                        .clone()
+                })
+                .collect::<Vec<_>>();
+
+            for (name, output_config) in infos.iter().map(|o| &o.connector).zip(configs.into_iter())
+            {
+                let output = outputs
+                    .iter()
+                    .find(|o| &o.name() == name)
+                    .unwrap()
+                    .clone();
+                *output
+                    .user_data()
+                    .get::<RefCell<OutputConfig>>()
+                    .unwrap()
+                    .borrow_mut() = output_config;
+                if let Err(err) = backend.apply_config_for_output(&output, false, shell) {
+                    slog_scope::warn!(
+                        "Failed to set new config for output {}: {}",
+                        output.name(),
+                        err
+                    );
+                    reset = true;
+                    break;
+                }
+            }
+
+            if reset {
+                for (output, output_config) in outputs
+                    .clone()
+                    .into_iter()
+                    .zip(known_good_configs.into_iter())
+                {
+                    *output
+                        .user_data()
+                        .get::<RefCell<OutputConfig>>()
+                        .unwrap()
+                        .borrow_mut() = output_config;
+                    if let Err(err) = backend.apply_config_for_output(&output, false, shell)
+                    {
+                        slog_scope::error!(
+                            "Failed to reset config for output {}: {}",
+                            output.name(),
+                            err
+                        );
+                    }
+                }
+            }
+        }
+    }
+    
+    pub fn write_outputs<'a>(&mut self, outputs: impl Iterator<Item=impl std::borrow::Borrow<Output>>) {
+        let mut infos = outputs
+            .map(|o| {
+                let o = o.borrow();
+                (
+                    Into::<crate::config::OutputInfo>::into(o.clone()),
+                    o.user_data()
+                        .get::<RefCell<OutputConfig>>()
+                        .unwrap()
+                        .borrow()
+                        .clone(),
+                )
+            })
+            .collect::<Vec<(OutputInfo, OutputConfig)>>();
+        infos.sort_by(|&(ref a, _), &(ref b, _)| a.cmp(b));
+        let (infos, configs) = infos.into_iter().unzip();
+        self
+            .dynamic_conf
+            .outputs_mut()
+            .config
+            .insert(infos, configs);
+    }
 }
 
 pub struct PersistenceGuard<'a, T: Serialize>(Option<PathBuf>, &'a mut T);

src/shell/mod.rs

@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: GPL-3.0-only
 
 use crate::{
-    config::{Config, OutputConfig, OutputInfo},
+    config::{Config, OutputConfig},
     input::active_output,
-    state::{BackendData, Common},
+    state::Common,
 };
 pub use smithay::{
     desktop::{PopupGrab, PopupManager, PopupUngrabStrategy, Space, Window},
@@ -92,129 +92,38 @@ impl Shell {
         }
     }
 
-    fn refresh_config(&mut self, backend: &mut BackendData, config: &mut Config) -> bool {
+    pub fn refresh_outputs(&mut self) {
-        let mut infos = self
+        if let Mode::Global { active } = self.mode {
-            .outputs()
+            let workspace = &mut self.spaces[active];
-            .cloned()
+            for output in self.outputs.iter() {
-            .map(Into::<crate::config::OutputInfo>::into)
+                let config = output
-            .collect::<Vec<_>>();
-        infos.sort();
-        if let Some(configs) = config.dynamic_conf.outputs().config.get(&infos).cloned() {
-            let mut reset = false;
-            let known_good_configs = self
-                .outputs
-                .iter()
-                .map(|output| {
-                    output
-                        .user_data()
-                        .get::<RefCell<OutputConfig>>()
-                        .unwrap()
-                        .borrow()
-                        .clone()
-                })
-                .collect::<Vec<_>>();
-
-            for (name, output_config) in infos.iter().map(|o| &o.connector).zip(configs.into_iter())
-            {
-                let output = self
-                    .outputs
-                    .iter()
-                    .find(|o| &o.name() == name)
-                    .unwrap()
-                    .clone();
-                *output
                     .user_data()
                     .get::<RefCell<OutputConfig>>()
                     .unwrap()
-                    .borrow_mut() = output_config;
+                    .borrow();
-                if let Err(err) = backend.apply_config_for_output(&output, false, config, self) {
+                workspace
-                    slog_scope::warn!(
+                    .space
-                        "Failed to set new config for output {}: {}",
+                    .map_output(output, config.scale, config.position);
-                        output.name(),
-                        err
-                    );
-                    reset = true;
-                    break;
-                }
             }
-
-            if reset {
-                for (output, output_config) in self
-                    .outputs
-                    .clone()
-                    .into_iter()
-                    .zip(known_good_configs.into_iter())
-                {
-                    *output
-                        .user_data()
-                        .get::<RefCell<OutputConfig>>()
-                        .unwrap()
-                        .borrow_mut() = output_config;
-                    if let Err(err) = backend.apply_config_for_output(&output, false, config, self)
-                    {
-                        slog_scope::error!(
-                            "Failed to reset config for output {}: {}",
-                            output.name(),
-                            err
-                        );
-                    }
-                }
-            }
-
-            if let Mode::Global { active } = self.mode {
-                let workspace = &mut self.spaces[active];
-                for output in self.outputs.iter() {
-                    let config = output
-                        .user_data()
-                        .get::<RefCell<OutputConfig>>()
-                        .unwrap()
-                        .borrow();
-                    workspace
-                        .space
-                        .map_output(output, config.scale, config.position);
-                }
-            } else {
-                for output in self.outputs.iter() {
-                    let config = output
-                        .user_data()
-                        .get::<RefCell<OutputConfig>>()
-                        .unwrap()
-                        .borrow();
-                    let workspace = Self::assign_next_free_output(&mut self.spaces, output);
-                    workspace.space.map_output(output, config.scale, (0, 0));
-                }
-            }
-
-            true
         } else {
-            false
+            for output in self.outputs.iter() {
+                let config = output
+                    .user_data()
+                    .get::<RefCell<OutputConfig>>()
+                    .unwrap()
+                    .borrow();
+                let active = output
+                    .user_data()
+                    .get::<ActiveWorkspace>()
+                    .unwrap()
+                    .get()
+                    .unwrap();
+                let workspace = &mut self.spaces[active];
+                workspace.space.map_output(output, config.scale, (0, 0));
+            }
         }
     }
 
-    pub fn save_config(&self, config: &mut Config) {
-        let mut infos = self
-            .outputs()
-            .cloned()
-            .map(|o| {
-                (
-                    Into::<crate::config::OutputInfo>::into(o.clone()),
-                    o.user_data()
-                        .get::<RefCell<OutputConfig>>()
-                        .unwrap()
-                        .borrow()
-                        .clone(),
-                )
-            })
-            .collect::<Vec<(OutputInfo, OutputConfig)>>();
-        infos.sort_by(|&(ref a, _), &(ref b, _)| a.cmp(b));
-        let (infos, configs) = infos.into_iter().unzip();
-        config
-            .dynamic_conf
-            .outputs_mut()
-            .config
-            .insert(infos, configs);
-    }
-
     fn assign_next_free_output<'a>(
         spaces: &'a mut [Workspace],
         output: &Output,
@@ -236,7 +145,8 @@ impl Shell {
         workspace
     }
 
-    fn add_output(&mut self, output: &Output) {
+    pub fn add_output(&mut self, output: &Output) {
+        self.outputs.push(output.clone());
         let config = output
             .user_data()
             .get::<RefCell<OutputConfig>>()
@@ -255,9 +165,10 @@ impl Shell {
                     .map_output(output, config.scale, config.position);
             }
         }
+        output.change_current_state(None, None, Some(config.scale.ceil() as i32), None);
     }
 
-    fn remove_output(&mut self, output: &Output) {
+    pub fn remove_output(&mut self, output: &Output) {
         match self.mode {
             Mode::OutputBound => {
                 if let Some(idx) = output
@@ -277,65 +188,6 @@ impl Shell {
         }
     }
 
-    pub fn map_output(&mut self, output: &Output, backend: &mut BackendData, config: &mut Config) {
-        self.outputs.push(output.clone());
-        if !self.refresh_config(backend, config) {
-            let new_pos_x = self
-                .outputs()
-                .take(self.outputs.len() - 1)
-                .map(|o| {
-                    let logical_size = self
-                        .active_space(o)
-                        .space
-                        .output_geometry(o)
-                        .map(|x| x.size)
-                        .unwrap_or((0, 0).into());
-                    o.user_data()
-                        .get::<RefCell<OutputConfig>>()
-                        .unwrap()
-                        .borrow()
-                        .position
-                        .0
-                        + logical_size.w
-                })
-                .max()
-                .unwrap_or(0);
-
-            let new_pos = (new_pos_x, 0);
-            output
-                .user_data()
-                .get::<RefCell<OutputConfig>>()
-                .unwrap()
-                .borrow_mut()
-                .position = new_pos;
-            output.change_current_state(None, None, None, Some(new_pos.into()));
-
-            self.add_output(output);
-            self.save_config(config);
-        }
-    }
-
-    pub fn unmap_output(
-        &mut self,
-        output: &Output,
-        backend: &mut BackendData,
-        config: &mut Config,
-    ) {
-        self.remove_output(output);
-        self.refresh_config(backend, config);
-    }
-
-    pub fn enable_output(&mut self, output: &Output, config: &mut Config) {
-        self.outputs.push(output.clone());
-        self.add_output(output);
-        self.save_config(config);
-    }
-
-    pub fn disable_output(&mut self, output: &Output, config: &mut Config) {
-        self.remove_output(output);
-        self.save_config(config);
-    }
-
     pub fn output_size(&self, output: &Output) -> Size<i32, Logical> {
         let workspace = self.active_space(output);
         workspace

src/state.rs

@@ -116,12 +116,11 @@ impl BackendData {
         &mut self,
         output: &Output,
         test_only: bool,
-        config: &mut Config,
         shell: &mut Shell,
     ) -> Result<(), anyhow::Error> {
         let result = match self {
             BackendData::Kms(ref mut state) => {
-                state.apply_config_for_output(output, config, shell, test_only)
+                state.apply_config_for_output(output, shell, test_only)
             }
             BackendData::Winit(ref mut state) => state.apply_config_for_output(output, test_only),
             BackendData::X11(ref mut state) => state.apply_config_for_output(output, test_only),
@@ -150,7 +149,6 @@ impl BackendData {
             let location =
                 Some(final_config.position.into()).filter(|x| *x != output.current_location());
             output.change_current_state(mode, transform, scale, location);
-            shell.save_config(config);
         }
 
         result
@@ -263,7 +261,6 @@ impl State {
                     if let Err(err) = state.backend.apply_config_for_output(
                         output,
                         test_only,
-                        &mut state.common.config,
                         &mut state.common.shell,
                     ) {
                         slog_scope::warn!(
@@ -284,7 +281,6 @@ impl State {
                                 if let Err(err) = state.backend.apply_config_for_output(
                                     output,
                                     false,
-                                    &mut state.common.config,
                                     &mut state.common.shell,
                                 ) {
                                     slog_scope::error!(
@@ -297,6 +293,7 @@ impl State {
                         }
                         return false;
                     }
+
                 }
 
                 for output in conf.iter().filter(|(_, c)| c.is_some()).map(|(o, _)| o) {
@@ -305,6 +302,7 @@ impl State {
                 for output in conf.iter().filter(|(_, c)| c.is_none()).map(|(o, _)| o) {
                     wlr_configuration::disable_head(output);
                 }
+                state.common.config.write_outputs(state.common.output_conf.outputs());
                 state.common.event_loop_handle.insert_idle(move |state| {
                     state
                         .common