- don't add a new SysEx canvas item every time we zoom or drag.
- speed up redisplay generally using PatchChange-like method
for finding items (find_canvas_sys_ex() in boost::unordered_map).
snap now fills in a struct (MusicFrame) which contins a snapped frame
along with a music divisor.
this gives useful information wrt magnetic snap which may or may not
have rounded to an exact musical position.
region position may now be set musically (using quarter notes for now).
this patch fixes several problems in the current code:
- dragging a list of music-locked regions now maintains correct
musical offsets within the list.
- splitting regions using magnetic snap works correctly (#7192)
- cut drag should now work correctly with magnetic snap.
- musical length of split midi regions is no longer frame based.
It is slightly questionable whether type specific methods like
velocity() belong on Event at all, these may be better off as free
functions. However the code currently uses them as methods in many
places, and it seems like a step in the right direction, since, for
example, we might some day have events that have a velocity but aren't
stored as MIDI messages (e.g. if Ardour uses an internal musical model
that is more expressive).
In any case, the former inheritance and plethora of sloppy casts is
definitely not the right thing.
- this allows a midi region drag to update the visible notes
correctly while crossing MIDI streamviews with a differing
note range.
as a side effect, fixes a bug where changing
note range on a track did not draw some notes
(apply_note_range redisplays the model).
- add more debugging output detecting regions whose
beat and frame position do not align on a playlist.
this is required as a check as we have never used
frame rounding on constant tempi before 8884a5723dc
- hide ghost note when add dragging.
- new note length snaps as per ghost note start (shifted snap).
- prevent ghost note from appearing before region start.
- Evoral::Beats operator!= would prevent an increment
of start_beats by intervals of less than a tick,
so its possible that other subtle problems
existed due to this kind of thing.
This is not caused by commit_reversible_command, but because
NoteDrag::total_dx calls Evoral::Note<Evoral::Beats>::time()
with invalid MIDI note (deleted).
- for those not in the know, this series provides a way to
remove the temporal distortion introduced when using an
audio frame-based gui for music-locked objects.
In short, the gui uses an audio frame representation to move
objects. It displays the object using frame_at_beat(), quantizing
the time value to audio frames. This is fine until the user selects
that frame but expects it to be interpreted as a beat.
Thus beat_at_frame() would not produce the user-expected beat
(temporal quantization error of up to 0.5 audio samples).
This is one method of mapping audio time to music time accurately.
Each MidiRegionView(MRV) is connected to the Selection::ClearMidiNoteSelection
signal that is used to notify the all MRV instances to clear their note
selection.
The MRV class also has a private static SelectionCleared signal that is used to
signal other MRV instances when their selection has been cleared. When the
Selection::ClearMidiNoteSelection signal is emitted it causes each MRV to also
emit the SelectionCleared signal. So the emission takes quadratic time.
With 1500 MRV instances emission takes about 2.2 seconds on my machine, and
some operations like track selection cause it to be emitted 3 times(another
issue).
The Selection class in the Editor knows which MRV instances have note
selections, as it is notified by MidiRegionView whenever the selection count
becomes zero or becomes non-zero. Clearing the Note selection should then just
be O(N) and direct calls can be used rather than signals.
This change removes both the signals and uses the existing references between
Selection and MRV class to control note selection. There should be no
behavioural changes in Midi note selection with this change.
I don't think this is necessary, if some synth cannot deal with that,
perhaps Ardour could try to send the note off right before the next
note on, even-though they supposedly occur simultaneously.
The shortcut for 'change velocities together' had to be changed to
achieve that. The new shortcut is now primary+tertiary modifier +scroll,
i.e. ctrl+shift+scroll for the default keys.
Currently when a GhostRegion is deleted by its "parent" RegionView it emits the
static GhostRegion::CatchDeletion signal which is connected to the
RegionView::remove_ghost method of every RegionView instance.
With a static GhostRegion::CatchDeletion signal a particular test session
causes 31 Million calls of RegionView::remove_ghost on Session deletion and the
session takes 70 seconds to close with a debug build.
The lifetime of a ghost region is tied to both the TimeAxisView(TAV) and
RegionView(RV) in that when a RegionView is deleted all GhostRegion instances
associated with the RegionView should be deleted or when a TimeAxisView is
deleted all ghost regions that are contained in the view should be deleted.
This means that there needs to be notification between GhostRegion and both
classes. Instead of using a signal for this as we know there are only two
listeners and GhostRegion already holds a reference to the TimeAxisView, also
take a reference to the parent RegionView in the GhostRegion constructor and
use it to notify the RegionView when GhostRegion destructor is called so it can
drop any references it holds.
Using a direct function call in the GhostRegion destructor to notify the
TimeAxisView and RegionView "parents" brings the unload/close time down for the
test session from 70 seconds to 4.5 seconds.
The GhostRegion also references canvas items that are added to the TimeAxisView
canvas group or at least a canvas group that it manages. So when the
TimeAxisView is destroyed and the canvas group that is the parent of those
items is destroyed, the GhostRegion's canvas items will also be
deleted/destroyed by the parent canvas item/group. This means the GhostRegions
must be destroyed when the TimeAxisView they are contained in is destroyed or
there will be dangling references to canvas items that have already been
deleted and trying to delete them again will be bad.
NoteBaseDeleted signal is static so each MidiRegionView(MRV) gets notified
about the deletion of each NodeBase instance even if it is contained in another
MRV
The NoteBase and MRV classes are currently coupled anyway, so this change uses
the reference to the MRV parent to directly call the parent when the NoteBase
is deleted. This is all in the GUI thread so I'm not sure why a PBD::Signal was
being used?
If the MRV class is the only reference holder to the NoteBase class
then I'm not sure if a callback is needed, perhaps the MRV should just remove
the note from the selection before deleting it but I'm not that familiar with
the code.
Signal emission/calls static NoteBaseDeleted signal vs direct with 10540
NoteBase instances.
static:
After Load Session: 6360638
After Unload Session: 12221026(5860388)
direct:
After load Session: 10540
After unload Session: 21080
Session Load/Unload time in master, debug/release with ~10000 Notes(seconds)
Load Debug: 32, 26
Unload Debug: 83
Load Release 32, 20, 42
Unload Release 26, 25
Session Load/Unload time with direct call debug/release(seconds)
Load Debug: 21.7, 18.1
Unload Debug: 69.4, 71
Load Release: 22.6, 13.4, 17.7
Unload Release: 24, 23.5
This is not a large Session, 1500 regions, 10000 notes so there is probably
some other funky stuff going on that needs fixing.
This moves MIDI channel filtering into a reusable class and moves filtering to
the source, rather than modifying the buffer afterwards. This is necessary so
that the playlist trackers reflect the emitted notes (and thus are able to stop
them in situations like mute).
As a perk, this is also faster because events are just dropped on read, rather
than pushed into a buffer then later removed (which is very slow).
Really hammering on mute or solo still seems to produce stuck notes
occasionally (perhaps related to multiple-on warnings). I am not yet sure why,
but occasional beats always.
During DnD, the region uses the 'old/current'
midi_stream_view()'s range and its position/height calculation.
Ideally DnD would decouple the midi_stream_view() for the
region(s) being dragged and set it to the target's range
(or in case of the drop-zone, FullRange).
but I don't see how this can be done without major rework.
For now, just prevent visual bleeding of events in case
the target-track is smaller.
This is debatable, the "sustained until mouse release" behaviour is handy
sometimes, but this way seems like what most people probably want.
Also, this "fire it and forget it and let it delete itself a bit later" thing
with MidiPlayer makes me nervous. I guess it's unlikely someone manages to
select a note then delete a track within 100ms, but, well...
This avoids stuck notes if active notes are edited, but without stopping all
active notes in the region on any edit as before.
This implementation injects note ons in places that aren't actually note
starts. Depending on how percussive the instrument is, this may not be
desired. In the future, an option for this would be an improvement, but there
are other places where "start notes in the middle" is a reasonable option. I
think that should be handled universally if we're to do it at all, so not
considering it a part of this fix for now.
Fix several other cases where a single mouse click could cause several
(not nested) selection ops.
Fix missing selection memento for midi notes and midi commands.
Rename some variables.
Fix random style issues.
The reasonable value 1 tick doesn't seem to work here, presumably it gets lost
in rounding conversion somewhere. Instead use a really small power of two
reciprocal. Once we use actual beats and ticks we can fix this to be a minimum
of one tick (the actual minimum length for a note).
... almost. There are some artifacts when you zoom out while recording that I
can't figure out, but whatever.
Also fix performance issues caused by last attempt at rec display while zoom.
