Skip to content

Commit

Permalink
revamp-events note.
Browse files Browse the repository at this point in the history
It looks like this refactorign does not provide clear benefits. The
change makes some parts simpler while others get more complicated.

The primary hindrance is support for intersecting events (which are
required for smooth editing) which prevents simpler implementations.
Another problem is support for edit animations which may require having
separate note view model in stave.

See events-revamp branch for a working version of such refactoring.
  • Loading branch information
PetrGlad committed Dec 16, 2024
1 parent d179d92 commit 53a4ef2
Showing 1 changed file with 10 additions and 35 deletions.
45 changes: 10 additions & 35 deletions src/track.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,20 @@ pub fn is_cc_switch_on(x: Level) -> bool {
x >= 64
}

#[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)]
#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone, Serialize, Deserialize)]
pub struct Note {
pub pitch: Pitch,
pub velocity: Level,
pub duration: Time,
}

#[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)]
#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone, Serialize, Deserialize)]
pub struct ControllerSetValue {
pub controller_id: ControllerId,
pub value: Level,
}

#[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)]
#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone, Serialize, Deserialize)]
pub enum TrackEventType {
Note(Note),
Controller(ControllerSetValue),
Expand All @@ -68,46 +68,21 @@ impl TrackEvent {

impl PartialOrd for TrackEvent {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
// TODO Maybe consider complete comparison (including actual events)
// to avoid ambiguities in sorting.
Some(self.at.cmp(&other.at))
Some(self.cmp(&other))
}
}

impl Ord for TrackEvent {
fn cmp(&self, other: &Self) -> Ordering {
self.partial_cmp(&other).unwrap()
/*
Ordering events by track time, while ensuring sorting always produces the same
sequence every time. The time ordering is important for playback and editing while unique
sort order ensures we do not have any surprises when changing or serializing the track.
*/
(self.at, &self.event, self.id).cmp(&(other.at, &other.event, other.id))
}
}

/*
TODO (refactoring, a big one, ???) Use MIDI-like events directly in the track.
Currently notes have duration and correspond to 2 MIDI events ("on" and "off").
New implementation would:
0. Keep events separated by lanes (one for each pitch, CC control, or UI marker type).
Events in each lane should be ordered by time ascending.
1. Instead of notes hold MIDI events of structs that directly correspond to a MIDI events.
2. Treat track-time or note related markers (like bookmarks) as another event type.
3. (optimally) Keep unsupported types of MIDI on the track also.
4. (???) How selection and diff patches will refer to events then? Should we still have
project-unique event ids? Or event id should be "lane_id:event_id"?
5. Map events to some internal convenient structs or provide some view functions directly
into SMD events? Dealing with 7 bit integers is cumbersome, I'd rather avoid that.
Expected result:
1. Export/import and playback would be more complex. In particular export procedure and
track source for playback engine will have to scan all (non UI) lanes to see which event
comes next. Import/load procedure will have to take single MIDI stream and group events by lane.
On the other hand mapping to notes and back will be unnecessary.
2. Have ability to keep unsupported events. More fidelity to input MIDI file in the exported data.
Although this looks odd but it is possible to have two "on" events in a row for the same note.
Not sure what "off" velocity affects, but adjusting it will also be possible.
The new implementation may have a dedicated lane for events that are unsupported or ignored.
3. CC events can be used as is without handling them as special case.
4. Zoomed display optimization: only visible events can be selected for painting.
5. Simplify some time operations at expense of note selection which will be trickier.
For example playback resuming may need to look up previous or next note. At the moment
this may require to scan whole track to the beginning or to the end.
*/
#[derive(Debug, Default, Clone)]
pub struct Track {
/* Events must always be kept ordered by start time ascending.
Expand Down

0 comments on commit 53a4ef2

Please sign in to comment.