various fixes for moving markers, fixes a crash reported by tim blechmann and also likely #5232 and #5241

git-svn-id: svn://localhost/ardour2/branches/3.0@13754 d708f5d6-7413-0410-9779-e7cbd77b26cf
This commit is contained in:
Paul Davis 2013-01-02 23:54:06 +00:00
parent bcab772257
commit 67265c6d90
6 changed files with 135 additions and 36 deletions

View file

@ -2543,11 +2543,18 @@ MarkerDrag::MarkerDrag (Editor* e, ArdourCanvas::Item* i)
MarkerDrag::~MarkerDrag () MarkerDrag::~MarkerDrag ()
{ {
for (list<Location*>::iterator i = _copied_locations.begin(); i != _copied_locations.end(); ++i) { for (CopiedLocationInfo::iterator i = _copied_locations.begin(); i != _copied_locations.end(); ++i) {
delete *i; delete i->location;
} }
} }
MarkerDrag::CopiedLocationMarkerInfo::CopiedLocationMarkerInfo (Location* l, Marker* m)
{
location = new Location (*l);
markers.push_back (m);
move_both = false;
}
void void
MarkerDrag::start_grab (GdkEvent* event, Gdk::Cursor* cursor) MarkerDrag::start_grab (GdkEvent* event, Gdk::Cursor* cursor)
{ {
@ -2573,7 +2580,7 @@ MarkerDrag::start_grab (GdkEvent* event, Gdk::Cursor* cursor)
switch (op) { switch (op) {
case Selection::Toggle: case Selection::Toggle:
_editor->selection->toggle (_marker); /* we toggle on the button release */
break; break;
case Selection::Set: case Selection::Set:
if (!_editor->selection->selected (_marker)) { if (!_editor->selection->selected (_marker)) {
@ -2615,11 +2622,37 @@ MarkerDrag::start_grab (GdkEvent* event, Gdk::Cursor* cursor)
break; break;
} }
/* Set up copies for us to manipulate during the drag */ /* Set up copies for us to manipulate during the drag
*/
for (MarkerSelection::iterator i = _editor->selection->markers.begin(); i != _editor->selection->markers.end(); ++i) { for (MarkerSelection::iterator i = _editor->selection->markers.begin(); i != _editor->selection->markers.end(); ++i) {
Location* l = _editor->find_location_from_marker (*i, is_start); Location* l = _editor->find_location_from_marker (*i, is_start);
_copied_locations.push_back (new Location (*l));
if (!l) {
continue;
}
if (l->is_mark()) {
_copied_locations.push_back (CopiedLocationMarkerInfo (l, *i));
} else {
/* range: check that the other end of the range isn't
already there.
*/
CopiedLocationInfo::iterator x;
for (x = _copied_locations.begin(); x != _copied_locations.end(); ++x) {
if (*(*x).location == *l) {
break;
}
}
if (x == _copied_locations.end()) {
_copied_locations.push_back (CopiedLocationMarkerInfo (l, *i));
} else {
(*x).markers.push_back (*i);
(*x).move_both = true;
}
}
} }
} }
@ -2637,33 +2670,31 @@ MarkerDrag::motion (GdkEvent* event, bool)
framecnt_t f_delta = 0; framecnt_t f_delta = 0;
bool is_start; bool is_start;
bool move_both = false; bool move_both = false;
Marker* marker;
Location *real_location; Location *real_location;
Location *copy_location = 0; Location *copy_location = 0;
framepos_t const newframe = adjusted_current_frame (event); framepos_t const newframe = adjusted_current_frame (event);
framepos_t next = newframe; framepos_t next = newframe;
if (Keyboard::modifier_state_equals (event->button.state, Keyboard::PrimaryModifier)) { if (Keyboard::modifier_state_equals (event->button.state, Keyboard::PrimaryModifier)) {
move_both = true; move_both = true;
} }
MarkerSelection::iterator i; CopiedLocationInfo::iterator x;
list<Location*>::iterator x;
/* find the marker we're dragging, and compute the delta */ /* find the marker we're dragging, and compute the delta */
for (i = _editor->selection->markers.begin(), x = _copied_locations.begin(); for (x = _copied_locations.begin(); x != _copied_locations.end(); ++x) {
x != _copied_locations.end() && i != _editor->selection->markers.end();
++i, ++x) {
copy_location = *x; copy_location = (*x).location;
marker = *i;
if (marker == _marker) { if (find (x->markers.begin(), x->markers.end(), _marker) != x->markers.end()) {
if ((real_location = _editor->find_location_from_marker (marker, is_start)) == 0) { /* this marker is represented by this
* CopiedLocationMarkerInfo
*/
if ((real_location = _editor->find_location_from_marker (_marker, is_start)) == 0) {
/* que pasa ?? */ /* que pasa ?? */
return; return;
} }
@ -2673,7 +2704,7 @@ MarkerDrag::motion (GdkEvent* event, bool)
} else { } else {
switch (marker->type()) { switch (_marker->type()) {
case Marker::SessionStart: case Marker::SessionStart:
case Marker::RangeStart: case Marker::RangeStart:
case Marker::LoopStart: case Marker::LoopStart:
@ -2692,27 +2723,25 @@ MarkerDrag::motion (GdkEvent* event, bool)
return; return;
} }
} }
break; break;
} }
} }
if (i == _editor->selection->markers.end()) { if (x == _copied_locations.end()) {
/* hmm, impossible - we didn't find the dragged marker */ /* hmm, impossible - we didn't find the dragged marker */
return; return;
} }
/* now move them all */ /* now move them all */
for (i = _editor->selection->markers.begin(), x = _copied_locations.begin(); for (x = _copied_locations.begin(); x != _copied_locations.end(); ++x) {
x != _copied_locations.end() && i != _editor->selection->markers.end();
++i, ++x) {
copy_location = *x; copy_location = x->location;
marker = *i;
/* call this to find out if its the start or end */ /* call this to find out if its the start or end */
if ((real_location = _editor->find_location_from_marker (marker, is_start)) == 0) { if ((real_location = _editor->find_location_from_marker (x->markers.front(), is_start)) == 0) {
continue; continue;
} }
@ -2727,13 +2756,22 @@ MarkerDrag::motion (GdkEvent* event, bool)
copy_location->set_start (copy_location->start() + f_delta); copy_location->set_start (copy_location->start() + f_delta);
} else { } else {
framepos_t new_start = copy_location->start() + f_delta; framepos_t new_start = copy_location->start() + f_delta;
framepos_t new_end = copy_location->end() + f_delta; framepos_t new_end = copy_location->end() + f_delta;
/* if we are moving multiple markers, we can have
* forced earlier ones back before zero ... don't
* do this
*/
if (new_start < 0 || new_end < 0) {
continue;
}
if (is_start) { // start-of-range marker if (is_start) { // start-of-range marker
if (move_both) { if (move_both || (*x).move_both) {
copy_location->set_start (new_start); copy_location->set_start (new_start);
copy_location->set_end (new_end); copy_location->set_end (new_end);
} else if (new_start < copy_location->end()) { } else if (new_start < copy_location->end()) {
@ -2746,7 +2784,7 @@ MarkerDrag::motion (GdkEvent* event, bool)
} else { // end marker } else { // end marker
if (move_both) { if (move_both || (*x).move_both) {
copy_location->set_end (new_end); copy_location->set_end (new_end);
copy_location->set_start (new_start); copy_location->set_start (new_start);
} else if (new_end > copy_location->start()) { } else if (new_end > copy_location->start()) {
@ -2760,12 +2798,20 @@ MarkerDrag::motion (GdkEvent* event, bool)
} }
update_item (copy_location); update_item (copy_location);
/* now lookup the actual GUI items used to display this
* location and move them to wherever the copy of the location
* is now. This means that the logic in ARDOUR::Location is
* still enforced, even though we are not (yet) modifying
* the real Location itself.
*/
Editor::LocationMarkers* lm = _editor->find_location_markers (real_location); Editor::LocationMarkers* lm = _editor->find_location_markers (real_location);
if (lm) { if (lm) {
lm->set_position (copy_location->start(), copy_location->end()); lm->set_position (copy_location->start(), copy_location->end());
} }
} }
assert (!_copied_locations.empty()); assert (!_copied_locations.empty());
@ -2796,6 +2842,10 @@ MarkerDrag::finished (GdkEvent* event, bool movement_occurred)
break; break;
case Selection::Toggle: case Selection::Toggle:
/* we toggle on the button release, click only */
_editor->selection->toggle (_marker);
break;
case Selection::Extend: case Selection::Extend:
case Selection::Add: case Selection::Add:
break; break;
@ -2810,7 +2860,7 @@ MarkerDrag::finished (GdkEvent* event, bool movement_occurred)
XMLNode &before = _editor->session()->locations()->get_state(); XMLNode &before = _editor->session()->locations()->get_state();
MarkerSelection::iterator i; MarkerSelection::iterator i;
list<Location*>::iterator x; CopiedLocationInfo::iterator x;
bool is_start; bool is_start;
for (i = _editor->selection->markers.begin(), x = _copied_locations.begin(); for (i = _editor->selection->markers.begin(), x = _copied_locations.begin();
@ -2826,9 +2876,9 @@ MarkerDrag::finished (GdkEvent* event, bool movement_occurred)
} }
if (location->is_mark()) { if (location->is_mark()) {
location->set_start ((*x)->start()); location->set_start (((*x).location)->start());
} else { } else {
location->set ((*x)->start(), (*x)->end()); location->set (((*x).location)->start(), ((*x).location)->end());
} }
} }
} }

