From ed3bb8bc9892c4cef60c6eee1fcd5991d06c7287 Mon Sep 17 00:00:00 2001 From: wiiznokes <78230769+wiiznokes@users.noreply.github.com> Date: Tue, 25 Jun 2024 22:20:12 +0200 Subject: [PATCH 1/3] fix: graph logic --- Cargo.toml | 3 + data/src/config/graph.rs | 139 ++++++++++++++++++--------------------- data/src/utils.rs | 33 ++++++++++ ui/src/lib.rs | 10 ++- 4 files changed, 104 insertions(+), 81 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b9aa0842..02acad51 100644 --- a/Cargo.toml +++ b/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" \ No newline at end of file diff --git a/data/src/config/graph.rs b/data/src/config/graph.rs index b75cf4b5..0bf99e60 100644 --- a/data/src/config/graph.rs +++ b/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,21 +81,22 @@ 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) } @@ -102,6 +104,8 @@ impl ToNode for 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 { + 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; diff --git a/data/src/utils.rs b/data/src/utils.rs index b31fcb1e..477fb549 100644 --- a/data/src/utils.rs +++ b/data/src/utils.rs @@ -1,5 +1,7 @@ use std::cmp::Ordering; +use std::collections::BTreeSet; + pub trait RemoveElem { fn remove_elem(&mut self, predicate: F) -> Option where @@ -17,6 +19,36 @@ impl RemoveElem for Vec { } } +pub fn has_duplicate(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(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 > ¤t { + 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 { + /// Don't allow duplicate fn insert_sorted(&mut self, predicate: F, element: T) -> Option where F: Fn(&T) -> Ordering; diff --git a/ui/src/lib.rs b/ui/src/lib.rs index 9b6f740b..34023fd5 100644 --- a/ui/src/lib.rs +++ b/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 cosmic::Application for Ui { 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); } } } From ce3f2156ed110545704f6a850e89d3ee445410d9 Mon Sep 17 00:00:00 2001 From: wiiznokes <78230769+wiiznokes@users.noreply.github.com> Date: Tue, 25 Jun 2024 22:22:08 +0200 Subject: [PATCH 2/3] Update PULL_REQUEST_TEMPLATE.md --- .github/PULL_REQUEST_TEMPLATE.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 1e2f2d31..f3225e4d 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.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. \ No newline at end of file From 1dec73b6b68c64831c9e8dfbe3dd233fef2e14c0 Mon Sep 17 00:00:00 2001 From: wiiznokes <78230769+wiiznokes@users.noreply.github.com> Date: Tue, 25 Jun 2024 22:22:50 +0200 Subject: [PATCH 3/3] Update PULL_REQUEST_TEMPLATE.md --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index f3225e4d..4510d7d2 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -2,4 +2,4 @@ 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. \ No newline at end of file +You also need to run `just pull` to format and check that all test will pass on CI.