0xfe / vexflow

A JavaScript library for rendering music notation and guitar tablature.
http://www.vexflow.com
Other
3.89k stars 662 forks source link

Easy element builder for tests and other simple use cases #419

Closed 0xfe closed 8 years ago

0xfe commented 8 years ago

We need a simple way to build common elements (notes, accidentals, beams, dots, etc.) This would vastly reduce the amount of code in the tests, and make the simple stuff easy to do.

Also, now that we have the Factory and System classes, there's a place to put it.

Proposal

I propose a new method, Factory.easyVoice(line, options) that takes a string expressing a short sequence of music and creates the relevant VF objects.

Usage

Using the factory to build everything:

let vf = VF.Factory();
vf.addStave();
vf.easyVoice('C5/8, D#5/8, Bbb4/w', options: {stem: up});
vf.draw()

Using the factory for just easyVoice:

let voice, elements = new VF.Factory().easyVoice('C5/8, D#5/8, Bbb4/w', options: {stem: up});
voice.setContext(ctx).setStave(stave);
new VF.Formatter().joinVoices([voice]).format([voice], 400);
voice.draw();

// Render beams, ties, etc.
elements.forEach(el => el.setContext(ctx).setStave(stave).draw())

Examples

easyVoice("C5/8, D#5/8, Bbb4/w")

Constructs the three stave notes, C5, D5, and B5, of durations 8, 8, and w, and attaches the # and bb accidentals to the latter two. It sticks them in a voice, and returns {voice: ..., elements: [...]}.

easyVoice("(C#5 G5 E5)/w, D#5/8, Bb4/w")

Same as above, except the first StaveNote is a chord.

easyVoice("C#5 G5 E5)/w, D#5/8[id='foo', stem='up'], Bb4/w")

Same as above, except also set the id to foo and its stem facing upwards. The [key=val,...] syntax allows you to attach arbitrary properties to a note.

Options

The options object passed into Factory.easyVoice can have properties that affect all the notes, like stem or stave.

Grammar

The grammar for the line is straightforward, and can be built with a simple hand parser:

SLASH: /
LPAREN: (
RPAREN: )
LBRACKET: [
RBRACKET: ]
COMMA: ,
EQUALS: =
QUOTE: '
DOT: .
SPACE: \s+

