From bc7acc18e02cdfee5d1ae72bf6b5d3b483479850 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Thu, 23 Mar 2023 21:40:01 -0700 Subject: [PATCH] fixup images with the multiplexer * Translate from File to EncodedFile as needed * Adopt blob leases in the mux server * Fix an issue where the first image sent by the mux server would be replaced on the client by its background image, if configured. Removed the ImageData::id field to resolve this: you should use the hash field instead to identify and disambiguate images. Bumped the termwiz API version because this is conceptually a breaking change to the API refs: https://github.com/wez/wezterm/issues/3343 --- Cargo.lock | 3 +- codec/src/lib.rs | 2 +- docs/changelog.md | 1 + tabout/Cargo.toml | 2 +- term/Cargo.toml | 2 +- termwiz/Cargo.toml | 2 +- termwiz/src/image.rs | 33 +++++++++++------- wezterm-client/src/pane/renderable.rs | 27 +++++++++------ wezterm-gui/src/glyphcache.rs | 50 +++++++++++++++------------ wezterm-mux-server/Cargo.toml | 1 + wezterm-mux-server/src/main.rs | 6 ++++ wezterm-ssh/Cargo.toml | 2 +- 12 files changed, 80 insertions(+), 51 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 603af50a0..38b8f58bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5045,7 +5045,7 @@ checksum = "3369f5ac52d5eb6ab48c6b4ffdc8efbcad6b89c765749064ba298f2c68a16a76" [[package]] name = "termwiz" -version = "0.21.0" +version = "0.22.0" dependencies = [ "anyhow", "base64 0.21.0", @@ -6106,6 +6106,7 @@ dependencies = [ "portable-pty", "promise", "umask", + "wezterm-blob-leases", "wezterm-gui-subcommands", "wezterm-mux-server-impl", "wezterm-term", diff --git a/codec/src/lib.rs b/codec/src/lib.rs index 7e36965bf..6e5d072d9 100644 --- a/codec/src/lib.rs +++ b/codec/src/lib.rs @@ -417,7 +417,7 @@ macro_rules! pdu { /// The overall version of the codec. /// This must be bumped when backwards incompatible changes /// are made to the types and protocol. -pub const CODEC_VERSION: usize = 32; +pub const CODEC_VERSION: usize = 33; // Defines the Pdu enum. // Each struct has an explicit identifying number. diff --git a/docs/changelog.md b/docs/changelog.md index 036e0137c..2df466850 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -52,6 +52,7 @@ As features stabilize some brief notes about them will accumulate here. * Lingering openconsole.exe processes on Windows. Thanks to @mbikovitsky! #3339 * macos: command line parameters beyond the first were treated as terminal command scripts and opened spurious windows. #3340 +* imgcat broken with multiplexer protocol #3343 ### 20230320-124340-559cb7b0 diff --git a/tabout/Cargo.toml b/tabout/Cargo.toml index 593963930..4ec65e9ca 100644 --- a/tabout/Cargo.toml +++ b/tabout/Cargo.toml @@ -9,4 +9,4 @@ license = "MIT" documentation = "https://docs.rs/tabout" [dependencies] -termwiz = { path = "../termwiz", version="0.21"} +termwiz = { path = "../termwiz", version="0.22"} diff --git a/term/Cargo.toml b/term/Cargo.toml index ca74be8dc..f5948e3e8 100644 --- a/term/Cargo.toml +++ b/term/Cargo.toml @@ -40,6 +40,6 @@ env_logger = "0.10" k9 = "0.11.0" [dependencies.termwiz] -version = "0.21" +version = "0.22" path = "../termwiz" features = ["use_image"] diff --git a/termwiz/Cargo.toml b/termwiz/Cargo.toml index bc2f5212a..8c0927db0 100644 --- a/termwiz/Cargo.toml +++ b/termwiz/Cargo.toml @@ -1,7 +1,7 @@ [package] authors = ["Wez Furlong"] name = "termwiz" -version = "0.21.0" +version = "0.22.0" edition = "2018" repository = "https://github.com/wez/wezterm" description = "Terminal Wizardry for Unix and Windows" diff --git a/termwiz/src/image.rs b/termwiz/src/image.rs index da069169f..b37d2a423 100644 --- a/termwiz/src/image.rs +++ b/termwiz/src/image.rs @@ -471,20 +471,35 @@ impl ImageDataType { } } -static IMAGE_ID: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::AtomicUsize::new(0); - #[cfg_attr(feature = "use_serde", derive(Serialize, Deserialize))] -#[derive(Debug)] pub struct ImageData { - id: usize, data: Mutex, hash: [u8; 32], } +struct HexSlice<'a>(&'a [u8]); +impl<'a> std::fmt::Display for HexSlice<'a> { + fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { + for byte in self.0 { + write!(fmt, "{byte:x}")?; + } + Ok(()) + } +} + +impl std::fmt::Debug for ImageData { + fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { + fmt.debug_struct("ImageData") + .field("data", &self.data) + .field("hash", &format_args!("{}", HexSlice(&self.hash))) + .finish() + } +} + impl Eq for ImageData {} impl PartialEq for ImageData { fn eq(&self, rhs: &Self) -> bool { - self.id == rhs.id + self.hash == rhs.hash } } @@ -496,19 +511,15 @@ impl ImageData { } fn with_data_and_hash(data: ImageDataType, hash: [u8; 32]) -> Self { - let id = IMAGE_ID.fetch_add(1, ::std::sync::atomic::Ordering::Relaxed); Self { - id, data: Mutex::new(data), hash, } } pub fn with_data(data: ImageDataType) -> Self { - let id = IMAGE_ID.fetch_add(1, ::std::sync::atomic::Ordering::Relaxed); let hash = data.compute_hash(); Self { - id, data: Mutex::new(data), hash, } @@ -528,10 +539,6 @@ impl ImageData { self.data.lock().unwrap() } - pub fn id(&self) -> usize { - self.id - } - pub fn hash(&self) -> [u8; 32] { self.hash } diff --git a/wezterm-client/src/pane/renderable.rs b/wezterm-client/src/pane/renderable.rs index a72f59205..2fd39e24d 100644 --- a/wezterm-client/src/pane/renderable.rs +++ b/wezterm-client/src/pane/renderable.rs @@ -666,16 +666,23 @@ pub(crate) async fn hydrate_lines( } for (_, request) in requests { - log::trace!("requesting {:?}", request); - if let Ok(GetImageCellResponse { - data: Some(data), .. - }) = client.client.get_image_cell(request).await - { - IMAGE_LRU - .lock() - .unwrap() - .put(data.hash(), Arc::clone(&data)); - data_by_hash.insert(data.hash(), data); + match client.client.get_image_cell(request).await { + Ok(GetImageCellResponse { + data: Some(data), .. + }) => { + IMAGE_LRU + .lock() + .unwrap() + .put(data.hash(), Arc::clone(&data)); + data_by_hash.insert(data.hash(), data); + } + Ok(GetImageCellResponse { data: None, .. }) => { + log::error!("no image data!"); + } + + Err(err) => { + log::error!("failed to retrieve image {err:#}"); + } } } diff --git a/wezterm-gui/src/glyphcache.rs b/wezterm-gui/src/glyphcache.rs index c34f97048..c824d8184 100644 --- a/wezterm-gui/src/glyphcache.rs +++ b/wezterm-gui/src/glyphcache.rs @@ -9,7 +9,7 @@ use config::{AllowSquareGlyphOverflow, TextStyle}; use euclid::num::Zero; use image::io::Limits; use image::{AnimationDecoder, Frame, Frames, ImageDecoder, ImageFormat, ImageResult}; -use lfucache::LfuCacheU64; +use lfucache::LfuCache; use once_cell::sync::Lazy; use ordered_float::NotNan; use std::cell::RefCell; @@ -441,27 +441,33 @@ impl DecodedImage { } } + fn start_frame_decoder(lease: BlobLease, image_data: &Arc) -> Self { + match FrameDecoder::start(lease.clone()) { + Ok(rx) => Self { + frame_start: RefCell::new(Instant::now()), + current_frame: RefCell::new(0), + image: Arc::clone(image_data), + frames: RefCell::new(Some(FrameState::new(rx))), + }, + Err(err) => { + log::error!("failed to start FrameDecoder: {err:#}"); + Self::placeholder() + } + } + } + fn load(image_data: &Arc) -> Self { match &*image_data.data() { - ImageDataType::EncodedLease(lease) => match FrameDecoder::start(lease.clone()) { - Ok(rx) => Self { - frame_start: RefCell::new(Instant::now()), - current_frame: RefCell::new(0), - image: Arc::clone(image_data), - frames: RefCell::new(Some(FrameState::new(rx))), - }, + ImageDataType::EncodedLease(lease) => { + Self::start_frame_decoder(lease.clone(), image_data) + } + ImageDataType::EncodedFile(data) => match BlobManager::store(&data) { + Ok(lease) => Self::start_frame_decoder(lease, image_data), Err(err) => { - log::error!("failed to start FrameDecoder: {err:#}"); + log::error!("Unable to move file data to blob manager: {err:#}"); Self::placeholder() } }, - ImageDataType::EncodedFile(_) => { - log::error!("Unexpected EncodedFile; should have File here"); - // The swap_out call happens in the terminal layer - // when normalizing/caching the data just prior to - // applying it to the terminal model - Self::placeholder() - } ImageDataType::AnimRgba8 { durations, .. } => { let current_frame = if durations.len() > 1 && durations[0].as_millis() == 0 { // Skip possible 0-duration root frame @@ -493,7 +499,7 @@ pub struct GlyphCache { glyph_cache: HashMap>, pub atlas: Atlas, pub fonts: Rc, - pub image_cache: LfuCacheU64, + pub image_cache: LfuCache<[u8; 32], DecodedImage>, frame_cache: HashMap<[u8; 32], Sprite>, line_glyphs: HashMap, pub block_glyphs: HashMap, @@ -510,7 +516,7 @@ impl GlyphCache { Ok(Self { fonts: Rc::clone(fonts), glyph_cache: HashMap::new(), - image_cache: LfuCacheU64::new( + image_cache: LfuCache::new( "glyph_cache.image_cache.hit.rate", "glyph_cache.image_cache.miss.rate", |config| config.glyph_cache_image_cache_size, @@ -539,7 +545,7 @@ impl GlyphCache { Ok(Self { fonts: Rc::clone(fonts), glyph_cache: HashMap::new(), - image_cache: LfuCacheU64::new( + image_cache: LfuCache::new( "glyph_cache.image_cache.hit.rate", "glyph_cache.image_cache.miss.rate", |config| config.glyph_cache_image_cache_size, @@ -947,9 +953,9 @@ impl GlyphCache { image_data: &Arc, padding: Option, ) -> anyhow::Result<(Sprite, Option)> { - let id = image_data.id() as u64; + let hash = image_data.hash(); - if let Some(decoded) = self.image_cache.get(&id) { + if let Some(decoded) = self.image_cache.get(&hash) { Self::cached_image_impl( &mut self.frame_cache, &mut self.atlas, @@ -966,7 +972,7 @@ impl GlyphCache { padding, self.min_frame_duration, )?; - self.image_cache.put(id, decoded); + self.image_cache.put(hash, decoded); Ok(res) } } diff --git a/wezterm-mux-server/Cargo.toml b/wezterm-mux-server/Cargo.toml index 24687acb7..b9ed64850 100644 --- a/wezterm-mux-server/Cargo.toml +++ b/wezterm-mux-server/Cargo.toml @@ -21,6 +21,7 @@ openssl = "0.10" portable-pty = { path = "../pty", features = ["serde_support"]} promise = { path = "../promise" } umask = { path = "../umask" } +wezterm-blob-leases = { path = "../wezterm-blob-leases", version="0.1", features=["simple_tempdir"] } wezterm-mux-server-impl = { path = "../wezterm-mux-server-impl" } wezterm-gui-subcommands = { path = "../wezterm-gui-subcommands" } wezterm-term = { path = "../term" } diff --git a/wezterm-mux-server/src/main.rs b/wezterm-mux-server/src/main.rs index 3b293ea43..721dee290 100644 --- a/wezterm-mux-server/src/main.rs +++ b/wezterm-mux-server/src/main.rs @@ -60,9 +60,11 @@ struct Opt { fn main() { if let Err(err) = run() { + wezterm_blob_leases::clear_storage(); log::error!("{:#}", err); std::process::exit(1); } + wezterm_blob_leases::clear_storage(); } fn run() -> anyhow::Result<()> { @@ -159,6 +161,10 @@ fn run() -> anyhow::Result<()> { std::env::remove_var(name); } + wezterm_blob_leases::register_storage(Arc::new( + wezterm_blob_leases::simple_tempdir::SimpleTempDir::new()?, + ))?; + let need_builder = !opts.prog.is_empty() || opts.cwd.is_some(); let cmd = if need_builder { diff --git a/wezterm-ssh/Cargo.toml b/wezterm-ssh/Cargo.toml index 88407338d..49affadcb 100644 --- a/wezterm-ssh/Cargo.toml +++ b/wezterm-ssh/Cargo.toml @@ -49,5 +49,5 @@ env_logger = "0.10" rstest = "0.17" shell-words = "1.1" smol-potat = "1.1.2" -termwiz = { version = "0.21", path = "../termwiz" } +termwiz = { version = "0.22", path = "../termwiz" } whoami = "1.1"