fix: graph logic

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,3 +1,5 @@
 Please explain briefly what the PR does. It's not always obvious for maintainers.
 
 Also, please check the case "allow edits by maintainers", this make it easy for maintainers to do a few commit before merging.
+
+You also need to run `just pull` to format and check that all test will pass on CI.

Cargo.toml

@@ -86,3 +86,6 @@ cargo-packager-resource-resolver = { version = "0.1", features = [
 
 # [patch."https://github.com/pop-os/libcosmic"]
 # libcosmic = { path = "../libcosmic" }
+
+[profile.release]
+lto = "fat"

data/src/config/graph.rs

@@ -1,4 +1,4 @@
-use std::{collections::HashSet, hash::Hash, vec};
+use std::{collections::BTreeSet, hash::Hash, vec};
 
 use hardware::{Hardware, Value};
 use serde::{Deserialize, Serialize};
@@ -7,6 +7,7 @@ use crate::{
     app_graph::AppGraph,
     node::{IsValid, Node, NodeType, ToNode},
     update::UpdateError,
+    utils::{has_duplicate, is_sorted, InsertSorted, RemoveElem},
 };
 
 use super::utils::affine::Affine;
@@ -80,28 +81,31 @@ impl Default for Graph {
 
 impl ToNode for Graph {
     fn to_node(mut self, app_graph: &mut AppGraph, _hardware: &Hardware) -> Node {
-        let mut deduplicator = HashSet::new();
+        let mut deduplicator = BTreeSet::new();
 
-        for c in &self.coords {
-            deduplicator.insert(*c);
-        }
-
-        self.coords.retain_mut(|c| {
+        for mut c in self.coords.clone() {
             if c.percent > 100 {
-                warn!("coord percent is superior at 100");
+                warn!("coord percent is superior to 100");
                 c.percent = 100;
             }
-            deduplicator.contains(c)
-        });
+            if !deduplicator.insert(c) {
+                warn!("2 coords share the same temp");
+            }
+        }
+
+        self.coords = deduplicator.into_iter().collect();
 
-        self.coords.sort();
+        debug_assert!(!has_duplicate(&self.coords));
+        debug_assert!(is_sorted(&self.coords));
 
         Node::new(NodeType::Graph(self), app_graph)
     }
 }
 
 impl IsValid for Graph {
     fn is_valid(&self) -> bool {
+        debug_assert!(!has_duplicate(&self.coords));
+        debug_assert!(is_sorted(&self.coords));
         self.input.is_some() && !self.coords.is_empty()
     }
 }
@@ -134,6 +138,9 @@ impl Graph {
     }
 
     pub fn get_value(&self, value: Value) -> Result<Value, UpdateError> {
+        debug_assert!(!has_duplicate(&self.coords));
+        debug_assert!(is_sorted(&self.coords));
+
         let dummy_coord = Coord {
             temp: value as u8,
             percent: 0,
@@ -163,79 +170,61 @@ impl Graph {
 
         Ok(res)
     }
+
+    pub fn add_coord(&mut self, new: Coord) {
+        self.coords.insert_sorted(|c| c.cmp(&new), new);
+    }
+    pub fn remove_coord(&mut self, coord: &Coord) {
+        self.coords.remove_elem(|c| c.exact_same(coord));
+    }
+    pub fn replace_coord(&mut self, prev: &Coord, new: Coord) {
+        self.remove_coord(prev);
+        self.add_coord(new);
+    }
 }
 
 #[cfg(test)]
 mod test {
-    use crate::config::graph::Coord;
-
-    #[test]
-    fn test() {
-        let coord1 = Coord {
-            temp: 10,
-            percent: 10,
-        };
-
-        let coord2 = Coord {
-            temp: 20,
-            percent: 20,
-        };
+    use crate::{config::graph::Coord, node::IsValid};
 
-        let coord3 = Coord {
-            temp: 30,
-            percent: 30,
-        };
+    use super::Graph;
 
-        let coord4 = Coord {
-            temp: 40,
-            percent: 40,
+    #[test]
+    fn test_logic() {
+        let graph = Graph {
+            name: "name".into(),
+            coords: vec![
+                Coord {
+                    temp: 10,
+                    percent: 10,
+                },
+                Coord {
+                    temp: 20,
+                    percent: 30,
+                },
+                Coord {
+                    temp: 25,
+                    percent: 20,
+                },
+                Coord {
+                    temp: 30,
+                    percent: 25,
+                },
+                Coord {
+                    temp: 40,
+                    percent: 5,
+                },
+            ],
+            input: None,
         };
 
-        let coords = [coord1, coord2, coord3, coord4];
+        graph.is_valid();
 
-        let dummy_coord = Coord {
-            temp: 50,
-            percent: 0,
-        };
-
-        let res = coords.binary_search(&dummy_coord);
+        assert_eq!(graph.get_value(9).unwrap(), 10);
+        assert_eq!(graph.get_value(50).unwrap(), 5);
 
-        match res {
-            Ok(index) => {
-                println!("use {}", index);
-            }
-            Err(index) => {
-                if index == 0 {
-                    println!("use {}", index);
-                } else if index == coords.len() {
-                    println!("use {}", index - 1);
-                } else {
-                    println!("use {} and {}", index - 1, index);
-                }
-            }
-        }
-        dbg!(&res);
+        assert_eq!(graph.get_value(22).unwrap(), 26);
+        assert_eq!(graph.get_value(27).unwrap(), 22);
+        assert_eq!(graph.get_value(35).unwrap(), 15);
     }
 }
-
-// #[derive(PartialEq)]
-// enum DupState {
-//     Init,
-//     Prev { temp: u8 },
-//     DuplicateFound,
-// }
-
-// self.coords
-//     .iter()
-//     .fold(DupState::Init, |prev, coord| match prev {
-//         DupState::Init => DupState::Prev { temp: coord.temp },
-//         DupState::Prev { temp } => {
-//             if temp == coord.temp {
-//                 DupState::DuplicateFound
-//             } else {
-//                 DupState::Prev { temp: coord.temp }
-//             }
-//         }
-//         DupState::DuplicateFound => DupState::DuplicateFound,
-//     })
-//     != DupState::DuplicateFound;

data/src/utils.rs

@@ -1,5 +1,7 @@
 use std::cmp::Ordering;
 
+use std::collections::BTreeSet;
+
 pub trait RemoveElem<T> {
     fn remove_elem<F>(&mut self, predicate: F) -> Option<T>
     where
@@ -17,6 +19,36 @@ impl<T> RemoveElem<T> for Vec<T> {
     }
 }
 
+pub fn has_duplicate<T>(iter: T) -> bool
+where
+    T: IntoIterator,
+    T::Item: Ord,
+{
+    let mut uniq = BTreeSet::new();
+    !iter.into_iter().all(move |x| uniq.insert(x))
+}
+
+pub fn is_sorted<T>(iter: T) -> bool
+where
+    T: IntoIterator,
+    T::Item: Ord,
+{
+    let mut iter = iter.into_iter();
+
+    let mut prev = iter.next();
+
+    for current in iter {
+        if let Some(ref prev) = prev {
+            if prev > &current {
+                return false;
+            }
+        }
+        prev.replace(current);
+    }
+
+    true
+}
+
 #[cfg(test)]
 pub fn init_test_logging() {
     let _ = env_logger::builder()
@@ -26,6 +58,7 @@ pub fn init_test_logging() {
 }
 
 pub trait InsertSorted<T> {
+    /// Don't allow duplicate
     fn insert_sorted<F>(&mut self, predicate: F, element: T) -> Option<T>
     where
         F: Fn(&T) -> Ordering;

ui/src/lib.rs

@@ -5,7 +5,7 @@ use data::{
     config::Config,
     node::{validate_name, IsValid, NodeType},
     settings::AppTheme,
-    utils::{InsertSorted, RemoveElem},
+    utils::RemoveElem,
     AppState,
 };
 use graph::GraphWindow;
@@ -314,15 +314,13 @@ impl<H: HardwareBridge + 'static> cosmic::Application for Ui<H> {
 
                         match graph_msg {
                             message::GraphMsg::RemoveCoord(coord) => {
-                                graph.coords.remove_elem(|c| c.exact_same(&coord));
+                                graph.remove_coord(&coord);
                             }
                             message::GraphMsg::AddCoord(coord) => {
-                                graph.coords.insert_sorted(|c| coord.cmp(c), coord);
+                                graph.add_coord(coord);
                             }
                             message::GraphMsg::ReplaceCoord { previous, new } => {
-                                graph.coords.remove_elem(|c| c.exact_same(&previous));
-
-                                graph.coords.insert_sorted(|c| new.cmp(c), new);
+                                graph.replace_coord(&previous, new);
                             }
                         }
                     }