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

@@ -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",

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.

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
 

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"}

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"]

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"

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<ImageDataType>,
     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
     }

wezterm-client/src/pane/renderable.rs

@@ -666,17 +666,24 @@ pub(crate) async fn hydrate_lines(
     }
 
     for (_, request) in requests {
-        log::trace!("requesting {:?}", request);
-        if let Ok(GetImageCellResponse {
+        match client.client.get_image_cell(request).await {
+            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);
             }
+            Ok(GetImageCellResponse { data: None, .. }) => {
+                log::error!("no image data!");
+            }
+
+            Err(err) => {
+                log::error!("failed to retrieve image {err:#}");
+            }
+        }
     }
 
     let mut line_by_idx = HashMap::new();

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,9 +441,8 @@ impl DecodedImage {
         }
     }
 
-    fn load(image_data: &Arc<ImageData>) -> Self {
-        match &*image_data.data() {
-            ImageDataType::EncodedLease(lease) => match FrameDecoder::start(lease.clone()) {
+    fn start_frame_decoder(lease: BlobLease, image_data: &Arc<ImageData>) -> Self {
+        match FrameDecoder::start(lease.clone()) {
             Ok(rx) => Self {
                 frame_start: RefCell::new(Instant::now()),
                 current_frame: RefCell::new(0),
@@ -454,14 +453,21 @@ impl DecodedImage {
                 log::error!("failed to start FrameDecoder: {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
+        }
+    }
+
+    fn load(image_data: &Arc<ImageData>) -> Self {
+        match &*image_data.data() {
+            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!("Unable to move file data to blob manager: {err:#}");
                     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<GlyphKey, Rc<CachedGlyph>>,
     pub atlas: Atlas,
     pub fonts: Rc<FontConfiguration>,
-    pub image_cache: LfuCacheU64<DecodedImage>,
+    pub image_cache: LfuCache<[u8; 32], DecodedImage>,
     frame_cache: HashMap<[u8; 32], Sprite>,
     line_glyphs: HashMap<LineKey, Sprite>,
     pub block_glyphs: HashMap<SizedBlockKey, Sprite>,
@@ -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<ImageData>,
         padding: Option<usize>,
     ) -> anyhow::Result<(Sprite, Option<Instant>)> {
-        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)
         }
     }

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" }

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 {

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"