From df07892e764137cdfa70e71fc4ebff368ae23172 Mon Sep 17 00:00:00 2001 From: vcoppe Date: Wed, 7 Oct 2026 18:22:30 +0200 Subject: [PATCH] refactoring --- gpx-rs/engine/src/core/gpx/chunk.rs | 72 +++- gpx-rs/engine/src/core/gpx/chunked.rs | 328 ++++++++++++++++++ gpx-rs/engine/src/core/gpx/file.rs | 16 +- gpx-rs/engine/src/core/gpx/mod.rs | 4 + gpx-rs/engine/src/core/gpx/segment.rs | 290 +++------------- gpx-rs/engine/src/core/gpx/waypoints.rs | 316 +++++++++++++++++ gpx-rs/engine/src/core/io/parse.rs | 16 +- .../engine/src/engine/command/file/delete.rs | 20 +- .../engine/command/file/delete_waypoint.rs | 9 +- .../src/engine/command/file/duplicate.rs | 54 +-- .../src/engine/command/file/move_elements.rs | 6 +- .../engine/src/engine/command/file/paste.rs | 7 +- .../src/engine/command/file/transfer.rs | 12 +- .../command/pattern/edit_waypoint_chunks.rs | 40 --- .../command/pattern/insert_waypoints.rs | 218 ------------ .../engine/src/engine/command/pattern/mod.rs | 4 - .../engine/command/pattern/update_selected.rs | 25 +- .../engine/command/pattern/update_waypoint.rs | 5 +- .../engine/src/engine/command/tools/clean.rs | 20 +- .../src/engine/command/tools/edit_waypoint.rs | 10 +- .../src/engine/command/tools/move_waypoint.rs | 14 +- .../src/engine/command/tools/new_waypoint.rs | 17 +- .../engine/src/engine/command/tools/route.rs | 2 +- .../src/engine/derived/coordinates_cache.rs | 19 +- .../src/engine/derived/file_structure.rs | 21 +- .../src/engine/derived/routing_buffer.rs | 4 +- gpx-rs/engine/src/engine/engine.rs | 1 - gpx-rs/engine/src/engine/state/clipboard.rs | 28 +- gpx-rs/engine/src/engine/state/selection.rs | 45 +-- 29 files changed, 876 insertions(+), 747 deletions(-) create mode 100644 gpx-rs/engine/src/core/gpx/chunked.rs create mode 100644 gpx-rs/engine/src/core/gpx/waypoints.rs delete mode 100644 gpx-rs/engine/src/engine/command/pattern/edit_waypoint_chunks.rs delete mode 100644 gpx-rs/engine/src/engine/command/pattern/insert_waypoints.rs diff --git a/gpx-rs/engine/src/core/gpx/chunk.rs b/gpx-rs/engine/src/core/gpx/chunk.rs index f0878bf7d..bdf1e981f 100644 --- a/gpx-rs/engine/src/core/gpx/chunk.rs +++ b/gpx-rs/engine/src/core/gpx/chunk.rs @@ -2,6 +2,29 @@ use uuid::Uuid; use crate::{Trackpoint, Waypoint}; +/// A run of items, shared between the successive versions of a [`crate::Chunked`] list: a chunk +/// that is not touched by an edit is not copied. +/// +/// Chunks are equal when they have the same identity, which is cheap to compare and changes +/// whenever the content does (a modified chunk is a new one). +pub trait Chunk { + type Item: Clone; + + /// Maximum number of items of a chunk. + const MAX_SIZE: usize; + + /// A chunk with a new identity, holding `items`. + fn new(items: Vec) -> Self; + + fn items(&self) -> &Vec; + + fn items_mut(&mut self) -> &mut Vec; + + fn is_full(&self) -> bool { + self.items().len() >= Self::MAX_SIZE + } +} + const MAX_TRKPT_CHUNK_SIZE: usize = 4096; #[derive(Debug, PartialEq, Eq)] @@ -25,9 +48,23 @@ impl PartialEq for TrackpointChunk { } } -impl TrackpointChunk { - pub fn is_full(&self) -> bool { - self.trkpt.len() == MAX_TRKPT_CHUNK_SIZE +impl Chunk for TrackpointChunk { + type Item = Trackpoint; + const MAX_SIZE: usize = MAX_TRKPT_CHUNK_SIZE; + + fn new(trkpt: Vec) -> Self { + Self { + trkpt, + ..Default::default() + } + } + + fn items(&self) -> &Vec { + &self.trkpt + } + + fn items_mut(&mut self) -> &mut Vec { + &mut self.trkpt } } @@ -54,9 +91,23 @@ impl PartialEq for WaypointChunk { } } -impl WaypointChunk { - pub fn is_full(&self) -> bool { - self.wpt.len() == MAX_WPT_CHUNK_SIZE +impl Chunk for WaypointChunk { + type Item = Waypoint; + const MAX_SIZE: usize = MAX_WPT_CHUNK_SIZE; + + fn new(wpt: Vec) -> Self { + Self { + wpt, + ..Default::default() + } + } + + fn items(&self) -> &Vec { + &self.wpt + } + + fn items_mut(&mut self) -> &mut Vec { + &mut self.wpt } } @@ -86,6 +137,15 @@ mod tests { assert!(chunk.is_full()); } + #[test] + fn test_new_chunks_have_their_own_identity() { + let a = TrackpointChunk::new(vec![Trackpoint::default()]); + let b = TrackpointChunk::new(vec![Trackpoint::default()]); + assert_ne!(a, b); + assert_eq!(a.items().len(), 1); + assert_ne!(WaypointChunk::new(vec![]), WaypointChunk::new(vec![])); + } + #[test] fn test_chunk_equality_is_by_id() { let a = TrackpointChunk::default(); diff --git a/gpx-rs/engine/src/core/gpx/chunked.rs b/gpx-rs/engine/src/core/gpx/chunked.rs new file mode 100644 index 000000000..116d1142c --- /dev/null +++ b/gpx-rs/engine/src/core/gpx/chunked.rs @@ -0,0 +1,328 @@ +use std::{ops::Index, rc::Rc}; + +use crate::Chunk; + +/// A list of items, stored in [`Chunk`]s that are shared between its versions: editing it only +/// copies the chunks around the change, so that a version costs little more than the change. +/// +/// There are no empty chunks. +#[derive(Debug, PartialEq)] +pub struct Chunked { + chunks: Vec>, + cumul_length: Vec, +} + +impl Clone for Chunked { + fn clone(&self) -> Self { + Self { + chunks: self.chunks.clone(), + cumul_length: self.cumul_length.clone(), + } + } +} + +impl Default for Chunked { + fn default() -> Self { + Self { + chunks: vec![], + cumul_length: vec![], + } + } +} + +/// The position of an item: in which chunk, where in it, and among all the items. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, PartialOrd, Ord)] +pub struct ChunkIndex { + pub chunk: usize, + pub pos: usize, + pub flat: usize, +} + +impl Chunked { + /// Adds a chunk at the end, which is dropped if it is empty. + pub fn push(&mut self, chunk: C) { + self.push_shared(Rc::new(chunk)); + } + + pub fn push_shared(&mut self, chunk: Rc) { + if chunk.items().is_empty() { + return; + } + self.cumul_length + .push(self.cumul_length.last().copied().unwrap_or_default() + chunk.items().len()); + self.chunks.push(chunk); + } + + pub fn chunks(&self) -> &[Rc] { + &self.chunks + } + + /// Replaces the items in `start..end` by `items`. Panics if the range is out of bounds. + /// + /// Chunks that are not concerned are kept as they are (shared), only the chunks around the + /// range are copied and refilled. + pub fn splice(&mut self, start: usize, end: usize, items: Vec) { + assert!( + start <= end && end <= self.len(), + "splice range out of bounds" + ); + let old = std::mem::take(&mut self.chunks); + self.cumul_length.clear(); + + let mut items = Some(items); + let mut pending = vec![]; + let mut offset = 0; + for chunk in old { + let (lo, hi) = (offset, offset + chunk.items().len()); + offset = hi; + // a chunk ending at `start` is extended unless it is full, to avoid tiny chunks + if hi < start || (hi == start && chunk.is_full()) { + self.push_shared(chunk); + continue; + } + if let Some(items) = items.take() { + self.fill(&mut pending, chunk.items()[..start - lo].iter().cloned()); + self.fill(&mut pending, items); + } + if hi <= end { + continue; + } + if lo >= end { + self.flush(&mut pending); + self.push_shared(chunk); + } else { + self.fill(&mut pending, chunk.items()[end - lo..].iter().cloned()); + } + } + if let Some(items) = items { + self.fill(&mut pending, items); + } + self.flush(&mut pending); + } + + fn fill(&mut self, pending: &mut Vec, items: impl IntoIterator) { + for item in items { + pending.push(item); + if pending.len() >= C::MAX_SIZE { + self.flush(pending); + } + } + } + + fn flush(&mut self, pending: &mut Vec) { + self.push(C::new(std::mem::take(pending))); + } + + /// Edits the items of the chunks that contain an item accepted by `filter`. + /// + /// `f` receives a copy of the items of such a chunk and returns whether it changed them. A + /// changed chunk is replaced by a new one (so that what is derived from it is computed + /// again) and the chunks left empty are dropped. The other chunks are kept. Returns whether + /// anything changed. + pub fn edit( + &mut self, + filter: impl Fn(&C::Item) -> bool, + mut f: impl FnMut(&mut Vec) -> bool, + ) -> bool { + let mut changed = false; + let old = std::mem::take(&mut self.chunks); + self.cumul_length.clear(); + for chunk in old { + if !chunk.items().iter().any(&filter) { + self.push_shared(chunk); + continue; + } + let mut items = chunk.items().clone(); + if !f(&mut items) { + self.push_shared(chunk); + continue; + } + changed = true; + self.push(C::new(items)); + } + changed + } + + /// Changes what is in the chunk `chunk`, but not how many items there are. A chunk that is + /// shared is copied first, so that the other versions do not change. + fn replace_chunk(&mut self, chunk: usize, f: impl FnOnce(&mut Vec)) { + let len = self.chunks[chunk].items().len(); + match Rc::get_mut(&mut self.chunks[chunk]) { + Some(unique) => f(unique.items_mut()), + None => { + let mut items = self.chunks[chunk].items().clone(); + f(&mut items); + self.chunks[chunk] = Rc::new(C::new(items)); + } + } + debug_assert_eq!(self.chunks[chunk].items().len(), len); + } + + /// Applies `f` to the item at `index`, which is copied in a new chunk. Panics if there is no + /// such item. + pub fn update(&mut self, index: usize, f: impl FnOnce(&mut C::Item)) { + let ChunkIndex { chunk, pos, .. } = self.locate(index).unwrap(); + self.replace_chunk(chunk, |items| f(&mut items[pos])); + } + + /// Applies `f` to every item, with its index, in new chunks. + pub fn update_all(&mut self, mut f: impl FnMut(usize, &mut C::Item)) { + let mut offset = 0; + for chunk in 0..self.chunks.len() { + let len = self.chunks[chunk].items().len(); + self.replace_chunk(chunk, |items| { + for (i, item) in items.iter_mut().enumerate() { + f(offset + i, item); + } + }); + offset += len; + } + } + + pub fn len(&self) -> usize { + self.cumul_length.last().copied().unwrap_or_default() + } + + pub fn is_empty(&self) -> bool { + self.chunks.is_empty() + } + + pub fn iter(&self) -> ChunkedIter<'_, C> { + ChunkedIter::new(self) + } + + pub fn first_index(&self) -> Option { + self.next_index(None) + } + + pub fn last_index(&self) -> Option { + self.prev_index(None) + } + + pub fn next_index(&self, cur: Option) -> Option { + let mut next = cur.map_or_default(|idx| ChunkIndex { + chunk: idx.chunk, + pos: idx.pos + 1, + flat: idx.flat + 1, + }); + loop { + if next.chunk >= self.chunks.len() { + return None; + } + if next.pos == self.chunks[next.chunk].items().len() { + next.chunk += 1; + next.pos = 0; + } else { + return Some(next); + } + } + } + + pub fn prev_index(&self, cur: Option) -> Option { + let mut prev = cur.unwrap_or(ChunkIndex { + chunk: self.chunks.len(), + pos: 0, + flat: self.cumul_length.last().copied().unwrap_or_default(), + }); + if prev.pos == 0 { + while prev.chunk > 0 { + prev.chunk -= 1; + if !self.chunks[prev.chunk].items().is_empty() { + prev.pos = self.chunks[prev.chunk].items().len() - 1; + prev.flat -= 1; + return Some(prev); + } + } + None + } else { + prev.pos -= 1; + prev.flat -= 1; + Some(prev) + } + } + + /// The position of the item `idx` among all the items. + pub fn locate(&self, idx: usize) -> Option { + let chunk = self.cumul_length.partition_point(|l| idx >= *l); + if chunk >= self.chunks.len() { + return None; + } + let pos = if chunk > 0 { + idx - self.cumul_length[chunk - 1] + } else { + idx + }; + if pos >= self.chunks[chunk].items().len() { + None + } else { + Some(ChunkIndex { + chunk, + pos, + flat: idx, + }) + } + } +} + +impl Index for Chunked { + type Output = C::Item; + + fn index(&self, idx: ChunkIndex) -> &Self::Output { + &self.chunks[idx.chunk].items()[idx.pos] + } +} + +impl Index for Chunked { + type Output = C::Item; + + fn index(&self, idx: usize) -> &Self::Output { + &self[self.locate(idx).unwrap()] + } +} + +impl<'a, C: Chunk> IntoIterator for &'a Chunked { + type Item = &'a C::Item; + type IntoIter = ChunkedIter<'a, C>; + + fn into_iter(self) -> Self::IntoIter { + self.iter() + } +} + +pub struct ChunkedIter<'a, C: Chunk> { + chunked: &'a Chunked, + idx: Option, +} + +impl Clone for ChunkedIter<'_, C> { + fn clone(&self) -> Self { + Self { + chunked: self.chunked, + idx: self.idx, + } + } +} + +impl<'a, C: Chunk> ChunkedIter<'a, C> { + pub fn new(chunked: &'a Chunked) -> Self { + Self { + chunked, + idx: Default::default(), + } + } +} + +impl<'a, C: Chunk> Iterator for ChunkedIter<'a, C> { + type Item = &'a C::Item; + + fn next(&mut self) -> Option { + self.idx = self.chunked.next_index(self.idx); + self.idx.map(|idx| &self.chunked[idx]) + } + + fn nth(&mut self, n: usize) -> Option { + let idx = self.idx.map_or_default(|idx| idx.flat) + n; + self.idx = self.chunked.locate(idx); + self.idx.map(|idx| &self.chunked[idx]) + } +} diff --git a/gpx-rs/engine/src/core/gpx/file.rs b/gpx-rs/engine/src/core/gpx/file.rs index 09b1b8f55..3d2960a48 100644 --- a/gpx-rs/engine/src/core/gpx/file.rs +++ b/gpx-rs/engine/src/core/gpx/file.rs @@ -1,8 +1,6 @@ -use std::rc::Rc; - use uuid::Uuid; -use crate::{Link, Track, WaypointChunk}; +use crate::{Link, Track, Waypoints}; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct FileId(pub Uuid); @@ -13,22 +11,12 @@ impl Default for FileId { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct FileWaypointsRevisionId(pub Uuid); - -impl Default for FileWaypointsRevisionId { - fn default() -> Self { - Self(Uuid::new_v4()) - } -} - #[derive(Debug, Default, Clone, PartialEq)] pub struct File { pub id: FileId, pub info: FileInfo, pub trk: Vec, - pub wpt: Vec>, - pub wpt_rev_id: FileWaypointsRevisionId, + pub wpt: Waypoints, // TODO routes } diff --git a/gpx-rs/engine/src/core/gpx/mod.rs b/gpx-rs/engine/src/core/gpx/mod.rs index a702aa650..3ba1c5cc5 100644 --- a/gpx-rs/engine/src/core/gpx/mod.rs +++ b/gpx-rs/engine/src/core/gpx/mod.rs @@ -1,17 +1,21 @@ mod categories; mod chunk; +mod chunked; mod common; mod file; mod segment; mod track; mod trackpoint; mod waypoint; +mod waypoints; pub use categories::*; pub use chunk::*; +pub use chunked::*; pub use common::*; pub use file::*; pub use segment::*; pub use track::*; pub use trackpoint::*; pub use waypoint::*; +pub use waypoints::*; diff --git a/gpx-rs/engine/src/core/gpx/segment.rs b/gpx-rs/engine/src/core/gpx/segment.rs index d178fa827..34136a5cb 100644 --- a/gpx-rs/engine/src/core/gpx/segment.rs +++ b/gpx-rs/engine/src/core/gpx/segment.rs @@ -1,8 +1,8 @@ -use std::{ops::Index, rc::Rc}; +use std::ops::{Deref, DerefMut}; use uuid::Uuid; -use crate::{Trackpoint, TrackpointChunk, compute_anchors}; +use crate::{ChunkIndex, Chunked, ChunkedIter, Trackpoint, TrackpointChunk, compute_anchors}; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct TrackSegmentId(pub Uuid); @@ -26,66 +26,33 @@ impl Default for TrackSegmentRevisionId { pub struct TrackSegment { pub id: TrackSegmentId, pub rev_id: TrackSegmentRevisionId, - chunks: Vec>, - cumul_length: Vec, + points: Chunked, +} + +/// The position of a trackpoint in a segment. +pub type TrackSegmentIndex = ChunkIndex; + +pub type TrackSegmentIterator<'a> = ChunkedIter<'a, TrackpointChunk>; + +impl Deref for TrackSegment { + type Target = Chunked; + + fn deref(&self) -> &Self::Target { + &self.points + } +} + +impl DerefMut for TrackSegment { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.points + } } impl TrackSegment { - pub fn push(&mut self, chunk: TrackpointChunk) { - self.push_shared(Rc::new(chunk)); - } - - fn push_shared(&mut self, chunk: Rc) { - if chunk.trkpt.is_empty() { - return; - } - self.cumul_length - .push(self.cumul_length.last().copied().unwrap_or_default() + chunk.trkpt.len()); - self.chunks.push(chunk); - } - - /// Replaces the points in `start..end` by `points`. Panics if the range is out of bounds. - /// - /// Chunks that are not concerned are kept as they are (shared), only the chunks around the - /// range are copied and refilled. + /// Replaces the points in `start..end` by `points`, see [`Chunked::splice`]. The first and + /// last trackpoints are anchors afterwards. pub fn splice(&mut self, start: usize, end: usize, points: Vec) { - assert!( - start <= end && end <= self.len(), - "splice range out of bounds" - ); - let old = std::mem::take(&mut self.chunks); - self.cumul_length.clear(); - - let mut pending = TrackpointChunk::default(); - let mut inserted = false; - let mut offset = 0; - for chunk in old { - let (lo, hi) = (offset, offset + chunk.trkpt.len()); - offset = hi; - // a chunk ending at `start` is extended unless it is full, to avoid tiny chunks - if hi < start || (hi == start && chunk.is_full()) { - self.push_shared(chunk); - continue; - } - if !inserted { - self.fill(&mut pending, chunk.trkpt[..start - lo].iter().cloned()); - self.fill(&mut pending, points.iter().cloned()); - inserted = true; - } - if hi <= end { - continue; - } - if lo >= end { - self.flush(&mut pending); - self.push_shared(chunk); - } else { - self.fill(&mut pending, chunk.trkpt[end - lo..].iter().cloned()); - } - } - if !inserted { - self.fill(&mut pending, points); - } - self.flush(&mut pending); + self.points.splice(start, end, points); self.ensure_end_anchors(); } @@ -99,7 +66,7 @@ impl TrackSegment { }; for index in [0, last] { if self[index].anchor != Some(0) { - self.point_mut(index).anchor = Some(0); + self.set_anchor(index, 0); } } } @@ -107,7 +74,7 @@ impl TrackSegment { /// Makes the trackpoint at `index` an anchor shown from the map zoom level `zoom`. Panics if /// there is no such trackpoint. pub fn set_anchor(&mut self, index: usize, zoom: u8) { - self.point_mut(index).anchor = Some(zoom); + self.points.update(index, |trkpt| trkpt.anchor = Some(zoom)); } /// Sets the anchors of the trackpoints from the details of the path of the segment (see @@ -115,7 +82,7 @@ impl TrackSegment { pub fn compute_anchors(&mut self) { let anchors = compute_anchors(self); let mut anchors = anchors.into_iter().peekable(); - self.map_points(|index, trkpt| { + self.points.update_all(|index, trkpt| { trkpt.anchor = match anchors.peek() { Some(&(anchor, zoom)) if anchor == index => { anchors.next(); @@ -125,191 +92,12 @@ impl TrackSegment { }; }); } - - /// A chunk that can be modified: the shared chunks are copied first. - fn chunk_mut(&mut self, chunk: usize) -> &mut TrackpointChunk { - let shared = &mut self.chunks[chunk]; - if Rc::get_mut(shared).is_none() { - *shared = Rc::new(TrackpointChunk { - trkpt: shared.trkpt.clone(), - ..Default::default() - }); - } - Rc::get_mut(shared).unwrap() - } - - fn point_mut(&mut self, index: usize) -> &mut Trackpoint { - let TrackSegmentIndex { chunk, pos, .. } = self.locate(index).unwrap(); - &mut self.chunk_mut(chunk).trkpt[pos] - } - - /// Applies `f` to every trackpoint, with its index in the segment. - fn map_points(&mut self, mut f: impl FnMut(usize, &mut Trackpoint)) { - let mut offset = 0; - for chunk in 0..self.chunks.len() { - for (i, trkpt) in self.chunk_mut(chunk).trkpt.iter_mut().enumerate() { - f(offset + i, trkpt); - } - offset += self.chunks[chunk].trkpt.len(); - } - } - - fn fill( - &mut self, - pending: &mut TrackpointChunk, - points: impl IntoIterator, - ) { - for trkpt in points { - pending.trkpt.push(trkpt); - if pending.is_full() { - self.flush(pending); - } - } - } - - fn flush(&mut self, pending: &mut TrackpointChunk) { - self.push(std::mem::take(pending)); - } - - pub fn len(&self) -> usize { - self.cumul_length.last().copied().unwrap_or_default() - } - - pub fn is_empty(&self) -> bool { - self.chunks.is_empty() - } - - pub fn iter(&self) -> TrackSegmentIterator<'_> { - TrackSegmentIterator::new(self) - } - - pub fn first_index(&self) -> Option { - self.next_index(None) - } - - pub fn last_index(&self) -> Option { - self.prev_index(None) - } - - pub fn next_index(&self, cur: Option) -> Option { - let mut next = cur.map_or_default(|idx| TrackSegmentIndex { - chunk: idx.chunk, - pos: idx.pos + 1, - flat: idx.flat + 1, - }); - loop { - if next.chunk >= self.chunks.len() { - return None; - } - if next.pos == self.chunks[next.chunk].trkpt.len() { - next.chunk += 1; - next.pos = 0; - } else { - return Some(next); - } - } - } - - pub fn prev_index(&self, cur: Option) -> Option { - let mut prev = cur.unwrap_or(TrackSegmentIndex { - chunk: self.chunks.len(), - pos: 0, - flat: self.cumul_length.last().copied().unwrap_or_default(), - }); - if prev.pos == 0 { - while prev.chunk > 0 { - prev.chunk -= 1; - if !self.chunks[prev.chunk].trkpt.is_empty() { - prev.pos = self.chunks[prev.chunk].trkpt.len() - 1; - prev.flat -= 1; - return Some(prev); - } - } - None - } else { - prev.pos -= 1; - prev.flat -= 1; - Some(prev) - } - } - - fn locate(&self, idx: usize) -> Option { - let chunk = self.cumul_length.partition_point(|l| idx >= *l); - if chunk >= self.chunks.len() { - return None; - } - let pos = if chunk > 0 { - idx - self.cumul_length[chunk - 1] - } else { - idx - }; - if pos >= self.chunks[chunk].trkpt.len() { - None - } else { - Some(TrackSegmentIndex { - chunk, - pos, - flat: idx, - }) - } - } -} - -impl Index for TrackSegment { - type Output = Trackpoint; - - fn index(&self, idx: TrackSegmentIndex) -> &Self::Output { - &self.chunks[idx.chunk].trkpt[idx.pos] - } -} - -impl Index for TrackSegment { - type Output = Trackpoint; - - fn index(&self, idx: usize) -> &Self::Output { - &self[self.locate(idx).unwrap()] - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, PartialOrd, Ord)] -pub struct TrackSegmentIndex { - pub chunk: usize, - pub pos: usize, - pub flat: usize, -} - -#[derive(Debug, Clone)] -pub struct TrackSegmentIterator<'a> { - trkseg: &'a TrackSegment, - idx: Option, -} - -impl<'a> TrackSegmentIterator<'a> { - pub fn new(trkseg: &'a TrackSegment) -> Self { - Self { - trkseg, - idx: Default::default(), - } - } -} - -impl<'a> Iterator for TrackSegmentIterator<'a> { - type Item = &'a Trackpoint; - - fn next(&mut self) -> Option { - self.idx = self.trkseg.next_index(self.idx); - self.idx.map(|idx| &self.trkseg[idx]) - } - - fn nth(&mut self, n: usize) -> Option { - let idx = self.idx.map_or_default(|idx| idx.flat) + n; - self.idx = self.trkseg.locate(idx); - self.idx.map(|idx| &self.trkseg[idx]) - } } #[cfg(test)] mod tests { + use std::rc::Rc; + use super::*; fn create_track_segment(nb_chunks: usize) -> TrackSegment { @@ -427,14 +215,14 @@ mod tests { let mut trkseg = create_track_segment(5); // the ends are anchors already, or their chunks would be copied trkseg.ensure_end_anchors(); - let before = trkseg.chunks.clone(); + let before = trkseg.chunks().to_vec(); // inside the third chunk only trkseg.splice(4, 5, points(&[-1.0])); - assert!(Rc::ptr_eq(&trkseg.chunks[0], &before[0])); - assert!(Rc::ptr_eq(&trkseg.chunks[1], &before[1])); - assert!(Rc::ptr_eq(trkseg.chunks.last().unwrap(), &before[4])); + assert!(Rc::ptr_eq(&trkseg.chunks()[0], &before[0])); + assert!(Rc::ptr_eq(&trkseg.chunks()[1], &before[1])); + assert!(Rc::ptr_eq(trkseg.chunks().last().unwrap(), &before[4])); assert!(Rc::ptr_eq( - &trkseg.chunks[trkseg.chunks.len() - 2], + &trkseg.chunks()[trkseg.chunks().len() - 2], &before[3] )); } @@ -447,10 +235,10 @@ mod tests { trkseg.splice(len, len, points(&[i as f64])); } assert_eq!(trkseg.len(), 10_000); - assert!(trkseg.chunks.len() <= 3); - assert!(trkseg.chunks.iter().all(|c| c.trkpt.len() <= 4096)); + assert!(trkseg.chunks().len() <= 3); + assert!(trkseg.chunks().iter().all(|c| c.trkpt.len() <= 4096)); assert_eq!(trkseg[9_999].ele, 9_999.0); - assert_eq!(*trkseg.cumul_length.last().unwrap(), 10_000); + assert_eq!(trkseg.len(), 10_000); } #[test] @@ -496,7 +284,7 @@ mod tests { let nb_chunks = 10; let trkseg = create_track_segment(nb_chunks); let _ = trkseg[TrackSegmentIndex { - chunk: trkseg.chunks.len(), + chunk: trkseg.chunks().len(), pos: 0, flat: 0, }]; diff --git a/gpx-rs/engine/src/core/gpx/waypoints.rs b/gpx-rs/engine/src/core/gpx/waypoints.rs new file mode 100644 index 000000000..81ff1189f --- /dev/null +++ b/gpx-rs/engine/src/core/gpx/waypoints.rs @@ -0,0 +1,316 @@ +use std::ops::Deref; + +use uuid::Uuid; + +use crate::{Chunked, Waypoint, WaypointChunk, WaypointId}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct FileWaypointsRevisionId(pub Uuid); + +impl Default for FileWaypointsRevisionId { + fn default() -> Self { + Self(Uuid::new_v4()) + } +} + +/// The waypoints of a file, in chunks that are shared between the versions of the file (see +/// [`Chunked`]). +/// +/// Everything that changes the waypoints changes the revision too, which is why there is no +/// mutable access to the list itself: the revision tells what is derived from the waypoints +/// (coordinates for the map...) that it is out of date. +#[derive(Debug, Default, Clone, PartialEq)] +pub struct Waypoints { + chunks: Chunked, + /// Changes when the waypoints do. + pub rev_id: FileWaypointsRevisionId, +} + +impl Deref for Waypoints { + type Target = Chunked; + + fn deref(&self) -> &Self::Target { + &self.chunks + } +} + +impl Waypoints { + /// Waypoints made of the given chunks, which are used as they are. + pub fn new(chunks: impl IntoIterator) -> Self { + let mut waypoints = Self::default(); + for chunk in chunks { + waypoints.push(chunk); + } + waypoints + } + + /// Adds a chunk after the last waypoint. + pub fn push(&mut self, chunk: WaypointChunk) { + self.chunks.push(chunk); + self.rev_id = Default::default(); + } + + /// Replaces the waypoints in `start..end` by `waypoints`, see [`Chunked::splice`]. + pub fn splice(&mut self, start: usize, end: usize, waypoints: Vec) { + self.chunks.splice(start, end, waypoints); + self.rev_id = Default::default(); + } + + /// Inserts waypoints, so that the first one is at `index` among the waypoints (at the end if + /// the index is past it). Only the chunks around the insertion are copied. + pub fn insert_at(&mut self, index: usize, waypoints: Vec) { + if waypoints.is_empty() { + return; + } + let index = index.min(self.len()); + self.splice(index, index, waypoints); + } + + /// Inserts waypoints right after the waypoint `after`, or at the end if there is none (or if + /// it is not there). See [`Waypoints::insert_at`]. + pub fn insert_after(&mut self, after: Option, waypoints: Vec) { + let index = after + .and_then(|after| self.position(after)) + .map_or(usize::MAX, |i| i + 1); + self.insert_at(index, waypoints); + } + + /// The position of the waypoint among the waypoints. + pub fn position(&self, id: WaypointId) -> Option { + self.iter().position(|wpt| wpt.id == id) + } + + /// Edits the waypoints of the chunks that contain one accepted by `filter`, see + /// [`Chunked::edit`]. Returns whether anything changed. + pub fn edit( + &mut self, + filter: impl Fn(&Waypoint) -> bool, + f: impl FnMut(&mut Vec) -> bool, + ) -> bool { + let changed = self.chunks.edit(filter, f); + if changed { + self.rev_id = Default::default(); + } + changed + } +} + +#[cfg(test)] +mod tests { + use std::rc::Rc; + + use super::*; + + fn waypoint() -> Waypoint { + Waypoint::default() + } + + fn waypoints_with(chunks: &[usize]) -> (Waypoints, Vec) { + let mut waypoints = Waypoints::default(); + let mut ids = vec![]; + for n in chunks { + let wpt: Vec<_> = (0..*n).map(|_| waypoint()).collect(); + ids.extend(wpt.iter().map(|w| w.id)); + waypoints.push(WaypointChunk { + wpt, + ..Default::default() + }); + } + (waypoints, ids) + } + + fn ids(waypoints: &Waypoints) -> Vec { + waypoints.iter().map(|w| w.id).collect() + } + + #[test] + fn test_new_and_push() { + let (waypoints, ids_) = waypoints_with(&[2, 1]); + assert_eq!(waypoints.len(), 3); + assert_eq!(waypoints.chunks().len(), 2); + assert_eq!(ids(&waypoints), ids_); + assert_eq!(waypoints.position(ids_[2]), Some(2)); + assert_eq!(waypoints.position(WaypointId::default()), None); + // empty chunks are dropped + let waypoints = Waypoints::new([WaypointChunk::default()]); + assert!(waypoints.is_empty()); + } + + #[test] + fn test_insert_at_the_end() { + let (mut waypoints, before) = waypoints_with(&[2, 1]); + let first = waypoints.chunks()[0].clone(); + let rev = waypoints.rev_id; + let new = vec![waypoint(), waypoint()]; + let new_ids: Vec<_> = new.iter().map(|w| w.id).collect(); + waypoints.insert_after(None, new); + assert_eq!(ids(&waypoints), [before.clone(), new_ids].concat()); + // the chunk that is not touched is kept, the last one is extended + assert_eq!(waypoints.chunks().len(), 2); + assert!(Rc::ptr_eq(&waypoints.chunks()[0], &first)); + assert_ne!(waypoints.rev_id, rev); + // an unknown waypoint is an insertion at the end too + let (mut waypoints, before) = waypoints_with(&[2]); + let one = waypoint(); + let one_id = one.id; + waypoints.insert_after(Some(WaypointId::default()), vec![one]); + assert_eq!(ids(&waypoints), [before, vec![one_id]].concat()); + } + + #[test] + fn test_insert_after_the_last_waypoint_of_a_chunk_keeps_the_next_ones() { + let (mut waypoints, before) = waypoints_with(&[2, 2]); + let last = waypoints.chunks()[1].clone(); + let one = waypoint(); + let one_id = one.id; + waypoints.insert_after(Some(before[1]), vec![one]); + assert_eq!( + ids(&waypoints), + [&before[..2], &[one_id], &before[2..]].concat() + ); + assert_eq!(waypoints.chunks().len(), 2); + assert!(Rc::ptr_eq(&waypoints.chunks()[1], &last)); + } + + #[test] + fn test_insert_in_the_middle_of_a_chunk_cuts_it() { + let (mut waypoints, before) = waypoints_with(&[3, 1]); + let last = waypoints.chunks()[1].clone(); + let new = vec![waypoint(), waypoint()]; + let new_ids: Vec<_> = new.iter().map(|w| w.id).collect(); + waypoints.insert_after(Some(before[0]), new); + assert_eq!( + ids(&waypoints), + [&before[..1], &new_ids[..], &before[1..]].concat() + ); + // the cut chunk is refilled, the other one is kept + assert_eq!(waypoints.chunks().len(), 2); + assert!(Rc::ptr_eq(&waypoints.chunks()[1], &last)); + } + + #[test] + fn test_insert_nothing() { + let (mut waypoints, before) = waypoints_with(&[2]); + let rev = waypoints.rev_id; + waypoints.insert_after(Some(before[0]), vec![]); + assert_eq!(ids(&waypoints), before); + assert_eq!(waypoints.rev_id, rev); + } + + #[test] + fn test_insert_many_fills_chunks() { + let mut waypoints = Waypoints::default(); + let new: Vec<_> = (0..300).map(|_| waypoint()).collect(); + let new_ids: Vec<_> = new.iter().map(|w| w.id).collect(); + waypoints.insert_after(None, new); + assert_eq!(ids(&waypoints), new_ids); + assert_eq!(waypoints.chunks().len(), 3); + assert!(waypoints.chunks()[..2].iter().all(|chunk| { + use crate::Chunk; + chunk.is_full() + })); + } + + #[test] + fn test_insert_at_an_index() { + // at the start + let (mut waypoints, before) = waypoints_with(&[2, 1]); + let first = waypoints.chunks()[0].clone(); + let one = waypoint(); + let one_id = one.id; + waypoints.insert_at(0, vec![one]); + assert_eq!(ids(&waypoints), [vec![one_id], before.clone()].concat()); + // the existing chunks are kept + assert_eq!(waypoints.chunks().len(), 3); + assert!(Rc::ptr_eq(&waypoints.chunks()[1], &first)); + + // between two chunks + let (mut waypoints, before) = waypoints_with(&[2, 1]); + let last = waypoints.chunks()[1].clone(); + let one = waypoint(); + let one_id = one.id; + waypoints.insert_at(2, vec![one]); + assert_eq!( + ids(&waypoints), + [&before[..2], &[one_id], &before[2..]].concat() + ); + assert!(Rc::ptr_eq(&waypoints.chunks()[1], &last)); + + // inside a chunk + let (mut waypoints, before) = waypoints_with(&[3]); + let one = waypoint(); + let one_id = one.id; + waypoints.insert_at(1, vec![one]); + assert_eq!( + ids(&waypoints), + [&before[..1], &[one_id], &before[1..]].concat() + ); + + // at the end, or past it + for index in [3, 100, usize::MAX] { + let (mut waypoints, before) = waypoints_with(&[3]); + let one = waypoint(); + let one_id = one.id; + waypoints.insert_at(index, vec![one]); + assert_eq!(ids(&waypoints), [before, vec![one_id]].concat()); + } + + // in a file without waypoints + let mut waypoints = Waypoints::default(); + let one = waypoint(); + let one_id = one.id; + waypoints.insert_at(0, vec![one]); + assert_eq!(ids(&waypoints), vec![one_id]); + } + + #[test] + fn test_splice_replaces_waypoints() { + let (mut waypoints, before) = waypoints_with(&[2, 2]); + let rev = waypoints.rev_id; + waypoints.splice(1, 3, vec![]); + assert_eq!(ids(&waypoints), [before[0], before[3]]); + assert_ne!(waypoints.rev_id, rev); + } + + #[test] + fn test_edit_changes_the_chunks_with_a_match_only() { + let (mut waypoints, before) = waypoints_with(&[2, 2, 1]); + let (first, last) = (waypoints.chunks()[0].clone(), waypoints.chunks()[2].clone()); + let rev = waypoints.rev_id; + let target = before[2]; + let changed = waypoints.edit( + |wpt| wpt.id == target, + |wpts| { + wpts.retain(|wpt| wpt.id != target); + true + }, + ); + assert!(changed); + assert_ne!(waypoints.rev_id, rev); + assert_eq!(ids(&waypoints), [&before[..2], &before[3..]].concat()); + // the chunks without a match are the same ones, the changed one is new + assert!(Rc::ptr_eq(&waypoints.chunks()[0], &first)); + assert!(Rc::ptr_eq(&waypoints.chunks()[2], &last)); + assert_eq!(waypoints.chunks().len(), 3); + + // a chunk left empty is dropped + let (mut waypoints, before) = waypoints_with(&[1, 2]); + waypoints.edit( + |wpt| wpt.id == before[0], + |wpts| { + wpts.clear(); + true + }, + ); + assert_eq!(waypoints.chunks().len(), 1); + assert_eq!(ids(&waypoints), before[1..]); + + // a chunk that is not changed is kept, and so is the revision + let (mut waypoints, before) = waypoints_with(&[2]); + let (chunk, rev) = (waypoints.chunks()[0].clone(), waypoints.rev_id); + assert!(!waypoints.edit(|wpt| wpt.id == before[0], |_| false)); + assert!(Rc::ptr_eq(&waypoints.chunks()[0], &chunk)); + assert_eq!(waypoints.rev_id, rev); + assert!(!waypoints.edit(|_| false, |_| true)); + } +} diff --git a/gpx-rs/engine/src/core/io/parse.rs b/gpx-rs/engine/src/core/io/parse.rs index 07ba40635..f3e8a9064 100644 --- a/gpx-rs/engine/src/core/io/parse.rs +++ b/gpx-rs/engine/src/core/io/parse.rs @@ -1,7 +1,5 @@ -use std::rc::Rc; - use crate::{ - Author, File, Link, LngLat, Track, TrackSegment, Trackpoint, TrackpointCategories, + Author, Chunk, File, Link, LngLat, Track, TrackSegment, Trackpoint, TrackpointCategories, TrackpointChunk, Waypoint, WaypointChunk, }; use chrono::DateTime; @@ -133,7 +131,7 @@ pub fn parse(data: &[u8], categories: &mut TrackpointCategories) -> Result (), @@ -141,7 +139,7 @@ pub fn parse(data: &[u8], categories: &mut TrackpointCategories) -> Result match e.name().as_ref() { "gpx" => { if !wpt_chunk.wpt.is_empty() { - gpx.wpt.push(Rc::new(std::mem::take(&mut wpt_chunk))); + gpx.wpt.push(std::mem::take(&mut wpt_chunk)); } } "metadata" => { @@ -197,7 +195,7 @@ pub fn parse(data: &[u8], categories: &mut TrackpointCategories) -> Result = gpx.wpt.iter().flat_map(|chunk| &chunk.wpt).collect(); + let wpt: Vec<_> = gpx.wpt.iter().collect(); assert_eq!(wpt.len(), 3); assert_eq!(wpt[0].coordinates.lat, 50.0); assert_eq!(wpt[0].name, None); @@ -550,9 +548,7 @@ mod tests { let gpx = parse_data("with_waypoint"); assert_eq!(gpx.wpt.len(), 1); - let chunk = &gpx.wpt[0]; - assert_eq!(chunk.wpt.len(), 1); - let wpt = &chunk.wpt[0]; + let wpt = &gpx.wpt[0]; assert_eq!(wpt.coordinates.lat, 50.7836710064975); assert_eq!(wpt.coordinates.lng, 4.410764082658738); assert!(wpt.name.as_ref().is_some_and(|n| n == "waypoint name")); diff --git a/gpx-rs/engine/src/engine/command/file/delete.rs b/gpx-rs/engine/src/engine/command/file/delete.rs index 0918c4b9e..b60ac4f57 100644 --- a/gpx-rs/engine/src/engine/command/file/delete.rs +++ b/gpx-rs/engine/src/engine/command/file/delete.rs @@ -1,8 +1,6 @@ use std::rc::Rc; -use crate::{ - Apply, CommandError, FileId, Selection, StackEntry, State, Waypoint, edit_waypoint_chunks, -}; +use crate::{Apply, CommandError, FileId, Selection, StackEntry, State, Waypoint}; /// Deletes the selected elements. With `whole_files`, the files holding the selected elements /// are deleted instead, even if only a track or a waypoint is selected. @@ -83,7 +81,7 @@ pub(crate) fn delete_waypoints( ) -> 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| { + let changed = file.wpt.edit(&filter, |wpts| { wpts.retain(|wpt| !filter(wpt)); true }); @@ -191,10 +189,10 @@ mod tests { 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 { + file.wpt = crate::Waypoints::new([crate::WaypointChunk { wpt: wpts, ..Default::default() - })]; + }]); fx.files.insert(id, Rc::new(file)); ids } @@ -207,16 +205,12 @@ mod tests { file_id: id, wpt_ids: HashSet::from([ids[1]]), }; - let rev = fx.files[&id].wpt_rev_id; + let rev = fx.files[&id].wpt.rev_id; Delete { whole_files: false } .apply(&mut fx.state()) .unwrap(); - assert_ne!(fx.files[&id].wpt_rev_id, rev); - let left: Vec<_> = fx.files[&id] - .wpt - .iter() - .flat_map(|c| c.wpt.iter().map(|w| w.id)) - .collect(); + assert_ne!(fx.files[&id].wpt.rev_id, rev); + let left: Vec<_> = fx.files[&id].wpt.iter().map(|w| w.id).collect(); assert_eq!(left, vec![ids[0], ids[2]]); assert_eq!(fx.selected_files(), HashSet::from([id])); } diff --git a/gpx-rs/engine/src/engine/command/file/delete_waypoint.rs b/gpx-rs/engine/src/engine/command/file/delete_waypoint.rs index 5976c2d67..d74794f58 100644 --- a/gpx-rs/engine/src/engine/command/file/delete_waypoint.rs +++ b/gpx-rs/engine/src/engine/command/file/delete_waypoint.rs @@ -37,7 +37,7 @@ mod tests { fn test_deletes_one_waypoint_without_touching_the_selection() { let mut fx = Fixture::default(); let mut file = File::default(); - file.wpt.push(Rc::new(WaypointChunk { + file.wpt.push(WaypointChunk { wpt: (0..3) .map(|i| Waypoint { name: Some(format!("w{i}")), @@ -45,7 +45,7 @@ mod tests { }) .collect(), ..Default::default() - })); + }); let id = file.id; let ids: Vec<_> = waypoint_ids(&file).collect(); fx.files.insert(id, Rc::new(file)); @@ -64,7 +64,6 @@ mod tests { let names: Vec<_> = fx.files[&id] .wpt .iter() - .flat_map(|chunk| &chunk.wpt) .map(|wpt| wpt.name.clone().unwrap()) .collect(); assert_eq!(names, ["w0", "w2"]); @@ -80,10 +79,10 @@ mod tests { fn test_deleting_the_selected_waypoint_selects_its_file() { let mut fx = Fixture::default(); let mut file = File::default(); - file.wpt.push(Rc::new(WaypointChunk { + file.wpt.push(WaypointChunk { wpt: vec![Waypoint::default(), Waypoint::default()], ..Default::default() - })); + }); let id = file.id; let ids: Vec<_> = waypoint_ids(&file).collect(); fx.files.insert(id, Rc::new(file)); diff --git a/gpx-rs/engine/src/engine/command/file/duplicate.rs b/gpx-rs/engine/src/engine/command/file/duplicate.rs index afea09546..91d400ab4 100644 --- a/gpx-rs/engine/src/engine/command/file/duplicate.rs +++ b/gpx-rs/engine/src/engine/command/file/duplicate.rs @@ -2,7 +2,7 @@ use std::{collections::HashSet, rc::Rc}; use crate::{ Apply, CommandError, FileId, Selection, StackEntry, State, Waypoint, copy_file, copy_segment, - copy_track, copy_waypoint, insert_waypoints, + copy_track, copy_waypoint, }; #[derive(Debug)] @@ -112,19 +112,14 @@ fn duplicate_waypoints( filter: impl Fn(&Waypoint) -> bool, ) -> Result { let file = files.get(&file_id).ok_or(CommandError::NothingToDo)?; - let selected: Vec<&Waypoint> = file - .wpt - .iter() - .flat_map(|chunk| &chunk.wpt) - .filter(|wpt| filter(wpt)) - .collect(); + let selected: Vec<&Waypoint> = file.wpt.iter().filter(|wpt| filter(wpt)).collect(); let last = selected.last().ok_or(CommandError::NothingToDo)?.id; let copies: Vec = selected.into_iter().map(copy_waypoint).collect(); let copy_ids = copies.iter().map(|wpt| wpt.id).collect(); // The copies go right after the last selected waypoint. let mut file = (**file).clone(); - insert_waypoints(&mut file, Some(last), copies); + file.wpt.insert_after(Some(last), copies); files.insert(file_id, Rc::new(file)); Ok(Selection::Waypoint { file_id, @@ -243,57 +238,44 @@ mod tests { 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 { + file.wpt = crate::Waypoints::new([crate::WaypointChunk { wpt: wpts, ..Default::default() - })]; + }]); fx.files.insert(id, Rc::new(file)); - let original_chunk = fx.files[&id].wpt[0].clone(); + let original_chunk = fx.files[&id].wpt.chunks()[0].clone(); fx.selection = Selection::Waypoint { file_id: id, wpt_ids: HashSet::from([ids[0]]), }; - let rev = fx.files[&id].wpt_rev_id; + let rev = fx.files[&id].wpt.rev_id; Duplicate.apply(&mut fx.state()).unwrap(); - assert_ne!(fx.files[&id].wpt_rev_id, rev); - let all: Vec<_> = fx.files[&id] - .wpt - .iter() - .flat_map(|c| c.wpt.iter().map(|w| w.id)) - .collect(); + assert_ne!(fx.files[&id].wpt.rev_id, rev); + let all: Vec<_> = fx.files[&id].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] + // the copy comes right after the selected waypoint, in a new chunk 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); + assert_eq!(fx.files[&id].wpt.chunks().len(), 1); + assert_ne!(fx.files[&id].wpt.chunks()[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(); + // selecting the last waypoint: the copy goes at the end + let last = fx.files[&id].wpt.iter().last().unwrap().id; 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)) - ); + let all: Vec<_> = fx.files[&id].wpt.iter().map(|w| w.id).collect(); + assert_eq!(all.len(), 5); + assert_eq!(all[..4], [ids[0], all[1], ids[1], ids[2]]); 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 - ); + assert_eq!(fx.files[&id].wpt.len(), 10); } #[test] diff --git a/gpx-rs/engine/src/engine/command/file/move_elements.rs b/gpx-rs/engine/src/engine/command/file/move_elements.rs index fb4267125..b63630727 100644 --- a/gpx-rs/engine/src/engine/command/file/move_elements.rs +++ b/gpx-rs/engine/src/engine/command/file/move_elements.rs @@ -71,7 +71,7 @@ mod tests { use crate::{ File, Track, TrackInfo, TrackSegment, TrackSegmentId, Waypoint, WaypointChunk, WaypointId, - engine::command::fixture::Fixture, new_file, waypoint_ids, + Waypoints, engine::command::fixture::Fixture, new_file, waypoint_ids, }; use super::*; @@ -93,10 +93,10 @@ mod tests { .collect(), ); if waypoints > 0 { - file.wpt = vec![Rc::new(WaypointChunk { + file.wpt = Waypoints::new([WaypointChunk { wpt: (0..waypoints).map(|_| Waypoint::default()).collect(), ..Default::default() - })]; + }]); } file } diff --git a/gpx-rs/engine/src/engine/command/file/paste.rs b/gpx-rs/engine/src/engine/command/file/paste.rs index e371798ba..c49396158 100644 --- a/gpx-rs/engine/src/engine/command/file/paste.rs +++ b/gpx-rs/engine/src/engine/command/file/paste.rs @@ -140,7 +140,8 @@ mod tests { use crate::{ Clipboard, ClipboardIds, File, FileId, Track, TrackId, TrackInfo, TrackSegment, - TrackSegmentId, Waypoint, WaypointId, engine::command::fixture::Fixture, new_file, + TrackSegmentId, Waypoint, WaypointId, Waypoints, engine::command::fixture::Fixture, + new_file, }; use super::*; @@ -162,10 +163,10 @@ mod tests { .collect(), ); if waypoints > 0 { - file.wpt = vec![Rc::new(crate::WaypointChunk { + file.wpt = Waypoints::new([crate::WaypointChunk { wpt: (0..waypoints).map(|_| Waypoint::default()).collect(), ..Default::default() - })]; + }]); } file } diff --git a/gpx-rs/engine/src/engine/command/file/transfer.rs b/gpx-rs/engine/src/engine/command/file/transfer.rs index b3664afcc..fdd85ecf0 100644 --- a/gpx-rs/engine/src/engine/command/file/transfer.rs +++ b/gpx-rs/engine/src/engine/command/file/transfer.rs @@ -7,7 +7,7 @@ use std::{collections::HashSet, rc::Rc}; use crate::{ ClipboardContent, ClipboardSegment, ClipboardTrack, CommandError, File, FileId, Selection, State, Track, TrackId, TrackSegment, TrackSegmentId, Waypoint, WaypointId, copy_file, - copy_segment, copy_track, copy_waypoint, edit_waypoint_chunks, insert_waypoints_at, + copy_segment, copy_track, copy_waypoint, }; /// Where elements are put, among the others of the same list. @@ -52,10 +52,7 @@ pub enum Destination { } pub fn waypoint_ids(file: &File) -> impl Iterator + '_ { - file.wpt - .iter() - .flat_map(|chunk| &chunk.wpt) - .map(|wpt| wpt.id) + file.wpt.iter().map(|wpt| wpt.id) } fn file_mut<'a>(state: &'a mut State, id: FileId) -> Result<&'a mut File, CommandError> { @@ -119,8 +116,7 @@ pub fn remove_elements(state: &mut State, content: &ClipboardContent) { let ids: HashSet = waypoints.iter().map(|wpt| wpt.id).collect(); for file in state.files.values_mut() { if waypoint_ids(file).any(|id| ids.contains(&id)) { - edit_waypoint_chunks( - Rc::make_mut(file), + Rc::make_mut(file).wpt.edit( |wpt| ids.contains(&wpt.id), |wpts| { wpts.retain(|wpt| !ids.contains(&wpt.id)); @@ -334,7 +330,7 @@ fn transfer_waypoints( let ids = waypoints.iter().map(|wpt| wpt.id).collect(); let file = file_mut(state, file_id)?; let existing: Vec = waypoint_ids(file).collect(); - insert_waypoints_at(file, place.position(&existing), waypoints); + file.wpt.insert_at(place.position(&existing), waypoints); Ok(Selection::Waypoint { file_id, wpt_ids: ids, 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 deleted file mode 100644 index 4769b740c..000000000 --- a/gpx-rs/engine/src/engine/command/pattern/edit_waypoint_chunks.rs +++ /dev/null @@ -1,40 +0,0 @@ -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; - if changed { - file.wpt_rev_id = Default::default(); - } - changed -} diff --git a/gpx-rs/engine/src/engine/command/pattern/insert_waypoints.rs b/gpx-rs/engine/src/engine/command/pattern/insert_waypoints.rs deleted file mode 100644 index d49f34d76..000000000 --- a/gpx-rs/engine/src/engine/command/pattern/insert_waypoints.rs +++ /dev/null @@ -1,218 +0,0 @@ -use std::rc::Rc; - -use crate::{File, Waypoint, WaypointChunk, WaypointId}; - -/// Inserts waypoints in the file, right after the waypoint `after`, or at the end if there is -/// none (or if it is not in the file). -/// -/// See [`insert_waypoints_at`]. -pub fn insert_waypoints(file: &mut File, after: Option, waypoints: Vec) { - let index = after - .and_then(|after| { - file.wpt - .iter() - .flat_map(|chunk| &chunk.wpt) - .position(|wpt| wpt.id == after) - }) - .map_or(usize::MAX, |i| i + 1); - insert_waypoints_at(file, index, waypoints); -} - -/// Inserts waypoints in the file, so that the first one is at `index` among the waypoints of -/// the file (at the end if the index is past it). -/// -/// Only the chunk holding the waypoint that was at `index` is cut (when the waypoints are not -/// inserted between two chunks), all the other chunks are kept as they are. -pub fn insert_waypoints_at(file: &mut File, index: usize, waypoints: Vec) { - if waypoints.is_empty() { - return; - } - - let mut inserted = Vec::new(); - let mut chunk = WaypointChunk::default(); - for wpt in waypoints { - chunk.wpt.push(wpt); - if chunk.is_full() { - inserted.push(Rc::new(std::mem::take(&mut chunk))); - } - } - if !chunk.wpt.is_empty() { - inserted.push(Rc::new(chunk)); - } - - // the chunk holding the waypoint at `index`, and the position of that waypoint in it - let mut start = 0; - let position = file.wpt.iter().enumerate().find_map(|(i, chunk)| { - let offset = index - .checked_sub(start) - .filter(|offset| *offset < chunk.wpt.len()); - start += chunk.wpt.len(); - offset.map(|offset| (i, offset)) - }); - match position { - None => file.wpt.extend(inserted), - Some((i, 0)) => { - file.wpt.splice(i..i, inserted); - } - Some((i, offset)) => { - let cut = &file.wpt[i]; - let part = |wpt: &[Waypoint]| { - Rc::new(WaypointChunk { - wpt: wpt.to_vec(), - ..Default::default() - }) - }; - let mut chunks = vec![part(&cut.wpt[..offset])]; - chunks.extend(inserted); - chunks.push(part(&cut.wpt[offset..])); - file.wpt.splice(i..=i, chunks); - } - } - file.wpt_rev_id = Default::default(); -} - -#[cfg(test)] -mod tests { - use super::*; - - fn waypoint() -> Waypoint { - Waypoint::default() - } - - fn file_with(chunks: &[usize]) -> (File, Vec) { - let mut file = File::default(); - let mut ids = vec![]; - for n in chunks { - let wpt: Vec<_> = (0..*n).map(|_| waypoint()).collect(); - ids.extend(wpt.iter().map(|w| w.id)); - file.wpt.push(Rc::new(WaypointChunk { - wpt, - ..Default::default() - })); - } - (file, ids) - } - - fn ids(file: &File) -> Vec { - file.wpt - .iter() - .flat_map(|chunk| chunk.wpt.iter().map(|w| w.id)) - .collect() - } - - #[test] - fn test_insert_at_the_end() { - let (mut file, before) = file_with(&[2, 1]); - let rev = file.wpt_rev_id; - let new = vec![waypoint(), waypoint()]; - let new_ids: Vec<_> = new.iter().map(|w| w.id).collect(); - insert_waypoints(&mut file, None, new); - assert_eq!(ids(&file), [before.clone(), new_ids].concat()); - // the chunks that were there are kept - assert_eq!(file.wpt.len(), 3); - assert_ne!(file.wpt_rev_id, rev); - // an unknown waypoint is an insertion at the end too - let (mut file, before) = file_with(&[2]); - let one = waypoint(); - let one_id = one.id; - insert_waypoints(&mut file, Some(WaypointId::default()), vec![one]); - assert_eq!(ids(&file), [before, vec![one_id]].concat()); - } - - #[test] - fn test_insert_after_the_last_waypoint_of_a_chunk_keeps_the_chunks() { - let (mut file, before) = file_with(&[2, 2]); - let first = file.wpt[0].clone(); - let one = waypoint(); - let one_id = one.id; - insert_waypoints(&mut file, Some(before[1]), vec![one]); - assert_eq!(ids(&file), [&before[..2], &[one_id], &before[2..]].concat()); - // 2 chunks + the inserted one, nothing was cut - assert_eq!(file.wpt.len(), 3); - assert!(Rc::ptr_eq(&file.wpt[0], &first)); - } - - #[test] - fn test_insert_in_the_middle_of_a_chunk_cuts_it() { - let (mut file, before) = file_with(&[3, 1]); - let last = file.wpt[1].clone(); - let new = vec![waypoint(), waypoint()]; - let new_ids: Vec<_> = new.iter().map(|w| w.id).collect(); - insert_waypoints(&mut file, Some(before[0]), new); - assert_eq!( - ids(&file), - [&before[..1], &new_ids[..], &before[1..]].concat() - ); - // the cut chunk, the inserted one and the end of the cut one, then the untouched chunk - assert_eq!(file.wpt.len(), 4); - assert!(Rc::ptr_eq(&file.wpt[3], &last)); - } - - #[test] - fn test_insert_nothing() { - let (mut file, before) = file_with(&[2]); - let rev = file.wpt_rev_id; - insert_waypoints(&mut file, Some(before[0]), vec![]); - assert_eq!(ids(&file), before); - assert_eq!(file.wpt_rev_id, rev); - } - - #[test] - fn test_insert_many_fills_chunks() { - let mut file = File::default(); - let new: Vec<_> = (0..300).map(|_| waypoint()).collect(); - let new_ids: Vec<_> = new.iter().map(|w| w.id).collect(); - insert_waypoints(&mut file, None, new); - assert_eq!(ids(&file), new_ids); - assert_eq!(file.wpt.len(), 3); - assert!(file.wpt[..2].iter().all(|chunk| chunk.is_full())); - } - - #[test] - fn test_insert_at_an_index() { - // at the start - let (mut file, before) = file_with(&[2, 1]); - let first = file.wpt[0].clone(); - let one = waypoint(); - let one_id = one.id; - insert_waypoints_at(&mut file, 0, vec![one]); - assert_eq!(ids(&file), [vec![one_id], before.clone()].concat()); - // the existing chunks are kept - assert_eq!(file.wpt.len(), 3); - assert!(Rc::ptr_eq(&file.wpt[1], &first)); - - // between two chunks - let (mut file, before) = file_with(&[2, 1]); - let (first, last) = (file.wpt[0].clone(), file.wpt[1].clone()); - let one = waypoint(); - let one_id = one.id; - insert_waypoints_at(&mut file, 2, vec![one]); - assert_eq!(ids(&file), [&before[..2], &[one_id], &before[2..]].concat()); - assert_eq!(file.wpt.len(), 3); - assert!(Rc::ptr_eq(&file.wpt[0], &first) && Rc::ptr_eq(&file.wpt[2], &last)); - - // inside a chunk - let (mut file, before) = file_with(&[3]); - let one = waypoint(); - let one_id = one.id; - insert_waypoints_at(&mut file, 1, vec![one]); - assert_eq!(ids(&file), [&before[..1], &[one_id], &before[1..]].concat()); - assert_eq!(file.wpt.len(), 3); - - // at the end, or past it - for index in [3, 100] { - let (mut file, before) = file_with(&[3]); - let one = waypoint(); - let one_id = one.id; - insert_waypoints_at(&mut file, index, vec![one]); - assert_eq!(ids(&file), [before, vec![one_id]].concat()); - } - - // in a file without waypoints - let mut file = File::default(); - let one = waypoint(); - let one_id = one.id; - insert_waypoints_at(&mut file, 0, vec![one]); - assert_eq!(ids(&file), vec![one_id]); - } -} diff --git a/gpx-rs/engine/src/engine/command/pattern/mod.rs b/gpx-rs/engine/src/engine/command/pattern/mod.rs index 7855021e9..f4f58aaee 100644 --- a/gpx-rs/engine/src/engine/command/pattern/mod.rs +++ b/gpx-rs/engine/src/engine/command/pattern/mod.rs @@ -1,13 +1,9 @@ mod copy; -mod edit_waypoint_chunks; -mod insert_waypoints; mod produce; mod update_selected; mod update_waypoint; pub use copy::*; -pub use edit_waypoint_chunks::*; -pub use insert_waypoints::*; pub use produce::*; pub use update_selected::*; pub use update_waypoint::*; 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 3307e8254..b49245083 100644 --- a/gpx-rs/engine/src/engine/command/pattern/update_selected.rs +++ b/gpx-rs/engine/src/engine/command/pattern/update_selected.rs @@ -1,8 +1,6 @@ use std::rc::Rc; -use crate::{ - File, FileId, Selection, StackEntry, State, Track, TrackSegment, Waypoint, edit_waypoint_chunks, -}; +use crate::{File, FileId, Selection, StackEntry, State, Track, TrackSegment, Waypoint}; /// What an [`Editor`] hook did to the element it was given. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -87,7 +85,7 @@ fn edit_waypoints( filter: impl Fn(&Waypoint) -> bool, editor: &mut E, ) -> Edit { - let changed = edit_waypoint_chunks(file, &filter, |wpts| { + let changed = file.wpt.edit(&filter, |wpts| { edit_where(wpts, &filter, |wpt| editor.waypoint(wpt)) == Edit::Changed }); if changed { @@ -287,35 +285,34 @@ mod tests { 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 { + file.wpt = crate::Waypoints::new([crate::WaypointChunk { wpt: wpts, ..Default::default() - })]; + }]); fx.files.insert(id, Rc::new(file)); - let chunk_id = fx.files[&id].wpt[0].id; + let chunk_id = fx.files[&id].wpt.chunks()[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())); + assert!(fx.files[&id].wpt.iter().all(|w| w.name.is_none())); fx.selection = Selection::Waypoint { file_id: id, wpt_ids: HashSet::from([ids[1]]), }; - let rev = fx.files[&id].wpt_rev_id; + let rev = fx.files[&id].wpt.rev_id; update_selected(&mut fx.state(), &mut Name); - assert_ne!(fx.files[&id].wpt_rev_id, rev); - 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_ne!(fx.files[&id].wpt.rev_id, rev); + assert_ne!(fx.files[&id].wpt.chunks()[0].id, chunk_id); + let named: Vec<_> = fx.files[&id].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())); + assert!(fx.files[&id].wpt.iter().all(|w| w.name.is_some())); } #[test] diff --git a/gpx-rs/engine/src/engine/command/pattern/update_waypoint.rs b/gpx-rs/engine/src/engine/command/pattern/update_waypoint.rs index 69cfe5881..e1f4293a8 100644 --- a/gpx-rs/engine/src/engine/command/pattern/update_waypoint.rs +++ b/gpx-rs/engine/src/engine/command/pattern/update_waypoint.rs @@ -1,6 +1,6 @@ use std::rc::Rc; -use crate::{CommandError, FileId, State, Waypoint, WaypointId, edit_waypoint_chunks}; +use crate::{CommandError, FileId, State, Waypoint, WaypointId}; /// Changes one waypoint of a file with `f`, whatever is selected. Nothing to do if the file or /// the waypoint does not exist. @@ -12,8 +12,7 @@ pub fn update_waypoint( ) -> Result<(), CommandError> { let file = state.files.get(&file_id).ok_or(CommandError::NothingToDo)?; let mut file = (**file).clone(); - let changed = edit_waypoint_chunks( - &mut file, + let changed = file.wpt.edit( |wpt| wpt.id == waypoint_id, |wpts| { wpts.iter_mut() diff --git a/gpx-rs/engine/src/engine/command/tools/clean.rs b/gpx-rs/engine/src/engine/command/tools/clean.rs index bc1f7325a..6cfde55d6 100644 --- a/gpx-rs/engine/src/engine/command/tools/clean.rs +++ b/gpx-rs/engine/src/engine/command/tools/clean.rs @@ -2,8 +2,7 @@ use std::{collections::HashSet, rc::Rc}; use crate::{ Apply, CommandError, Edit, Editor, File, FileId, LngLat, LngLatBounds, Selection, State, - TrackSegment, TrackSegmentId, Trackpoint, Waypoint, delete_waypoints, edit_waypoint_chunks, - update_selected, + TrackSegment, TrackSegmentId, Trackpoint, Waypoint, delete_waypoints, update_selected, }; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -91,8 +90,7 @@ impl Editor for Cleaner<'_> { } } if self.clean.wpt - && edit_waypoint_chunks( - file, + && file.wpt.edit( |wpt| self.clean.removes(wpt.coordinates), |wpts| { wpts.retain(|wpt| !self.clean.removes(wpt.coordinates)); @@ -244,7 +242,7 @@ fn fixed_selection(state: &State) -> Selection { mod tests { use std::collections::HashSet; - use crate::{FileId, Load, engine::command::fixture::Fixture}; + use crate::{FileId, Load, WaypointChunk, Waypoints, engine::command::fixture::Fixture}; use super::*; @@ -363,18 +361,14 @@ mod tests { fn test_waypoints_flag_and_selection() { let (mut fx, id) = loaded(); let mut file = (*fx.files[&id]).clone(); - file.wpt = vec![std::rc::Rc::new(crate::WaypointChunk { + file.wpt = Waypoints::new([WaypointChunk { wpt: vec![Waypoint::default(), Waypoint::default()], ..Default::default() - })]; - let ids: Vec<_> = file.wpt[0].wpt.iter().map(|w| w.id).collect(); + }]); + let ids: Vec<_> = file.wpt.iter().map(|w| w.id).collect(); fx.files.insert(id, std::rc::Rc::new(file)); let everywhere = bounds(-180.0, -90.0, 180.0, 90.0); - let wpt_count = |fx: &Fixture| { - fx.files - .get(&id) - .map_or(0, |f| f.wpt.iter().map(|c| c.wpt.len()).sum::()) - }; + let wpt_count = |fx: &Fixture| fx.files.get(&id).map_or(0, |f| f.wpt.len()); // trackpoints only: waypoints are kept Clean { diff --git a/gpx-rs/engine/src/engine/command/tools/edit_waypoint.rs b/gpx-rs/engine/src/engine/command/tools/edit_waypoint.rs index a051d48cf..7828907bc 100644 --- a/gpx-rs/engine/src/engine/command/tools/edit_waypoint.rs +++ b/gpx-rs/engine/src/engine/command/tools/edit_waypoint.rs @@ -45,7 +45,7 @@ mod tests { fn test_edit_the_given_waypoint() { let mut fx = Fixture::default(); let mut file = File::default(); - file.wpt.push(Rc::new(WaypointChunk { + file.wpt.push(WaypointChunk { wpt: vec![ Waypoint { name: Some("old".into()), @@ -64,7 +64,7 @@ mod tests { }, ], ..Default::default() - })); + }); let id = file.id; let ids: Vec<_> = waypoint_ids(&file).collect(); fx.files.insert(id, Rc::new(file)); @@ -83,11 +83,7 @@ mod tests { .apply(&mut fx.state()) .unwrap(); - let wpts: Vec<_> = fx.files[&id] - .wpt - .iter() - .flat_map(|c| c.wpt.iter()) - .collect(); + let wpts: Vec<_> = fx.files[&id].wpt.iter().collect(); let edited = wpts[0]; assert_eq!(edited.id, ids[0]); assert_eq!(edited.name.as_deref(), Some("new")); diff --git a/gpx-rs/engine/src/engine/command/tools/move_waypoint.rs b/gpx-rs/engine/src/engine/command/tools/move_waypoint.rs index 13063309f..dc22eae3f 100644 --- a/gpx-rs/engine/src/engine/command/tools/move_waypoint.rs +++ b/gpx-rs/engine/src/engine/command/tools/move_waypoint.rs @@ -36,7 +36,7 @@ mod tests { fn fixture() -> (Fixture, crate::FileId, Vec) { let mut fx = Fixture::default(); let mut file = File::default(); - file.wpt.push(Rc::new(WaypointChunk { + file.wpt.push(WaypointChunk { wpt: (0..3) .map(|i| Waypoint { name: Some(format!("w{i}")), @@ -44,7 +44,7 @@ mod tests { }) .collect(), ..Default::default() - })); + }); let id = file.id; let ids = waypoint_ids(&file).collect(); fx.files.insert(id, Rc::new(file)); @@ -55,7 +55,7 @@ mod tests { #[test] fn test_only_the_given_waypoint_moves_whatever_is_selected() { let (mut fx, id, ids) = fixture(); - let rev = fx.files[&id].wpt_rev_id; + let rev = fx.files[&id].wpt.rev_id; MoveWaypoint { file_id: id, @@ -67,11 +67,7 @@ mod tests { .apply(&mut fx.state()) .unwrap(); - let wpts: Vec<_> = fx.files[&id] - .wpt - .iter() - .flat_map(|c| c.wpt.iter()) - .collect(); + let wpts: Vec<_> = fx.files[&id].wpt.iter().collect(); assert_eq!(wpts.len(), 3); assert_eq!( ( @@ -87,7 +83,7 @@ mod tests { assert_eq!((wpts[0].coordinates.lng, wpts[0].ele), (0.0, 0.0)); assert_eq!((wpts[2].coordinates.lat, wpts[2].ele), (0.0, 0.0)); // the coordinates changed, which is noticed by what is derived from them - assert_ne!(fx.files[&id].wpt_rev_id, rev); + assert_ne!(fx.files[&id].wpt.rev_id, rev); assert_eq!(fx.selection, Selection::Empty); } diff --git a/gpx-rs/engine/src/engine/command/tools/new_waypoint.rs b/gpx-rs/engine/src/engine/command/tools/new_waypoint.rs index 563b7fd17..599347e01 100644 --- a/gpx-rs/engine/src/engine/command/tools/new_waypoint.rs +++ b/gpx-rs/engine/src/engine/command/tools/new_waypoint.rs @@ -1,9 +1,6 @@ use std::rc::Rc; -use crate::{ - Apply, CommandError, File, FileId, Link, LngLat, Selection, State, Waypoint, - insert_waypoints_at, -}; +use crate::{Apply, CommandError, File, FileId, Link, LngLat, Selection, State, Waypoint}; /// Adds a waypoint at the end of the waypoints of each selected file (or of the file of the /// selected elements). The strings are empty when the waypoint does not have the field. @@ -77,7 +74,7 @@ impl Apply for NewWaypoint<'_> { self.link, ); let file: &mut File = Rc::make_mut(state.files.get_mut(&id).unwrap()); - insert_waypoints_at(file, usize::MAX, vec![waypoint]); + file.wpt.insert_at(usize::MAX, vec![waypoint]); } Ok(()) } @@ -104,11 +101,7 @@ mod tests { } fn waypoints(fx: &Fixture, id: FileId) -> Vec { - fx.files[&id] - .wpt - .iter() - .flat_map(|chunk| chunk.wpt.iter().cloned()) - .collect() + fx.files[&id].wpt.iter().cloned().collect() } #[test] @@ -127,7 +120,7 @@ mod tests { .apply(&mut fx.state()) .unwrap(); let (a, b) = (fx.order.0[0], fx.order.0[1]); - let rev = fx.files[&b].wpt_rev_id; + let rev = fx.files[&b].wpt.rev_id; // b is selected new_waypoint("peak").apply(&mut fx.state()).unwrap(); @@ -147,7 +140,7 @@ mod tests { wpt.link.as_ref().map(|l| l.href.as_str()), Some("https://example.com") ); - assert_ne!(fx.files[&b].wpt_rev_id, rev); + assert_ne!(fx.files[&b].wpt.rev_id, rev); // added after the others let mut second = new_waypoint("second"); diff --git a/gpx-rs/engine/src/engine/command/tools/route.rs b/gpx-rs/engine/src/engine/command/tools/route.rs index 176f4128e..892c8aa11 100644 --- a/gpx-rs/engine/src/engine/command/tools/route.rs +++ b/gpx-rs/engine/src/engine/command/tools/route.rs @@ -496,7 +496,7 @@ mod tests { let lens: Vec<_> = file .trk .iter() - .flat_map(|trk| trk.trkseg.iter().map(TrackSegment::len)) + .flat_map(|trk| trk.trkseg.iter().map(|segment| segment.len())) .collect(); assert!(lens.len() >= 2); diff --git a/gpx-rs/engine/src/engine/derived/coordinates_cache.rs b/gpx-rs/engine/src/engine/derived/coordinates_cache.rs index 6c5772c75..7ce53c346 100644 --- a/gpx-rs/engine/src/engine/derived/coordinates_cache.rs +++ b/gpx-rs/engine/src/engine/derived/coordinates_cache.rs @@ -23,14 +23,14 @@ impl CoordinatesCache { if self .waypoints .get(&file.id) - .is_none_or(|(r, _)| *r != file.wpt_rev_id) + .is_none_or(|(r, _)| *r != file.wpt.rev_id) { let mut coordinates = Vec::new(); - for wpt in file.wpt.iter().flat_map(|chunk| &chunk.wpt) { + for wpt in file.wpt.iter() { coordinates.extend([wpt.coordinates.lng, wpt.coordinates.lat]); } self.waypoints - .insert(file.id, (file.wpt_rev_id, coordinates)); + .insert(file.id, (file.wpt.rev_id, coordinates)); } for seg in file.trk.iter().flat_map(|trk| &trk.trkseg) { segments.insert(seg.id); @@ -138,17 +138,16 @@ mod tests { ..Default::default() }; let mut file = (*fx.files[&id]).clone(); - file.wpt_rev_id = Default::default(); - file.wpt = vec![ - Rc::new(WaypointChunk { + file.wpt = crate::Waypoints::new([ + WaypointChunk { wpt: vec![wpt(1.0, 2.0)], ..Default::default() - }), - Rc::new(WaypointChunk { + }, + WaypointChunk { wpt: vec![wpt(3.0, 4.0)], ..Default::default() - }), - ]; + }, + ]); fx.files.insert(id, Rc::new(file)); let mut cache = CoordinatesCache::default(); cache.update(Some(&fx.files)); diff --git a/gpx-rs/engine/src/engine/derived/file_structure.rs b/gpx-rs/engine/src/engine/derived/file_structure.rs index df68afa5c..82e522044 100644 --- a/gpx-rs/engine/src/engine/derived/file_structure.rs +++ b/gpx-rs/engine/src/engine/derived/file_structure.rs @@ -76,21 +76,19 @@ impl FileStructure { waypoints: file .wpt .iter() - .flat_map(|chunk| &chunk.wpt) .map(|wpt| WaypointNode { id: wpt.id, name: wpt.name.clone(), sym: wpt.sym.clone(), }) .collect(), - wpt_rev_id: file.wpt_rev_id, + wpt_rev_id: file.wpt.rev_id, } } } #[cfg(test)] mod tests { - use std::rc::Rc; use crate::{Apply, Load, Waypoint, WaypointChunk, engine::command::fixture::Fixture}; @@ -133,18 +131,17 @@ mod tests { name: Some(n.to_string()), ..Default::default() }; - let before = file.wpt_rev_id; - file.wpt_rev_id = Default::default(); - file.wpt = vec![ - Rc::new(WaypointChunk { + let before = file.wpt.rev_id; + file.wpt = crate::Waypoints::new([ + WaypointChunk { wpt: vec![wpt("a"), wpt("b")], ..Default::default() - }), - Rc::new(WaypointChunk { + }, + WaypointChunk { wpt: vec![wpt("c")], ..Default::default() - }), - ]; + }, + ]); let node = FileStructure::new(&file); let names: Vec<_> = node .waypoints @@ -153,6 +150,6 @@ mod tests { .collect(); assert_eq!(names, ["a", "b", "c"]); assert_ne!(node.wpt_rev_id, before); - assert_eq!(node.wpt_rev_id, file.wpt_rev_id); + assert_eq!(node.wpt_rev_id, file.wpt.rev_id); } } diff --git a/gpx-rs/engine/src/engine/derived/routing_buffer.rs b/gpx-rs/engine/src/engine/derived/routing_buffer.rs index 5c7df7c57..8181e4a46 100644 --- a/gpx-rs/engine/src/engine/derived/routing_buffer.rs +++ b/gpx-rs/engine/src/engine/derived/routing_buffer.rs @@ -66,7 +66,7 @@ impl RoutingBuffer { mod tests { use std::{collections::HashSet, rc::Rc}; - use crate::{TrackSegment, parse}; + use crate::parse; use super::*; @@ -91,7 +91,7 @@ mod tests { let lengths: Vec = files[&id] .trk .iter() - .flat_map(|trk| trk.trkseg.iter().map(TrackSegment::len)) + .flat_map(|trk| trk.trkseg.iter().map(|segment| segment.len())) .collect(); let mut buffer = RoutingBuffer::default(); buffer.update(Some(&files), &file_selection(id), &[id]); diff --git a/gpx-rs/engine/src/engine/engine.rs b/gpx-rs/engine/src/engine/engine.rs index dc7537732..bcbd55105 100644 --- a/gpx-rs/engine/src/engine/engine.rs +++ b/gpx-rs/engine/src/engine/engine.rs @@ -80,7 +80,6 @@ impl Engine { .get(file_id)? .wpt .iter() - .flat_map(|chunk| &chunk.wpt) .find(|wpt| wpt.id == *id) } diff --git a/gpx-rs/engine/src/engine/state/clipboard.rs b/gpx-rs/engine/src/engine/state/clipboard.rs index da3bc01f9..6f9684302 100644 --- a/gpx-rs/engine/src/engine/state/clipboard.rs +++ b/gpx-rs/engine/src/engine/state/clipboard.rs @@ -120,20 +120,14 @@ impl Clipboard { .collect(), ) } - Selection::Waypoints { file_id } => ClipboardContent::Waypoints( - files - .get(file_id)? - .wpt - .iter() - .flat_map(|chunk| chunk.wpt.iter().cloned()) - .collect(), - ), + Selection::Waypoints { file_id } => { + ClipboardContent::Waypoints(files.get(file_id)?.wpt.iter().cloned().collect()) + } Selection::Waypoint { file_id, wpt_ids } => ClipboardContent::Waypoints( files .get(file_id)? .wpt .iter() - .flat_map(|chunk| &chunk.wpt) .filter(|wpt| wpt_ids.contains(&wpt.id)) .cloned() .collect(), @@ -233,14 +227,14 @@ mod tests { ..Default::default() }); } - file.wpt.push(Rc::new(WaypointChunk { + file.wpt.push(WaypointChunk { wpt: vec![Waypoint::default(), Waypoint::default()], ..Default::default() - })); - file.wpt.push(Rc::new(WaypointChunk { + }); + file.wpt.push(WaypointChunk { wpt: vec![Waypoint::default()], ..Default::default() - })); + }); file } @@ -267,11 +261,7 @@ mod tests { let file = file("source"); let (t0, t2) = (file.trk[0].id, file.trk[2].id); let (s0, s1) = (file.trk[1].trkseg[0].id, file.trk[1].trkseg[1].id); - let wpt: Vec<_> = file - .wpt - .iter() - .flat_map(|c| c.wpt.iter().map(|w| w.id)) - .collect(); + let wpt: Vec<_> = file.wpt.iter().map(|w| w.id).collect(); let files = files_of(vec![file.clone()]); // tracks, in the order of the file, with the name of their file @@ -447,7 +437,7 @@ mod tests { let waypoints = Selection::Waypoints { file_id: file.id }; let waypoint = Selection::Waypoint { file_id: file.id, - wpt_ids: [file.wpt[0].wpt[0].id].into(), + wpt_ids: [file.wpt[0].id].into(), }; let targets = [ Selection::Empty, diff --git a/gpx-rs/engine/src/engine/state/selection.rs b/gpx-rs/engine/src/engine/state/selection.rs index eab395f07..a9689680c 100644 --- a/gpx-rs/engine/src/engine/state/selection.rs +++ b/gpx-rs/engine/src/engine/state/selection.rs @@ -126,12 +126,7 @@ impl Selection { Selection::Waypoints { file_id } => !files.contains_key(file_id), Selection::Waypoint { file_id, wpt_ids } => match files.get(file_id) { Some(file) => { - wpt_ids.retain(|id| { - file.wpt - .iter() - .flat_map(|chunk| &chunk.wpt) - .any(|wpt| wpt.id == *id) - }); + wpt_ids.retain(|id| file.wpt.iter().any(|wpt| wpt.id == *id)); wpt_ids.is_empty() } None => true, @@ -324,13 +319,7 @@ impl Selection { Selection::Waypoints { .. } => None, Selection::Waypoint { file_id, .. } => Some(Selection::Waypoint { file_id: *file_id, - wpt_ids: files - .get(file_id)? - .wpt - .iter() - .flat_map(|chunk| &chunk.wpt) - .map(|wpt| wpt.id) - .collect(), + wpt_ids: files.get(file_id)?.wpt.iter().map(|wpt| wpt.id).collect(), }), } } @@ -384,13 +373,8 @@ impl Selection { }) } Selection::Waypoint { file_id, wpt_ids } => { - let ids: Vec = files - .get(file_id)? - .wpt - .iter() - .flat_map(|chunk| &chunk.wpt) - .map(|wpt| wpt.id) - .collect(); + let ids: Vec = + files.get(file_id)?.wpt.iter().map(|wpt| wpt.id).collect(); Some(Selection::Waypoint { file_id: *file_id, wpt_ids: [neighbour(&ids, wpt_ids, down)?].into(), @@ -444,12 +428,12 @@ mod tests { trkseg: vec![TrackSegment::default()], ..Default::default() }); - file.wpt.push(Rc::new(WaypointChunk { + file.wpt.push(WaypointChunk { wpt: vec![Waypoint::default()], ..Default::default() - })); + }); let (file_id, trk_id) = (file.id, file.trk[0].id); - let (seg_id, wpt_id) = (file.trk[0].trkseg[0].id, file.wpt[0].wpt[0].id); + let (seg_id, wpt_id) = (file.trk[0].trkseg[0].id, file.wpt[0].id); let mut files = StackEntry::default(); files.insert(file_id, Rc::new(file)); @@ -595,14 +579,14 @@ mod tests { ..Default::default() }); } - file.wpt.push(Rc::new(WaypointChunk { + file.wpt.push(WaypointChunk { wpt: vec![Waypoint::default(), Waypoint::default()], ..Default::default() - })); - file.wpt.push(Rc::new(WaypointChunk { + }); + file.wpt.push(WaypointChunk { wpt: vec![Waypoint::default()], ..Default::default() - })); + }); let tree = Tree { file: file.id, tracks: file.trk.iter().map(|t| t.id).collect(), @@ -611,12 +595,7 @@ mod tests { .iter() .map(|t| t.trkseg.iter().map(|s| s.id).collect()) .collect(), - waypoints: file - .wpt - .iter() - .flat_map(|chunk| &chunk.wpt) - .map(|w| w.id) - .collect(), + waypoints: file.wpt.iter().map(|w| w.id).collect(), order: vec![file.id], files: StackEntry::from([(file.id, Rc::new(file))]), };