From 5f28ab12a3f40b556f454d8c16c1cf7383df6ce0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9mi=20Dupr=C3=A9?= <remi@dupre.io>
Date: Fri, 9 Feb 2024 15:33:51 +0100
Subject: [PATCH] fix members of a multibar always rendering

---
 src/draw_target.rs | 58 ++++++++++++++++++++++++++++++++++++++++------
 src/multi.rs       | 53 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 103 insertions(+), 8 deletions(-)

diff --git a/src/draw_target.rs b/src/draw_target.rs
index b99b2396..274340d2 100644
--- a/src/draw_target.rs
+++ b/src/draw_target.rs
@@ -146,6 +146,18 @@ impl ProgressDrawTarget {
         }
     }
 
+    /// Check if a call to `drawable` would actually give back a `Drawable`.
+    pub(crate) fn will_allow_draw(&self, now: Instant) -> bool {
+        match &self.kind {
+            TargetKind::Term { rate_limiter, .. } => rate_limiter.will_allow(now),
+            TargetKind::Multi { state, .. } => state.try_read().unwrap().will_allow_draw(now),
+            TargetKind::Hidden => false,
+            TargetKind::TermLike { rate_limiter, .. } => {
+                rate_limiter.as_ref().map_or(true, |r| r.will_allow(now))
+            }
+        }
+    }
+
     /// Apply the given draw state (draws it).
     pub(crate) fn drawable(&mut self, force_draw: bool, now: Instant) -> Option<Drawable<'_>> {
         match &mut self.kind {
@@ -169,13 +181,31 @@ impl ProgressDrawTarget {
                 }
             }
             TargetKind::Multi { idx, state, .. } => {
-                let state = state.write().unwrap();
-                Some(Drawable::Multi {
-                    idx: *idx,
-                    state,
-                    force_draw,
-                    now,
-                })
+                let mut state_guard = state.write().unwrap();
+
+                // Check if this multibar's inner draw target will rate limit
+                // the draw call. If so no rendering needs to be done as long
+                // as we mark the member for redraw. Either case no actual
+                // request needs to be performed on inner rate limiter otherwise
+                // the actual draw call won't be able to perform.
+                if state_guard.will_allow_draw(now) {
+                    state_guard.unmark_delayed(*idx);
+
+                    let state = MultiState::force_redraw_delayed(state_guard, now)
+                        .ok()
+                        .flatten()
+                        .unwrap_or_else(|| state.write().unwrap());
+
+                    Some(Drawable::Multi {
+                        idx: *idx,
+                        state,
+                        force_draw,
+                        now,
+                    })
+                } else {
+                    state_guard.mark_delayed(*idx); // needs to be drawn later
+                    None
+                }
             }
             TargetKind::TermLike {
                 inner,
@@ -412,6 +442,20 @@ impl RateLimiter {
         }
     }
 
+    /// Check ahead if a call to `allow` will return `true`.
+    fn will_allow(&self, now: Instant) -> bool {
+        if now < self.prev {
+            return false;
+        }
+
+        let elapsed = now - self.prev;
+
+        // If `capacity` is 0 and not enough time (`self.interval` ms) has passed since
+        // `self.prev` to add new capacity, return `false`. The goal of this method is to
+        // make this decision as efficient as possible.
+        self.capacity != 0 || elapsed >= Duration::from_millis(self.interval as u64)
+    }
+
     fn allow(&mut self, now: Instant) -> bool {
         if now < self.prev {
             return false;
diff --git a/src/multi.rs b/src/multi.rs
index 3a372866..0e6c434f 100644
--- a/src/multi.rs
+++ b/src/multi.rs
@@ -1,6 +1,6 @@
 use std::fmt::{Debug, Formatter};
 use std::io;
-use std::sync::{Arc, RwLock};
+use std::sync::{Arc, RwLock, RwLockWriteGuard};
 use std::thread::panicking;
 #[cfg(not(target_arch = "wasm32"))]
 use std::time::Instant;
@@ -9,6 +9,7 @@ use crate::draw_target::{
     visual_line_count, DrawState, DrawStateWrapper, LineAdjust, ProgressDrawTarget, VisualLines,
 };
 use crate::progress_bar::ProgressBar;
+use crate::WeakProgressBar;
 #[cfg(target_arch = "wasm32")]
 use instant::Instant;
 
@@ -263,6 +264,52 @@ impl MultiState {
         self.remove_idx(index);
     }
 
+    /// Mark that a given member has had its state changed without a redraw
+    pub(crate) fn mark_delayed(&mut self, idx: usize) {
+        self.members[idx].is_delayed = true;
+    }
+
+    /// Mark that a given member does not need to be redrawn
+    pub(crate) fn unmark_delayed(&mut self, idx: usize) {
+        self.members[idx].is_delayed = false;
+    }
+
+    /// Force the redraw of all delayed members. The input lock guard will be
+    /// given back if and only if there was no member that had to be redrawn.
+    pub(crate) fn force_redraw_delayed(
+        mut this: RwLockWriteGuard<'_, Self>,
+        now: Instant,
+    ) -> io::Result<Option<RwLockWriteGuard<'_, Self>>> {
+        let to_redraw: Vec<_> = this
+            .members
+            .iter_mut()
+            .filter(|mb| mb.is_delayed)
+            .filter_map(|mb| {
+                mb.is_delayed = false;
+                mb.pb_weak.upgrade()
+            })
+            .collect();
+
+        if to_redraw.is_empty() {
+            return Ok(Some(this));
+        }
+
+        // The members will need to borrow this multibar to redraw!
+        drop(this);
+
+        for pb in to_redraw {
+            pb.state().draw(true, now)?;
+        }
+
+        Ok(None)
+    }
+
+    /// Check if the draw target will allow to draw. If not, the member will
+    /// have to be marked as delayed to redraw it later.
+    pub(crate) fn will_allow_draw(&self, now: Instant) -> bool {
+        self.draw_target.will_allow_draw(now)
+    }
+
     pub(crate) fn draw(
         &mut self,
         mut force_draw: bool,
@@ -477,6 +524,10 @@ struct MultiStateMember {
     draw_state: Option<DrawState>,
     /// Whether the corresponding progress bar (more precisely, `BarState`) has been dropped.
     is_zombie: bool,
+    /// Mark the member if he has not been redrawn after its last state change
+    is_delayed: bool,
+    /// Used to trigger a redraw if this member has been delayed
+    pb_weak: WeakProgressBar,
 }
 
 impl Debug for MultiStateMember {