Shift bookmarks on tape insert/delets (no undo support!)

A proper implementation would complicate the editing logic. Leaving it
as is for now.
Will have to make bookmarks a type of stave events, then the proper
behaviour will be followed automatically.

Cargo.toml

@@ -31,6 +31,7 @@ wmidi = "4.0.8"
 # rodio = { git = "https://github.com/PetrGlad/rodio.git", branch = "configurable-buffer-size" }
 
 # https://docs.rs/alsa/latest/alsa/
+# apt install libasound2-dev
 # alsa = "0.8.1"
 
 # see also https://github.com/RustAudio/cpal

README.md

@@ -26,11 +26,8 @@ revamps.
 In case you get "No package 'freetype2' found" on Linux
 `apt install libxft-dev`.
 
-ALSA wrapper dependency
-`apt install libasound2-dev`.
-
 As an VST synth plugin you can use `amsynth`, for example.
-I personally use Pianoteq, but that is a commercial product.
+I use Pianoteq, but that is a commercial product.
 
 ## TODO
 

src/stave.rs

@@ -1,16 +1,15 @@
-use std::cell::RefCell;
-use std::collections::btree_set::Iter;
-use std::collections::{BTreeMap, BTreeSet, HashSet};
-use std::ops::Range;
-use std::path::PathBuf;
-
 use eframe::egui::{
     self, Color32, Context, Frame, Margin, Modifiers, Painter, PointerButton, Pos2, Rangef, Rect,
     Rounding, Sense, Stroke, Ui,
 };
 use egui::Rgba;
 use ordered_float::OrderedFloat;
 use serde::{Deserialize, Serialize};
+use std::cell::RefCell;
+use std::collections::{BTreeMap, HashSet};
+use std::ops::Range;
+use std::path::PathBuf;
+use std::slice::Iter;
 
 use crate::changeset::{Changeset, EventActionsList};
 use crate::common::Time;
@@ -23,7 +22,7 @@ use crate::track_edit::{
     stretch_selected_notes, tape_delete, tape_insert, transpose_selected_notes, AppliedCommand,
     EditCommandId,
 };
-use crate::track_history::TrackHistory;
+use crate::track_history::{CommandApplication, TrackHistory};
 use crate::{util, Pix};
 
 // Tone 60 is C3, tones start at C-2 (21).
