Always clamp non-auto window height with >1 windows in column

niri-visual-tests/src/test_window.rs

@@ -87,7 +87,7 @@ impl TestWindow {
 
         let mut new_size = inner.size;
 
-        if let Some(size) = inner.requested_size.take() {
+        if let Some(size) = inner.requested_size {
             assert!(size.w >= 0);
             assert!(size.h >= 0);
 
@@ -236,6 +236,10 @@ impl LayoutElement for TestWindow {
         self.inner.borrow().pending_fullscreen
     }
 
+    fn requested_size(&self) -> Option<Size<i32, Logical>> {
+        self.inner.borrow().requested_size
+    }
+
     fn refresh(&self) {}
 
     fn rules(&self) -> &ResolvedWindowRules {

src/layout/mod.rs

@@ -189,6 +189,9 @@ pub trait LayoutElement {
     /// This *will* switch immediately after a [`LayoutElement::request_fullscreen()`] call.
     fn is_pending_fullscreen(&self) -> bool;
 
+    /// Size previously requested through [`LayoutElement::request_size()`].
+    fn requested_size(&self) -> Option<Size<i32, Logical>>;
+
     fn rules(&self) -> &ResolvedWindowRules;
 
     /// Runs periodic clean-up tasks.
@@ -2777,7 +2780,7 @@ mod tests {
         }
 
         fn communicate(&self) -> bool {
-            if let Some(size) = self.0.requested_size.take() {
+            if let Some(size) = self.0.requested_size.get() {
                 assert!(size.w >= 0);
                 assert!(size.h >= 0);
 
@@ -2887,6 +2890,10 @@ mod tests {
             self.0.pending_fullscreen.get()
         }
 
+        fn requested_size(&self) -> Option<Size<i32, Logical>> {
+            self.0.requested_size.get()
+        }
+
         fn refresh(&self) {}
 
         fn rules(&self) -> &ResolvedWindowRules {
@@ -4696,6 +4703,36 @@ mod tests {
         check_ops(&ops);
     }
 
+    #[test]
+    fn fixed_height_takes_max_non_auto_into_account() {
+        let ops = [
+            Op::AddOutput(1),
+            Op::AddWindow {
+                id: 0,
+                bbox: Rectangle::from_loc_and_size((0, 0), (100, 200)),
+                min_max_size: Default::default(),
+            },
+            Op::SetWindowHeight { id: Some(0), change: SizeChange::SetFixed(704) },
+            Op::AddWindow {
+                id: 1,
+                bbox: Rectangle::from_loc_and_size((0, 0), (100, 200)),
+                min_max_size: Default::default(),
+            },
+            Op::ConsumeOrExpelWindowLeft,
+        ];
+
+        let options = Options {
+            border: niri_config::Border {
+                off: false,
+                width: niri_config::FloatOrInt(4.),
+                ..Default::default()
+            },
+            gaps: 0.,
+            ..Default::default()
+        };
+        check_ops_with_options(options, &ops);
+    }
+
     fn arbitrary_spacing() -> impl Strategy<Value = f64> {
         // Give equal weight to:
         // - 0: the element is disabled

src/layout/workspace.rs

@@ -3272,12 +3272,45 @@ impl<W: LayoutElement> Column<W> {
         let width = f64::max(f64::min(width, max_width), min_width);
         let height = self.working_area.size.h;
 
+        // If there are multiple windows in a column, clamp the non-auto window's height according
+        // to other windows' min sizes.
+        let mut max_non_auto_window_height = None;
+        if self.tiles.len() > 1 {
+            if let Some(non_auto_idx) = self
+                .data
+                .iter()
+                .position(|data| !matches!(data.height, WindowHeight::Auto { .. }))
+            {
+                let min_height_taken = min_size
+                    .iter()
+                    .enumerate()
+                    .filter(|(idx, _)| *idx != non_auto_idx)
+                    .map(|(_, min_size)| min_size.h + self.options.gaps)
+                    .sum::<f64>();
+
+                let tile = &self.tiles[non_auto_idx];
+                let height_left = self.working_area.size.h
+                    - self.options.gaps
+                    - min_height_taken
+                    - self.options.gaps;
+                max_non_auto_window_height = Some(f64::max(
+                    1.,
+                    tile.window_height_for_tile_height(height_left).round(),
+                ));
+            }
+        }
+
         // Compute the tile heights. Start by converting window heights to tile heights.
         let mut heights = zip(&self.tiles, &self.data)
             .map(|(tile, data)| match data.height {
                 auto @ WindowHeight::Auto { .. } => auto,
                 WindowHeight::Fixed(height) => {
-                    WindowHeight::Fixed(tile.tile_height_for_window_height(height.round().max(1.)))
+                    let mut window_height = height.round().max(1.);
+                    if let Some(max) = max_non_auto_window_height {
+                        window_height = f64::min(window_height, max);
+                    }
+
+                    WindowHeight::Fixed(tile.tile_height_for_window_height(window_height))
                 }
                 WindowHeight::Preset(idx) => {
                     let preset = self.options.preset_window_heights[idx];
@@ -3285,8 +3318,13 @@ impl<W: LayoutElement> Column<W> {
                         ResolvedSize::Tile(h) => tile.window_height_for_tile_height(h),
                         ResolvedSize::Window(h) => h,
                     };
-                    let tile_height = tile
+
-                        .tile_height_for_window_height(window_height.round().clamp(1., 100000.));
+                    let mut window_height = window_height.round().clamp(1., 100000.);
+                    if let Some(max) = max_non_auto_window_height {
+                        window_height = f64::min(window_height, max);
+                    }
+
+                    let tile_height = tile.tile_height_for_window_height(window_height);
                     WindowHeight::Fixed(tile_height)
                 }
             })
@@ -3488,7 +3526,8 @@ impl<W: LayoutElement> Column<W> {
             assert_eq!(self.tiles.len(), 1);
         }
 
-        if self.tiles.len() == 1 {
+        let tile_count = self.tiles.len();
+        if tile_count == 1 {
             if let WindowHeight::Auto { weight } = self.data[0].height {
                 assert_eq!(
                     weight, 1.,
@@ -3498,6 +3537,8 @@ impl<W: LayoutElement> Column<W> {
         }
 
         let mut found_fixed = false;
+        let mut total_height = 0.;
+        let mut total_min_height = 0.;
         for (tile, data) in zip(&self.tiles, &self.data) {
             assert!(Rc::ptr_eq(&self.options, &tile.options));
             assert_eq!(self.scale, tile.scale());
@@ -3524,6 +3565,25 @@ impl<W: LayoutElement> Column<W> {
             if let WindowHeight::Preset(idx) = data.height {
                 assert!(self.options.preset_window_heights.len() > idx);
             }
+
+            let requested_size = tile.window().requested_size().unwrap();
+            total_height += tile.tile_height_for_window_height(f64::from(requested_size.h));
+            total_min_height += f64::max(1., tile.min_size().h);
+        }
+
+        if tile_count > 1
+            && self.scale.round() == self.scale
+            && self.working_area.size.h.round() == self.working_area.size.h
+            && self.options.gaps.round() == self.options.gaps
+        {
+            total_height += self.options.gaps * (tile_count + 1) as f64;
+            total_min_height += self.options.gaps * (tile_count + 1) as f64;
+            let max_height = f64::max(total_min_height, self.working_area.size.h);
+            assert!(
+                total_height <= max_height,
+                "multiple tiles in a column mustn't go beyond working area height \
+                 (total height {total_height} > max height {max_height})"
+            );
         }
     }
 

src/window/mapped.rs

@@ -730,6 +730,10 @@ impl LayoutElement for Mapped {
             .with_pending_state(|state| state.states.contains(xdg_toplevel::State::Fullscreen))
     }
 
+    fn requested_size(&self) -> Option<Size<i32, Logical>> {
+        self.toplevel().with_pending_state(|state| state.size)
+    }
+
     fn refresh(&self) {
         self.window.refresh();
     }