simplify iterating with Cursor

[heuchi]

This adds to Cursor:
property 'inSelection' is true if the current cursor position is inside the selection
property 'hasSelection' is true if there is a selection

while(cursor.inSelection) { ... cursor.next(); }

will iterate over all segments of current track in the selection

function 'iterate(type)' resets the cursor to start of score (type==false) or selection (type==true)
function 'iterating()' remains true while there are still tracks/staves to be iterated
functions 'nextTrack()' and 'nextStaff()' setting the iterator to the beginning of the next track or staff. (according to type set when calling iterate).

 for (cursor.iterate(true); cursor.iterating(); cursor.nextTrack()) { ... }

will iterate over the selection track by track.

Below is a sample that iterates over all notes and grace notes of the selection or - if none - the whole score:

import QtQuick 2.0
import MuseScore 1.0

MuseScore {
  menuPath: "Plugins.newCursorTest"

  function fn(note) {
        note.color = "#ff0000";
 }

  onRun: {
        var cursor = curScore.newCursor();
        // This iterates through the selection if there is one
        // or through the whole score if there's no selection

        // iterate tracks
        for (cursor.iterate(cursor.hasSelection); cursor.iterating(); cursor.nextTrack()) {
              // iterate segments
              while (cursor.segment && (cursor.inSelection || (!cursor.hasSelection))) {
                    // if there's a chord, iterate all grace notes and notes
                    if (cursor.element && cursor.element.type == Element.CHORD) {
                          var chord = cursor.element;
                          // grace notes
                          var graceChords = chord.graceNotes;
                          for (var i = 0; i < graceChords.length; i++) {
                                var notes = graceChords[i].notes;
                                for (var j = 0; j < notes.length; j++) {
                                      fn(notes[j]);
                                }
                          }
                          // notes of chord
                          var notes = chord.notes;
                          for (var i = 0; i < notes.length; i++) {
                                fn(notes[i]);
                          }
                    }
                    cursor.next();
              }
        }

        Qt.quit()
        }
  }

[lasconic]

I would change "type" to "selection" or "throughSelection"

[lasconic]