groups: chord | chords
chords: chord COMMA
chord: (note | notes) (SLASH duration)? (SLASH type)? (dots)? (options)?
notes: LPAREN (note SPACE)+ RPAREN   
note: key accidental? octave?
key: [a-gA-G]
accidental: [#bn]*
octave: [0-9]+
duration: [0-9qhw]+
type: [rRsSxX]
dots: DOT+
options: LBRACKET (keyval | keyvals+) RBRACKET
keyvals: keyval COMMA
keyval: key EQUALS val
key: [a-zA-Z0-9-_]+
val: QUOTE [^']+ QUOTE
0xfe commented 8 years ago

@Silverwolf90 thoughts?

SalahAdDin commented 8 years ago

Awesome bro!

Hehehehhe now @Silverwolf90 is the master chief.

0xfe commented 8 years ago

Parser update: replaced dashes with commas, and quote the values.

mscuthbert commented 8 years ago

This is about as good a new format as any I've seen. BRAVO!

The qs that people will probably ask are:

Most formats aiming at simplicity allow a way for octaves and durations to be encoded implicitly based on the previous note: that way things like "C4/8 D E D C D E D" can be spelled out. I see octave and duration as optional. Is this the intent? Does the octave stay the same or does the closest octave to the previous note get chosen? (i.e., is "C B C B" a bunch of 7ths or 2nds?)

"." for dots is a very standard format, I'd recommend over "/1" for one dot. In theory, C3/8 is ambiguous between a C3 eighth note and a C3 unknown duration w/ 8 dots. (duration is undefined currently, but probably not too hard to do).

The history of the ABC, humdrum **kern, Lilypond formats and my work on TinyNotation suggests that the notation system is going to want to expand itself in the future (slurs, ties, tuplets; beam status; etc.; inclusion of time signatures and key signatures, etc.) -- it'd be good to plan for this expansion now because the grammar in the future can become a complete mess otherwise. My solution on TinyNotation was to leave three user definable grouping symbols (sort of like the [options=val] is here, but with user definable meanings in the absence of option=value) and say that those three can be used for whatever you want but if you want more than three items, upgrade to MusicXML or a more structured format. :-)

Amazing progress y'all. Sorry I've not been able to do anything but applaud.

0xfe commented 8 years ago

Thanks Michael.

Re: dots, I like your idea better and will update the grammar to use periods.

Re: octaves and durations, I went back and forth about whether they could be implied from prior notes, and decided on allowing it (which is why they're optional.)

I'm okay with the API expanding, but I really hope the grammar wont expand (I went through that with VexTab.) I explicitly put the keyval options in so that additional elements could be added by the factory. So, if you want to add a tie:

let vf = VF.Factory();
vf.addStave();
vf.easyVoice("C5/8[id='foo'], D#5/8[id='bar'], Bbb4/w", options: {stem: up});
vf.Tie({from: 'foo', to: 'bar'});
vf.draw();

Overall, this is supposed to be just a helper, and like TinyNotation, if people want more they should use something more structured. Thanks again for the feedback.

Silverwolf90 commented 8 years ago

I think this is a really good idea! I'm definitely in favor of keeping the scope of the grammar small, while relying on the factory methods for the more complex functionality. If I understand correctly this is mainly a convenience thing for simple cases. We're not trying to add another music syntax into the wild. A case with complexity that doesn't fit within the syntax should assemble Voice objects in the typical, more verbose, fashion. (@0xfe is this correct?)


To pick apart your examples:

let voice, elements = new VF.Factory().easyVoice('C5/8, D#5/8, Bbb4/w', options: {stem: up});

Is this missing destructuring? Does easyVoice return both the voice and the elements within the voice?

voice.setContext(ctx).setStave(stave);

Where did the stave come from? Isn't the factory supposed to take care of this?

new VF.Formatter().joinVoices([voice]).format([voice], 400);
voice.draw();
 // Render beams, ties, etc.
 elements.forEach(el => el.setContext(ctx).setStave(stave).draw())

Again, isn't the factory supposed to handle drawing?

Silverwolf90 commented 8 years ago

Ah I see now you had two distinct examples to showcase the isolated usage of easyVoice. Although the first line still confuses me.

mscuthbert commented 8 years ago

Makes great sense.

Of the most common uses for builder/Factory notation, I think that (1) the Tie mark is useful and would consider including it even though you can do it this way. (we use "~"). The other two really important thing to be able to specify are (2) rests (add R to key) and (3) pickup notes -- otherwise shifting everything over for an example can be very hard even though lots of simple music uses them.

Not including chords is probably the worst mistake I made for tinyNotation; it may happen in the future. :-) Glad you're including them as class one parts of the grammar.

Silverwolf90 commented 8 years ago

How does this handle the num_beats and beat_value when initializing a Voice?

0xfe commented 8 years ago

@Silverwolf90, to answer your questions:

Is this missing destructuring? Does easyVoice return both the voice and the elements within the voice?

I haven't fully fleshed this out yet, but I think it should return the voice and the additional elements that may need to be rendered. So the user can call voice.draw() and forEach(el => el.draw(). Perhaps we can return an object like {voice: ..., notes: ..., accidentals: ..., beams: ...}? I'm trying to balance between simple and flexible.

How does this handle the num_beats and beat_value when initializing a Voice?

This would be in the options parameter.

If I understand correctly this is mainly a convenience thing for simple cases. We're not trying to add another music syntax into the wild. A case with complexity that doesn't fit within the syntax should assemble Voice objects in the typical, more verbose, fashion.

Correct. This just makes the simple cases easy.

0xfe commented 8 years ago

@mscuthbert, @Silverwolf90 do you have thoughts on beams? What kinds of beam options should I expose in the API (in the options paramters)? E.g., 'autoBeam,beamGroups` etc.?

0xfe commented 8 years ago

@mscuthbert Thanks for mentioning rests, I forgot about that :-) I need to think about pickup notes and ties a bit more.

0xfe commented 8 years ago

I just realized that we'll need a syntax for triplets/tuplets too.

Silverwolf90 commented 8 years ago

The problem is that "simple" music notation is still very complex. And thinking about this more, I'm wondering if the syntax should be stripped down. It's very close to a normal javascript array, so maybe we can just leverage that rather than creating a grammar for entire voices. Maybe we only need a grammar for notes/chords.

// note
StaveNote.create('D#3/4');

// chord
StaveNote.create('D3 Gb3 B3/4.') 
const voice = (notes) => vf.Voice().addTickables(notes.map(StaveNote.create));

voice(['D#3/4', 'D3 Gb3 B3/2.', 'Eb4/8'])

And then you can start extending in any direction with regular functions.

Extremely hand-wavy strawman:

voice(
  beam([
    triplet(['C4/8', 'C4/8', 'C4/8']), 'C4/8', 'C4/8', 'C4/8', 'C4/8',
  ])
);

Re: Beams Config

I'd expose as much as possible for beams. Maybe a polymorphic beam config property:

  1. true auto beams using defaults
  2. array of [start, stop] note indexes for explicit beaming (maybe ids?)
  3. object that with same shape as the Beam.generateBeams() config

Even for simple cases, beam creation requires significant flexibility.


Not sure I understand the pick-up notes case. Can that be elaborated on?

0xfe commented 8 years ago

Just pushed a functional parser for this to the dev branch:

https://github.com/0xfe/vexflow/blob/dev/src/easyscore.js https://github.com/0xfe/vexflow/blob/dev/src/parser.js

It's well tested and seems to parse everything cleanly. It doesn't build elements yet, but that's the easy part.

0xfe commented 8 years ago

That might work, and we don't really need to strip down the grammar I think. Right now there's an EasyScore class in the dev branch that implements the proposed grammar, and we could easily do something like this.

vf = new Factory(...);
ev = new EasyScore(vf);

vf.addStave();
ev.voice(
  ev.tuplet(ev.notes('C5/8, G#6, (C4 E4 G4)/16'), 3),
  ev.notes('C5/8, G#6, (C4 E4 G4)').tuplet(3),
  { beams: }
);

vf.addStave();
ev.voice(
  ev.beam(ev.notes('C5/8, G#6, (C4 E4 G4)/16, C4')),
  ev.beam(ev.notes('C5/8, G#6, (C4 E4 G4), C4')),
  { beams: }
);

factory.draw();
Silverwolf90 commented 8 years ago

I'd be pretty content with something like that!

I'd like to point out that there's a pretty negligible difference between the custom list syntax and rest parameters.

vf.addStave();
ev.voice(
  ev.beam(ev.notes('C5/8', 'G#6', 'C4 E4 G4/16', 'C4')),
  ev.beam(ev.notes('C5/8', 'G#6', 'C4 E4 G4', 'C4')),
  { beams: }
);

So I really question the benefit. Seems like the cost of having a custom list syntax is much higher than quote characters it saves. And using rest parameters eliminates the need for parentheses to declare chords.

0xfe commented 8 years ago

It actually doesn't cost that much (probably no more than 5 - 10 lines of code), because the rules are really easy to define with the Parser class. And, yes, I agree that rest parameters makes the alternative slightly cheaper (although we can't use them in our tests because ES5.)

