re-render image if the last rectangle is different from the current o…

…ne (#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

@@ -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,18 +94,18 @@ 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 {
                 state: LibraryPageUIState::new(),
             }],
             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(),
         }
     }
 }

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(())
 }

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,16 +116,16 @@ 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() {
                     for y in rect.top()..rect.bottom() {
                         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(())