fix: change Cell::EMPTY to Cell::empty() and fix underline_color bug

This leads to more than a 50% speedup

BREAKING-CHANGES.md

@@ -11,6 +11,7 @@ GitHub with a [breaking change] label.
 This is a quick summary of the sections below:
 
 - [v0.29.0](#unreleased)
+  - `Terminal`, `Buffer`, `Cell`, `Frame` are no longer `Sync` / `RefUnwindSafe`
   - Removed public fields from `Rect` iterators
   - `Line` now implements `From<Cow<str>`
   - `Table::highlight_style` is now `Table::row_highlight_style`
@@ -72,6 +73,17 @@ This is a quick summary of the sections below:
 
 ## Unreleased
 
+### `Terminal`, `Buffer`, `Cell`, `Frame` are no longer `Sync` / `RefUnwindSafe` [#1339]
+
+[#1339]: https://github.com/ratatui/ratatui/pull/1339
+
+In #1339, we added a cache of the Cell width which uses a std::cell::Cell. This causes `Cell` and
+all types that contain this (`Terminal`, `Buffer`, `Frame`, `CompletedFrame`, `TestBackend`) to no
+longer be `Sync`
+
+This change is unlikely to cause problems as these types likely should not be sent between threads
+regardless due to their interaction with various things which mutated externally (e.g. stdio).
+
 ### Removed public fields from `Rect` iterators ([#1358])
 
 [#1358]: https://github.com/ratatui/ratatui/pull/1358

benches/main/buffer.rs

@@ -1,11 +1,14 @@
+use std::iter::zip;
+
 use criterion::{black_box, BenchmarkId, Criterion};
 use ratatui::{
     buffer::{Buffer, Cell},
     layout::Rect,
     text::Line,
+    widgets::Widget,
 };
 
-criterion::criterion_group!(benches, empty, filled, with_lines);
+criterion::criterion_group!(benches, empty, filled, with_lines, diff);
 
 const fn rect(size: u16) -> Rect {
     Rect::new(0, 0, size, size)
@@ -58,3 +61,37 @@ fn with_lines(c: &mut Criterion) {
     }
     group.finish();
 }
+
+fn diff(c: &mut Criterion) {
+    const AREA: Rect = Rect {
+        x: 0,
+        y: 0,
+        width: 200,
+        height: 50,
+    };
+    c.bench_function("buffer/diff", |b| {
+        let buffer_1 = create_random_buffer(AREA);
+        let buffer_2 = create_random_buffer(AREA);
+        b.iter(|| {
+            let _ = black_box(&buffer_1).diff(black_box(&buffer_2));
+        });
+    });
+}
+
+fn create_random_buffer(area: Rect) -> Buffer {
+    const PARAGRAPH_COUNT: i64 = 15;
+    const SENTENCE_COUNT: i64 = 5;
+    const WORD_COUNT: i64 = 20;
+    const SEPARATOR: &str = "\n\n";
+    let paragraphs = fakeit::words::paragraph(
+        PARAGRAPH_COUNT,
+        SENTENCE_COUNT,
+        WORD_COUNT,
+        SEPARATOR.to_string(),
+    );
+    let mut buffer = Buffer::empty(area);
+    for (line, row) in zip(paragraphs.lines(), area.rows()) {
+        Line::from(line).render(row, &mut buffer);
+    }
+    buffer
+}

src/backend/test.rs

@@ -6,8 +6,6 @@ use std::{
     io, iter,
 };
 
-use unicode_width::UnicodeWidthStr;
-
 use crate::{
     backend::{Backend, ClearType, WindowSize},
     buffer::{Buffer, Cell},
@@ -52,13 +50,13 @@ fn buffer_view(buffer: &Buffer) -> String {
         let mut overwritten = vec![];
         let mut skip: usize = 0;
         view.push('"');
-        for (x, c) in cells.iter().enumerate() {
+        for (x, cell) in cells.iter().enumerate() {
             if skip == 0 {
-                view.push_str(c.symbol());
+                view.push_str(cell.symbol());
             } else {
-                overwritten.push((x, c.symbol()));
+                overwritten.push((x, cell.symbol()));
             }
-            skip = std::cmp::max(skip, c.symbol().width()).saturating_sub(1);
+            skip = std::cmp::max(skip, cell.width()).saturating_sub(1);
         }
         view.push('"');
         if !overwritten.is_empty() {

src/buffer/buffer.rs

@@ -79,7 +79,7 @@ impl Buffer {
     /// Returns a Buffer with all cells set to the default one
     #[must_use]
     pub fn empty(area: Rect) -> Self {
-        Self::filled(area, Cell::EMPTY)
+        Self::filled(area, Cell::empty())
     }
 
     /// Returns a Buffer with all cells initialized with the attributes of the given Cell
@@ -414,7 +414,7 @@ impl Buffer {
         if self.content.len() > length {
             self.content.truncate(length);
         } else {
-            self.content.resize(length, Cell::EMPTY);
+            self.content.resize(length, Cell::empty());
         }
         self.area = area;
     }
@@ -429,7 +429,7 @@ impl Buffer {
     /// Merge an other buffer into this one
     pub fn merge(&mut self, other: &Self) {
         let area = self.area.union(other.area);
-        self.content.resize(area.area() as usize, Cell::EMPTY);
+        self.content.resize(area.area() as usize, Cell::empty());
 
         // Move original content to the appropriate space
         let size = self.area.area() as usize;
@@ -499,9 +499,8 @@ impl Buffer {
                 updates.push((x, y, &next_buffer[i]));
             }
 
-            to_skip = current.symbol().width().saturating_sub(1);
-
-            let affected_width = std::cmp::max(current.symbol().width(), previous.symbol().width());
+            to_skip = current.width().saturating_sub(1);
+            let affected_width = std::cmp::max(current.width(), previous.width());
             invalidated = std::cmp::max(affected_width, invalidated).saturating_sub(1);
         }
         updates
@@ -598,7 +597,7 @@ impl fmt::Debug for Buffer {
                 } else {
                     overwritten.push((x, c.symbol()));
                 }
-                skip = std::cmp::max(skip, c.symbol().width()).saturating_sub(1);
+                skip = std::cmp::max(skip, c.width()).saturating_sub(1);
                 #[cfg(feature = "underline-color")]
                 {
                     let style = (c.fg, c.bg, c.underline_color, c.modifier);

src/buffer/cell.rs

@@ -1,9 +1,12 @@
+use std::hash::{Hash, Hasher};
+
 use compact_str::CompactString;
+use unicode_width::UnicodeWidthStr;
 
 use crate::style::{Color, Modifier, Style};
 
 /// A buffer cell
-#[derive(Debug, Clone, Eq, PartialEq, Hash)]
+#[derive(Debug, Clone, Eq)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Cell {
     /// The string to be drawn in the cell.
@@ -31,11 +34,57 @@ pub struct Cell {
 
     /// Whether the cell should be skipped when copying (diffing) the buffer to the screen.
     pub skip: bool,
+
+    /// Cache the width of the cell.
+    width: std::cell::Cell<Option<usize>>,
+}
+
+impl PartialEq for Cell {
+    fn eq(&self, other: &Self) -> bool {
+        let eq = self.symbol == other.symbol
+            && self.fg == other.fg
+            && self.bg == other.bg
+            && self.modifier == other.modifier
+            && self.skip == other.skip;
+        // explicitly not comparing width, as it is a cache and may be not set
+        // && self.width == other.width
+
+        #[cfg(feature = "underline-color")]
+        return eq && self.underline_color == other.underline_color;
+        #[cfg(not(feature = "underline-color"))]
+        return eq;
+    }
+}
+
+impl Hash for Cell {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.symbol.hash(state);
+        self.fg.hash(state);
+        self.bg.hash(state);
+        #[cfg(feature = "underline-color")]
+        self.underline_color.hash(state);
+        self.modifier.hash(state);
+        self.skip.hash(state);
+
+        // explicitly not hashing width, as it is a cache and not part of the cell's identity
+        // self.width.hash(state);
+    }
 }
 
 impl Cell {
     /// An empty `Cell`
-    pub const EMPTY: Self = Self::new(" ");
+    pub const fn empty() -> Self {
+        Self {
+            symbol: CompactString::const_new(" "),
+            fg: Color::Reset,
+            bg: Color::Reset,
+            #[cfg(feature = "underline-color")]
+            underline_color: Color::Reset,
+            modifier: Modifier::empty(),
+            skip: false,
+            width: std::cell::Cell::new(Some(1)),
+        }
+    }
 
     /// Creates a new `Cell` with the given symbol.
     ///
@@ -52,6 +101,7 @@ impl Cell {
             underline_color: Color::Reset,
             modifier: Modifier::empty(),
             skip: false,
+            width: std::cell::Cell::new(None),
         }
     }
 
@@ -64,6 +114,7 @@ impl Cell {
     /// Sets the symbol of the cell.
     pub fn set_symbol(&mut self, symbol: &str) -> &mut Self {
         self.symbol = CompactString::new(symbol);
+        self.width.set(None);
         self
     }
 
@@ -72,6 +123,7 @@ impl Cell {
     /// This is particularly useful for adding zero-width characters to the cell.
     pub(crate) fn append_symbol(&mut self, symbol: &str) -> &mut Self {
         self.symbol.push_str(symbol);
+        self.width.set(None);
         self
     }
 
@@ -79,6 +131,7 @@ impl Cell {
     pub fn set_char(&mut self, ch: char) -> &mut Self {
         let mut buf = [0; 4];
         self.symbol = CompactString::new(ch.encode_utf8(&mut buf));
+        self.width.set(None);
         self
     }
 
@@ -148,18 +201,33 @@ impl Cell {
         }
         self.modifier = Modifier::empty();
         self.skip = false;
+        self.width.set(Some(1));
+    }
+
+    /// Returns the width of the cell.
+    ///
+    /// This value is cached and will only be recomputed when the cell is modified.
+    #[must_use]
+    pub fn width(&self) -> usize {
+        if let Some(width) = self.width.get() {
+            width
+        } else {
+            let width = self.symbol().width();
+            self.width.set(Some(width));
+            width
+        }
     }
 }
 
 impl Default for Cell {
     fn default() -> Self {
-        Self::EMPTY
+        Self::empty()
     }
 }
 
 impl From<char> for Cell {
     fn from(ch: char) -> Self {
-        let mut cell = Self::EMPTY;
+        let mut cell = Self::empty();
         cell.set_char(ch);
         cell
     }
@@ -182,19 +250,20 @@ mod tests {
                 underline_color: Color::Reset,
                 modifier: Modifier::empty(),
                 skip: false,
+                width: std::cell::Cell::new(None),
             }
         );
     }
 
     #[test]
     fn empty() {
-        let cell = Cell::EMPTY;
+        let cell = Cell::empty();
         assert_eq!(cell.symbol(), " ");
     }
 
     #[test]
     fn set_symbol() {
-        let mut cell = Cell::EMPTY;
+        let mut cell = Cell::empty();
         cell.set_symbol("あ"); // Multi-byte character
         assert_eq!(cell.symbol(), "あ");
         cell.set_symbol("👨‍👩‍👧‍👦"); // Multiple code units combined with ZWJ
@@ -203,7 +272,7 @@ mod tests {
 
     #[test]
     fn append_symbol() {
-        let mut cell = Cell::EMPTY;
+        let mut cell = Cell::empty();
         cell.set_symbol("あ"); // Multi-byte character
         cell.append_symbol("\u{200B}"); // zero-width space
         assert_eq!(cell.symbol(), "あ\u{200B}");
@@ -211,28 +280,28 @@ mod tests {
 
     #[test]
     fn set_char() {
-        let mut cell = Cell::EMPTY;
+        let mut cell = Cell::empty();
         cell.set_char('あ'); // Multi-byte character
         assert_eq!(cell.symbol(), "あ");
     }
 
     #[test]
     fn set_fg() {
-        let mut cell = Cell::EMPTY;
+        let mut cell = Cell::empty();
         cell.set_fg(Color::Red);
         assert_eq!(cell.fg, Color::Red);
     }
 
     #[test]
     fn set_bg() {
-        let mut cell = Cell::EMPTY;
+        let mut cell = Cell::empty();
         cell.set_bg(Color::Red);
         assert_eq!(cell.bg, Color::Red);
     }
 
     #[test]
     fn set_style() {
-        let mut cell = Cell::EMPTY;
+        let mut cell = Cell::empty();
         cell.set_style(Style::new().fg(Color::Red).bg(Color::Blue));
         assert_eq!(cell.fg, Color::Red);
         assert_eq!(cell.bg, Color::Blue);
@@ -240,14 +309,14 @@ mod tests {
 
     #[test]
     fn set_skip() {
-        let mut cell = Cell::EMPTY;
+        let mut cell = Cell::empty();
         cell.set_skip(true);
         assert!(cell.skip);
     }
 
     #[test]
     fn reset() {
-        let mut cell = Cell::EMPTY;
+        let mut cell = Cell::empty();
         cell.set_symbol("あ");
         cell.set_fg(Color::Red);
         cell.set_bg(Color::Blue);
@@ -261,7 +330,7 @@ mod tests {
 
     #[test]
     fn style() {
-        let cell = Cell::EMPTY;
+        let cell = Cell::empty();
         assert_eq!(
             cell.style(),
             Style {
@@ -294,4 +363,12 @@ mod tests {
         let cell2 = Cell::new("い");
         assert_ne!(cell1, cell2);
     }
+
+    #[test]
+    fn width() {
+        let cell = Cell::new("あ");
+        assert_eq!(cell.width, std::cell::Cell::new(None)); // not yet cached
+        assert_eq!(cell.width(), 2);
+        assert_eq!(cell.width, std::cell::Cell::new(Some(2))); // cached
+    }
 }