From 975d4109305c1e2b8dfece2d2a8798b90469f2fd Mon Sep 17 00:00:00 2001 From: David Robillard Date: Thu, 22 May 2008 01:02:04 +0000 Subject: [PATCH] Fix MIDI selection/tool issues (issue #0002415 and other bugs). Fix selection preservation across MIDI model editing commands (for both note moving and resizing). Fix selection breakage introduced by old selection preservation stuff (fix zombie selection). git-svn-id: svn://localhost/ardour2/branches/3.0@3381 d708f5d6-7413-0410-9779-e7cbd77b26cf --- gtk2_ardour/canvas-note-event.cc | 4 +- gtk2_ardour/midi_region_view.cc | 145 ++++++++++++++++++------------- gtk2_ardour/midi_region_view.h | 46 ++-------- libs/ardour/midi_model.cc | 4 +- 4 files changed, 98 insertions(+), 101 deletions(-) diff --git a/gtk2_ardour/canvas-note-event.cc b/gtk2_ardour/canvas-note-event.cc index 0e758b5489..c370530cfc 100644 --- a/gtk2_ardour/canvas-note-event.cc +++ b/gtk2_ardour/canvas-note-event.cc @@ -255,7 +255,9 @@ CanvasNoteEvent::on_event(GdkEvent* ev) switch (_state) { case Pressed: // Drag begin - if (_region.mouse_state() != MidiRegionView::SelectTouchDragging) { + if (_region.midi_view()->editor.current_midi_edit_mode() == Editing::MidiEditSelect + && _region.mouse_state() != MidiRegionView::SelectTouchDragging + && _region.mouse_state() != MidiRegionView::EraseTouchDragging) { _item->grab(GDK_POINTER_MOTION_MASK | GDK_BUTTON_RELEASE_MASK, Gdk::Cursor(Gdk::FLEUR), ev->motion.time); _state = Dragging; diff --git a/gtk2_ardour/midi_region_view.cc b/gtk2_ardour/midi_region_view.cc index a1823a4cfa..ab17acbaf7 100644 --- a/gtk2_ardour/midi_region_view.cc +++ b/gtk2_ardour/midi_region_view.cc @@ -196,13 +196,16 @@ MidiRegionView::canvas_event(GdkEvent* ev) case GDK_BUTTON_PRESS: if (_mouse_state != SelectTouchDragging && _mouse_state != EraseTouchDragging && - ev->button.button == 1 ) { + ev->button.button == 1) { _pressed_button = ev->button.button; _mouse_state = Pressed; return true; } _pressed_button = ev->button.button; - return false; + return true; + + case GDK_2BUTTON_PRESS: + return true; case GDK_ENTER_NOTIFY: /* FIXME: do this on switch to note tool, too, if the pointer is already in */ @@ -335,16 +338,15 @@ MidiRegionView::canvas_event(GdkEvent* ev) break; case MidiEditPencil: create_note_at(event_x, event_y, _default_note_length); - default: - break; + default: break; } _mouse_state = None; - return true; + break; case SelectRectDragging: // Select drag done _mouse_state = None; delete drag_rect; drag_rect = NULL; - return true; + break; case AddDragging: // Add drag done _mouse_state = None; if (drag_rect->property_x2() > drag_rect->property_x1() + 2) { @@ -357,13 +359,10 @@ MidiRegionView::canvas_event(GdkEvent* ev) delete drag_rect; drag_rect = NULL; - return true; - default: - break; + default: break; } - default: - break; + default: break; } return false; @@ -409,8 +408,6 @@ MidiRegionView::create_note_at(double x, double y, double duration) MidiModel::DeltaCommand* cmd = _model->new_delta_command("add note"); cmd->add(new_note); _model->apply_command(cmd); - - //add_note(new_note); } @@ -442,6 +439,60 @@ MidiRegionView::display_model(boost::shared_ptr model) if (_enable_display) redisplay_model(); } + + +void +MidiRegionView::start_delta_command(string name) +{ + if (!_delta_command) + _delta_command = _model->new_delta_command(name); +} + +void +MidiRegionView::command_add_note(const boost::shared_ptr note, bool selected) +{ + if (_delta_command) + _delta_command->add(note); + + if (selected) + _marked_for_selection.insert(note); +} + +void +MidiRegionView::command_remove_note(ArdourCanvas::CanvasNoteEvent* ev) +{ + if (_delta_command && ev->note()) { + _delta_command->remove(ev->note()); + } +} + +void +MidiRegionView::apply_command() +{ + if (!_delta_command) { + return; + } + + // Mark all selected notes for selection when model reloads + for (Selection::iterator i = _selection.begin(); i != _selection.end(); ++i) { + _marked_for_selection.insert((*i)->note()); + } + + _model->apply_command(_delta_command); + _delta_command = NULL; + midi_view()->midi_track()->diskstream()->playlist_modified(); + + _marked_for_selection.clear(); +} + + +void +MidiRegionView::abort_command() +{ + delete _delta_command; + _delta_command = NULL; + clear_selection(); +} void @@ -730,26 +781,21 @@ MidiRegionView::add_note(const boost::shared_ptr note) assert(midi_view()->note_mode() == Sustained || midi_view()->note_mode() == Percussive); // dont display notes beyond the region bounds - if ( - note->time() - _region->start() >= _region->length() || - note->time() < _region->start() || - note->note() < midi_stream_view()->lowest_note() || - note->note() > midi_stream_view()->highest_note() - ) { + if ( note->time() - _region->start() >= _region->length() || + note->time() < _region->start() || + note->note() < midi_stream_view()->lowest_note() || + note->note() > midi_stream_view()->highest_note() ) { return; } ArdourCanvas::Group* const group = (ArdourCanvas::Group*)get_canvas_group(); - CanvasNoteEvent *event = 0; + CanvasNoteEvent* event = 0; const double x = trackview.editor.frame_to_pixel((nframes_t)note->time() - _region->start()); if (midi_view()->note_mode() == Sustained) { - //cerr << "MRV::add_note sustained " << note->note() << " @ " << note->time() - // << " .. " << note->end_time() << endl; - const double y1 = midi_stream_view()->note_to_y(note->note()); const double note_endpixel = trackview.editor.frame_to_pixel((nframes_t)note->end_time() - _region->start()); @@ -817,8 +863,8 @@ MidiRegionView::add_note(const boost::shared_ptr note) } if (event) { - if (_marked_for_selection.find(event->note()) != _marked_for_selection.end()) { - note_selected(event, true); + if (_marked_for_selection.find(note) != _marked_for_selection.end()) { + note_selected(event, true); } event->on_channel_selection_change(_last_channel_selection); } @@ -965,15 +1011,10 @@ MidiRegionView::note_dropped(CanvasNoteEvent* ev, double dt, uint8_t dnote) uint8_t highest_note_difference = 0; // find highest and lowest notes first - for (Selection::iterator i = _selection.begin(); i != _selection.end() ; ) { - Selection::iterator next = i; - ++next; - + for (Selection::iterator i = _selection.begin(); i != _selection.end(); ++i) { uint8_t pitch = (*i)->note()->note(); lowest_note_in_selection = std::min(lowest_note_in_selection, pitch); highest_note_in_selection = std::max(highest_note_in_selection, pitch); - - i = next; } /* @@ -1031,21 +1072,20 @@ MidiRegionView::note_dropped(CanvasNoteEvent* ev, double dt, uint8_t dnote) copy->set_note(new_pitch); command_remove_note(*i); - command_add_note(copy); + command_add_note(copy, true); - _marked_for_selection.insert(copy); i = next; } + apply_command(); // care about notes being moved beyond the upper/lower bounds on the canvas if (lowest_note_in_selection < midi_stream_view()->lowest_note() || - highest_note_in_selection > midi_stream_view()->highest_note() - ) { + highest_note_in_selection > midi_stream_view()->highest_note()) { midi_stream_view()->set_note_range(MidiStreamView::ContentsRange); + } } } -} nframes_t MidiRegionView::snap_to_frame(double x) @@ -1109,7 +1149,7 @@ MidiRegionView::begin_resizing(CanvasNote::NoteEnd note_end) note->y2()); // calculate the colors: get the color settings - uint fill_color = + uint32_t fill_color = UINT_RGBA_CHANGE_A( ARDOUR_UI::config()->canvasvar_MidiNoteSelectedOutline.get(), 128); @@ -1174,10 +1214,10 @@ MidiRegionView::commit_resizing(CanvasNote::NoteEnd note_end, double event_x, bo start_delta_command(_("resize notes")); for (std::vector::iterator i = _resize_data.begin(); i != _resize_data.end(); ++i) { - CanvasNote *canvas_note = (*i)->canvas_note; - SimpleRect *resize_rect = (*i)->resize_rect; - double current_x = (*i)->current_x; - const double position = get_position_pixels(); + CanvasNote* canvas_note = (*i)->canvas_note; + SimpleRect* resize_rect = (*i)->resize_rect; + double current_x = (*i)->current_x; + const double position = get_position_pixels(); if (!relative) { // event_x is in track relative, transform it to region relative @@ -1196,16 +1236,13 @@ MidiRegionView::commit_resizing(CanvasNote::NoteEnd note_end, double event_x, bo if (note_end == CanvasNote::NOTE_ON && current_frame < copy->end_time()) { command_remove_note(canvas_note); copy->on_event().time() = current_frame; - command_add_note(copy); - _marked_for_selection.insert(copy); + command_add_note(copy, _selection.find(canvas_note) != _selection.end()); } // resize end of note if (note_end == CanvasNote::NOTE_OFF && current_frame > copy->time()) { - command_remove_note(canvas_note); command_remove_note(canvas_note); copy->off_event().time() = current_frame; - command_add_note(copy); - _marked_for_selection.insert(copy); + command_add_note(copy, _selection.find(canvas_note) != _selection.end()); } delete resize_rect; @@ -1238,17 +1275,11 @@ MidiRegionView::change_velocity(uint8_t velocity, bool relative) } command_remove_note(event); - command_add_note(copy); + command_add_note(copy, true); - _marked_for_selection.insert(copy); i = next; } - // dont keep notes selected if tweaking a single note - if (_marked_for_selection.size() == 1) { - _marked_for_selection.clear(); - } - apply_command(); } @@ -1266,17 +1297,11 @@ MidiRegionView::change_channel(uint8_t channel) copy->set_channel(channel); command_remove_note(event); - command_add_note(copy); + command_add_note(copy, true); - _marked_for_selection.insert(copy); i = next; } - // dont keep notes selected if tweaking a single note - if (_marked_for_selection.size() == 1) { - _marked_for_selection.clear(); - } - apply_command(); } diff --git a/gtk2_ardour/midi_region_view.h b/gtk2_ardour/midi_region_view.h index 4283483f04..c2475f00fa 100644 --- a/gtk2_ardour/midi_region_view.h +++ b/gtk2_ardour/midi_region_view.h @@ -92,39 +92,12 @@ class MidiRegionView : public RegionView void display_model(boost::shared_ptr model); - /* This stuff is a bit boilerplatey ATM. Work in progress. */ + void start_delta_command(string name = "midi edit"); + void command_add_note(const boost::shared_ptr note, bool selected); + void command_remove_note(ArdourCanvas::CanvasNoteEvent* ev); - inline void start_delta_command(string name = "midi edit") { - if (!_delta_command) - _delta_command = _model->new_delta_command(name); - } - - void command_add_note(const boost::shared_ptr note) { - if (_delta_command) - _delta_command->add(note); - } - - void command_remove_note(ArdourCanvas::CanvasNoteEvent* ev) { - if (_delta_command && ev->note()) { - _selection.erase(ev); - _delta_command->remove(ev->note()); - ev->selected(true); - } - } - - void abort_command() { - delete _delta_command; - _delta_command = NULL; - clear_selection(); - } - - void apply_command() { - if (_delta_command) { - _model->apply_command(_delta_command); - _delta_command = NULL; - } - midi_view()->midi_track()->diskstream()->playlist_modified(); - } + void apply_command(); + void abort_command(); void note_entered(ArdourCanvas::CanvasNoteEvent* ev); void unique_select(ArdourCanvas::CanvasNoteEvent* ev); @@ -261,15 +234,12 @@ class MidiRegionView : public RegionView MouseState _mouse_state; int _pressed_button; - /// currently selected CanvasNoteEvents typedef std::set Selection; + /// Currently selected CanvasNoteEvents Selection _selection; - /** - * this enables vanilla notes to be marked for selection - * they are added to _selection when redisplay_model is called - * this is necessary for selecting notes during/after model manipulations - */ + /** New notes (created in the current command) which should be selected + * when they appear after the command is applied. */ std::set< boost::shared_ptr > _marked_for_selection; std::vector _resize_data; diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc index 02b076a5e5..637b0ee38e 100644 --- a/libs/ardour/midi_model.cc +++ b/libs/ardour/midi_model.cc @@ -655,11 +655,11 @@ void MidiModel::remove_note_unlocked(const boost::shared_ptr note) // TODO: There is still the issue, that after restarting ardour // persisted undo does not work, because of rounding errors in the // event times after saving/restoring to/from MIDI files - cerr << "======================================= " << endl; + /*cerr << "======================================= " << endl; cerr << int(_n.note()) << "@" << int(_n.time()) << "[" << int(_n.channel()) << "] --" << int(_n.duration()) << "-- #" << int(_n.velocity()) << endl; cerr << int(_note.note()) << "@" << int(_note.time()) << "[" << int(_note.channel()) << "] --" << int(_note.duration()) << "-- #" << int(_note.velocity()) << endl; cerr << "Equal: " << bool(_n == _note) << endl; - cerr << endl << endl; + cerr << endl << endl;*/ if (_n == _note) { _notes.erase(n); // we have to break here, because erase invalidates all iterators, ie. n itself