BUGFIX: erasing unicode chars correctly

Author: togglebyte

CHANGES.md

@@ -4,6 +4,7 @@
     * `to_float` template function
     * `PathBuf`s can now be used as templates
     * BUGFIX: tick events no longer tries to use removed components
+    * BUGFIX: erasing characters correctly between frames
 * 0.2.5
     * BUGFIX: component reuse in if / else
     * `with` statement

anathema-backend/src/tui/buffer.rs

@@ -1,5 +1,6 @@
 #![deny(missing_docs)]
 use std::io::{Result, Write};
+use std::ops::Index;
 
 use anathema_geometry::Size;
 use anathema_widgets::Style;
@@ -11,7 +12,7 @@ use super::LocalPos;
 use super::style::write_style;
 
 #[derive(Debug, Copy, Clone, PartialEq)]
-pub(crate) struct Cell {
+pub struct Cell {
     pub(crate) style: Style,
     pub(crate) state: CellState,
 }
@@ -49,7 +50,7 @@ impl Cell {
 }
 
 /// Represent the state of a cell inside a [`Buffer`].
-#[derive(Debug, Copy, Clone, PartialEq)]
+#[derive(Copy, Clone, PartialEq)]
 pub(crate) enum CellState {
     /// Empty
     Empty,
@@ -60,6 +61,17 @@ pub(crate) enum CellState {
     Continuation,
 }
 
+impl std::fmt::Debug for CellState {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            CellState::Empty => write!(f, "<E>"),
+            CellState::Occupied(Glyph::Single(glyph, _width)) => write!(f, "{glyph}"),
+            CellState::Occupied(Glyph::Cluster(index, width)) => write!(f, "<C{index:?},{width}>"),
+            CellState::Continuation => write!(f, "<C>"),
+        }
+    }
+}
+
 /// A buffer contains a list of cells representing characters that can be rendered.
 /// This doesn't necessarily have to be `stdout`, it can be anything that implements
 /// [`std::io::Write`]
@@ -163,7 +175,7 @@ impl Buffer {
             return None;
         }
 
-        let index = self.index(pos);
+        let index = pos.to_index(self.size.width);
         let cell = self.inner.get(index)?;
         match &cell.state {
             CellState::Occupied(c) => Some((c, &cell.style)),
@@ -177,44 +189,28 @@ impl Buffer {
             return None;
         }
 
-        let index = self.index(pos);
+        let index = pos.to_index(self.size.width);
         let cell = self.inner.get_mut(index)?;
         match &mut cell.state {
             CellState::Occupied(c) => Some((c, &mut cell.style)),
             _ => None,
         }
     }
 
-    /// An iterator over all the rows in the buffer
-    pub fn rows(&mut self) -> impl Iterator<Item = impl Iterator<Item = Option<(Glyph, Style)>> + '_> {
-        self.cell_lines().map(|chunk| {
-            chunk.iter().map(|cell| match cell.state {
-                CellState::Occupied(c) => Some((c, cell.style)),
-                _ => None,
-            })
-        })
-    }
-
     pub(super) fn reset_cell(&mut self, pos: LocalPos) {
         let index = pos.to_index(self.size.width);
         self.inner[index] = Cell::empty();
     }
 