View file

@ -682,7 +682,16 @@ private:
void update_item (ARDOUR::Location *); void update_item (ARDOUR::Location *);
Marker* _marker; ///< marker being dragged Marker* _marker; ///< marker being dragged
std::list<ARDOUR::Location*> _copied_locations;
struct CopiedLocationMarkerInfo {
ARDOUR::Location* location;
std::vector<Marker*> markers;
bool move_both;
CopiedLocationMarkerInfo (ARDOUR::Location* l, Marker* m);
};
typedef std::list<CopiedLocationMarkerInfo> CopiedLocationInfo;
CopiedLocationInfo _copied_locations;
ArdourCanvas::Points _points; ArdourCanvas::Points _points;
}; };

View file

@ -1129,7 +1129,7 @@ void
Selection::set (Marker* m) Selection::set (Marker* m)
{ {
clear_time (); //enforce region/object exclusivity clear_time (); //enforce region/object exclusivity
clear_objects(); markers.clear ();
add (m); add (m);
} }
@ -1137,8 +1137,6 @@ Selection::set (Marker* m)
void void
Selection::toggle (Marker* m) Selection::toggle (Marker* m)
{ {
clear_time (); //enforce region/object exclusivity
MarkerSelection::iterator i; MarkerSelection::iterator i;
if ((i = find (markers.begin(), markers.end(), m)) == markers.end()) { if ((i = find (markers.begin(), markers.end(), m)) == markers.end()) {

View file

@ -63,6 +63,11 @@ static string poor_mans_glob (string path)
return copy; return copy;
} }
static void show_me (Gtk::FileChooserButton* fcb)
{
cerr << " Current folder of " << fcb << " changed to " << fcb->get_current_folder() << endl;
}
ArdourStartup::ArdourStartup (bool require_new, const std::string& session_name, const std::string& session_path, const std::string& template_name) ArdourStartup::ArdourStartup (bool require_new, const std::string& session_name, const std::string& session_path, const std::string& template_name)
: _response (RESPONSE_OK) : _response (RESPONSE_OK)
@ -94,6 +99,8 @@ Ardour will play NO role in monitoring"))
need_audio_setup = EngineControl::need_setup (); need_audio_setup = EngineControl::need_setup ();
need_session_info = (session_name.empty() || require_new); need_session_info = (session_name.empty() || require_new);
new_folder_chooser.signal_current_folder_changed().connect (sigc::bind (sigc::ptr_fun (show_me), &new_folder_chooser));
_provided_session_name = session_name; _provided_session_name = session_name;
_provided_session_path = session_path; _provided_session_path = session_path;
@ -298,6 +305,7 @@ ArdourStartup::session_folder ()
if (ic_new_session_button.get_active()) { if (ic_new_session_button.get_active()) {
std::string legal_session_folder_name = legalize_for_path (new_name_entry.get_text()); std::string legal_session_folder_name = legalize_for_path (new_name_entry.get_text());
cerr << "using NFC @ " << &new_folder_chooser << ' ' << new_folder_chooser.get_current_folder() << " file " << new_folder_chooser.get_filename() << endl;
return Glib::build_filename (new_folder_chooser.get_current_folder(), legal_session_folder_name); return Glib::build_filename (new_folder_chooser.get_current_folder(), legal_session_folder_name);
} else if (_existing_session_chooser_used) { } else if (_existing_session_chooser_used) {
/* existing session chosen from file chooser */ /* existing session chosen from file chooser */

View file

@ -56,6 +56,8 @@ class Location : public SessionHandleRef, public PBD::StatefulDestructible
Location (const Location& other); Location (const Location& other);
Location (Session &, const XMLNode&); Location (Session &, const XMLNode&);
Location* operator= (const Location& other); Location* operator= (const Location& other);
bool operator==(const Location& other);
bool locked() const { return _locked; } bool locked() const { return _locked; }
void lock (); void lock ();

View file

@ -105,6 +105,21 @@ Location::Location (Session& s, const XMLNode& node)
assert (_end >= 0); assert (_end >= 0);
} }
bool
Location::operator== (const Location& other)
{
if (_name != other._name ||
_start != other._start ||
_end != other._end ||
_bbt_start != other._bbt_start ||
_bbt_end != other._bbt_end ||
_flags != other._flags ||
_position_lock_style != other._position_lock_style) {
return false;
}
return true;
}
Location* Location*
Location::operator= (const Location& other) Location::operator= (const Location& other)
{ {
@ -140,6 +155,10 @@ Location::operator= (const Location& other)
int int
Location::set_start (framepos_t s, bool force, bool allow_bbt_recompute) Location::set_start (framepos_t s, bool force, bool allow_bbt_recompute)
{ {
if (s < 0) {
return -1;
}
if (_locked) { if (_locked) {
return -1; return -1;
} }
@ -196,6 +215,10 @@ Location::set_start (framepos_t s, bool force, bool allow_bbt_recompute)
int int
Location::set_end (framepos_t e, bool force, bool allow_bbt_recompute) Location::set_end (framepos_t e, bool force, bool allow_bbt_recompute)
{ {
if (e < 0) {
return -1;
}
if (_locked) { if (_locked) {
return -1; return -1;
} }
@ -224,6 +247,7 @@ Location::set_end (framepos_t e, bool force, bool allow_bbt_recompute)
} }
if (e != _end) { if (e != _end) {
framepos_t const old = _end; framepos_t const old = _end;
_end = e; _end = e;
@ -245,6 +269,10 @@ Location::set_end (framepos_t e, bool force, bool allow_bbt_recompute)
int int
Location::set (framepos_t start, framepos_t end, bool allow_bbt_recompute) Location::set (framepos_t start, framepos_t end, bool allow_bbt_recompute)
{ {
if (start < 0 || end < 0) {
return -1;
}
/* check validity */ /* check validity */
if (((is_auto_punch() || is_auto_loop()) && start >= end) || (!is_mark() && start > end)) { if (((is_auto_punch() || is_auto_loop()) && start >= end) || (!is_mark() && start > end)) {
return -1; return -1;
@ -260,6 +288,10 @@ Location::set (framepos_t start, framepos_t end, bool allow_bbt_recompute)
int int
Location::move_to (framepos_t pos) Location::move_to (framepos_t pos)
{ {
if (pos < 0) {
return -1;
}
if (_locked) { if (_locked) {
return -1; return -1;
} }