diff --git a/gpx-rs/engine/src/core/gpx/chunk.rs b/gpx-rs/engine/src/core/gpx/chunk.rs index 9deff2d0c..3fd661910 100644 --- a/gpx-rs/engine/src/core/gpx/chunk.rs +++ b/gpx-rs/engine/src/core/gpx/chunk.rs @@ -31,7 +31,7 @@ impl TrackpointChunk { } } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct WaypointChunkId(Uuid); impl Default for WaypointChunkId { diff --git a/gpx-rs/engine/src/core/gpx/waypoint.rs b/gpx-rs/engine/src/core/gpx/waypoint.rs index f83b39ed7..f4a65fdaa 100644 --- a/gpx-rs/engine/src/core/gpx/waypoint.rs +++ b/gpx-rs/engine/src/core/gpx/waypoint.rs @@ -2,7 +2,7 @@ use uuid::Uuid; use crate::{Link, LngLat}; -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct WaypointId(Uuid); impl Default for WaypointId { @@ -11,7 +11,7 @@ impl Default for WaypointId { } } -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub struct Waypoint { pub id: WaypointId, pub coordinates: LngLat, diff --git a/gpx-rs/engine/src/engine/command/file/delete.rs b/gpx-rs/engine/src/engine/command/file/delete.rs index 84cc16ad6..e9d6ff0d8 100644 --- a/gpx-rs/engine/src/engine/command/file/delete.rs +++ b/gpx-rs/engine/src/engine/command/file/delete.rs @@ -1,6 +1,8 @@ use std::rc::Rc; -use crate::{Apply, CommandError, Selection, State}; +use crate::{ + Apply, CommandError, FileId, Selection, StackEntry, State, Waypoint, edit_waypoint_chunks, +}; #[derive(Debug)] pub struct Delete; @@ -53,16 +55,37 @@ impl Apply for Delete { trk_ids: [*trk_id].into(), } } - // TODO waypoints - Selection::Empty | Selection::Waypoints { .. } | Selection::Waypoint { .. } => { - return Err(CommandError::NothingToDo); + Selection::Waypoints { file_id } => delete_waypoints(state.files, *file_id, |_| true)?, + Selection::Waypoint { file_id, wpt_ids } => { + delete_waypoints(state.files, *file_id, |wpt| wpt_ids.contains(&wpt.id))? } + Selection::Empty => return Err(CommandError::NothingToDo), }; *state.selection = next; Ok(()) } } +fn delete_waypoints( + files: &mut StackEntry, + file_id: FileId, + filter: impl Fn(&Waypoint) -> bool, +) -> Result { + let file = files.get(&file_id).ok_or(CommandError::NothingToDo)?; + let mut file = (**file).clone(); + let changed = edit_waypoint_chunks(&mut file, &filter, |wpts| { + wpts.retain(|wpt| !filter(wpt)); + true + }); + if !changed { + return Err(CommandError::NothingToDo); + } + files.insert(file_id, Rc::new(file)); + Ok(Selection::File { + file_ids: [file_id].into(), + }) +} + #[cfg(test)] mod tests { use std::collections::HashSet; @@ -138,6 +161,50 @@ mod tests { ); } + fn with_waypoints(fx: &mut Fixture, id: FileId, n: usize) -> Vec { + let wpts: Vec<_> = (0..n).map(|_| Waypoint::default()).collect(); + let ids = wpts.iter().map(|w| w.id).collect(); + let mut file = (*fx.files[&id]).clone(); + file.wpt = vec![Rc::new(crate::WaypointChunk { + wpt: wpts, + ..Default::default() + })]; + fx.files.insert(id, Rc::new(file)); + ids + } + + #[test] + fn test_delete_selected_waypoints() { + let (mut fx, id) = loaded(); + let ids = with_waypoints(&mut fx, id, 3); + fx.selection = Selection::Waypoint { + file_id: id, + wpt_ids: HashSet::from([ids[1]]), + }; + Delete.apply(&mut fx.state()).unwrap(); + let left: Vec<_> = fx.files[&id] + .wpt + .iter() + .flat_map(|c| c.wpt.iter().map(|w| w.id)) + .collect(); + assert_eq!(left, vec![ids[0], ids[2]]); + assert_eq!(fx.selected_files(), HashSet::from([id])); + } + + #[test] + fn test_delete_all_waypoints_of_file() { + let (mut fx, id) = loaded(); + with_waypoints(&mut fx, id, 3); + fx.selection = Selection::Waypoints { file_id: id }; + Delete.apply(&mut fx.state()).unwrap(); + assert!(fx.files[&id].wpt.is_empty()); + fx.selection = Selection::Waypoints { file_id: id }; + assert_eq!( + Delete.apply(&mut fx.state()), + Err(CommandError::NothingToDo) + ); + } + #[test] fn test_delete_unknown_ids_is_nothing_to_do() { let (mut fx, id) = loaded(); diff --git a/gpx-rs/engine/src/engine/command/file/duplicate.rs b/gpx-rs/engine/src/engine/command/file/duplicate.rs index ed9544837..0c6e1bfdf 100644 --- a/gpx-rs/engine/src/engine/command/file/duplicate.rs +++ b/gpx-rs/engine/src/engine/command/file/duplicate.rs @@ -1,6 +1,9 @@ use std::{collections::HashSet, rc::Rc}; -use crate::{Apply, CommandError, File, Selection, State, Track, TrackSegment}; +use crate::{ + Apply, CommandError, File, FileId, Selection, StackEntry, State, Track, TrackSegment, Waypoint, + WaypointChunk, +}; #[derive(Debug)] pub struct Duplicate; @@ -10,10 +13,9 @@ impl Apply for Duplicate { let next = match &*state.selection { Selection::File { file_ids } => { let mut copies = HashSet::new(); - let mut order = Vec::with_capacity(state.order.0.len() + file_ids.len()); - for id in &state.order.0 { - order.push(*id); - let Some(file) = state.files.get(id).filter(|_| file_ids.contains(id)) else { + let mut order = Vec::new(); + for id in state.order.0.iter().filter(|id| file_ids.contains(id)) { + let Some(file) = state.files.get(id) else { continue; }; let copy = copy_file(file); @@ -24,7 +26,13 @@ impl Apply for Duplicate { if copies.is_empty() { return Err(CommandError::NothingToDo); } - state.order.0 = order; + let at = state + .order + .0 + .iter() + .rposition(|id| file_ids.contains(id)) + .map_or(state.order.0.len(), |i| i + 1); + state.order.0.splice(at..at, order); Selection::File { file_ids: copies } } Selection::Track { file_id, trk_ids } => { @@ -34,7 +42,7 @@ impl Apply for Duplicate { .ok_or(CommandError::NothingToDo)?; let file = Rc::make_mut(file); let mut copies = HashSet::new(); - duplicate_after( + append_copies( &mut file.trk, |trk| trk_ids.contains(&trk.id), |trk| { @@ -67,7 +75,7 @@ impl Apply for Duplicate { .find(|trk| trk.id == *trk_id) .ok_or(CommandError::NothingToDo)?; let mut copies = HashSet::new(); - duplicate_after( + append_copies( &mut trk.trkseg, |seg| trkseg_ids.contains(&seg.id), |seg| { @@ -85,28 +93,88 @@ impl Apply for Duplicate { trkseg_ids: copies, } } - // TODO waypoints - Selection::Empty | Selection::Waypoints { .. } | Selection::Waypoint { .. } => { - return Err(CommandError::NothingToDo); + Selection::Waypoints { file_id } => { + duplicate_waypoints(state.files, *file_id, |_| true)? } + Selection::Waypoint { file_id, wpt_ids } => { + duplicate_waypoints(state.files, *file_id, |wpt| wpt_ids.contains(&wpt.id))? + } + Selection::Empty => return Err(CommandError::NothingToDo), }; *state.selection = next; Ok(()) } } -/// Inserts a copy right after each item matching `filter`. -fn duplicate_after( - items: &mut Vec, - filter: impl Fn(&T) -> bool, - mut copy: impl FnMut(&T) -> T, -) { - let old = std::mem::take(items); - for item in old { - let dup = filter(&item).then(|| copy(&item)); - items.push(item); - items.extend(dup); +fn duplicate_waypoints( + files: &mut StackEntry, + file_id: FileId, + filter: impl Fn(&Waypoint) -> bool, +) -> Result { + let file = files.get(&file_id).ok_or(CommandError::NothingToDo)?; + let mut file = (**file).clone(); + let last_chunk = file + .wpt + .iter() + .rposition(|chunk| chunk.wpt.iter().any(&filter)) + .ok_or(CommandError::NothingToDo)?; + + let mut copies = HashSet::new(); + let mut inserted = Vec::new(); + let mut chunk = WaypointChunk::default(); + for wpt in file + .wpt + .iter() + .flat_map(|chunk| &chunk.wpt) + .filter(|wpt| filter(wpt)) + { + let mut copy = wpt.clone(); + copy.id = Default::default(); + copies.insert(copy.id); + chunk.wpt.push(copy); + if chunk.is_full() { + inserted.push(Rc::new(std::mem::take(&mut chunk))); + } } + if !chunk.wpt.is_empty() { + inserted.push(Rc::new(chunk)); + } + + // The copies go right after the last selected waypoint: only the chunk holding it is cut + // (when waypoints follow it), all the other chunks are kept as they are. + let cut = &file.wpt[last_chunk]; + let split = cut.wpt.iter().rposition(&filter).unwrap() + 1; + let replacement = if split == cut.wpt.len() { + let mut chunks = vec![cut.clone()]; + chunks.extend(inserted); + chunks + } else { + let part = |wpt: &[Waypoint]| { + Rc::new(WaypointChunk { + wpt: wpt.to_vec(), + ..Default::default() + }) + }; + let mut chunks = vec![part(&cut.wpt[..split])]; + chunks.extend(inserted); + chunks.push(part(&cut.wpt[split..])); + chunks + }; + file.wpt.splice(last_chunk..=last_chunk, replacement); + files.insert(file_id, Rc::new(file)); + Ok(Selection::Waypoint { + file_id, + wpt_ids: copies, + }) +} + +/// Inserts a copy of each item matching `filter`, as a block after the last matching item. +fn append_copies(items: &mut Vec, filter: impl Fn(&T) -> bool, copy: impl FnMut(&T) -> T) { + let Some(last) = items.iter().rposition(&filter) else { + return; + }; + let copies: Vec = items.iter().filter(|item| filter(item)).map(copy).collect(); + items.splice(last + 1..last + 1, copies); } // Track points are shared chunks, so copies are cheap. @@ -180,10 +248,30 @@ mod tests { } #[test] - fn test_duplicate_track_inserts_after_original() { + fn test_duplicate_files_go_after_last_selected() { + let mut fx = Fixture::default(); + for name in ["a", "b", "c"] { + crate::New { name }.apply(&mut fx.state()).unwrap(); + } + let [a, b, c] = [fx.order.0[0], fx.order.0[1], fx.order.0[2]]; + fx.selection = Selection::File { + file_ids: [a, b].into(), + }; + Duplicate.apply(&mut fx.state()).unwrap(); + assert_eq!(fx.order.0.len(), 5); + assert_eq!(fx.order.0[..2], [a, b]); + assert_eq!(fx.order.0[4], c); + assert_eq!( + fx.selected_files(), + fx.order.0[2..4].iter().copied().collect() + ); + } + + #[test] + fn test_duplicate_track_appends_copy() { let (mut fx, id) = loaded(); let before = fx.files[&id].trk.len(); - let trk_id = fx.files[&id].trk[0].id; + let (trk_id, second_id) = (fx.files[&id].trk[0].id, fx.files[&id].trk[1].id); fx.selection = Selection::Track { file_id: id, trk_ids: [trk_id].into(), @@ -193,13 +281,72 @@ mod tests { assert_eq!(file.trk.len(), before + 1); assert_eq!(file.trk[0].id, trk_id); assert_ne!(file.trk[1].id, trk_id); + // copies go right after the selected track, not at the end + assert_eq!(file.trk[2].id, second_id); assert!( matches!(&fx.selection, Selection::Track { trk_ids, .. } if trk_ids.contains(&file.trk[1].id)) ); } #[test] - fn test_duplicate_segment_inserts_after_original() { + fn test_duplicate_selected_waypoints() { + let (mut fx, id) = loaded(); + let wpts: Vec<_> = (0..3).map(|_| Waypoint::default()).collect(); + let ids: Vec<_> = wpts.iter().map(|w| w.id).collect(); + let mut file = (*fx.files[&id]).clone(); + file.wpt = vec![Rc::new(crate::WaypointChunk { + wpt: wpts, + ..Default::default() + })]; + fx.files.insert(id, Rc::new(file)); + let original_chunk = fx.files[&id].wpt[0].clone(); + + fx.selection = Selection::Waypoint { + file_id: id, + wpt_ids: HashSet::from([ids[0]]), + }; + Duplicate.apply(&mut fx.state()).unwrap(); + let all: Vec<_> = fx.files[&id] + .wpt + .iter() + .flat_map(|c| c.wpt.iter().map(|w| w.id)) + .collect(); + assert_eq!(all.len(), 4); + assert!( + matches!(&fx.selection, Selection::Waypoint { wpt_ids, .. } if wpt_ids == &HashSet::from([all[1]])) + ); + // the chunk is cut after the selected waypoint: [w0] [copy] [w1 w2] + assert_eq!(all[..], [ids[0], all[1], ids[1], ids[2]]); + assert_eq!(fx.files[&id].wpt.len(), 3); + assert_ne!(fx.files[&id].wpt[0].id, original_chunk.id); + + // selecting the last waypoint of a chunk does not cut it + let last = fx.files[&id].wpt.last().unwrap().wpt.last().unwrap().id; + let chunks = fx.files[&id].wpt.clone(); + fx.selection = Selection::Waypoint { + file_id: id, + wpt_ids: HashSet::from([last]), + }; + Duplicate.apply(&mut fx.state()).unwrap(); + let file = &fx.files[&id]; + assert_eq!(file.wpt.len(), 4); + assert!( + file.wpt[..3] + .iter() + .zip(&chunks) + .all(|(a, b)| Rc::ptr_eq(a, b)) + ); + + fx.selection = Selection::Waypoints { file_id: id }; + Duplicate.apply(&mut fx.state()).unwrap(); + assert_eq!( + fx.files[&id].wpt.iter().map(|c| c.wpt.len()).sum::(), + 10 + ); + } + + #[test] + fn test_duplicate_segment_appends_copy() { let (mut fx, id) = loaded(); let trk = &fx.files[&id].trk[0]; let (trk_id, seg_id, before) = (trk.id, trk.trkseg[0].id, trk.trkseg.len()); diff --git a/gpx-rs/engine/src/engine/command/pattern/edit_waypoint_chunks.rs b/gpx-rs/engine/src/engine/command/pattern/edit_waypoint_chunks.rs new file mode 100644 index 000000000..919c55d97 --- /dev/null +++ b/gpx-rs/engine/src/engine/command/pattern/edit_waypoint_chunks.rs @@ -0,0 +1,37 @@ +use std::rc::Rc; + +use crate::{File, Waypoint, WaypointChunk}; + +/// Edits the waypoints of the chunks containing a waypoint accepted by `filter`. +/// +/// `f` receives a copy of the chunk's waypoints and returns whether it changed them. A +/// changed chunk is replaced by a new one (with a new id, so that derived data is recomputed) +/// and chunks left empty are dropped. Returns whether anything changed. +pub fn edit_waypoint_chunks( + file: &mut File, + filter: impl Fn(&Waypoint) -> bool, + mut f: impl FnMut(&mut Vec) -> bool, +) -> bool { + let mut changed = false; + let mut chunks = Vec::with_capacity(file.wpt.len()); + for chunk in file.wpt.drain(..) { + if !chunk.wpt.iter().any(&filter) { + chunks.push(chunk); + continue; + } + let mut wpt = chunk.wpt.clone(); + if !f(&mut wpt) { + chunks.push(chunk); + continue; + } + changed = true; + if !wpt.is_empty() { + chunks.push(Rc::new(WaypointChunk { + wpt, + ..Default::default() + })); + } + } + file.wpt = chunks; + changed +} diff --git a/gpx-rs/engine/src/engine/command/pattern/mod.rs b/gpx-rs/engine/src/engine/command/pattern/mod.rs index 2243f95cb..935506fa0 100644 --- a/gpx-rs/engine/src/engine/command/pattern/mod.rs +++ b/gpx-rs/engine/src/engine/command/pattern/mod.rs @@ -1,5 +1,7 @@ +mod edit_waypoint_chunks; mod produce; mod update_selected; +pub use edit_waypoint_chunks::*; pub use produce::*; pub use update_selected::*; diff --git a/gpx-rs/engine/src/engine/command/pattern/update_selected.rs b/gpx-rs/engine/src/engine/command/pattern/update_selected.rs index 5c54b6a1d..e05570003 100644 --- a/gpx-rs/engine/src/engine/command/pattern/update_selected.rs +++ b/gpx-rs/engine/src/engine/command/pattern/update_selected.rs @@ -1,6 +1,8 @@ use std::rc::Rc; -use crate::{File, FileId, Selection, StackEntry, State, Track, TrackSegment}; +use crate::{ + File, FileId, Selection, StackEntry, State, Track, TrackSegment, Waypoint, edit_waypoint_chunks, +}; /// What an [`Editor`] hook did to the element it was given. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -24,6 +26,7 @@ impl Edit { /// A selected file, track or segment is passed to the hook of its own level. By default a /// file is forwarded to its tracks and a track to its segments, so an editor only overrides /// the level(s) it cares about (metadata: file and track, style: track, reverse: segment). +/// Waypoints are only edited when selected, never through their file. pub trait Editor { fn file(&mut self, file: &mut File) -> Edit { edit_each(&mut file.trk, |trk| self.track(trk)) @@ -36,6 +39,10 @@ pub trait Editor { fn segment(&mut self, _segment: &mut TrackSegment) -> Edit { Edit::Unchanged } + + fn waypoint(&mut self, _waypoint: &mut Waypoint) -> Edit { + Edit::Unchanged + } } /// Calls the segment hook and gives the segment a new revision id if it changed, so that @@ -75,6 +82,21 @@ fn update_file(files: &mut StackEntry, id: FileId, f: impl FnOnce(&mut File) -> } } +fn edit_waypoints( + file: &mut File, + filter: impl Fn(&Waypoint) -> bool, + editor: &mut E, +) -> Edit { + let changed = edit_waypoint_chunks(file, &filter, |wpts| { + edit_where(wpts, &filter, |wpt| editor.waypoint(wpt)) == Edit::Changed + }); + if changed { + Edit::Changed + } else { + Edit::Unchanged + } +} + /// Applies the editor to every selected file, track or segment. pub fn update_selected(state: &mut State, editor: &mut E) { match &*state.selection { @@ -111,8 +133,17 @@ pub fn update_selected(state: &mut State, editor: &mut E) { ) }); } - // TODO waypoint-level hooks - Selection::Empty | Selection::Waypoints { .. } | Selection::Waypoint { .. } => {} + Selection::Waypoints { file_id } => { + update_file(state.files, *file_id, |file| { + edit_waypoints(file, |_| true, editor) + }); + } + Selection::Waypoint { file_id, wpt_ids } => { + update_file(state.files, *file_id, |file| { + edit_waypoints(file, |wpt| wpt_ids.contains(&wpt.id), editor) + }); + } + Selection::Empty => {} } } @@ -243,6 +274,48 @@ mod tests { } } + #[test] + fn test_waypoint_selection_edits_only_selected_waypoints() { + struct Name; + impl Editor for Name { + fn waypoint(&mut self, wpt: &mut Waypoint) -> Edit { + wpt.name = Some("x".into()); + Edit::Changed + } + } + let (mut fx, id) = fixture_with_tracks(); + let wpts: Vec<_> = (0..3).map(|_| Waypoint::default()).collect(); + let ids: Vec<_> = wpts.iter().map(|w| w.id).collect(); + let mut file = (*fx.files[&id]).clone(); + file.wpt = vec![Rc::new(crate::WaypointChunk { + wpt: wpts, + ..Default::default() + })]; + fx.files.insert(id, Rc::new(file)); + let chunk_id = fx.files[&id].wpt[0].id; + + // A file selection never touches waypoints. + fx.selection = Selection::File { + file_ids: HashSet::from([id]), + }; + update_selected(&mut fx.state(), &mut Name); + assert!(fx.files[&id].wpt[0].wpt.iter().all(|w| w.name.is_none())); + + fx.selection = Selection::Waypoint { + file_id: id, + wpt_ids: HashSet::from([ids[1]]), + }; + update_selected(&mut fx.state(), &mut Name); + let chunk = &fx.files[&id].wpt[0]; + assert_ne!(chunk.id, chunk_id); + let named: Vec<_> = chunk.wpt.iter().map(|w| w.name.is_some()).collect(); + assert_eq!(named, [false, true, false]); + + fx.selection = Selection::Waypoints { file_id: id }; + update_selected(&mut fx.state(), &mut Name); + assert!(fx.files[&id].wpt[0].wpt.iter().all(|w| w.name.is_some())); + } + #[test] fn test_missing_files_and_other_selections_are_ignored() { let mut fx = Fixture::default(); @@ -255,7 +328,6 @@ mod tests { Selection::File { file_ids: HashSet::from([FileId::default()]), }, - Selection::Waypoints { file_id: id }, Selection::Empty, ] { fx.selection = selection;