However, part of the intent here is to future-proof it a bit and cover as many common cases as possible, and I think adding sequences of notes (in tests, from UIs, from tools) is a common enough case to warrant it.

Silverwolf90 commented 8 years ago

Fair enough :) This is going to be cool!

0xfe commented 8 years ago

I got it working. The grammar fully parses and the builder can render almost everything specified. Now need to add support for beams and tuplets.

The API still needs to be fleshed out, but here's a working example:

const score = new VF.EasyScore({factory: vf});
const system = vf.System();

system.addStave({
  voices: [
    score.voice('c#4/q, c4/q, c4/q/r, c4/q', {stem: 'down'}),
    score.voice('c#5/h., c5/q', {stem: 'up'})
]}).addClef('treble');

system.addStave({
  voices: [
    score.voice('c#4/q, cn4/q, bb4/q, d##4/q'),
]}).addClef('bass');
system.addConnector().setType(VF.StaveConnector.type.BRACKET);

vf.draw();
screen shot 2016-08-19 at 7 41 41 pm
0xfe commented 8 years ago

Got beams and tuplets working:

system.addStave({
  voices: [
    voice(
      tuplet(notes('(c4 e4 g4)/q, cbb4/q, c4/q', {stem: 'down'}), {location: VF.Tuplet.LOCATION_BOTTOM})
      .concat(notes('c4/h', {stem: 'down'}))),
    voice(notes('c#5/h.', {stem: 'up'})
      .concat(tuplet(beam(notes('cb5/8, cn5/8, c5/8', {stem: 'up'})))))
]}).addClef('treble');
screen shot 2016-08-19 at 9 10 35 pm
0xfe commented 8 years ago

@Silverwolf90 PR sent for your review.

0xfe commented 8 years ago

This has been merged and released in version 1.2.78.