-    fn index(&self, pos: LocalPos) -> usize {
-        (pos.y * self.size.width + pos.x) as usize
-    }
-
     fn put(&mut self, mut cell: Cell, pos: LocalPos) {
         let index = pos.to_index(self.size.width);
 
         if let CellState::Occupied(c) = cell.state {
             // If this is a unicode char that is wider than one cell,
             // add a continuation cell if it fits, this way if we overwrite it
             // we can set the continuation cell to `Empty`.
-            if pos.x < self.size.width {
-                if let 2.. = c.width() {
-                    self.put(Cell::continuation(cell.style), LocalPos::new(pos.x + 1, pos.y));
-                }
+            if pos.x < self.size.width && c.width() >= 2 {
+                self.put(Cell::continuation(cell.style), LocalPos::new(pos.x + 1, pos.y));
             }
         }
 
@@ -239,21 +235,33 @@ impl Buffer {
         }
     }
 
-    fn cell_lines(&mut self) -> impl Iterator<Item = &mut [Cell]> {
+    pub(super) fn cell_lines(&mut self) -> impl Iterator<Item = &mut [Cell]> {
         self.inner.chunks_mut(self.size.width as usize)
     }
 }
 
+impl Index<(usize, usize)> for Buffer {
+    type Output = Cell;
+
+    fn index(&self, pos: (usize, usize)) -> &Self::Output {
+        let pos = LocalPos::from(pos);
+        let index = pos.to_index(self.size.width);
+        &self.inner[index]
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub(crate) enum Change {
     Remove,
+    Continuation,
     Insert(Glyph),
 }
 
 impl Change {
     fn width(self) -> usize {
         match self {
             Self::Remove => 1,
+            Self::Continuation => 0,
             Self::Insert(c) => c.width(),
         }
     }
@@ -274,7 +282,7 @@ pub(crate) fn diff(
 
             let change = match new_cell.state {
                 CellState::Empty => Change::Remove,
-                CellState::Continuation => continue,
+                CellState::Continuation => Change::Continuation,
                 CellState::Occupied(c) => Change::Insert(c),
             };
 
@@ -336,6 +344,7 @@ pub(crate) fn draw_changes(
                     }
                 }
             },
+            Change::Continuation => (), // Don't write on continuation, just advance
             Change::Remove => {
                 w.queue(Print(' '))?;
             }

anathema-backend/src/tui/screen.rs

@@ -162,26 +162,26 @@ impl WidgetRenderer for Screen {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::tui::buffer::Cell;
-
-    fn make_screen(size: Size) -> Screen {
-        let mut screen = Screen::new(size);
-        for y in 0..size.height {
-            let c = y.to_string().chars().next().unwrap();
-            for x in 0..size.width {
-                screen.paint_glyph(Glyph::from_char(c, 1), LocalPos::new(x, y));
-            }
-        }
+    use crate::tui::buffer::{Cell, CellState};
 
-        screen
+    fn state_at(buffer: &Buffer, x: usize, y: usize) -> CellState {
+        let cell = buffer[(x, y)];
+        cell.state
+    }
+
+    fn char_at(buffer: &Buffer, x: usize, y: usize) -> char {
+        match state_at(buffer, x, y) {
+            CellState::Occupied(Glyph::Single(c, _)) => c,
+            _ => panic!(),
+        }
     }
 
     #[test]
     fn render() {
         // Render a character
         let mut render_output = vec![];
         let glyph_map = GlyphMap::empty();
-        let mut screen = make_screen(Size::new(1, 1));
+        let mut screen = Screen::new(Size::new(1, 1));
         screen.paint_glyph(Glyph::from_char('x', 1), LocalPos::ZERO);
         screen.render(&mut render_output, &glyph_map).unwrap();
 
@@ -194,14 +194,14 @@ mod test {
     fn erase_region() {
         let mut render_output = vec![];
         let glyph_map = GlyphMap::empty();
-        let mut screen = make_screen(Size::new(2, 2));
+        let mut screen = Screen::new(Size::new(2, 2));
         screen.render(&mut render_output, &glyph_map).unwrap();
 
         // Erase the bottom right corner of the 2x2 region
         screen.erase_region(LocalPos::new(1, 1), Size::new(2, 2));
 
         let top_left = screen.new_buffer.inner[0];
-        assert_eq!(Cell::new(Glyph::from_char('0', 1), Style::reset()), top_left);
+        assert_eq!(Cell::empty(), top_left);
         let bottom_right = screen.new_buffer.inner[3];
         assert_eq!(Cell::empty(), bottom_right);
     }
@@ -211,8 +211,48 @@ mod test {
     fn put_outside_of_screen() {
         // Put a character outside of the screen should panic
         let glyph_map = GlyphMap::empty();
-        let mut screen = make_screen(Size::new(1, 1));
+        let mut screen = Screen::new(Size::new(1, 1));
         screen.paint_glyph(Glyph::from_char('x', 1), LocalPos::new(3, 0));
         screen.render(&mut vec![], &glyph_map).unwrap();
     }
+
+    #[test]
+    fn erasing_unicode_with_continuation_cell() {
+        // Paint a bunny in the next cell
+
+        let glyph_map = GlyphMap::empty();
+        let mut screen = Screen::new(Size::new(4, 1));
+        let bunny = '🐇';
+        // Where B = Bunny, c = continuation, 1 = some character
+        //
+        // Buffer
+        // B c 1 0
+        //
+        // Next buffer
+        // - B c 1
+
+        // First frame: Bc10
+        screen.paint_glyph(Glyph::from_char('1', 1), LocalPos::new(2, 0));
+        screen.paint_glyph(Glyph::from_char('0', 1), LocalPos::new(3, 0));
+        screen.paint_glyph(Glyph::from_char(bunny, 2), LocalPos::new(0, 0));
+
+        screen.render(&mut vec![], &glyph_map).unwrap();
+
+        assert_eq!(char_at(&screen.new_buffer, 0, 0), bunny);
+        assert_eq!(state_at(&screen.new_buffer, 1, 0), CellState::Continuation);
+        assert_eq!(char_at(&screen.new_buffer, 2, 0), '1');
+        assert_eq!(char_at(&screen.new_buffer, 3, 0), '0');
+
+        // // Second frame: -Bc1
+        screen.erase();
+
+        screen.paint_glyph(Glyph::from_char('1', 1), LocalPos::new(3, 0));
+        screen.paint_glyph(Glyph::from_char(bunny, 2), LocalPos::new(1, 0));
+        screen.render(&mut vec![], &glyph_map).unwrap();
+
+        assert_eq!(state_at(&screen.new_buffer, 0, 0), CellState::Empty);
+        assert_eq!(char_at(&screen.new_buffer, 1, 0), bunny);
+        assert_eq!(state_at(&screen.new_buffer, 2, 0), CellState::Continuation);
+        assert_eq!(char_at(&screen.new_buffer, 3, 0), '1');
+    }
 }

anathema-runtime/src/runtime/mod.rs

@@ -327,6 +327,7 @@ impl<'rt, 'bp, G: GlobalEventHandler> Frame<'rt, 'bp, G> {
         let now = Instant::now();
         self.init_new_components();
         let elapsed = self.handle_messages(now);
+
         // Pre cycle events
         self.poll_events(elapsed, now, backend);
         self.drain_deferred_commands();