From 1f818c614b9f86caf23160cef517ac875e877c7b Mon Sep 17 00:00:00 2001 From: Thang Pham Date: Mon, 11 Mar 2024 22:36:34 -0400 Subject: [PATCH] re-render image if the last rectangle is different from the current one (#390) Resolves #389 This PR also reverts the rendering order back to the before `v0.17.0` in which playback window is rendered before any popups. --- spotify_player/src/state/ui/mod.rs | 16 ++++++-- spotify_player/src/ui/mod.rs | 14 ++++--- spotify_player/src/ui/playback.rs | 64 ++++++++++++++++++------------ 3 files changed, 60 insertions(+), 34 deletions(-) diff --git a/spotify_player/src/state/ui/mod.rs b/spotify_player/src/state/ui/mod.rs index 0fc0f28d..c9bf1bac 100644 --- a/spotify_player/src/state/ui/mod.rs +++ b/spotify_player/src/state/ui/mod.rs @@ -10,6 +10,14 @@ use super::*; pub use page::*; pub use popup::*; +#[derive(Default, Debug)] +pub struct ImageRenderInfo { + pub url: String, + pub render_area: tui::layout::Rect, + /// indicates if the image is rendered + pub rendered: bool, +} + /// Application's UI state #[derive(Debug)] pub struct UIState { @@ -25,7 +33,7 @@ pub struct UIState { pub playback_progress_bar_rect: tui::layout::Rect, #[cfg(feature = "image")] - pub last_cover_image_render_info: Option<(String, tui::layout::Rect)>, + pub last_cover_image_render_info: ImageRenderInfo, } impl UIState { @@ -86,7 +94,7 @@ impl Default for UIState { fn default() -> Self { Self { is_running: true, - theme: config::Theme::default(), + theme: Default::default(), input_key_sequence: key::KeySequence { keys: vec![] }, history: vec![PageState::Library { @@ -94,10 +102,10 @@ impl Default for UIState { }], popup: None, - playback_progress_bar_rect: tui::layout::Rect::default(), + playback_progress_bar_rect: Default::default(), #[cfg(feature = "image")] - last_cover_image_render_info: None, + last_cover_image_render_info: Default::default(), } } } diff --git a/spotify_player/src/ui/mod.rs b/spotify_player/src/ui/mod.rs index 55fbd47a..9167b350 100644 --- a/spotify_player/src/ui/mod.rs +++ b/spotify_player/src/ui/mod.rs @@ -32,7 +32,7 @@ pub fn run(state: SharedState) -> Result<()> { #[cfg(feature = "image")] { // redraw the cover image when the terminal's size changes - ui.last_cover_image_render_info = None; + ui.last_cover_image_render_info = Default::default(); } } @@ -88,15 +88,19 @@ fn render_application( ui: &mut UIStateGuard, rect: Rect, ) -> Result<()> { - // rendering order: help popup -> other popups -> playback window -> main layout + // rendering order: playback window -> help popup -> other popups -> main layout + // + // Playback window is rendered first to make sure the window's position is fixed + // which avoids any rendering issues with the cover image area. + // See: https://github.com/aome510/spotify-player/issues/389 + + let (playback_rect, rect) = playback::split_rect_for_playback_window(rect, state); + playback::render_playback_window(frame, state, ui, playback_rect)?; let rect = popup::render_shortcut_help_popup(frame, state, ui, rect); let (rect, is_active) = popup::render_popup(frame, state, ui, rect); - let (playback_rect, rect) = playback::split_rect_for_playback_window(rect, state); - playback::render_playback_window(frame, state, ui, playback_rect)?; - render_main_layout(is_active, frame, state, ui, rect)?; Ok(()) } diff --git a/spotify_player/src/ui/playback.rs b/spotify_player/src/ui/playback.rs index 9cd2340f..258a1add 100644 --- a/spotify_player/src/ui/playback.rs +++ b/spotify_player/src/ui/playback.rs @@ -48,23 +48,40 @@ pub fn render_playback_window( (hor_chunks[1], ver_chunks[0]) }; - // set the `skip` state of cells in the cover image area - // to prevent buffer from overwriting image cells - for x in cover_img_rect.left()..cover_img_rect.right() { - for y in cover_img_rect.top()..cover_img_rect.bottom() { - frame.buffer_mut().get_mut(x, y).set_skip(true); - } - } - let url = crate::utils::get_track_album_image_url(track).map(String::from); if let Some(url) = url { - let needs_render = match &ui.last_cover_image_render_info { - Some((last_url, _)) => *last_url != url, - None => true, + let needs_clear = if ui.last_cover_image_render_info.url != url + || ui.last_cover_image_render_info.render_area != cover_img_rect + { + ui.last_cover_image_render_info = ImageRenderInfo { + url, + render_area: cover_img_rect, + rendered: false, + }; + true + } else { + false }; - if needs_render { - render_playback_cover_image(state, ui, cover_img_rect, url) - .context("render playback's cover image")?; + + if needs_clear { + // clear the image's area to ensure no remaining artifacts before rendering the image + // See: https://github.com/aome510/spotify-player/issues/389 + frame.render_widget(Clear, cover_img_rect); + } else { + if !ui.last_cover_image_render_info.rendered { + render_playback_cover_image(state, ui) + .context("render playback's cover image")?; + } + + // set the `skip` state of cells in the cover image area + // to prevent buffer from overwriting the image's rendered area + // NOTE: `skip` should not be set when clearing the render area. + // Otherwise, nothing will be clear as the buffer doesn't handle cells with `skip=true`. + for x in cover_img_rect.left()..cover_img_rect.right() { + for y in cover_img_rect.top()..cover_img_rect.bottom() { + frame.buffer_mut().get_mut(x, y).set_skip(true); + } + } } } @@ -99,8 +116,8 @@ pub fn render_playback_window( // clear the previous widget's area before rendering the text. #[cfg(feature = "image")] { - if let Some((_, rect)) = ui.last_cover_image_render_info { - frame.render_widget(Clear, rect); + if ui.last_cover_image_render_info.rendered { + let rect = ui.last_cover_image_render_info.render_area; // reset the `skip` state of cells in the cover image area // in order to render the "No playback found" message for x in rect.left()..rect.right() { @@ -108,7 +125,7 @@ pub fn render_playback_window( frame.buffer_mut().get_mut(x, y).set_skip(false); } } - ui.last_cover_image_render_info = None; + ui.last_cover_image_render_info = Default::default(); } } @@ -264,12 +281,7 @@ fn render_playback_progress_bar( } #[cfg(feature = "image")] -fn render_playback_cover_image( - state: &SharedState, - ui: &mut UIStateGuard, - rect: Rect, - url: String, -) -> Result<()> { +fn render_playback_cover_image(state: &SharedState, ui: &mut UIStateGuard) -> Result<()> { fn remove_temp_files() -> Result<()> { // Clean up temp files created by `viuer`'s kitty printer to avoid // possible freeze because of too many temp files in the temp folder. @@ -288,7 +300,9 @@ fn render_playback_cover_image( remove_temp_files().context("remove temp files")?; let data = state.data.read(); - if let Some(image) = data.caches.images.get(&url) { + if let Some(image) = data.caches.images.get(&ui.last_cover_image_render_info.url) { + let rect = ui.last_cover_image_render_info.render_area; + // `viuer` renders image using `sixel` in a different scale compared to other methods. // Scale the image to make the rendered image more fit if needed. // This scaling factor is user configurable as the scale works differently @@ -311,7 +325,7 @@ fn render_playback_cover_image( ) .context("print image to the terminal")?; - ui.last_cover_image_render_info = Some((url, rect)); + ui.last_cover_image_render_info.rendered = true; } Ok(())