Split off location setting from setup

api/rust/src/output.rs

@@ -199,6 +199,9 @@ impl Output {
                     setup.apply(output, &tag_mod);
                 }
             }
+            if let Some(tag) = output.tags().first() {
+                tag.set_active(true);
+            }
         };
 
         let outputs = self.get_all();
@@ -211,10 +214,18 @@ impl Output {
         })));
     }
 
-    pub fn setup_locs(&self, setup: OutputLocSetup) {
+    pub fn setup_locs(
+        &self,
+        update_locs_on: UpdateLocsOn,
+        setup: impl IntoIterator<Item = (impl ToString, OutputLoc)>,
+    ) {
+        let setup: HashMap<_, _> = setup
+            .into_iter()
+            .map(|(name, align)| (name.to_string(), align))
+            .collect();
+
         let api = self.api.get().unwrap().clone();
         let layout_outputs = move || {
-            let setups = setups_clone.clone().into_iter().collect::<Vec<_>>();
             let outputs = api.output.get_all();
 
             let mut rightmost_output_and_x: Option<(OutputHandle, i32)> = None;
@@ -225,23 +236,19 @@ impl Output {
 
             // Place outputs with OutputSetupLoc::Point
             for output in outputs.iter() {
-                for setup in setups.iter() {
-                    if setup.output.matches(output) {
-                        if let Some(OutputLoc::Point(x, y)) = setup.loc {
-                            output.set_location(x, y);
+                if let Some(&OutputLoc::Point(x, y)) = setup.get(output.name()) {
+                    output.set_location(x, y);
 
-                            placed_outputs.insert(output.clone());
-                            let props = output.props();
-                            let x = props.x.unwrap();
-                            let width = props.logical_width.unwrap() as i32;
-                            if rightmost_output_and_x.is_none()
-                                || rightmost_output_and_x
-                                    .as_ref()
-                                    .is_some_and(|(_, rm_x)| x + width > *rm_x)
-                            {
-                                rightmost_output_and_x = Some((output.clone(), x + width));
-                            }
-                        }
+                    placed_outputs.insert(output.clone());
+                    let props = output.props();
+                    let x = props.x.unwrap();
+                    let width = props.logical_width.unwrap() as i32;
+                    if rightmost_output_and_x.is_none()
+                        || rightmost_output_and_x
+                            .as_ref()
+                            .is_some_and(|(_, rm_x)| x + width > *rm_x)
+                    {
+                        rightmost_output_and_x = Some((output.clone(), x + width));
                     }
                 }
             }
@@ -252,48 +259,48 @@ impl Output {
                 .filter(|op| !placed_outputs.contains(op))
                 .collect::<Vec<_>>()
             {
-                for setup in setups.iter() {
-                    if setup.output.matches(output) && setup.loc.is_none() {
-                        if let Some((rm_op, _)) = rightmost_output_and_x.as_ref() {
-                            output.set_loc_adj_to(rm_op, Alignment::RightAlignTop);
-                        } else {
-                            output.set_location(0, 0);
-                            println!("SET LOC FOR {} TO (0, 0)", output.name());
-                        }
+                if setup.get(output.name()).is_none() {
+                    if let Some((rm_op, _)) = rightmost_output_and_x.as_ref() {
+                        output.set_loc_adj_to(rm_op, Alignment::RightAlignTop);
+                    } else {
+                        output.set_location(0, 0);
+                    }
 
-                        placed_outputs.insert(output.clone());
-                        let props = output.props();
-                        let x = props.x.unwrap();
-                        let width = props.logical_width.unwrap() as i32;
-                        if rightmost_output_and_x.is_none()
-                            || rightmost_output_and_x
-                                .as_ref()
-                                .is_some_and(|(_, rm_x)| x + width > *rm_x)
-                        {
-                            rightmost_output_and_x = Some((output.clone(), x + width));
-                        }
+                    placed_outputs.insert(output.clone());
+                    let props = output.props();
+                    let x = props.x.unwrap();
+                    let width = props.logical_width.unwrap() as i32;
+                    if rightmost_output_and_x.is_none()
+                        || rightmost_output_and_x
+                            .as_ref()
+                            .is_some_and(|(_, rm_x)| x + width > *rm_x)
+                    {
+                        rightmost_output_and_x = Some((output.clone(), x + width));
                     }
                 }
             }
 
             // Attempt to place relative outputs
-            while let Some((output, relative_to, alignment)) = setups.iter().find_map(|setup| {
-                outputs.iter().find_map(|op| {
-                    if !placed_outputs.contains(op) && setup.output.matches(op) {
-                        match &setup.loc {
-                            Some(OutputLoc::RelativeTo(matcher, alignment)) => {
-                                let first_matched_op = outputs
-                                    .iter()
-                                    .find(|o| matcher.matches(o) && placed_outputs.contains(o))?;
-                                Some((op, first_matched_op, alignment))
+            while let Some((output, relative_to, alignment)) =
+                setup.iter().find_map(|(setup_op_name, loc)| {
+                    outputs
+                        .iter()
+                        .find(|setup_op| {
+                            !placed_outputs.contains(setup_op) && setup_op.name() == setup_op_name
+                        })
+                        .and_then(|setup_op| match loc {
+                            OutputLoc::RelativeTo(relative_tos) => {
+                                relative_tos.iter().find_map(|(rel_name, align)| {
+                                    placed_outputs.iter().find_map(|pl_op| {
+                                        (pl_op.name() == rel_name)
+                                            .then_some((setup_op, pl_op, align))
+                                    })
+                                })
                             }
                             _ => None,
-                        }
-                    } else {
-                        None
-                    }
+                        })
                 })
-            }) {
+            {
                 output.set_loc_adj_to(relative_to, *alignment);
 
                 placed_outputs.insert(output.clone());
@@ -349,17 +356,23 @@ impl Output {
         let layout_outputs_clone1 = layout_outputs.clone();
         let layout_outputs_clone2 = layout_outputs.clone();
 
-        self.connect_signal(OutputSignal::Connect(Box::new(move |output| {
-            layout_outputs_clone2();
-        })));
+        if update_locs_on.contains(UpdateLocsOn::CONNECT) {
+            self.connect_signal(OutputSignal::Connect(Box::new(move |_| {
+                layout_outputs_clone2();
+            })));
+        }
 
-        self.connect_signal(OutputSignal::Disconnect(Box::new(move |_| {
-            layout_outputs_clone1();
-        })));
+        if update_locs_on.contains(UpdateLocsOn::DISCONNECT) {
+            self.connect_signal(OutputSignal::Disconnect(Box::new(move |_| {
+                layout_outputs_clone1();
+            })));
+        }
 
-        self.connect_signal(OutputSignal::Resize(Box::new(move |_, _, _| {
-            layout_outputs();
-        })));
+        if update_locs_on.contains(UpdateLocsOn::RESIZE) {
+            self.connect_signal(OutputSignal::Resize(Box::new(move |_, _, _| {
+                layout_outputs();
+            })));
+        }
     }
 }
 
@@ -481,45 +494,36 @@ impl OutputSetup {
             output.set_scale(scale);
         }
         if let Some(tag_names) = &self.tag_names {
-            let tags = tag.add(output, tag_names);
-            if let Some(tag) = tags.first() {
-                tag.set_active(true);
-            }
+            tag.add(output, tag_names);
         }
     }
 }
 
+/// A location for an output.
+#[derive(Clone, Debug)]
 pub enum OutputLoc {
+    /// A specific point in the global space.
     Point(i32, i32),
+    /// A location relative to another output.
+    ///
+    /// This holds a `Vec` of output names to alignments.
+    /// The output that is relative to will be chosen from the first
+    /// connected and placed output in this `Vec`.
     RelativeTo(Vec<(String, Alignment)>),
 }
 
-pub struct OutputLocSetup {
-    update_locs_on: UpdateLocsOn,
-    setup: HashMap<String, OutputLoc>,
-}
-
-impl OutputLocSetup {
-    pub fn new(
-        update_locs_on: UpdateLocsOn,
-        setup: impl IntoIterator<Item = (impl ToString, OutputLoc)>,
-    ) -> Self {
-        Self {
-            update_locs_on,
-            setup: setup
-                .into_iter()
-                .map(|(s, loc)| (s.to_string(), loc))
-                .collect(),
-        }
-    }
-}
-
 impl OutputLoc {
-    pub fn from_point(x: i32, y: i32) -> Self {
-        Self::Point(x, y)
+    /// Creates an `OutputLoc` that will place outputs relative to
+    /// the output with the given name using the given alignment.
+    pub fn relative_to(name: impl ToString, alignment: Alignment) -> Self {
+        Self::RelativeTo(vec![(name.to_string(), alignment)])
     }
 
-    pub fn from_relatives(relatives: impl IntoIterator<Item = (impl ToString, Alignment)>) -> Self {
+    /// Creates an `OutputLoc` from multiple (output_name, alignment) pairs
+    /// that serve as fallbacks.
+    pub fn relative_to_with_fallbacks(
+        relatives: impl IntoIterator<Item = (impl ToString, Alignment)>,
+    ) -> Self {
         Self::RelativeTo(
             relatives
                 .into_iter()

tests/common/mod.rs

@@ -1,9 +1,12 @@
 use std::{panic::UnwindSafe, time::Duration};
 
 use pinnacle::{backend::dummy::setup_dummy, state::State};
-use smithay::reexports::calloop::{
-    self,
-    channel::{Event, Sender},
+use smithay::{
+    output::Output,
+    reexports::calloop::{
+        self,
+        channel::{Event, Sender},
+    },
 };
 
 #[allow(clippy::type_complexity)]
@@ -74,3 +77,12 @@ pub fn test_api(
 
     Ok(())
 }
+
+pub fn output_for_name(state: &State, name: &str) -> Output {
+    state
+        .space
+        .outputs()
+        .find(|op| op.name() == name)
+        .unwrap()
+        .clone()
+}

tests/rust_api.rs

@@ -5,6 +5,7 @@ use std::thread::JoinHandle;
 use pinnacle_api::ApiModules;
 use test_log::test;
 
+use crate::common::output_for_name;
 use crate::common::{sleep_secs, test_api, with_state};
 
 #[tokio::main]
@@ -23,42 +24,27 @@ fn setup_rust(run: impl FnOnce(ApiModules) + Send + 'static) -> JoinHandle<()> {
 }
 
 mod output {
-    use pinnacle_api::output::{Alignment, OutputMatcher, OutputSetup};
-    use smithay::utils::Rectangle;
+    use pinnacle::state::WithState;
+    use pinnacle_api::output::{Alignment, OutputLoc, OutputSetup, UpdateLocsOn};
+    use smithay::{output::Output, utils::Rectangle};
 
     use super::*;
 
     #[tokio::main]
     #[self::test]
     async fn setup() -> anyhow::Result<()> {
-        test_api(|sender| {
-            setup_rust(|api| {
-                api.output
-                    .setup([OutputSetup::new_with_matcher(|_| true).with_tags(["1", "2", "3"])]);
-            });
-
-            sleep_secs(1);
-
-            with_state(&sender, |state| {
-                state.new_output("First", (300, 200).into());
-            });
-        })
-    }
-
-    #[tokio::main]
-    #[self::test]
-    async fn setup_with_cyclic_relative_locs_work() -> anyhow::Result<()> {
         test_api(|sender| {
             setup_rust(|api| {
                 api.output.setup([
-                    OutputSetup::new("Pinnacle Window"),
-                    OutputSetup::new("First").with_relative_loc(
-                        OutputMatcher::Name("Second".into()),
-                        Alignment::RightAlignTop,
-                    ),
-                    OutputSetup::new("Second").with_relative_loc(
-                        OutputMatcher::Name("First".into()),
-                        Alignment::LeftAlignTop,
+                    OutputSetup::new_with_matcher(|_| true).with_tags(["1", "2", "3"]),
+                    OutputSetup::new_with_matcher(|op| op.name().contains("Test"))
+                        .with_tags(["Test 4", "Test 5"]),
+                    OutputSetup::new("Second").with_scale(2.0).with_mode(
+                        pinnacle_api::output::Mode {
+                            pixel_width: 6900,
+                            pixel_height: 420,
+                            refresh_rate_millihertz: 69420,
+                        },
                     ),
                 ]);
             });
@@ -67,24 +53,91 @@ mod output {
 
             with_state(&sender, |state| {
                 state.new_output("First", (300, 200).into());
+                state.new_output("Second", (300, 200).into());
+                state.new_output("Test Third", (300, 200).into());
             });
 
             sleep_secs(1);
 
             with_state(&sender, |state| {
-                let original_op = state
-                    .space
-                    .outputs()
-                    .find(|op| op.name() == "Pinnacle Window")
-                    .unwrap();
-                let first_op = state
-                    .space
-                    .outputs()
-                    .find(|op| op.name() == "First")
-                    .unwrap();
+                let original_op = output_for_name(state, "Pinnacle Window");
+                let first_op = output_for_name(state, "First");
+                let second_op = output_for_name(state, "Second");
+                let test_third_op = output_for_name(state, "Test Third");
 
-                let original_geo = state.space.output_geometry(original_op).unwrap();
-                let first_geo = state.space.output_geometry(first_op).unwrap();
+                let tags_for = |output: &Output| {
+                    output
+                        .with_state(|state| state.tags.iter().map(|t| t.name()).collect::<Vec<_>>())
+                };
+
+                let focused_tags_for = |output: &Output| {
+                    output.with_state(|state| {
+                        state.focused_tags().map(|t| t.name()).collect::<Vec<_>>()
+                    })
+                };
+
+                assert_eq!(tags_for(&original_op), vec!["1", "2", "3"]);
+                assert_eq!(tags_for(&first_op), vec!["1", "2", "3"]);
+                assert_eq!(tags_for(&second_op), vec!["1", "2", "3"]);
+                assert_eq!(
+                    tags_for(&test_third_op),
+                    vec!["1", "2", "3", "Test 4", "Test 5"]
+                );
+
+                assert_eq!(focused_tags_for(&original_op), vec!["1"]);
+                assert_eq!(focused_tags_for(&test_third_op), vec!["1"]);
+
+                assert_eq!(second_op.current_scale().fractional_scale(), 2.0);
+
+                let second_mode = second_op.current_mode().unwrap();
+                assert_eq!(second_mode.size.w, 6900);
+                assert_eq!(second_mode.size.h, 420);
+                assert_eq!(second_mode.refresh, 69420);
+            });
+        })
+    }
+
+    #[tokio::main]
+    #[self::test]
+    async fn setup_loc_with_cyclic_relative_locs_work() -> anyhow::Result<()> {
+        test_api(|sender| {
+            setup_rust(|api| {
+                api.output.setup_locs(
+                    UpdateLocsOn::all(),
+                    [
+                        ("Pinnacle Window", OutputLoc::Point(0, 0)),
+                        (
+                            "First",
+                            OutputLoc::relative_to_with_fallbacks([(
+                                "Second",
+                                Alignment::LeftAlignTop,
+                            )]),
+                        ),
+                        (
+                            "Second",
+                            OutputLoc::relative_to_with_fallbacks([(
+                                "First",
+                                Alignment::RightAlignTop,
+                            )]),
+                        ),
+                    ],
+                );
+            });
+
+            sleep_secs(1);
+
+            with_state(&sender, |state| {
+                state.new_output("First", (300, 200).into());
+            });
+
+            sleep_secs(1);
+
+            with_state(&sender, |state| {
+                let original_op = output_for_name(state, "Pinnacle Window");
+                let first_op = output_for_name(state, "First");
+
+                let original_geo = state.space.output_geometry(&original_op).unwrap();
+                let first_geo = state.space.output_geometry(&first_op).unwrap();
 
                 assert_eq!(
                     original_geo,
@@ -101,25 +154,13 @@ mod output {
             sleep_secs(1);
 
             with_state(&sender, |state| {
-                let original_op = state
-                    .space
-                    .outputs()
-                    .find(|op| op.name() == "Pinnacle Window")
-                    .unwrap();
-                let first_op = state
-                    .space
-                    .outputs()
-                    .find(|op| op.name() == "First")
-                    .unwrap();
-                let second_op = state
-                    .space
-                    .outputs()
-                    .find(|op| op.name() == "Second")
-                    .unwrap();
+                let original_op = output_for_name(state, "Pinnacle Window");
+                let first_op = output_for_name(state, "First");
+                let second_op = output_for_name(state, "Second");
 
-                let original_geo = state.space.output_geometry(original_op).unwrap();
-                let first_geo = state.space.output_geometry(first_op).unwrap();
-                let second_geo = state.space.output_geometry(second_op).unwrap();
+                let original_geo = state.space.output_geometry(&original_op).unwrap();
+                let first_geo = state.space.output_geometry(&first_op).unwrap();
+                let second_geo = state.space.output_geometry(&second_op).unwrap();
 
                 assert_eq!(
                     original_geo,
@@ -139,45 +180,97 @@ mod output {
 
     #[tokio::main]
     #[self::test]
-    async fn setup_with_relative_output_matcher_works() -> anyhow::Result<()> {
+    async fn setup_loc_with_relative_locs_with_more_than_one_relative_works() -> anyhow::Result<()>
+    {
         test_api(|sender| {
             setup_rust(|api| {
-                api.output.setup([
-                    OutputSetup::new("Pinnacle Window"),
-                    OutputSetup::new_with_matcher(|_| true)
-                        .with_relative_loc(|_: &_| true, Alignment::BottomAlignLeft),
-                ]);
+                api.output.setup_locs(
+                    UpdateLocsOn::all(),
+                    [
+                        ("Pinnacle Window", OutputLoc::Point(0, 0)),
+                        (
+                            "First",
+                            OutputLoc::relative_to("Pinnacle Window", Alignment::BottomAlignLeft),
+                        ),
+                        (
+                            "Second",
+                            OutputLoc::relative_to("First", Alignment::BottomAlignLeft),
+                        ),
+                        (
+                            "Third",
+                            OutputLoc::relative_to_with_fallbacks([
+                                ("Second", Alignment::BottomAlignLeft),
+                                ("First", Alignment::BottomAlignLeft),
+                            ]),
+                        ),
+                    ],
+                );
             });
 
             sleep_secs(1);
 
             with_state(&sender, |state| {
                 state.new_output("First", (300, 200).into());
-                state.new_output("Second", (400, 600).into());
-                state.new_output("Third", (400, 300).into());
+                state.new_output("Second", (300, 700).into());
+                state.new_output("Third", (300, 400).into());
             });
 
             sleep_secs(1);
 
             with_state(&sender, |state| {
-                for output in state.space.outputs() {
-                    let geo = state.space.output_geometry(output).unwrap();
-                    match output.name().as_str() {
-                        "Pinnacle Window" => {
-                            assert_eq!(geo, Rectangle::from_loc_and_size((0, 0), (1920, 1080)));
-                        }
-                        "First" => {
-                            assert_eq!(geo, Rectangle::from_loc_and_size((0, 1080), (300, 200)));
-                        }
-                        "Second" => {
-                            assert_eq!(geo, Rectangle::from_loc_and_size((0, 1080), (400, 600)));
-                        }
-                        "Third" => {
-                            assert_eq!(geo, Rectangle::from_loc_and_size((0, 1080), (400, 300)));
-                        }
-                        _ => unreachable!(),
-                    }
-                }
+                let original_op = output_for_name(state, "Pinnacle Window");
+                let first_op = output_for_name(state, "First");
+                let second_op = output_for_name(state, "Second");
+                let third_op = output_for_name(state, "Third");
+
+                let original_geo = state.space.output_geometry(&original_op).unwrap();
+                let first_geo = state.space.output_geometry(&first_op).unwrap();
+                let second_geo = state.space.output_geometry(&second_op).unwrap();
+                let third_geo = state.space.output_geometry(&third_op).unwrap();
+
+                assert_eq!(
+                    original_geo,
+                    Rectangle::from_loc_and_size((0, 0), (1920, 1080))
+                );
+                assert_eq!(
+                    first_geo,
+                    Rectangle::from_loc_and_size((0, 1080), (300, 200))
+                );
+                assert_eq!(
+                    second_geo,
+                    Rectangle::from_loc_and_size((0, 1080 + 200), (300, 700))
+                );
+                assert_eq!(
+                    third_geo,
+                    Rectangle::from_loc_and_size((0, 1080 + 200 + 700), (300, 400))
+                );
+
+                state.remove_output(&second_op);
+            });
+
+            sleep_secs(1);
+
+            with_state(&sender, |state| {
+                let original_op = output_for_name(state, "Pinnacle Window");
+                let first_op = output_for_name(state, "First");
+                let third_op = output_for_name(state, "Third");
+
+                let original_geo = state.space.output_geometry(&original_op).unwrap();
+                let first_geo = state.space.output_geometry(&first_op).unwrap();
+                let third_geo = state.space.output_geometry(&third_op).unwrap();
+
+                assert_eq!(
+                    original_geo,
+                    Rectangle::from_loc_and_size((0, 0), (1920, 1080))
+                );
+                assert_eq!(
+                    first_geo,
+                    Rectangle::from_loc_and_size((0, 1080), (300, 200))
+                );
+                assert_eq!(
+                    third_geo,
+                    Rectangle::from_loc_and_size((0, 1080 + 200), (300, 400))
+                );
             });
         })
     }