I believe it's going in the right selection.
1/ I wonder how well it will integrate with #1271
2/ it tastes for more. Maybe you could apply the same treatment to while (cursor.segment && (cursor.inSelection || (!cursor.hasSelection))) {. In particular "cursor.segment", maybe it could be covered by cursor.inSelection ? Maybe a cursor should be initialiazed to work on the selection or the full score right from the start like the other PR does for repeat list?

[heuchi]

1/ I don't really know #1271. It seems not yet finished.
2/ The line you're refering to catches two flies at once, that's why it's a bit complicated. I don't think it would be a good idea to let inSelection do anything other than what the name suggests. If we want to get this simpler, we'll no longer have a Cursor. Anyway, what I did here is probably already out of the scope of a Cursor. And to keep the API 'clear' we should probably change this:

So, I'd propose: Remove the iterator stuff from Cursor and create a new class Iterator which will do the whole job (including walking through segments) in one loop. It can make sure it's not going to interfere with #1271, because it can initialize a local cursor to not count for repeats, and we can decide later if we want to integrate it. So we'd get something like:

for (var it = curScore.newIterator(overSelection); it.iterating(); it.next()) { ... }

One could also provide a second constructor newIterator(overSelection, elementFilter). The filter could be set to Element.CHORD, for example if we're not interested in anything else.
Iterator would provide these other properties and functions:

 void add(Element*)  // passed to the Cursor
 void addNote(pitch) 
 void setDuration(z,n)
 track, voice, staffIdx  // read only 
 element, segment, measure, tick, time, keySignature, score // as in Cursor

There needs to be a way to set a cursor to the same position of the Iterator to be able to work on other tracks at the same position. I can think of two ways doing this:

 bool Cursor.gotoTick(tick); // could be called like cursor.gotoTick(it.tick);

 var Iterator.getCursor();   // this should probably not expose the Iterator's own cursor

The second version has the disadvantage of creating a new object every time it's called.

Thinking of it: We could also add an Iterator type that would not go through the score track by track but segment by segment (the type needed for courtesy accidentals). This could be done in the same class. We'd just need a different constructor: Score.newSegmentIterator(elementFilter) or something like that?

Won't be able to do this today anyway. So, I'll wait for your comments.
By the way: What about #1264 which would also help here?

[lasconic]

I don't understand the difference between a Cursor and an Iterator in your example. Also I don't get how it would simplify things to have two classes to go through a score.

What if a Cursor would be defined like this

var cursor = curScore.newCursor(onSelection, expandrepeat);
cursor.nextTrack()
cursor.nextStaff()
cursor.nextSegment(type) //type being optional
cursor.next(type) // type being optional, to be defined but would loop through track and segment in a given order
cursor.atEnd() //(true if at end of score or selection), similar to your iterating()
// other methods for keysig etc... are kept.

// possibly we can add previousXXX methods too

[vgstef]

My commit #1271 is done. It simply expose the measure number and allows the cursor to iterate on expanded repeats.

[heuchi]

I thought it would be a nice to have two different classes for different ways of accessing the score. Cursor for random access, and Iterator for an ordered list like access. But maybe we don't need that.

I have some questions concerning your (Lasconic's) definition:
Would we still need Cursor.nextTrack() when Cursor.next() does the job of iterating over tracks?
I assume we'll keep rewind(), setTrack() and friends. If so (and if the Cursor takes 'onSelection' as parameter), I think we'll have to define what next() is supposed to do, if onSelection == true and the user moves the cursor out of the selection manually. Or if there is no selection at all.
Should we move 'hasSelection' to Score, to be able to know before creating a cursor? Providing a simpler way of getting startStaff and endStaff of the selection (without having to use rewind() twice) would also be nice.

Concerning 'expandrepeat': What should a Cursor do, when called with onSelection == true and expandrepeat == true, if the selection starts in the middle of a repeated section? Once it hits the repeat bar line, it cannot go back to the beginning of the repeated section, because that is not part of the selection, repeating only the selected part doesn't make much sense either. So would we just skip that repeat?

[lasconic]

First, let me state I'm glad to have this discussion with someone and I hope we will manage to make something simple and usable.

If we can manage to have both random access and iteration in the same class, I believe it's better. If the class becomes too complicated then ok, let's go for two classes.

I would see Cursor.next(type) as a shortcut and Cursor.nextTrack() as a finer iteration to allow segment first or track first iteration. Cursor.next() would be hardwired to do segment first if we take your first example. #1279 (comment). If we go for two classes, it's also valid for the Iterator.

Should we have hasSelection in score?
What about expandrepeat and onSelection= true?
Yes could be nice to have score.hasSelection for other reasons but not necessarly here. I'm less fan of start/end staffindex.
Score::newCursor(onSelection, expandrepeat) would work on the full score if onSelection is true. ExpandRepeat would be ignored if onSelection is true and there is a selection.

[heuchi]

Yes, let's see that we come up with a powerful yet simple to use Cursor. :-)
Just to make sure we're on the same page, I'll try to sum up the definition of Cursor:

Constructors:

 Score::newCursor() 
      implies onSelection=false,expandrepeat=false
 Score::newCursor(onSelection)  
      implies expandrepeat=false; 
      if there's no selection create a cursor with onSelection = false
 Score::newCursor(onSelection,expandRepeat) 
      if onSelection==true and there's a selection ignore expandRepeat.
      otherwise create cursor over full score with expandRepeat=true.

next group ('type' always optional):

 Cursor::next(type)
      goto next segment where current track has element of given type. 
      If no such segment exists in current track advance to next track and rewind.
      If there's no next track in score/selection set _segment=0.
 Cursor::nextTrack(type)
      goto next track that has element of given type in current segment.
      If no such track exists in current segment advance to next segment and first track.
      If there's no such segment set _segment=0.
 Cursor::nextStaff(type)
      same as nextTrack() but advance to first voice of next staff.
 Cursor::nextSegment() // no type here
      advance to next segment. Set _segment=0 at end of score/selection.
 Cursor::atEnd()
      returns true when _segment==0.

This extra nextTrack() would be needed for iterating parts. (Actually my courtesy accidental plugin would only be able to use nextTrack if this was provided):

 Cursor::nextTrack(type,startTrack,endTrack)
      same as nextTrack() but only work on tracks in given range.
      startTrack <= track < endTrack

keep:

 Cursor::track, Cursor::staffIdx, Cursor::voice
 Cursor::rewind()
 Cursor::inSelection()

 Cursor::tick, Cursor::time, Cursor::keySignature, Cursor::score
 Cursor::element, Cursor::segment, Cursor::measure

editing group:

 Cursor::add(), Cursor::addNote(), Cursor::setDuration()

I think we should also discuss this last group. But maybe not here.

[lasconic]

I don't get Cursor::nextTrack(type,startTrack,endTrack)
you could set the cursor track and then test for endTrack after nextTrack(type)?

Since we are at it, I would revise Cursor::rewind() and remove the param. Rewind would rewind to the start of the cursor as defined by the newCursor "constructor". And maybe toEnd() or similar to go to the end of the selection/score.

Also, could be good to know if the cursor is operating on the full score or not once initialized. Same for expandRepeat. But I would make them read only. So no way to switch from expandRepeat to not expand repeat with the same cursor.

[heuchi]

About Cursor::nextTrack(type,startTrack,endTrack)
Yes, but the code would probably look less readable:

 for ( ...; !cursor.atEnd(); cursor.nextTrack(Element.CHORD)) {
      if (cursor.track < startTrack) {
           // already at new segment but not in selected track range
           cursor.track = startTrack;         
      }          
      if (cursor.track >= endTrack) {
           // left selected track range but not yet at new segment
           cursor.track = startTrack;
           cursor.nextSegment();
      }
      if ((! cursor.element) || (cursor.element.type != Element.CHORD))
           continue;
      ...
 }

On the other hand:

 for ( ...; !cursor.atEnd(); cursor.nextTrack(Element.CHORD,startTrack,endTrack)) {
      ...
 }

Well, I like the second version better.

Changes to the definition of Cursor:

 Cursor::rewind()
      reset the cursor to beginning of score/selection; former rewind(0) or rewind(1)
 Cursor::toEnd()
      set the cursor to end of score/selection; new or former rewind(2)

 Cursor::onSelection
      true if this cursor is running on the selection (read only)
 Cursor::expandRepeat
      true if this cursor expands repeats (read only)

[heuchi]

One more thought about Cursor::toEnd()
If onSelection==false this would set the cursor behind the last segment, thus _segment=0 and invalidating the Cursor. Seems not very useful. However, there's probably no sensible alternative.

[lasconic]

About cursor.nextTrack(Element.CHORD,startTrack,endTrack)
I wonder if it wouldn't be better at the cursor level then? Like a way to have a staff cursor?

[heuchi]

I'd need a part cursor. For a flute that would be a staff, but for a piano it would be two staves.
So maybe something like:

 Score::newPartCursor(partIdx, onSelection,expandRepeat)

Of course we could also provide a staff cursor with little more effort.

 Score::newStaffCursor(staffIdx, onSelection, expandRepeat)

[lasconic]

Yes. I don't have strong opinion on adding newStaff/PartCursor or providing nextXXX variants with start/end tracks.

[heuchi]

I think I'd vote for nextTrack(type,start,end), because it's more flexible and easier to implement. And I don't think we'd need it for the other next functions.

 for (cr.track = startStaff; !cr.atEnd() && cr.track < endTrack; cr.next()) { ... }

This would do the trick for next().

[heuchi]

I think I was just complicating things here...
Since a plugin, that requests elements of different types will have to find out the type of the element it got anyway, let's stick to this:
nextXXX() will return elements of all types, not just chord or rest as it does now.
nextXXX(type) will return elements of given type only.

Old comment:
About elementType in the nextXXX functions. I always wrote elementType. Since we cannot use the 'or' (operator |) on these as we can with segmentType: what about allowing the nextXXX functions to use a variable number of arguments:

  next(); // will return next Chord or Rest
  next(Element.CHORD,Element.CLEF, ...); 

Or allow the argument elementType to be an array. This wouldn't kill my nextTrack(type,start,end) idea.
Oh, I just realized, that I only tested this using javascript. Is either of these possible with a C++ function?

[dmitrio95]

Needs a rework to be applied for current master