fix(rendering): occasional glitches while resizing (#2621)
This commit is contained in:
parent
385cc1c81b
commit
0825cb65a7
9 changed files with 120 additions and 33 deletions
|
|
@ -10,6 +10,7 @@ use nix::{
|
|||
},
|
||||
unistd,
|
||||
};
|
||||
|
||||
use signal_hook::consts::*;
|
||||
use sysinfo::{ProcessExt, ProcessRefreshKind, System, SystemExt};
|
||||
use zellij_utils::{
|
||||
|
|
@ -840,6 +841,34 @@ pub fn get_server_os_input() -> Result<ServerOsInputOutput, nix::Error> {
|
|||
})
|
||||
}
|
||||
|
||||
use crate::pty_writer::PtyWriteInstruction;
|
||||
use crate::thread_bus::ThreadSenders;
|
||||
|
||||
pub struct ResizeCache {
|
||||
senders: ThreadSenders,
|
||||
}
|
||||
|
||||
impl ResizeCache {
|
||||
pub fn new(senders: ThreadSenders) -> Self {
|
||||
senders
|
||||
.send_to_pty_writer(PtyWriteInstruction::StartCachingResizes)
|
||||
.unwrap_or_else(|e| {
|
||||
log::error!("Failed to cache resizes: {}", e);
|
||||
});
|
||||
ResizeCache { senders }
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for ResizeCache {
|
||||
fn drop(&mut self) {
|
||||
self.senders
|
||||
.send_to_pty_writer(PtyWriteInstruction::ApplyCachedResizes)
|
||||
.unwrap_or_else(|e| {
|
||||
log::error!("Failed to apply cached resizes: {}", e);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/// Process id's for forked terminals
|
||||
#[derive(Debug)]
|
||||
pub struct ChildId {
|
||||
|
|
|
|||
|
|
@ -2,9 +2,16 @@ use zellij_utils::errors::{prelude::*, ContextType, PtyWriteContext};
|
|||
|
||||
use crate::thread_bus::Bus;
|
||||
|
||||
// we separate these instruction to a different thread because some programs get deadlocked if
|
||||
// you write into their STDIN while reading from their STDOUT (I'm looking at you, vim)
|
||||
// while the same has not been observed to happen with resizes, it could conceivably happen and we have this
|
||||
// here anyway, so
|
||||
#[derive(Debug, Clone, Eq, PartialEq)]
|
||||
pub enum PtyWriteInstruction {
|
||||
Write(Vec<u8>, u32),
|
||||
ResizePty(u32, u16, u16, Option<u16>, Option<u16>), // terminal_id, columns, rows, pixel width, pixel height
|
||||
StartCachingResizes,
|
||||
ApplyCachedResizes,
|
||||
Exit,
|
||||
}
|
||||
|
||||
|
|
@ -12,6 +19,9 @@ impl From<&PtyWriteInstruction> for PtyWriteContext {
|
|||
fn from(tty_write_instruction: &PtyWriteInstruction) -> Self {
|
||||
match *tty_write_instruction {
|
||||
PtyWriteInstruction::Write(..) => PtyWriteContext::Write,
|
||||
PtyWriteInstruction::ResizePty(..) => PtyWriteContext::ResizePty,
|
||||
PtyWriteInstruction::ApplyCachedResizes => PtyWriteContext::ApplyCachedResizes,
|
||||
PtyWriteInstruction::StartCachingResizes => PtyWriteContext::StartCachingResizes,
|
||||
PtyWriteInstruction::Exit => PtyWriteContext::Exit,
|
||||
}
|
||||
}
|
||||
|
|
@ -23,7 +33,7 @@ pub(crate) fn pty_writer_main(bus: Bus<PtyWriteInstruction>) -> Result<()> {
|
|||
loop {
|
||||
let (event, mut err_ctx) = bus.recv().with_context(err_context)?;
|
||||
err_ctx.add_call(ContextType::PtyWrite((&event).into()));
|
||||
let os_input = bus
|
||||
let mut os_input = bus
|
||||
.os_input
|
||||
.clone()
|
||||
.context("no OS input API found")
|
||||
|
|
@ -39,6 +49,38 @@ pub(crate) fn pty_writer_main(bus: Bus<PtyWriteInstruction>) -> Result<()> {
|
|||
.with_context(err_context)
|
||||
.non_fatal();
|
||||
},
|
||||
PtyWriteInstruction::ResizePty(
|
||||
terminal_id,
|
||||
columns,
|
||||
rows,
|
||||
width_in_pixels,
|
||||
height_in_pixels,
|
||||
) => {
|
||||
os_input
|
||||
.set_terminal_size_using_terminal_id(
|
||||
terminal_id,
|
||||
columns,
|
||||
rows,
|
||||
width_in_pixels,
|
||||
height_in_pixels,
|
||||
)
|
||||
.with_context(err_context)
|
||||
.non_fatal();
|
||||
},
|
||||
PtyWriteInstruction::StartCachingResizes => {
|
||||
// we do this because there are some logic traps inside the screen/tab/layout code
|
||||
// the cause multiple resizes to be sent to the pty - while the last one is always
|
||||
// the correct one, many programs and shells debounce those (I guess due to the
|
||||
// trauma of dealing with GUI resizes of the controlling terminal window), and this
|
||||
// then causes glitches and missing redraws
|
||||
// so we do this to play nice and always only send the last resize instruction to
|
||||
// each pane
|
||||
// the logic for this happens in the main Screen event loop
|
||||
os_input.cache_resizes();
|
||||
},
|
||||
PtyWriteInstruction::ApplyCachedResizes => {
|
||||
os_input.apply_cached_resizes();
|
||||
},
|
||||
PtyWriteInstruction::Exit => {
|
||||
return Ok(());
|
||||
},
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ use zellij_utils::{
|
|||
position::Position,
|
||||
};
|
||||
|
||||
use crate::os_input_output::ResizeCache;
|
||||
use crate::panes::alacritty_functions::xparse_color;
|
||||
use crate::panes::terminal_character::AnsiCode;
|
||||
|
||||
|
|
@ -1583,6 +1584,7 @@ pub(crate) fn screen_thread_main(
|
|||
config_options.copy_on_select.unwrap_or(true),
|
||||
);
|
||||
|
||||
let thread_senders = bus.senders.clone();
|
||||
let mut screen = Screen::new(
|
||||
bus,
|
||||
&client_attributes,
|
||||
|
|
@ -1611,6 +1613,9 @@ pub(crate) fn screen_thread_main(
|
|||
.recv()
|
||||
.context("failed to receive event on channel")?;
|
||||
err_ctx.add_call(ContextType::Screen((&event).into()));
|
||||
// here we start caching resizes, so that we'll send them in bulk at the end of each event
|
||||
// when this cache is Dropped, for more information, see the comments in PtyWriter
|
||||
let _resize_cache = ResizeCache::new(thread_senders.clone());
|
||||
|
||||
match event {
|
||||
ScreenInstruction::PtyBytes(pid, vte_bytes) => {
|
||||
|
|
|
|||
|
|
@ -59,13 +59,17 @@ use zellij_utils::{
|
|||
macro_rules! resize_pty {
|
||||
($pane:expr, $os_input:expr, $senders:expr) => {{
|
||||
match $pane.pid() {
|
||||
PaneId::Terminal(ref pid) => $os_input.set_terminal_size_using_terminal_id(
|
||||
PaneId::Terminal(ref pid) => {
|
||||
$senders
|
||||
.send_to_pty_writer(PtyWriteInstruction::ResizePty(
|
||||
*pid,
|
||||
$pane.get_content_columns() as u16,
|
||||
$pane.get_content_rows() as u16,
|
||||
None,
|
||||
None,
|
||||
),
|
||||
))
|
||||
.with_context(err_context);
|
||||
},
|
||||
PaneId::Plugin(ref pid) => {
|
||||
let err_context = || format!("failed to resize plugin {pid}");
|
||||
$senders
|
||||
|
|
@ -93,13 +97,19 @@ macro_rules! resize_pty {
|
|||
}
|
||||
};
|
||||
match $pane.pid() {
|
||||
PaneId::Terminal(ref pid) => $os_input.set_terminal_size_using_terminal_id(
|
||||
PaneId::Terminal(ref pid) => {
|
||||
use crate::PtyWriteInstruction;
|
||||
let err_context = || format!("Failed to send resize pty instruction");
|
||||
$senders
|
||||
.send_to_pty_writer(PtyWriteInstruction::ResizePty(
|
||||
*pid,
|
||||
$pane.get_content_columns() as u16,
|
||||
$pane.get_content_rows() as u16,
|
||||
width_in_pixels,
|
||||
height_in_pixels,
|
||||
),
|
||||
))
|
||||
.with_context(err_context)
|
||||
},
|
||||
PaneId::Plugin(ref pid) => {
|
||||
let err_context = || format!("failed to resize plugin {pid}");
|
||||
$senders
|
||||
|
|
@ -758,16 +768,15 @@ impl Tab {
|
|||
Ok(())
|
||||
}
|
||||
pub fn previous_swap_layout(&mut self, client_id: Option<ClientId>) -> Result<()> {
|
||||
// warning, here we cache resizes rather than sending them to the pty, we do that in
|
||||
// apply_cached_resizes below - beware when bailing on this function early!
|
||||
self.os_api.cache_resizes();
|
||||
let search_backwards = true;
|
||||
if self.floating_panes.panes_are_visible() {
|
||||
self.relayout_floating_panes(client_id, search_backwards, true)?;
|
||||
} else {
|
||||
self.relayout_tiled_panes(client_id, search_backwards, true, false)?;
|
||||
}
|
||||
self.os_api.apply_cached_resizes();
|
||||
self.senders
|
||||
.send_to_pty_writer(PtyWriteInstruction::ApplyCachedResizes)
|
||||
.with_context(|| format!("failed to update plugins with mode info"))?;
|
||||
Ok(())
|
||||
}
|
||||
pub fn next_swap_layout(
|
||||
|
|
@ -775,16 +784,15 @@ impl Tab {
|
|||
client_id: Option<ClientId>,
|
||||
refocus_pane: bool,
|
||||
) -> Result<()> {
|
||||
// warning, here we cache resizes rather than sending them to the pty, we do that in
|
||||
// apply_cached_resizes below - beware when bailing on this function early!
|
||||
self.os_api.cache_resizes();
|
||||
let search_backwards = false;
|
||||
if self.floating_panes.panes_are_visible() {
|
||||
self.relayout_floating_panes(client_id, search_backwards, refocus_pane)?;
|
||||
} else {
|
||||
self.relayout_tiled_panes(client_id, search_backwards, refocus_pane, false)?;
|
||||
}
|
||||
self.os_api.apply_cached_resizes();
|
||||
self.senders
|
||||
.send_to_pty_writer(PtyWriteInstruction::ApplyCachedResizes)
|
||||
.with_context(|| format!("failed to update plugins with mode info"))?;
|
||||
Ok(())
|
||||
}
|
||||
pub fn apply_buffered_instructions(&mut self) -> Result<()> {
|
||||
|
|
@ -1807,9 +1815,6 @@ impl Tab {
|
|||
selectable_tiled_panes.count() > 0
|
||||
}
|
||||
pub fn resize_whole_tab(&mut self, new_screen_size: Size) -> Result<()> {
|
||||
// warning, here we cache resizes rather than sending them to the pty, we do that in
|
||||
// apply_cached_resizes below - beware when bailing on this function early!
|
||||
self.os_api.cache_resizes();
|
||||
let err_context = || format!("failed to resize whole tab (index {})", self.index);
|
||||
self.floating_panes.resize(new_screen_size);
|
||||
// we need to do this explicitly because floating_panes.resize does not do this
|
||||
|
|
@ -1829,7 +1834,9 @@ impl Tab {
|
|||
let _ = self.relayout_tiled_panes(None, false, false, true);
|
||||
}
|
||||
self.should_clear_display_before_rendering = true;
|
||||
let _ = self.os_api.apply_cached_resizes();
|
||||
self.senders
|
||||
.send_to_pty_writer(PtyWriteInstruction::ApplyCachedResizes)
|
||||
.with_context(|| format!("failed to update plugins with mode info"))?;
|
||||
Ok(())
|
||||
}
|
||||
pub fn resize(&mut self, client_id: ClientId, strategy: ResizeStrategy) -> Result<()> {
|
||||
|
|
|
|||
|
|
@ -175,6 +175,7 @@ impl MockPtyInstructionBus {
|
|||
.unwrap()
|
||||
.push(String::from_utf8_lossy(&msg).to_string()),
|
||||
PtyWriteInstruction::Exit => break,
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
---
|
||||
source: zellij-server/src/./unit/screen_tests.rs
|
||||
assertion_line: 1487
|
||||
assertion_line: 1825
|
||||
expression: "format!(\"{:?}\", * received_pty_instructions.lock().unwrap())"
|
||||
---
|
||||
[Write([102, 111, 111], 0), Write([102, 111, 111], 1), Exit]
|
||||
[StartCachingResizes, ApplyCachedResizes, StartCachingResizes, ResizePty(0, 59, 18, None, None), ResizePty(1, 58, 18, None, None), ResizePty(0, 59, 18, None, None), ResizePty(1, 58, 18, None, None), ResizePty(0, 59, 18, None, None), ResizePty(1, 58, 18, None, None), ApplyCachedResizes, StartCachingResizes, ApplyCachedResizes, StartCachingResizes, ApplyCachedResizes, StartCachingResizes, Write([102, 111, 111], 0), Write([102, 111, 111], 1), ApplyCachedResizes, Exit]
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
---
|
||||
source: zellij-server/src/./unit/screen_tests.rs
|
||||
assertion_line: 879
|
||||
assertion_line: 1065
|
||||
expression: "format!(\"{:?}\", * received_pty_instructions.lock().unwrap())"
|
||||
---
|
||||
[Write([102, 111, 111], 0), Exit]
|
||||
[StartCachingResizes, ApplyCachedResizes, StartCachingResizes, ResizePty(0, 119, 18, None, None), ResizePty(0, 119, 18, None, None), ResizePty(0, 119, 18, None, None), ApplyCachedResizes, StartCachingResizes, ApplyCachedResizes, StartCachingResizes, Write([102, 111, 111], 0), ApplyCachedResizes, Exit]
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
---
|
||||
source: zellij-server/src/./unit/screen_tests.rs
|
||||
assertion_line: 846
|
||||
assertion_line: 1039
|
||||
expression: "format!(\"{:?}\", * received_pty_instructions.lock().unwrap())"
|
||||
---
|
||||
[Write([105, 110, 112, 117, 116, 32, 102, 114, 111, 109, 32, 116, 104, 101, 32, 99, 108, 105], 0), Exit]
|
||||
[StartCachingResizes, ApplyCachedResizes, StartCachingResizes, ResizePty(0, 119, 18, None, None), ResizePty(0, 119, 18, None, None), ResizePty(0, 119, 18, None, None), ApplyCachedResizes, StartCachingResizes, ApplyCachedResizes, StartCachingResizes, Write([105, 110, 112, 117, 116, 32, 102, 114, 111, 109, 32, 116, 104, 101, 32, 99, 108, 105], 0), ApplyCachedResizes, Exit]
|
||||
|
|
|
|||
|
|
@ -413,6 +413,9 @@ pub enum ServerContext {
|
|||
#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]
|
||||
pub enum PtyWriteContext {
|
||||
Write,
|
||||
ResizePty,
|
||||
StartCachingResizes,
|
||||
ApplyCachedResizes,
|
||||
Exit,
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue