fix members of a multibar always rendering

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.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,32 @@ 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 force_draw || state_guard.will_allow_draw(now) {
+                    // Ensures no deadlock appears by trying to redraw current bar
+                    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 +443,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;

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;
 
@@ -159,7 +160,7 @@ impl MultiProgress {
 
     fn internalize(&self, location: InsertLocation, pb: ProgressBar) -> ProgressBar {
         let mut state = self.state.write().unwrap();
-        let idx = state.insert(location);
+        let idx = state.insert(location, &pb);
         drop(state);
 
         pb.set_draw_target(ProgressDrawTarget::new_remote(self.state.clone(), idx));
@@ -263,6 +264,54 @@ 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,
@@ -399,12 +448,12 @@ impl MultiState {
         self.draw_target.width()
     }
 
-    fn insert(&mut self, location: InsertLocation) -> usize {
+    fn insert(&mut self, location: InsertLocation, pb: &ProgressBar) -> usize {
         let idx = if let Some(idx) = self.free_set.pop() {
-            self.members[idx] = MultiStateMember::default();
+            self.members[idx] = MultiStateMember::new(pb);
             idx
         } else {
-            self.members.push(MultiStateMember::default());
+            self.members.push(MultiStateMember::new(pb));
             self.members.len() - 1
         };
 
@@ -454,7 +503,7 @@ impl MultiState {
             return;
         }
 
-        self.members[idx] = MultiStateMember::default();
+        self.members[idx] = MultiStateMember::inactive();
         self.free_set.push(idx);
         self.ordering.retain(|&x| x != idx);
 
@@ -470,13 +519,36 @@ impl MultiState {
     }
 }
 
-#[derive(Default)]
 struct MultiStateMember {
     /// Draw state will be `None` for members that haven't been drawn before, or for entries that
     /// correspond to something in the free set.
     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 MultiStateMember {
+    fn new(pb: &ProgressBar) -> Self {
+        Self {
+            draw_state: None,
+            is_zombie: false,
+            is_delayed: false,
+            pb_weak: pb.downgrade(),
+        }
+    }
+
+    fn inactive() -> Self {
+        Self {
+            draw_state: None,
+            is_zombie: false,
+            is_delayed: false,
+            pb_weak: WeakProgressBar::default(),
+        }
+    }
 }
 
 impl Debug for MultiStateMember {