From 5e6857881b290a6aca2a60a300844ef0f3f1c5fa Mon Sep 17 00:00:00 2001 From: itsjunetime Date: Sun, 8 Jun 2025 21:10:18 -0600 Subject: [PATCH] incorporate recovering from deleted images --- .github/workflows/rust.yml | 2 + Cargo.lock | 1 - Cargo.toml | 1 - src/converter.rs | 25 +++++---- src/kitty.rs | 55 +++++++++---------- src/main.rs | 71 +++++++++++++------------ src/renderer.rs | 98 ++++++++++++++++++++-------------- src/skip.rs | 106 +++++++++++++++++++++++++++++++++++++ src/tui.rs | 2 +- 9 files changed, 248 insertions(+), 113 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 61ccf7b..2f8eada 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -32,6 +32,8 @@ jobs: run: rustup component add clippy rustfmt - name: Clippy run: cargo clippy -- -D warnings + - name: Tests + run: cargo test - name: Check fmt run: cargo fmt -- --check - name: Run tests diff --git a/Cargo.lock b/Cargo.lock index 4a2196c..dd0f51e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3015,7 +3015,6 @@ dependencies = [ "flume", "futures-util", "image", - "itertools 0.14.0", "kittage", "log", "memmap2", diff --git a/Cargo.toml b/Cargo.toml index 6064bf7..7b24149 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,6 @@ image = { version = "0.25.1", features = ["pnm", "rayon", "png"], default-featur notify = { version = "8.0.0", features = ["crossbeam-channel"] } tokio = { version = "1.37.0", features = ["rt-multi-thread", "macros"] } futures-util = { version = "0.3.30", default-features = false } -itertools = "*" flume = { version = "0.11.0", default-features = false, features = ["async"] } xflags = "0.4.0-pre.2" mimalloc = "0.1.43" diff --git a/src/converter.rs b/src/converter.rs index 131e333..1f281a6 100644 --- a/src/converter.rs +++ b/src/converter.rs @@ -1,9 +1,8 @@ -use std::num::NonZeroU32; +use std::num::{NonZeroU32, NonZeroUsize}; use flume::{Receiver, SendError, Sender, TryRecvError}; use futures_util::stream::StreamExt; use image::DynamicImage; -use itertools::Itertools; use kittage::NumberOrId; use ratatui::layout::Rect; use ratatui_image::{ @@ -13,9 +12,11 @@ use ratatui_image::{ }; use rayon::iter::ParallelIterator; -use crate::renderer::{PageInfo, RenderError, fill_default}; +use crate::{ + renderer::{PageInfo, RenderError, fill_default}, + skip::InterleavedAroundWithMax +}; -#[derive(Debug)] pub enum MaybeTransferred { NotYet(kittage::image::Image<'static>), Transferred(kittage::ImageId) @@ -74,13 +75,19 @@ pub async fn run_conversion_loop( let idx_start = page.saturating_sub(prerender / 2); let idx_end = idx_start.saturating_add(prerender).min(images.len()); + // If there's none to render, then why bother. + let Some(idx_end) = NonZeroUsize::new(idx_end) else { + return Ok(None); + }; + // then we go through all the indices available to us and find the first one that has an // image available to steal - let Some((page_info, new_iter, page_num)) = (idx_start..page) - .interleave(page..idx_end) - .enumerate() - // .skip(*iteration) - .find_map(|(i_idx, p_idx)| images[p_idx].take().map(|p| (p, i_idx, p_idx))) + let Some((page_info, new_iter, page_num)) = + InterleavedAroundWithMax::new(page, idx_start, idx_end) + .enumerate() + .take(prerender) + // .skip(*iteration) + .find_map(|(i_idx, p_idx)| images[p_idx].take().map(|p| (p, i_idx, p_idx))) else { return Ok(None); }; diff --git a/src/kitty.rs b/src/kitty.rs index af8456f..a5ce954 100644 --- a/src/kitty.rs +++ b/src/kitty.rs @@ -14,7 +14,6 @@ use ratatui::prelude::Rect; use crate::converter::MaybeTransferred; -#[derive(Debug)] pub struct DbgWriter { w: W, #[cfg(debug_assertions)] @@ -44,8 +43,7 @@ impl Write for DbgWriter { pub async fn run_action<'image, 'data, 'es>( action: Action<'image, 'data>, ev_stream: &'es mut EventStream -) -> Result::Error>> -{ +) -> Result::Error>> { let writer = DbgWriter { w: std::io::stdout().lock(), #[cfg(debug_assertions)] @@ -57,14 +55,17 @@ pub async fn run_action<'image, 'data, 'es>( .map(|(_, i)| i) } -pub async fn display_kitty_images( +pub async fn display_kitty_images<'es>( images: Vec<(usize, &mut MaybeTransferred, Rect)>, - ev_stream: &mut EventStream -) -> Result<(), (Vec, String)> { - if images.is_empty() { - return Ok(()); - } - + ev_stream: &'es mut EventStream +) -> Result< + (), + ( + Vec, + &'static str, + TransmitError<<&'es mut EventStream as AsyncInputReader>::Error> + ) +> { run_action( Action::Delete(DeleteConfig { effect: ClearOrDelete::Clear, @@ -73,7 +74,7 @@ pub async fn display_kitty_images( ev_stream ) .await - .map_err(|e| (vec![], format!("Couldn't clear previous images: {e}")))?; + .map_err(|e| (vec![], "Couldn't clear previous images", e))?; let mut err = None; for (page_num, img, area) in images { @@ -114,34 +115,30 @@ pub async fn display_kitty_images( *img = MaybeTransferred::Transferred(img_id); Ok(()) } - Err(e) => Err((Some(page_num), e.to_string())), + Err(e) => Err((page_num, e)) } } - MaybeTransferred::Transferred(image_id) => { - run_action( - Action::Display { - image_id: *image_id, - placement_id: *image_id, - config - }, - ev_stream - ) - .await - .map(|_| ()) - .map_err(|e| (None, e.to_string())) - } + MaybeTransferred::Transferred(image_id) => run_action( + Action::Display { + image_id: *image_id, + placement_id: *image_id, + config + }, + ev_stream + ) + .await + .map(|_| ()) + .map_err(|e| (page_num, e)) }; if let Err((id, e)) = this_err { let e = err.get_or_insert_with(|| (vec![], e)); - if let Some(id) = id { - e.0.push(id); - } + e.0.push(id); } } match err { - Some(e) => Err(e), + Some((replace, e)) => Err((replace, "Couldn't transfer image to the terminal", e)), None => Ok(()) } } diff --git a/src/main.rs b/src/main.rs index aec99d2..61db985 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,7 +2,7 @@ use core::error::Error; use std::{ borrow::Cow, ffi::OsString, - io::{BufReader, Read, Write, stdout}, + io::{stdout, BufReader, Read, Stdout, Write}, num::{NonZeroU32, NonZeroUsize}, path::PathBuf }; @@ -20,7 +20,8 @@ use flexi_logger::FileSpec; use futures_util::{FutureExt, stream::StreamExt}; use kittage::{ action::Action, - delete::{ClearOrDelete, DeleteConfig, WhichToDelete} + delete::{ClearOrDelete, DeleteConfig, WhichToDelete}, + error::{TerminalError, TransmitError} }; use notify::{Event, EventKind, RecursiveMode, Watcher}; use ratatui::{Terminal, backend::CrosstermBackend}; @@ -103,17 +104,17 @@ async fn main() -> Result<(), WrappedErr> { maybe_logger = Some( flexi_logger::Logger::try_with_env() - .map_err(|e| WrappedErr(format!("Couldn't create initial logger: {e}")))? + .map_err(|e| WrappedErr(format!("Couldn't create initial logger: {e}").into()))? .log_to_file(FileSpec::try_from("./debug.log").map_err(|e| { - WrappedErr(format!("Couldn't create FileSpec for logger: {e}")) + WrappedErr(format!("Couldn't create FileSpec for logger: {e}").into()) })?) .start() - .map_err(|e| WrappedErr(format!("Can't start logger: {e}")))? + .map_err(|e| WrappedErr(format!("Can't start logger: {e}").into()))? ); } let (watch_to_render_tx, render_rx) = flume::unbounded(); - let tui_tx = watch_to_render_tx.clone(); + let to_renderer = watch_to_render_tx.clone(); let (render_tx, tui_rx) = flume::unbounded(); let watch_to_tui_tx = render_tx.clone(); @@ -247,7 +248,7 @@ async fn main() -> Result<(), WrappedErr> { ) }); - let ev_stream = crossterm::event::EventStream::new(); + let mut ev_stream = crossterm::event::EventStream::new(); let (to_converter, from_main) = flume::unbounded(); let (to_main, from_converter) = flume::unbounded(); @@ -294,12 +295,13 @@ async fn main() -> Result<(), WrappedErr> { }), &mut ev_stream ) - .await?; + .await + .map_err(|e| WrappedErr(format!("Couldn't delete all previous images from memory: {e}").into()))?; } let fullscreen = flags.fullscreen.unwrap_or_default(); let main_area = Tui::main_layout(&term.get_frame(), fullscreen); - tui_tx + to_renderer .send(RenderNotif::Area(main_area.page_area)) .map_err(|e| { WrappedErr( @@ -312,7 +314,7 @@ async fn main() -> Result<(), WrappedErr> { enter_redraw_loop( ev_stream, - tui_tx, + to_renderer, tui_rx, to_converter, from_converter, @@ -339,6 +341,8 @@ async fn main() -> Result<(), WrappedErr> { .unwrap(); disable_raw_mode().unwrap(); + drop(maybe_logger); + Ok(()) } @@ -346,7 +350,7 @@ async fn main() -> Result<(), WrappedErr> { #[expect(clippy::too_many_arguments)] async fn enter_redraw_loop( mut ev_stream: EventStream, - tui_tx: Sender, + to_renderer: Sender, mut tui_rx: RecvStream<'_, Result>, to_converter: Sender, mut from_converter: RecvStream<'_, Result>, @@ -370,11 +374,11 @@ async fn enter_redraw_loop( InputAction::Redraw => (), InputAction::QuitApp => return Ok(()), InputAction::JumpingToPage(page) => { - tui_tx.send(RenderNotif::JumpToPage(page))?; + to_renderer.send(RenderNotif::JumpToPage(page))?; to_converter.send(ConverterMsg::GoToPage(page))?; }, - InputAction::Search(term) => tui_tx.send(RenderNotif::Search(term))?, - InputAction::Invert => tui_tx.send(RenderNotif::Invert)?, + InputAction::Search(term) => to_renderer.send(RenderNotif::Search(term))?, + InputAction::Invert => to_renderer.send(RenderNotif::Invert)?, InputAction::Fullscreen => fullscreen = !fullscreen, } } @@ -408,7 +412,7 @@ async fn enter_redraw_loop( let new_area = Tui::main_layout(&term.get_frame(), fullscreen); if new_area != main_area { main_area = new_area; - tui_tx.send(RenderNotif::Area(main_area.page_area))?; + to_renderer.send(RenderNotif::Area(main_area.page_area))?; needs_redraw = true; } @@ -418,32 +422,33 @@ async fn enter_redraw_loop( to_display = tui.render(f, &main_area); })?; - let maybe_err = display_kitty_images(to_display, &mut ev_stream).await; + if !to_display.is_empty() { + let maybe_err = display_kitty_images(to_display, &mut ev_stream).await; - if let Err((to_replace, e)) = maybe_err { - tui.set_msg(MessageSetting::Some(BottomMessage::Error(format!( - "Couldn't transfer image to the terminal: {e}" - )))); + if let Err((to_replace, err_desc, enum_err)) = maybe_err { + match enum_err { + // This is the error that kitty provides us when it deletes an image due to + // memory constraints, so if we get it, we just fix it by re-rendering and + // don't display it to the user + TransmitError::Terminal(TerminalError::NoEntity(e)) + if e.contains("refers to non-existent image") => + (), + _ => tui.set_msg(MessageSetting::Some(BottomMessage::Error(format!( + "{err_desc}: {enum_err}" + )))) + } - for page_num in to_replace { - tui.page_failed_display(page_num); + for page_num in to_replace { + tui.page_failed_display(page_num); + // So that they get re-rendered and sent over again + to_renderer.send(RenderNotif::PageNeedsReRender(page_num))?; + } } } execute!(stdout().lock(), EndSynchronizedUpdate)?; } } - - execute!( - term.backend_mut(), - LeaveAlternateScreen, - crossterm::cursor::Show - )?; - disable_raw_mode()?; - - drop(maybe_logger); - - Ok(()) } fn on_notify_ev( diff --git a/src/renderer.rs b/src/renderer.rs index 708eb25..379ce70 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -1,17 +1,17 @@ -use std::{thread::sleep, time::Duration}; +use std::{collections::VecDeque, num::NonZeroUsize, thread::sleep, time::Duration}; use flume::{Receiver, SendError, Sender, TryRecvError}; -use itertools::Itertools; use mupdf::{ Colorspace, Document, Matrix, Page, Pixmap, Quad, TextPageOptions, text_page::SearchHitResponse }; use ratatui::layout::Rect; -use crate::PrerenderLimit; +use crate::{PrerenderLimit, skip::InterleavedAroundWithMax}; pub enum RenderNotif { Area(Rect), JumpToPage(usize), + PageNeedsReRender(usize), Search(String), Reload, Invert @@ -72,7 +72,7 @@ pub fn fill_default(vec: &mut Vec, size: usize) { // We're allowing passing by value here because this is only called once, at the beginning of the // program, and the arguments that 'should' be passed by value (`receiver` and `size`) would // probably be more performant if accessed by-value instead of through a reference. Probably. -#[allow(clippy::needless_pass_by_value)] +#[allow(clippy::needless_pass_by_value, clippy::too_many_arguments)] pub fn start_rendering( path: &str, sender: Sender>, @@ -94,6 +94,8 @@ pub fn start_rendering( let mut invert = false; let mut preserved_area = None; + let mut need_rerender = VecDeque::new(); + 'reload: loop { let doc = match Document::open(path) { Err(e) => { @@ -126,7 +128,13 @@ pub fn start_rendering( }; let n_pages = match doc.page_count() { - Ok(n) => n as usize, + Ok(n) => match NonZeroUsize::new(n as usize) { + Some(n) => n, + None => { + sleep(Duration::from_secs(1)); + continue 'reload; + } + }, Err(e) => { sender.send(Err(RenderError::Doc(e)))?; // just basic backoff i think @@ -135,7 +143,7 @@ pub fn start_rendering( } }; - sender.send(Ok(RenderInfo::NumPages(n_pages)))?; + sender.send(Ok(RenderInfo::NumPages(n_pages.get())))?; // We're using this vec of bools to indicate which page numbers have already been rendered, // to support people jumping to specific pages and having quick rendering results. We @@ -143,7 +151,7 @@ pub fn start_rendering( // doing basically nothing, but if we get a notification that something has been jumped to, // then we can split at that page and render at both sides of it let mut rendered = Vec::new(); - fill_default::(&mut rendered, n_pages); + fill_default::(&mut rendered, n_pages.get()); let mut start_point = 0; // This is kinda a weird way of doing this, but if we get a notification that the area @@ -163,6 +171,9 @@ pub fn start_rendering( new_area }); + let area_w = f32::from(area.width) * f32::from(col_w); + let area_h = f32::from(area.height) * f32::from(col_h); + // what we do with a notif is the same regardless of if we're in the middle of // rendering the list of pages or we're all done macro_rules! handle_notif { @@ -178,13 +189,18 @@ pub fn start_rendering( } RenderNotif::Area(new_area) => { preserved_area = Some(new_area); - fill_default(&mut rendered, n_pages); + fill_default(&mut rendered, n_pages.get()); continue 'render_pages; } RenderNotif::JumpToPage(page) => { start_point = page; continue 'render_pages; } + RenderNotif::PageNeedsReRender(page) => { + rendered[page].successful = false; + need_rerender.push_back(page); + continue 'render_pages; + } RenderNotif::Search(term) => { if term.is_empty() { // If the term is set to nothing, then we don't need to re-render @@ -214,28 +230,21 @@ pub fn start_rendering( } let any_not_searched = rendered.iter().any(|r| r.num_search_found.is_none()); - let (left, right) = rendered.split_at_mut(start_point); // This is our iterator over all the pages we want to look at and render. It uses this // weird 'interleave' thing to render pages on *both sides* of the currently-displayed // page in case they device to go forward or backwards. - let page_iter = right - .iter_mut() - .enumerate() - .map(move |(idx, p)| (idx + start_point, p)) - .interleave( - left.iter_mut() - .rev() - .enumerate() - .map(move |(idx, p)| (start_point - (idx + 1), p)) - ) - .take(match (&prerender, &search_term) { + let page_iter = PopOnNext { + inner: &mut need_rerender + } + .chain(InterleavedAroundWithMax::new(start_point, 0, n_pages).take( + match (&prerender, &search_term) { // If the user has limited the amount of pages they want to prerender, then we // just do what they ask. Nice and easy. (PrerenderLimit::Limited(l), _) => l.get(), // If they haven't limited it, but we don't have any search term that we're // currently looking for, just go for all of it - (PrerenderLimit::All, None) => n_pages, + (PrerenderLimit::All, None) => n_pages.get(), // If they haven't limited it, and we DO have a search term we need to look // for, just do 20 so that we don't dramatically slow down the search process // since they've specifically initiated that and so we want it to take priority @@ -243,15 +252,15 @@ pub fn start_rendering( if any_not_searched { 20 } else { - n_pages + n_pages.get() }, - }); - - let area_w = f32::from(area.width) * f32::from(col_w); - let area_h = f32::from(area.height) * f32::from(col_h); + } + )); // we go through each page - for (num, rendered) in page_iter { + for page_num in page_iter { + let rendered = &mut rendered[page_num]; + // we only want to continue if one of the following is met: // 1. It failed to render last time (we want to retry) // 2. The `contained_term` is set to Unknown, meaning that we need to at least @@ -260,18 +269,9 @@ pub fn start_rendering( continue; } - // check if we've been told to change the area that we're rendering to, - // or if we're told to rerender - match receiver.try_recv() { - // If it's disconnected, then the main loop is done, so we should just give up - Err(TryRecvError::Disconnected) => return Ok(()), - Ok(notif) => handle_notif!(notif), - Err(TryRecvError::Empty) => () - }; - // We know this is in range 'cause we're iterating over it but we still just want // to be safe - let page = match doc.load_page(num as i32) { + let page = match doc.load_page(page_num as i32) { Err(e) => { sender.send(Err(RenderError::Doc(e)))?; continue; @@ -310,13 +310,22 @@ pub fn start_rendering( cell_w: (ctx.surface_w / f32::from(col_w)) as u16, cell_h: (ctx.surface_h / f32::from(col_h)) as u16 }, - page_num: num, + page_num, result_rects: ctx.result_rects })))?; } // And if we got an error, then obviously we need to propagate that Err(e) => sender.send(Err(RenderError::Doc(e)))? } + + // check if we've been told to change the area that we're rendering to, + // or if we're told to rerender + match receiver.try_recv() { + // If it's disconnected, then the main loop is done, so we should just give up + Err(TryRecvError::Disconnected) => return Ok(()), + Ok(notif) => handle_notif!(notif), + Err(TryRecvError::Empty) => () + }; } // Now, if we have a search term, we want to look through the rest of the document past @@ -371,7 +380,7 @@ pub fn start_rendering( // now, we want to check if we've gone past the end - if so, we go back to the // beginning so we can get the pages before the current one. - if search_start > n_pages { + if search_start > n_pages.get() { if start_point == 0 { break; } @@ -544,3 +553,14 @@ fn count_search_results(page: &Page, search_term: &str) -> Result { + inner: &'a mut VecDeque +} + +impl<'a> Iterator for PopOnNext<'a> { + type Item = usize; + fn next(&mut self) -> Option { + self.inner.pop_front() + } +} diff --git a/src/skip.rs b/src/skip.rs index 5d0898d..9ac6c7a 100644 --- a/src/skip.rs +++ b/src/skip.rs @@ -1,3 +1,5 @@ +use std::num::NonZeroUsize; + use ratatui::widgets::Widget; pub struct Skip { @@ -19,3 +21,107 @@ impl Widget for Skip { } } } + +enum PlusOrMinus { + Plus, + Minus +} + +pub struct InterleavedAroundWithMax { + // starts at this number + around: usize, + inclusive_min: usize, + // this iterator can only produce values in [0..max) + exclusive_max: NonZeroUsize, + // the next time we call `next()`, this value should be combined with `around` according to + // `next_op`, then, after next_op is inverted, incremented if next_op was negative before being + // inverted. + next_change: usize, + // How `next_change` should be applied to `around` next time `next()` is called + next_op: PlusOrMinus +} + +impl InterleavedAroundWithMax { + /// the following must hold or else this is liable to panic or produce nonsense values: + /// - inclusive_min < exclusive_max + /// - inclusive_min <= around <= exclusive_max + pub fn new(around: usize, inclusive_min: usize, exclusive_max: NonZeroUsize) -> Self { + Self { + around, + inclusive_min, + exclusive_max, + next_change: 0, + next_op: PlusOrMinus::Minus + } + } +} + +impl Iterator for InterleavedAroundWithMax { + type Item = usize; + fn next(&mut self) -> Option { + let actual_change = self.next_change % (self.exclusive_max.get() - self.inclusive_min); + + let to_return = match self.next_op { + // If we're supposed to add them and we need it to wrap, then try to add them together + // 'cause we need special behavior if it overflows usize's limits + PlusOrMinus::Plus => match self.around.checked_add(actual_change) { + // If we added it and it's within the range, we're chillin + Some(next_val) if next_val < self.exclusive_max.get() => next_val, + // If we added it and it's not within the range, do next_val % (self.max + 1), e.g. + // if max is 20, we were at 15, and we added 7, we should get 1 (because +5 would + // hit the max, then 0, then 1). So adding 1 before the modulo makes it hit the + // right numbers. And we can be sure the + here doesn't overflow 'cause we already + // checked the `usize::MAX` up above + Some(next_val) => (next_val % self.exclusive_max.get()) + self.inclusive_min, + // If we added them and it would've overflowed usize::MAX, then we see how much + // of the change would be remaining after reaching `max` + None => + (actual_change - (self.exclusive_max.get() - actual_change)) + + self.inclusive_min, + }, + PlusOrMinus::Minus => match self.around.checked_sub(actual_change) { + // If we can just minus it, cool cool. All is good. + Some(next_val) if next_val >= self.inclusive_min => next_val, + // If we can minus it but it goes below our min, then see how much below it went + // and just manually wrap it around + Some(next_val) => self.exclusive_max.get() - (self.inclusive_min - next_val), + // If we can't... + None => { + // then we see how much of the change would be remaining after hitting the + // minimum + let remaining = actual_change - (self.around - self.inclusive_min); + + // and then we take that away from the top! + self.exclusive_max.get() - remaining + } + } + }; + + self.next_op = match self.next_op { + PlusOrMinus::Plus => PlusOrMinus::Minus, + PlusOrMinus::Minus => { + self.next_change = (self.next_change + 1) % self.exclusive_max.get(); + PlusOrMinus::Plus + } + }; + + Some(to_return) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn iter_works() { + let got = InterleavedAroundWithMax::new(5, 2, NonZeroUsize::new(21).unwrap()) + .take(30) + .collect::>(); + + assert_eq!(got, vec![ + 5, 6, 4, 7, 3, 8, 2, 9, 20, 10, 19, 11, 18, 12, 17, 13, 16, 14, 15, 15, 14, 16, 13, 17, + 12, 18, 11, 19, 10, 20 + ]); + } +} diff --git a/src/tui.rs b/src/tui.rs index 34fa2ec..2224cff 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -41,7 +41,7 @@ pub struct Tui { showing_help_msg: bool } -#[derive(Default, Debug)] +#[derive(Default)] struct LastRender { // Used as a way to track if we need to draw the images, to save ratatui from doing a lot of // diffing work