allow locking ImageData's ImageDataType for mutation

refs: #986

term/src/terminalstate/kitty.rs

@@ -97,7 +97,7 @@ impl TerminalState {
                 .ok_or_else(|| anyhow::anyhow!("no matching image id"))?,
         );
 
-        let (image_width, image_height) = match img.data() {
+        let (image_width, image_height) = match &*img.data() {
             ImageDataType::EncodedFile(data) => {
                 let decoded = ::image::load_from_memory(data).context("decode png")?;
                 decoded.dimensions()

termwiz/src/image.rs

@@ -14,7 +14,7 @@
 use ordered_float::NotNan;
 #[cfg(feature = "use_serde")]
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
-use std::sync::Arc;
+use std::sync::{Arc, Mutex, MutexGuard};
 use std::time::Duration;
 
 #[cfg(feature = "use_serde")]
@@ -312,11 +312,17 @@ impl ImageDataType {
 static IMAGE_ID: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::AtomicUsize::new(0);
 
 #[cfg_attr(feature = "use_serde", derive(Serialize, Deserialize))]
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Debug)]
 pub struct ImageData {
     id: usize,
-    hash: [u8; 32],
-    data: ImageDataType,
+    data: Mutex<ImageDataType>,
+}
+
+impl Eq for ImageData {}
+impl PartialEq for ImageData {
+    fn eq(&self, rhs: &Self) -> bool {
+        self.id == rhs.id
+    }
 }
 
 impl ImageData {
@@ -327,22 +333,23 @@ impl ImageData {
 
     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, hash, data }
+        Self {
+            id,
+            data: Mutex::new(data),
+        }
     }
 
     /// Returns the in-memory footprint
     pub fn len(&self) -> usize {
-        match &self.data {
+        match &*self.data() {
             ImageDataType::EncodedFile(d) => d.len(),
             ImageDataType::Rgba8 { data, .. } => data.len(),
             ImageDataType::AnimRgba8 { frames, .. } => frames.len() * frames[0].len(),
         }
     }
 
-    #[inline]
-    pub fn data(&self) -> &ImageDataType {
-        &self.data
+    pub fn data(&self) -> MutexGuard<ImageDataType> {
+        self.data.lock().unwrap()
     }
 
     pub fn id(&self) -> usize {
@@ -350,6 +357,6 @@ impl ImageData {
     }
 
     pub fn hash(&self) -> [u8; 32] {
-        self.hash
+        self.data().compute_hash()
     }
 }

termwiz/src/render/terminfo.rs

@@ -567,7 +567,7 @@ impl TerminfoRenderer {
                         {
                             // The whole image is requested, so we can send the
                             // original image bytes over
-                            match image.image.data() {
+                            match &*image.image.data() {
                                 ImageDataType::EncodedFile(data) => data.to_vec(),
                                 ImageDataType::AnimRgba8 { .. } | ImageDataType::Rgba8 { .. } => {
                                     unimplemented!()

wezterm-gui/src/glyphcache.rs

@@ -17,7 +17,7 @@ use euclid::num::Zero;
 use std::collections::HashMap;
 use std::convert::TryInto;
 use std::rc::Rc;
-use std::sync::Arc;
+use std::sync::{Arc, MutexGuard};
 use std::time::Instant;
 use termwiz::image::{ImageData, ImageDataType};
 use wezterm_font::units::*;
@@ -131,9 +131,16 @@ struct LineKey {
     overline: bool,
 }
 
-impl BitmapImage for DecodedImage {
+/// A helper struct to implement BitmapImage for ImageDataType while
+/// holding the mutex for the sake of safety.
+struct DecodedImageHandle<'a> {
+    current_frame: usize,
+    h: MutexGuard<'a, ImageDataType>,
+}
+
+impl<'a> BitmapImage for DecodedImageHandle<'a> {
     unsafe fn pixel_data(&self) -> *const u8 {
-        match self.image.data() {
+        match &*self.h {
             ImageDataType::Rgba8 { data, .. } => data.as_ptr(),
             ImageDataType::AnimRgba8 { frames, .. } => frames[self.current_frame].as_ptr(),
             ImageDataType::EncodedFile(_) => unreachable!(),
@@ -145,7 +152,7 @@ impl BitmapImage for DecodedImage {
     }
 
     fn image_dimensions(&self) -> (usize, usize) {
-        match self.image.data() {
+        match &*self.h {
             ImageDataType::Rgba8 { width, height, .. }
             | ImageDataType::AnimRgba8 { width, height, .. } => (*width as usize, *height as usize),
             ImageDataType::EncodedFile(_) => unreachable!(),
@@ -176,7 +183,7 @@ impl DecodedImage {
     }
 
     fn load(image_data: &Arc<ImageData>) -> Self {
-        match image_data.data() {
+        match &*image_data.data() {
             ImageDataType::EncodedFile(_) => {
                 log::warn!("Unexpected ImageDataType::EncodedFile; either file is unreadable or we missed a .decode call somewhere");
                 Self::placeholder()
@@ -516,6 +523,7 @@ impl<T: Texture2d> GlyphCache<T> {
 
         Ok(Rc::new(glyph))
     }
+
     fn cached_image_impl(
         frame_cache: &mut HashMap<(usize, usize), Sprite<T>>,
         atlas: &mut Atlas<T>,
@@ -523,12 +531,16 @@ impl<T: Texture2d> GlyphCache<T> {
         padding: Option<usize>,
     ) -> anyhow::Result<(Sprite<T>, Option<Instant>)> {
         let id = decoded.image.id();
-        match decoded.image.data() {
+        let mut handle = DecodedImageHandle {
+            h: decoded.image.data(),
+            current_frame: decoded.current_frame,
+        };
+        match &*handle.h {
             ImageDataType::Rgba8 { .. } => {
                 if let Some(sprite) = frame_cache.get(&(id, 0)) {
                     return Ok((sprite.clone(), None));
                 }
-                let sprite = atlas.allocate_with_padding(decoded, padding)?;
+                let sprite = atlas.allocate_with_padding(&handle, padding)?;
                 frame_cache.insert((id, 0), sprite.clone());
 
                 return Ok((sprite, None));
@@ -548,6 +560,7 @@ impl<T: Texture2d> GlyphCache<T> {
                         }
                         decoded.frame_start = now;
                         next_due = decoded.frame_start + durations[decoded.current_frame];
+                        handle.current_frame = decoded.current_frame;
                     }
 
                     next.replace(next_due);
@@ -557,7 +570,7 @@ impl<T: Texture2d> GlyphCache<T> {
                     return Ok((sprite.clone(), next));
                 }
 
-                let sprite = atlas.allocate_with_padding(decoded, padding)?;
+                let sprite = atlas.allocate_with_padding(&handle, padding)?;
 
                 frame_cache.insert((id, decoded.current_frame), sprite.clone());
 

wezterm-gui/src/termwindow/mod.rs

@@ -359,15 +359,13 @@ fn reload_background_image(
     match &config.window_background_image {
         Some(p) => match std::fs::read(p) {
             Ok(data) => {
-                if let Some(existing) = image {
-                    match existing.data() {
-                        ImageDataType::EncodedFile(d) if &**d == &*data => {
-                            return Some(Arc::clone(existing));
-                        }
-                        _ => {}
+                let data = ImageDataType::EncodedFile(data).decode();
+                match image {
+                    Some(existing) if existing.hash() == data.compute_hash() => {
+                        Some(Arc::clone(existing))
                     }
+                    _ => Some(Arc::new(ImageData::with_data(data))),
                 }
-                Some(Arc::new(ImageData::with_raw_data(data)))
             }
             Err(err) => {
                 log::error!(