refactor: rewrite column algorithm

ClementTsang · ClementTsang · Sep 10, 2020 · Sep 8, 2020 · Sep 8, 2020 · Sep 9, 2020
commit 5da26bfca9956dc273e97068973c4ecb709ad750
diff --git a/src/app.rs b/src/app.rs
@@ -298,6 +298,7 @@ impl App {
                                 .columns
                                 .toggle(&processes::ProcessSorting::Pid);
 
+                            proc_widget_state.requires_redraw = true;
                             self.proc_state.force_update = Some(self.current_widget.widget_id);
                         }
                     }
@@ -502,6 +503,7 @@ impl App {
             }
 
             self.proc_state.force_update = Some(self.current_widget.widget_id);
+            proc_widget_state.requires_redraw = true;
         }
     }
 

diff --git a/src/app/states.rs b/src/app/states.rs
@@ -109,6 +109,13 @@ impl AppSearchState {
     }
 }
 
+/// Meant for canvas operations involving table column widths.
+#[derive(Default)]
+pub struct CanvasTableWidthState {
+    pub desired_column_widths: Vec<u16>,
+    pub calculated_column_widths: Vec<u16>,
+}
+
 /// ProcessSearchState only deals with process' search's current settings and state.
 pub struct ProcessSearchState {
     pub search_state: AppSearchState,
@@ -321,6 +328,9 @@ impl ProcColumn {
     pub fn get_column_headers(
         &self, proc_sorting_type: &ProcessSorting, sort_reverse: bool,
     ) -> Vec<String> {
+        const DOWN_ARROW: char = '▼';
+        const UP_ARROW: char = '▲';
+
         // TODO: Gonna have to figure out how to do left/right GUI notation if we add it.
         self.ordered_columns
             .iter()
@@ -333,11 +343,14 @@ impl ProcColumn {
 
                 if mapping.enabled {
                     Some(if proc_sorting_type == column_type {
-                        column_type.to_string()
-                            + command_str.as_str()
-                            + if sort_reverse { "▼" } else { "▲" }
+                        format!(
+                            "{}{}{}",
+                            column_type.to_string(),
+                            command_str.as_str(),
+                            if sort_reverse { DOWN_ARROW } else { UP_ARROW }
+                        )
                     } else {
-                        column_type.to_string() + command_str.as_str()
+                        format!("{}{}", column_type.to_string(), command_str.as_str(),)
                     })
                 } else {
                     None
@@ -358,6 +371,8 @@ pub struct ProcWidgetState {
     pub is_sort_open: bool,
     pub columns: ProcColumn,
     pub is_tree_mode: bool,
+    pub table_width_state: CanvasTableWidthState,
+    pub requires_redraw: bool,
 }
 
 impl ProcWidgetState {
@@ -397,6 +412,8 @@ impl ProcWidgetState {
             is_sort_open: false,
             columns,
             is_tree_mode: false,
+            table_width_state: CanvasTableWidthState::default(),
+            requires_redraw: false,
         }
     }
 
@@ -595,6 +612,7 @@ pub struct CpuWidgetState {
     pub autohide_timer: Option<Instant>,
     pub scroll_state: AppScrollWidgetState,
     pub is_multi_graph_mode: bool,
+    pub table_width_state: CanvasTableWidthState,
 }
 
 impl CpuWidgetState {
@@ -605,6 +623,7 @@ impl CpuWidgetState {
             autohide_timer,
             scroll_state: AppScrollWidgetState::default(),
             is_multi_graph_mode: false,
+            table_width_state: CanvasTableWidthState::default(),
         }
     }
 }
@@ -668,12 +687,14 @@ impl MemState {
 
 pub struct TempWidgetState {
     pub scroll_state: AppScrollWidgetState,
+    pub table_width_state: CanvasTableWidthState,
 }
 
 impl TempWidgetState {
     pub fn init() -> Self {
         TempWidgetState {
             scroll_state: AppScrollWidgetState::default(),
+            table_width_state: CanvasTableWidthState::default(),
         }
     }
 }
@@ -698,12 +719,14 @@ impl TempState {
 
 pub struct DiskWidgetState {
     pub scroll_state: AppScrollWidgetState,
+    pub table_width_state: CanvasTableWidthState,
 }
 
 impl DiskWidgetState {
     pub fn init() -> Self {
         DiskWidgetState {
             scroll_state: AppScrollWidgetState::default(),
+            table_width_state: CanvasTableWidthState::default(),
         }
     }
 }

diff --git a/src/bin/main.rs b/src/bin/main.rs
@@ -106,6 +106,7 @@ fn main() -> Result<()> {
         termination_hook();
     })
     .unwrap();
+    let mut first_run = true;
 
     while !is_terminated.load(Ordering::SeqCst) {
         if let Ok(recv) = receiver.recv_timeout(Duration::from_millis(TICK_RATE_IN_MILLISECONDS)) {
@@ -123,6 +124,13 @@ fn main() -> Result<()> {
                 BottomEvent::Update(data) => {
                     app.data_collection.eat_data(&data);
 
+                    // This thing is required as otherwise, some widgets can't draw correctly w/o
+                    // some data (or they need to be re-drawn).
+                    if first_run {
+                        first_run = false;
+                        app.is_force_redraw = true;
+                    }
+
                     if !app.is_frozen {
                         // Convert all data into tui-compliant components
 

diff --git a/src/canvas.rs b/src/canvas.rs
@@ -67,7 +67,6 @@ pub struct Painter {
     widget_layout: BottomLayout,
     derived_widget_draw_locs: Vec<Vec<Vec<Vec<Rect>>>>,
     table_height_offset: u16,
-    requires_boundary_recalculation: bool,
 }
 
 impl Painter {
@@ -152,7 +151,6 @@ impl Painter {
             widget_layout,
             derived_widget_draw_locs: Vec::default(),
             table_height_offset: if is_basic_mode { 2 } else { 4 } + table_gap,
-            requires_boundary_recalculation: true,
         }
     }
 
@@ -401,25 +399,18 @@ impl Painter {
                 // Basic mode.  This basically removes all graphs but otherwise
                 // the same info.
 
-                let cpu_height = (app_state.canvas_data.cpu_data.len() / 4) as u16
-                    + (if app_state.canvas_data.cpu_data.len() % 4 == 0 {
-                        0
-                    } else {
-                        1
-                    });
-
-                // A little hack to force the widget boundary recalculation.  This is required here
-                // as basic mode has a height of 0 initially, which breaks things.
-                if self.requires_boundary_recalculation {
-                    app_state.is_determining_widget_boundary = true;
-                }
-                self.requires_boundary_recalculation = cpu_height == 0;
-
                 let vertical_chunks = Layout::default()
                     .direction(Direction::Vertical)
                     .constraints(
                         [
-                            Constraint::Length(cpu_height),
+                            Constraint::Length(
+                                (app_state.canvas_data.cpu_data.len() / 4) as u16
+                                    + (if app_state.canvas_data.cpu_data.len() % 4 == 0 {
+                                        0
+                                    } else {
+                                        1
+                                    }),
+                            ),
                             Constraint::Length(1),
                             Constraint::Length(2),
                             Constraint::Length(2),
@@ -486,7 +477,7 @@ impl Painter {
                     self.draw_basic_table_arrows(&mut f, app_state, vertical_chunks[3], widget_id);
                 }
             } else {
-                // Draws using the passed in (or default) layout.  NOT basic so far.
+                // Draws using the passed in (or default) layout.
                 if self.derived_widget_draw_locs.is_empty() || app_state.is_force_redraw {
                     let row_draw_locs = Layout::default()
                         .margin(0)

diff --git a/src/canvas/drawing_utils.rs b/src/canvas/drawing_utils.rs
@@ -1,78 +1,65 @@
 use crate::app;
-use itertools::izip;
+use std::cmp::min;
 
-// TODO: Reverse intrinsic?
-/// A somewhat jury-rigged solution to simulate a variable intrinsic layout for
-/// table widths.  Note that this will do one main pass to try to properly
-/// allocate widths.  This will thus potentially cut off latter elements
-/// (return size of 0) if it is too small (threshold), but will try its best.
+/// Return a (hard)-width vector for column widths.
+/// * `total_width` is how much width we have to work with overall.
+/// * `desired_widths` is the width that is *desired*, but may not be reached.
+/// * `width_thresholds` is the maximal percentage we allow a column to take in the table.  If it is
+///    negative, we assume it can take whatever size it wants.
+/// * `column_bias` is how we determine which columns are more important.  A higher value on a
+///   column means it is more important.
 ///
-/// `width thresholds` and `desired_widths_ratio` should be the same length.
-/// Otherwise bad things happen.
-pub fn get_variable_intrinsic_widths(
-    total_width: u16, desired_widths_ratio: &[f64], width_thresholds: &[usize],
-) -> (Vec<u16>, usize) {
-    let num_widths = desired_widths_ratio.len();
-    let mut resulting_widths: Vec<u16> = vec![0; num_widths];
-    let mut last_index = 0;
+/// **NOTE:** This function ASSUMES THAT ALL PASSED SLICES ARE OF THE SAME SIZE.
+/// **NOTE:** This function automatically takes away 2 from the width as part of the left/right bounds.
+pub fn get_column_widths(
+    total_width: u16, desired_widths: &[u16], width_thresholds: Option<&[f64]>,
+    column_bias: &[usize],
+) -> Vec<u16> {
+    debug_assert_eq!(desired_widths.len(), column_bias.len());
+    if let Some(width_thresholds) = width_thresholds {
+        debug_assert_eq!(desired_widths.len(), width_thresholds.len());
+    }
 
-    let mut remaining_width = (total_width - (num_widths as u16 - 1)) as i32; // Required for spaces...
-    let desired_widths = desired_widths_ratio
-        .iter()
-        .map(|&desired_width_ratio| (desired_width_ratio * total_width as f64) as i32);
+    let mut total_width_left = total_width.saturating_sub(desired_widths.len() as u16) + 1 - 2;
+    let mut column_widths: Vec<u16> = vec![0; desired_widths.len()];
 
-    for (desired_width, resulting_width, width_threshold) in izip!(
-        desired_widths,
-        resulting_widths.iter_mut(),
-        width_thresholds
-    ) {
-        *resulting_width = if desired_width < *width_threshold as i32 {
-            // Try to take threshold, else, 0
-            if remaining_width < *width_threshold as i32 {
-                0
+    // Let's sort out our bias into a sorted list (reverse to get descending order).
+    let mut bias_list = column_bias.iter().enumerate().collect::<Vec<_>>();
+    bias_list.sort_by(|a, b| a.1.cmp(b.1));
+    bias_list.reverse();
+
+    // Now, let's do a first pass.
+    for itx in column_bias {
+        let itx = *itx;
+        let desired_width = if let Some(width_thresholds) = width_thresholds {
+            if width_thresholds[itx].is_sign_negative() {
+                desired_widths[itx]
             } else {
-                remaining_width -= *width_threshold as i32;
-                *width_threshold as u16
+                min(
+                    desired_widths[itx],
+                    (width_thresholds[itx] * total_width as f64).ceil() as u16,
+                )
             }
         } else {
-            // Take as large as possible
-            if remaining_width < desired_width {
-                // Check the biggest chunk possible
-                if remaining_width < *width_threshold as i32 {
-                    0
-                } else {
-                    let temp_width = remaining_width;
-                    remaining_width = 0;
-                    temp_width as u16
-                }
-            } else {
-                remaining_width -= desired_width;
-                desired_width as u16
-            }
+            desired_widths[itx]
         };
-
-        if *resulting_width == 0 {
-            break;
-        } else {
-            last_index += 1;
-        }
+        let remaining_width = min(total_width_left, desired_width);
+        column_widths[itx] = remaining_width;
+        total_width_left -= remaining_width;
     }
 
-    // Simple redistribution tactic - if there's any space left, split it evenly amongst all members
-    if last_index < num_widths && last_index != 0 {
-        let for_all_widths = (remaining_width / last_index as i32) as u16;
-        let mut remainder = remaining_width % last_index as i32;
-
-        for resulting_width in &mut resulting_widths {
-            *resulting_width += for_all_widths;
-            if remainder > 0 {
-                *resulting_width += 1;
-                remainder -= 1;
+    // Second pass to fill in gaps and spaces
+    while total_width_left > 0 {
+        for itx in column_bias {
+            column_widths[*itx] += 1;
+            total_width_left -= 1;
+            if total_width_left == 0 {
+                break;
             }
         }
     }
 
-    (resulting_widths, last_index)
+    column_widths.into_iter().filter(|x| *x > 0).collect()
 }
 
 pub fn get_search_start_position(