@@ -86,27 +85,43 @@ pub struct Bookmark {
 
 #[derive(Debug, Deserialize, Serialize)]
 pub struct Bookmarks {
-    // Maybe bookmarks should also be events in the track.
-    pub list: BTreeSet<Bookmark>,
+    // (refactoring) Maybe bookmarks should also be events in the track. The logic is about the same.
+    pub list: Vec<Bookmark>,
     file_path: PathBuf,
 }
 
 impl Bookmarks {
     pub fn new(file_path: &PathBuf) -> Bookmarks {
         Bookmarks {
-            list: BTreeSet::default(),
+            list: vec![],
             file_path: file_path.to_owned(),
         }
     }
 
     pub fn set(&mut self, at: Time) {
-        self.list.insert(Bookmark { at });
-        self.store_to(&self.file_path);
+        let bm = Bookmark { at };
+        let idx = self.list.binary_search(&bm);
+        if let Err(idx) = idx {
+            self.list.insert(idx, bm);
+            self.store_to(&self.file_path);
+        }
     }
 
     pub fn remove(&mut self, at: &Time) {
-        self.list.remove(&Bookmark { at: *at });
-        self.store_to(&self.file_path);
+        let bm = Bookmark { at: at.clone() };
+        let idx = self.list.binary_search(&bm);
+        if let Ok(idx) = idx {
+            self.list.remove(idx);
+            self.store_to(&self.file_path);
+        }
+    }
+
+    pub fn shift(&mut self, after: Time, delta: Time) {
+        for m in self.list.iter_mut() {
+            if m.at >= after {
+                m.at += delta;
+            }
+        }
     }
 
     pub fn previous(&self, here: &Time) -> Option<Time> {
@@ -508,6 +523,9 @@ impl Stave {
 
     const KEYBOARD_TIME_STEP: Time = 10_000;
 
+    /**
+     * Applies the command and returns time to move the stave cursor to.
+     */
     fn handle_commands(&mut self, response: &egui::Response) -> Option<Time> {
         // TODO Have to see if duplication here can be reduced. Likely the dispatch needs some
         //   hash map that for each input state defines a unique command.
@@ -528,9 +546,23 @@ impl Stave {
             ))
         }) {
             if let Some(time_selection) = &self.time_selection.clone() {
-                self.do_edit_command(&response.ctx, response.id, |_stave, track| {
-                    tape_delete(track, &(time_selection.start, time_selection.end))
-                });
+                if self
+                    .do_edit_command(&response.ctx, response.id, |stave, track| {
+                        tape_delete(track, &(time_selection.start, time_selection.end))
+                    })
+                    .is_some()
+                {
+                    // TODO (refactoring) If bookmarks were represented as events they could
+                    //      be handled uniformly along with notes and CC changes. In that case the
+                    //      following will not need to be handled as a special case.
+                    //      See also tape_insert.
+                    // TODO (implementation) Ideally should also delete bookmarks in the selection.
+                    //      But there is no way to undo this.
+                    self.bookmarks.shift(
+                        time_selection.end,
+                        -(time_selection.end - time_selection.start),
+                    );
+                }
             }
             if !self.note_selection.selected.is_empty() {
                 self.do_edit_command(&response.ctx, response.id, |stave, track| {
@@ -546,9 +578,18 @@ impl Stave {
             ))
         }) {
             if let Some(time_selection) = &self.time_selection.clone() {
-                self.do_edit_command(&response.ctx, response.id, |_stave, _track| {
-                    tape_insert(&(time_selection.start, time_selection.end))
-                });
+                if self
+                    .do_edit_command(&response.ctx, response.id, |_stave, _track| {
+                        tape_insert(&(time_selection.start, time_selection.end))
+                    })
+                    .is_some()
+                {
+                    // TODO (refactoring) See comment in tape_delete case.
+                    self.bookmarks.shift(
+                        time_selection.start,
+                        time_selection.end - time_selection.start,
+                    );
+                }
             }
         }
 
@@ -743,9 +784,9 @@ impl Stave {
     fn animate_edit(
         context: &Context,
         transition_id: egui::Id,
-        edit_state: Option<(EditCommandId, EventActionsList)>,
+        diff: Option<(EditCommandId, EventActionsList)>,
     ) -> Option<EditTransition> {
-        if let Some((command_id, changes)) = edit_state {
+        if let Some((command_id, changes)) = diff {
             let mut changeset = Changeset::empty();
             changeset.add_all(&changes);
             Some(EditTransition::start(
@@ -764,14 +805,17 @@ impl Stave {
         context: &Context,
         transition_id: egui::Id,
         action: Action,
-    ) {
+    ) -> CommandApplication {
+        let diff = self
+            .history
+            .borrow_mut()
+            .update_track(|track| action(&self, track));
         self.transition = Self::animate_edit(
             context,
             transition_id,
-            self.history
-                .borrow_mut()
-                .update_track(|track| action(&self, track)),
-        )
+            diff.clone().map(|diff| (diff.0 .0, diff.1)),
+        );
+        diff
     }
 
     fn max_time(&self) -> Time {

src/track_edit.rs

@@ -48,6 +48,7 @@ pub enum CommandDiff {
     TailShift { at: Time, delta: Time },
 }
 
+// TODO (refactoring) make this a struct toi have named fields.
 pub type AppliedCommand = (EditCommandId, Vec<CommandDiff>);
 
 pub fn apply_diffs(track: &mut Track, diffs: &Vec<CommandDiff>, changes: &mut EventActionsList) {
@@ -106,9 +107,9 @@ fn do_shift_tail(track: &Track, at: &Time, delta: &Time, changes: &mut EventActi
 }
 
 pub fn tape_insert(range: &Range<Time>) -> Option<AppliedCommand> {
-    let mut diffs = vec![];
     let delta = range.1 - range.0;
     assert!(delta >= 0);
+    let mut diffs = vec![];
     diffs.push(CommandDiff::TailShift { at: range.0, delta });
     Some((EditCommandId::TapeInsert, diffs))
 }

src/track_history.rs

@@ -30,6 +30,8 @@ struct Version {
     diff_path: Option<PathBuf>,
 }
 
+pub type CommandApplication = Option<(AppliedCommand, EventActionsList)>;
+
 impl Version {
     pub fn is_empty(&self) -> bool {
         self.snapshot_path.is_none() && self.diff_path.is_none()
@@ -48,20 +50,20 @@ impl TrackHistory {
     pub fn update_track<Action: FnOnce(&Track) -> Option<AppliedCommand>>(
         &mut self,
         action: Action,
-    ) -> Option<(EditCommandId, EventActionsList)> {
+    ) -> CommandApplication {
         let applied_command = {
             let track = self.track.write().expect("read track");
             action(&track)
         };
-        if let Some(applied_command) = &applied_command {
+        if let Some(applied_command) = applied_command {
             let mut changes = vec![];
             {
                 let mut track = self.track.write().expect("write to track");
                 apply_diffs(&mut track, &applied_command.1, &mut changes);
                 track.commit();
             }
-            self.update(applied_command);
-            Some((applied_command.0, changes))
+            self.update(&applied_command);
+            Some((applied_command, changes))
         } else {
             None
         }