From b7cc3f3a62930b233b36a5a51cf73c7a1be08bff Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Sun, 2 Mar 2025 13:18:12 +0100 Subject: [PATCH] fix(panes): various fixes for rendering stacked panes without pane frames (#4035) * initial work * fix rendering issues with stacked panes without pane frames * make mouse clicking work * test: rendering stacked panes without frames * style(fmt): rustfmt * docs(changelog): update pr --- CHANGELOG.md | 1 + zellij-server/src/panes/plugin_pane.rs | 12 +- zellij-server/src/panes/terminal_pane.rs | 4 + zellij-server/src/panes/tiled_panes/mod.rs | 70 ++++++-- .../src/panes/tiled_panes/stacked_panes.rs | 32 ++++ zellij-server/src/screen.rs | 38 +++-- zellij-server/src/tab/mod.rs | 54 +++++- ...ts__render_stacks_without_pane_frames.snap | 46 +++++ .../src/tab/unit/tab_integration_tests.rs | 159 +++++++++++++++++- zellij-server/src/ui/boundaries.rs | 67 ++++++-- zellij-server/src/ui/pane_boundaries_frame.rs | 24 ++- zellij-server/src/ui/pane_contents_and_ui.rs | 12 +- zellij-utils/src/pane_size.rs | 16 ++ 13 files changed, 479 insertions(+), 56 deletions(-) create mode 100644 zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__render_stacks_without_pane_frames.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 02174025..6ea2a2c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) * chore(repo): update some dependencies (https://github.com/zellij-org/zellij/pull/4019) * fix(grid): reap sixel images on clear (https://github.com/zellij-org/zellij/pull/3982) * chore(repo): remove compile warnings (https://github.com/zellij-org/zellij/pull/4026) +* fix(panes): properly render stacked panes when pane frames are disabled (https://github.com/zellij-org/zellij/pull/4035) ## [0.41.2] - 2024-11-19 * fix(input): keypresses not being identified properly with kitty keyboard protocol in some terminals (https://github.com/zellij-org/zellij/pull/3725) diff --git a/zellij-server/src/panes/plugin_pane.rs b/zellij-server/src/panes/plugin_pane.rs index 276c2c33..3bfe9c24 100644 --- a/zellij-server/src/panes/plugin_pane.rs +++ b/zellij-server/src/panes/plugin_pane.rs @@ -412,13 +412,7 @@ impl Pane for PluginPane { self.pane_name.clone() }; - let mut frame_geom = self.current_geom(); - if !frame_params.should_draw_pane_frames { - // in this case the width of the frame needs not include the pane corners - frame_geom - .cols - .set_inner(frame_geom.cols.as_usize().saturating_sub(1)); - } + let frame_geom = self.current_geom(); let is_pinned = frame_geom.is_pinned; let mut frame = PaneFrame::new( frame_geom.into(), @@ -613,6 +607,10 @@ impl Pane for PluginPane { self.resize_grids(); } + fn get_content_offset(&self) -> Offset { + self.content_offset + } + fn store_pane_name(&mut self) { if self.pane_name != self.prev_pane_name { self.prev_pane_name = self.pane_name.clone() diff --git a/zellij-server/src/panes/terminal_pane.rs b/zellij-server/src/panes/terminal_pane.rs index 186b8ac5..e1de0e89 100644 --- a/zellij-server/src/panes/terminal_pane.rs +++ b/zellij-server/src/panes/terminal_pane.rs @@ -612,6 +612,10 @@ impl Pane for TerminalPane { self.reflow_lines(); } + fn get_content_offset(&self) -> Offset { + self.content_offset + } + fn store_pane_name(&mut self) { if self.pane_name != self.prev_pane_name { self.prev_pane_name = self.pane_name.clone() diff --git a/zellij-server/src/panes/tiled_panes/mod.rs b/zellij-server/src/panes/tiled_panes/mod.rs index 42bbe7f7..4ca428b9 100644 --- a/zellij-server/src/panes/tiled_panes/mod.rs +++ b/zellij-server/src/panes/tiled_panes/mod.rs @@ -443,6 +443,11 @@ impl TiledPanes { pub fn set_pane_frames(&mut self, draw_pane_frames: bool) { self.draw_pane_frames = draw_pane_frames; let viewport = *self.viewport.borrow(); + let position_and_sizes_of_stacks = { + StackedPanes::new_from_btreemap(&mut self.panes, &self.panes_to_hide) + .positions_and_sizes_of_all_stacks() + .unwrap_or_else(|| Default::default()) + }; for pane in self.panes.values_mut() { if !pane.borderless() { pane.set_frame(draw_pane_frames); @@ -464,12 +469,27 @@ impl TiledPanes { // no draw_pane_frames and this pane should have a separation to other panes // according to its position in the viewport (eg. no separation if its at the // viewport bottom) - offset its content accordingly - let position_and_size = pane.current_geom(); + let mut position_and_size = pane.current_geom(); + let is_stacked = position_and_size.is_stacked(); + let is_flexible = !position_and_size.rows.is_fixed(); + if let Some(position_and_size_of_stack) = position_and_size + .stacked + .and_then(|s_id| position_and_sizes_of_stacks.get(&s_id)) + { + // we want to check the offset against the position_and_size of the whole + // stack rather than the pane, because the stack needs to have a consistent + // offset with itself + position_and_size = *position_and_size_of_stack; + }; let (pane_columns_offset, pane_rows_offset) = pane_content_offset(&position_and_size, &viewport); - if !draw_pane_frames && pane.current_geom().is_stacked() { - // stacked panes should always leave 1 top row for a title - pane.set_content_offset(Offset::shift_right_and_top(pane_columns_offset, 1)); + if is_stacked && is_flexible { + // 1 to save room for the pane title + pane.set_content_offset(Offset::shift_right_top_and_bottom( + pane_columns_offset, + 1, + pane_rows_offset, + )); } else { pane.set_content_offset(Offset::shift(pane_rows_offset, pane_columns_offset)); } @@ -848,10 +868,14 @@ impl TiledPanes { .collect() }; let (stacked_pane_ids_under_flexible_pane, stacked_pane_ids_over_flexible_pane) = { - // TODO: do not recalculate this every time on render StackedPanes::new_from_btreemap(&mut self.panes, &self.panes_to_hide) .stacked_pane_ids_under_and_over_flexible_panes() - .unwrap() // TODO: no unwrap + .with_context(err_context)? + }; + let (stacked_pane_ids_on_top_of_stacks, stacked_pane_ids_on_bottom_of_stacks) = { + StackedPanes::new_from_btreemap(&mut self.panes, &self.panes_to_hide) + .stacked_pane_ids_on_top_and_bottom_of_stacks() + .with_context(err_context)? }; for (kind, pane) in self.panes.iter_mut() { if !self.panes_to_hide.contains(&pane.pid()) { @@ -859,8 +883,14 @@ impl TiledPanes { stacked_pane_ids_under_flexible_pane.contains(&pane.pid()); let pane_is_stacked_over = stacked_pane_ids_over_flexible_pane.contains(&pane.pid()); + let pane_is_on_top_of_stack = + stacked_pane_ids_on_top_of_stacks.contains(&pane.pid()); + let pane_is_on_bottom_of_stack = + stacked_pane_ids_on_bottom_of_stacks.contains(&pane.pid()); let should_draw_pane_frames = self.draw_pane_frames; let pane_is_stacked = pane.current_geom().is_stacked(); + let pane_is_one_liner_in_stack = + pane_is_stacked && pane.current_geom().rows.is_fixed(); let mut pane_contents_and_ui = PaneContentsAndUi::new( pane, output, @@ -882,9 +912,11 @@ impl TiledPanes { let err_context = || format!("failed to render tiled panes for client {client_id}"); if let PaneId::Plugin(..) = kind { - pane_contents_and_ui - .render_pane_contents_for_client(*client_id) - .with_context(err_context)?; + if !pane_is_one_liner_in_stack { + pane_contents_and_ui + .render_pane_contents_for_client(*client_id) + .with_context(err_context)?; + } } let is_floating = false; if self.draw_pane_frames { @@ -916,6 +948,8 @@ impl TiledPanes { client_mode, boundaries, self.session_is_mirrored, + pane_is_on_top_of_stack, + pane_is_on_bottom_of_stack, ); } else { let boundaries = client_id_to_boundaries @@ -926,6 +960,8 @@ impl TiledPanes { client_mode, boundaries, self.session_is_mirrored, + pane_is_on_top_of_stack, + pane_is_on_bottom_of_stack, ); } pane_contents_and_ui.render_terminal_title_if_needed( @@ -940,9 +976,13 @@ impl TiledPanes { .with_context(err_context)?; } if let PaneId::Terminal(..) = kind { - pane_contents_and_ui - .render_pane_contents_to_multiple_clients(connected_clients.iter().copied()) - .with_context(err_context)?; + if !pane_is_one_liner_in_stack { + pane_contents_and_ui + .render_pane_contents_to_multiple_clients( + connected_clients.iter().copied(), + ) + .with_context(err_context)?; + } } } } @@ -2426,6 +2466,12 @@ impl TiledPanes { fn is_connected(&self, client_id: &ClientId) -> bool { self.connected_clients.borrow().contains(&client_id) } + pub fn stacked_pane_ids_under_and_over_flexible_panes( + &mut self, + ) -> Result<(HashSet, HashSet)> { + StackedPanes::new_from_btreemap(&mut self.panes, &self.panes_to_hide) + .stacked_pane_ids_under_and_over_flexible_panes() + } } #[allow(clippy::borrowed_box)] diff --git a/zellij-server/src/panes/tiled_panes/stacked_panes.rs b/zellij-server/src/panes/tiled_panes/stacked_panes.rs index 81d15ea3..ecdfea71 100644 --- a/zellij-server/src/panes/tiled_panes/stacked_panes.rs +++ b/zellij-server/src/panes/tiled_panes/stacked_panes.rs @@ -416,6 +416,25 @@ impl<'a> StackedPanes<'a> { stacked_pane_ids_over_flexible_panes, )) } + pub fn stacked_pane_ids_on_top_and_bottom_of_stacks( + &self, + ) -> Result<(HashSet, HashSet)> { + let mut stacked_pane_ids_on_top_of_stacks = HashSet::new(); + let mut stacked_pane_ids_on_bottom_of_stacks = HashSet::new(); + let all_stacks = self.get_all_stacks()?; + for stack in all_stacks { + if let Some((first_pane_id, _pane)) = stack.iter().next() { + stacked_pane_ids_on_top_of_stacks.insert(*first_pane_id); + } + if let Some((last_pane_id, _pane)) = stack.iter().last() { + stacked_pane_ids_on_bottom_of_stacks.insert(*last_pane_id); + } + } + Ok(( + stacked_pane_ids_on_top_of_stacks, + stacked_pane_ids_on_bottom_of_stacks, + )) + } pub fn make_room_for_new_pane(&mut self) -> Result { let err_context = || format!("Failed to add pane to stack"); let all_stacks = self.get_all_stacks()?; @@ -804,6 +823,19 @@ impl<'a> StackedPanes<'a> { } highest_stack_id } + pub fn positions_and_sizes_of_all_stacks(&self) -> Option> { + let panes = self.panes.borrow(); + let mut positions_and_sizes_of_all_stacks = HashMap::new(); + for pane in panes.values() { + if let Some(stack_id) = pane.current_geom().stacked { + if !positions_and_sizes_of_all_stacks.contains_key(&stack_id) { + positions_and_sizes_of_all_stacks + .insert(stack_id, self.position_and_size_of_stack(&pane.pid())?); + } + } + } + Some(positions_and_sizes_of_all_stacks) + } fn reset_stack_size( &self, new_position_and_size_of_stack: &PaneGeom, diff --git a/zellij-server/src/screen.rs b/zellij-server/src/screen.rs index 16cc3d39..ace0dea7 100644 --- a/zellij-server/src/screen.rs +++ b/zellij-server/src/screen.rs @@ -3859,23 +3859,31 @@ pub(crate) fn screen_thread_main( screen.unblock_input()?; }, ScreenInstruction::MouseEvent(event, client_id) => { - let mouse_effect = screen + match screen .get_active_tab_mut(client_id) - .and_then(|tab| tab.handle_mouse_event(&event, client_id))?; - if mouse_effect.state_changed { - screen.log_and_report_session_state()?; + .and_then(|tab| tab.handle_mouse_event(&event, client_id)) + { + Ok(mouse_effect) => { + if mouse_effect.state_changed { + screen.log_and_report_session_state()?; + } + if !mouse_effect.leave_clipboard_message { + let _ = + screen + .bus + .senders + .send_to_plugin(PluginInstruction::Update(vec![( + None, + Some(client_id), + Event::InputReceived, + )])); + } + screen.render(None).non_fatal(); + }, + Err(e) => { + log::error!("Failed to process MouseEvent: {}", e); + }, } - if !mouse_effect.leave_clipboard_message { - let _ = screen - .bus - .senders - .send_to_plugin(PluginInstruction::Update(vec![( - None, - Some(client_id), - Event::InputReceived, - )])); - } - screen.render(None)?; }, ScreenInstruction::Copy(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab diff --git a/zellij-server/src/tab/mod.rs b/zellij-server/src/tab/mod.rs index b760c447..02c19987 100644 --- a/zellij-server/src/tab/mod.rs +++ b/zellij-server/src/tab/mod.rs @@ -308,6 +308,9 @@ pub trait Pane { fn set_active_at(&mut self, instant: Instant); fn set_frame(&mut self, frame: bool); fn set_content_offset(&mut self, offset: Offset); + fn get_content_offset(&self) -> Offset { + Offset::default() + } fn cursor_shape_csi(&self) -> String { "\u{1b}[0 q".to_string() // default to non blinking block } @@ -328,9 +331,15 @@ pub trait Pane { fn right_boundary_x_coords(&self) -> usize { self.x() + self.cols() } + fn right_boundary_x_content_coords(&self) -> usize { + self.get_content_x() + self.get_content_columns() + } fn bottom_boundary_y_coords(&self) -> usize { self.y() + self.rows() } + fn bottom_boundary_y_content_coords(&self) -> usize { + self.get_content_y() + self.get_content_rows() + } fn is_right_of(&self, other: &dyn Pane) -> bool { self.x() > other.x() } @@ -3350,7 +3359,11 @@ impl Tab { } } - fn get_pane_id_at(&self, point: &Position, search_selectable: bool) -> Result> { + fn get_pane_id_at( + &mut self, + point: &Position, + search_selectable: bool, + ) -> Result> { let err_context = || format!("failed to get id of pane at position {point:?}"); if self.tiled_panes.fullscreen_is_active() @@ -3368,15 +3381,50 @@ impl Tab { .with_context(err_context)?; return Ok(self.tiled_panes.get_active_pane_id(first_client_id)); } + + let (stacked_pane_ids_under_flexible_pane, _stacked_pane_ids_over_flexible_pane) = { + self.tiled_panes + .stacked_pane_ids_under_and_over_flexible_panes() + .with_context(err_context)? + }; + let pane_contains_point = |p: &Box, + point: &Position, + stacked_pane_ids_under_flexible_pane: &HashSet| + -> bool { + let is_flexible_in_stack = + p.current_geom().is_stacked() && !p.current_geom().rows.is_fixed(); + let is_stacked_under = stacked_pane_ids_under_flexible_pane.contains(&p.pid()); + let geom_to_compare_against = if is_stacked_under && !self.draw_pane_frames { + // these sort of panes are one-liner panes under a flexible pane in a stack when we + // don't draw pane frames - because the whole stack's content is offset to allow + // room for the boundary between panes, they are actually drawn 1 line above where + // they are + let mut geom = p.current_geom(); + geom.y = geom.y.saturating_sub(p.get_content_offset().bottom); + geom + } else if is_flexible_in_stack && !self.draw_pane_frames { + // these sorts of panes are flexible panes inside a stack when we don't draw pane + // frames - because the whole stack's content is offset to give room for the + // boundary between panes, we need to take this offset into account when figuring + // out whether the position is inside them + let mut geom = p.current_geom(); + geom.rows.decrease_inner(p.get_content_offset().bottom); + geom + } else { + p.current_geom() + }; + geom_to_compare_against.contains(point) + }; + if search_selectable { Ok(self .get_selectable_tiled_panes() - .find(|(_, p)| p.contains(point)) + .find(|(_, p)| pane_contains_point(p, point, &stacked_pane_ids_under_flexible_pane)) .map(|(&id, _)| id)) } else { Ok(self .get_tiled_panes() - .find(|(_, p)| p.contains(point)) + .find(|(_, p)| pane_contains_point(p, point, &stacked_pane_ids_under_flexible_pane)) .map(|(&id, _)| id)) } } diff --git a/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__render_stacks_without_pane_frames.snap b/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__render_stacks_without_pane_frames.snap new file mode 100644 index 00000000..8ce91b92 --- /dev/null +++ b/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__render_stacks_without_pane_frames.snap @@ -0,0 +1,46 @@ +--- +source: zellij-server/src/tab/./unit/tab_integration_tests.rs +assertion_line: 1134 +expression: snapshot +--- +00 (C): ─ Pane #1 ────────────────────────────────────────────┼─ Pane #7 ─────────────────────────────────── +01 (C): ─ Pane #9 ────────────────────────────────────────────┤ +02 (C): ─ Pane #10 ───────────────────────────────────────────┤ +03 (C): │ +04 (C): │ +05 (C): │ +06 (C): │ +07 (C): │ +08 (C): │ +09 (C): │ +10 (C): │ +11 (C): │ +12 (C): │ +13 (C): │ +14 (C): │ +15 (C): │ +16 (C): │ +17 (C): │ +18 (C): ├─ Pane #8 ─────────────────────────────────── +19 (C): ─────────────────────────────────────────────────┬────┴───────────────────────────────────────────── +20 (C): ─ Pane #2 ───────────────────────────────────────┼─ Pane #4 ──────────────────────────────────────── +21 (C): ─ Pane #3 ───────────────────────────────────────┼─ Pane #5 ──────────────────────────────────────── +22 (C): │ +23 (C): │ +24 (C): │ +25 (C): │ +26 (C): │ +27 (C): │ +28 (C): │ +29 (C): │ +30 (C): │ +31 (C): │ +32 (C): │ +33 (C): │ +34 (C): │ +35 (C): │ +36 (C): │ +37 (C): │ +38 (C): │ +39 (C): ├─ Pane #6 ──────────────────────────────────────── + diff --git a/zellij-server/src/tab/unit/tab_integration_tests.rs b/zellij-server/src/tab/unit/tab_integration_tests.rs index 6e2bca6c..743d99f1 100644 --- a/zellij-server/src/tab/unit/tab_integration_tests.rs +++ b/zellij-server/src/tab/unit/tab_integration_tests.rs @@ -201,7 +201,6 @@ impl MockPtyInstructionBus { } } -// TODO: move to shared thingy with other test file fn create_new_tab(size: Size, default_mode: ModeInfo) -> Tab { set_session_name("test".into()); let index = 0; @@ -270,6 +269,74 @@ fn create_new_tab(size: Size, default_mode: ModeInfo) -> Tab { tab } +fn create_new_tab_without_pane_frames(size: Size, default_mode: ModeInfo) -> Tab { + set_session_name("test".into()); + let index = 0; + let position = 0; + let name = String::new(); + let os_api = Box::new(FakeInputOutput::default()); + let senders = ThreadSenders::default().silently_fail_on_send(); + let max_panes = None; + let mode_info = default_mode; + let style = Style::default(); + let draw_pane_frames = false; + let auto_layout = true; + let client_id = 1; + let session_is_mirrored = true; + let mut connected_clients = HashSet::new(); + connected_clients.insert(client_id); + let connected_clients = Rc::new(RefCell::new(connected_clients)); + let character_cell_info = Rc::new(RefCell::new(None)); + let stacked_resize = Rc::new(RefCell::new(true)); + let terminal_emulator_colors = Rc::new(RefCell::new(Palette::default())); + let copy_options = CopyOptions::default(); + let terminal_emulator_color_codes = Rc::new(RefCell::new(HashMap::new())); + let sixel_image_store = Rc::new(RefCell::new(SixelImageStore::default())); + let debug = false; + let arrow_fonts = true; + let styled_underlines = true; + let explicitly_disable_kitty_keyboard_protocol = false; + let mut tab = Tab::new( + index, + position, + name, + size, + character_cell_info, + stacked_resize, + sixel_image_store, + os_api, + senders, + max_panes, + style, + mode_info, + draw_pane_frames, + auto_layout, + connected_clients, + session_is_mirrored, + Some(client_id), + copy_options, + terminal_emulator_colors, + terminal_emulator_color_codes, + (vec![], vec![]), + PathBuf::from("my_default_shell"), + debug, + arrow_fonts, + styled_underlines, + explicitly_disable_kitty_keyboard_protocol, + None, + ); + tab.apply_layout( + TiledPaneLayout::default(), + vec![], + vec![(1, None)], + vec![], + HashMap::new(), + client_id, + ) + .unwrap(); + tab +} + fn create_new_tab_with_swap_layouts( size: Size, default_mode: ModeInfo, @@ -980,6 +1047,96 @@ fn split_stack_horizontally() { assert_snapshot!(snapshot); } +#[test] +fn render_stacks_without_pane_frames() { + // this checks various cases and gotchas that have to do with rendering stacked panes when we + // don't draw frames around panes + let size = Size { + cols: 100, + rows: 40, + }; + let client_id = 1; + let mut tab = create_new_tab_without_pane_frames(size, ModeInfo::default()); + let mut output = Output::default(); + for i in 2..4 { + let new_pane_id_1 = PaneId::Terminal(i); + tab.new_pane( + new_pane_id_1, + None, + None, + None, + None, + false, + Some(client_id), + ) + .unwrap(); + } + // the below resizes will end up stacking the panes + tab.resize(client_id, ResizeStrategy::new(Resize::Increase, None)) + .unwrap(); + tab.resize(client_id, ResizeStrategy::new(Resize::Increase, None)) + .unwrap(); + tab.vertical_split(PaneId::Terminal(4), None, client_id) + .unwrap(); + for i in 5..7 { + let new_pane_id_1 = PaneId::Terminal(i); + tab.new_pane( + new_pane_id_1, + None, + None, + None, + None, + false, + Some(client_id), + ) + .unwrap(); + } + tab.focus_pane_with_id(PaneId::Terminal(1), false, client_id); + for i in 7..9 { + let new_pane_id_1 = PaneId::Terminal(i); + tab.new_pane( + new_pane_id_1, + None, + None, + None, + None, + false, + Some(client_id), + ) + .unwrap(); + } + let _ = tab.focus_pane_with_id(PaneId::Terminal(1), false, client_id); + for i in 9..11 { + let new_pane_id_1 = PaneId::Terminal(i); + tab.new_pane( + new_pane_id_1, + None, + None, + None, + None, + false, + Some(client_id), + ) + .unwrap(); + } + tab.resize( + client_id, + ResizeStrategy::new(Resize::Increase, Some(Direction::Right)), + ) + .unwrap(); + let _ = tab.focus_pane_with_id(PaneId::Terminal(7), false, client_id); + let _ = tab.focus_pane_with_id(PaneId::Terminal(5), false, client_id); + + tab.render(&mut output).unwrap(); + let snapshot = take_snapshot( + output.serialize().unwrap().get(&client_id).unwrap(), + size.rows, + size.cols, + Palette::default(), + ); + assert_snapshot!(snapshot); +} + #[test] fn dump_screen() { let size = Size { diff --git a/zellij-server/src/ui/boundaries.rs b/zellij-server/src/ui/boundaries.rs index 7f70b1c7..f0fdebd6 100644 --- a/zellij-server/src/ui/boundaries.rs +++ b/zellij-server/src/ui/boundaries.rs @@ -1,4 +1,4 @@ -use zellij_utils::pane_size::Viewport; +use zellij_utils::pane_size::{Offset, Viewport}; use crate::output::CharacterChunk; use crate::panes::terminal_character::{TerminalCharacter, EMPTY_TERMINAL_CHARACTER, RESET_STYLES}; @@ -444,25 +444,40 @@ impl Boundaries { boundary_characters: HashMap::new(), } } - pub fn add_rect(&mut self, rect: &dyn Pane, color: Option) { + pub fn add_rect( + &mut self, + rect: &dyn Pane, + color: Option, + pane_is_on_top_of_stack: bool, + pane_is_on_bottom_of_stack: bool, + pane_is_stacked_under: bool, + ) { let pane_is_stacked = rect.current_geom().is_stacked(); + let should_skip_top_boundary = pane_is_stacked && !pane_is_on_top_of_stack; + let should_skip_bottom_boundary = pane_is_stacked && !pane_is_on_bottom_of_stack; + let content_offset = rect.get_content_offset(); if !self.is_fully_inside_screen(rect) { return; } if rect.x() > self.viewport.x { // left boundary let boundary_x_coords = rect.x() - 1; - let first_row_coordinates = self.rect_right_boundary_row_start(rect); + let first_row_coordinates = + self.rect_right_boundary_row_start(rect, pane_is_stacked_under, content_offset); let last_row_coordinates = self.rect_right_boundary_row_end(rect); for row in first_row_coordinates..last_row_coordinates { let coordinates = Coordinates::new(boundary_x_coords, row); let symbol_to_add = if row == first_row_coordinates && row != self.viewport.y { - BoundarySymbol::new(boundary_type::TOP_LEFT).color(color) + if pane_is_stacked { + BoundarySymbol::new(boundary_type::VERTICAL_RIGHT).color(color) + } else { + BoundarySymbol::new(boundary_type::TOP_LEFT).color(color) + } } else if row == first_row_coordinates && pane_is_stacked { BoundarySymbol::new(boundary_type::TOP_LEFT).color(color) } else if row == last_row_coordinates - 1 && row != self.viewport.y + self.viewport.rows - 1 - && !pane_is_stacked + && content_offset.bottom > 0 { BoundarySymbol::new(boundary_type::BOTTOM_LEFT).color(color) } else { @@ -476,7 +491,7 @@ impl Boundaries { self.boundary_characters.insert(coordinates, next_symbol); } } - if rect.y() > self.viewport.y && !pane_is_stacked { + if rect.y() > self.viewport.y && !should_skip_top_boundary { // top boundary let boundary_y_coords = rect.y() - 1; let first_col_coordinates = self.rect_bottom_boundary_col_start(rect); @@ -501,15 +516,22 @@ impl Boundaries { if self.rect_right_boundary_is_before_screen_edge(rect) { // right boundary let boundary_x_coords = rect.right_boundary_x_coords() - 1; - let first_row_coordinates = self.rect_right_boundary_row_start(rect); + let first_row_coordinates = + self.rect_right_boundary_row_start(rect, pane_is_stacked_under, content_offset); let last_row_coordinates = self.rect_right_boundary_row_end(rect); for row in first_row_coordinates..last_row_coordinates { let coordinates = Coordinates::new(boundary_x_coords, row); - let symbol_to_add = if row == first_row_coordinates && row != self.viewport.y { - BoundarySymbol::new(boundary_type::TOP_RIGHT).color(color) + let symbol_to_add = if row == first_row_coordinates && pane_is_stacked { + BoundarySymbol::new(boundary_type::VERTICAL_LEFT).color(color) + } else if row == first_row_coordinates && row != self.viewport.y { + if pane_is_stacked { + BoundarySymbol::new(boundary_type::VERTICAL_LEFT).color(color) + } else { + BoundarySymbol::new(boundary_type::TOP_RIGHT).color(color) + } } else if row == last_row_coordinates - 1 && row != self.viewport.y + self.viewport.rows - 1 - && !pane_is_stacked + && content_offset.bottom > 0 { BoundarySymbol::new(boundary_type::BOTTOM_RIGHT).color(color) } else { @@ -523,7 +545,7 @@ impl Boundaries { self.boundary_characters.insert(coordinates, next_symbol); } } - if self.rect_bottom_boundary_is_before_screen_edge(rect) && !pane_is_stacked { + if self.rect_bottom_boundary_is_before_screen_edge(rect) && !should_skip_bottom_boundary { // bottom boundary let boundary_y_coords = rect.bottom_boundary_y_coords() - 1; let first_col_coordinates = self.rect_bottom_boundary_col_start(rect); @@ -575,11 +597,28 @@ impl Boundaries { fn rect_bottom_boundary_is_before_screen_edge(&self, rect: &dyn Pane) -> bool { rect.y() + rect.rows() < self.viewport.y + self.viewport.rows } - fn rect_right_boundary_row_start(&self, rect: &dyn Pane) -> usize { + fn rect_right_boundary_row_start( + &self, + rect: &dyn Pane, + pane_is_stacked_under: bool, + content_offset: Offset, + ) -> usize { let pane_is_stacked = rect.current_geom().is_stacked(); - let horizontal_frame_offset = if pane_is_stacked { 0 } else { 1 }; + let horizontal_frame_offset = if pane_is_stacked_under { + // these panes - panes that are in a stack below the flexible pane - need to have their + // content offset taken into account when rendering them (i.e. they are rendered + // one line above their actual y coordinates, since they are only 1 line) + // as opposed to panes that are in a stack above the flexible pane who do not because + // they are rendered in place (the content offset of the stack is "absorbed" by the + // flexible pane below them) + content_offset.bottom + } else if pane_is_stacked { + 0 + } else { + 1 + }; if rect.y() > self.viewport.y { - rect.y() - horizontal_frame_offset + rect.y().saturating_sub(horizontal_frame_offset) } else { self.viewport.y } diff --git a/zellij-server/src/ui/pane_boundaries_frame.rs b/zellij-server/src/ui/pane_boundaries_frame.rs index 4e2c01fb..295e3bce 100644 --- a/zellij-server/src/ui/pane_boundaries_frame.rs +++ b/zellij-server/src/ui/pane_boundaries_frame.rs @@ -4,7 +4,7 @@ use crate::ui::boundaries::boundary_type; use crate::ClientId; use zellij_utils::data::{client_id_to_colors, PaletteColor, Style}; use zellij_utils::errors::prelude::*; -use zellij_utils::pane_size::Viewport; +use zellij_utils::pane_size::{Offset, Viewport}; use zellij_utils::position::Position; use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; @@ -62,6 +62,7 @@ pub struct FrameParams { pub pane_is_stacked_over: bool, pub should_draw_pane_frames: bool, pub pane_is_floating: bool, + pub content_offset: Offset, } #[derive(Default, PartialEq)] @@ -82,6 +83,7 @@ pub struct PaneFrame { should_draw_pane_frames: bool, is_pinned: bool, is_floating: bool, + content_offset: Offset, } impl PaneFrame { @@ -108,6 +110,7 @@ impl PaneFrame { should_draw_pane_frames: frame_params.should_draw_pane_frames, is_pinned: false, is_floating: frame_params.pane_is_floating, + content_offset: frame_params.content_offset, } } pub fn is_pinned(mut self, is_pinned: bool) -> Self { @@ -776,11 +779,26 @@ impl PaneFrame { // if this is a stacked pane with pane frames off (and it doesn't necessarily have only // 1 row because it could also be a flexible stacked pane) // in this case we should always draw the pane title line, and only the title line - let one_line_title = self.render_one_line_title().with_context(err_context)?; + let mut one_line_title = self.render_one_line_title().with_context(err_context)?; + + if self.content_offset.right != 0 && !self.should_draw_pane_frames { + // here what happens is that the title should be offset to the right + // in order to give room to the boundaries between the panes to be drawn + one_line_title.pop(); + } + let y_coords_of_title = if self.pane_is_stacked_under && !self.should_draw_pane_frames { + // we only want to use the bottom offset in this case because panes that are + // stacked above the flexible pane should actually appear exactly where they are on + // screen, the content offset being "absorbed" by the flexible pane below them + self.geom.y.saturating_sub(self.content_offset.bottom) + } else { + self.geom.y + }; + character_chunks.push(CharacterChunk::new( one_line_title, self.geom.x, - self.geom.y, + y_coords_of_title, )); } else { for row in 0..self.geom.rows { diff --git a/zellij-server/src/ui/pane_contents_and_ui.rs b/zellij-server/src/ui/pane_contents_and_ui.rs index 1be18fd8..c47e68d8 100644 --- a/zellij-server/src/ui/pane_contents_and_ui.rs +++ b/zellij-server/src/ui/pane_contents_and_ui.rs @@ -223,6 +223,7 @@ impl<'a> PaneContentsAndUi<'a> { pane_is_stacked_under: self.pane_is_stacked_under, should_draw_pane_frames: self.should_draw_pane_frames, pane_is_floating, + content_offset: self.pane.get_content_offset(), } } else { FrameParams { @@ -236,6 +237,7 @@ impl<'a> PaneContentsAndUi<'a> { pane_is_stacked_under: self.pane_is_stacked_under, should_draw_pane_frames: self.should_draw_pane_frames, pane_is_floating, + content_offset: self.pane.get_content_offset(), } }; @@ -260,9 +262,17 @@ impl<'a> PaneContentsAndUi<'a> { client_mode: InputMode, boundaries: &mut Boundaries, session_is_mirrored: bool, + pane_is_on_top_of_stack: bool, + pane_is_on_bottom_of_stack: bool, ) { let color = self.frame_color(client_id, client_mode, session_is_mirrored); - boundaries.add_rect(self.pane.as_ref(), color); + boundaries.add_rect( + self.pane.as_ref(), + color, + pane_is_on_top_of_stack, + pane_is_on_bottom_of_stack, + self.pane_is_stacked_under, + ); } fn frame_color( &self, diff --git a/zellij-utils/src/pane_size.rs b/zellij-utils/src/pane_size.rs index d337e9ac..50c4229a 100644 --- a/zellij-utils/src/pane_size.rs +++ b/zellij-utils/src/pane_size.rs @@ -400,6 +400,22 @@ impl Offset { } } + pub fn shift_right(right: usize) -> Self { + Self { + right, + ..Default::default() + } + } + + pub fn shift_right_top_and_bottom(right: usize, top: usize, bottom: usize) -> Self { + Self { + right, + top, + bottom, + ..Default::default() + } + } + // FIXME: This should be top and left, not bottom and right, but `boundaries.rs` would need // some changing pub fn shift(bottom: usize, right: usize) -> Self {