Add ScoreVisualizer to display NoteSequences as musical scores in staffs

[rogerpasky]

Current Visualizer class is cool (great job @notwaldorf ), and it could evolve to display sequences not just as pianoroll, but music staff as well. A derived ScoreVisualizer is proposed.

It must fit current pianoroll functionality in a polymorphic way introducing minimal changes:

• constructor(sequence, canvas, config) which:
  • Constructs all the internals according to the given config.
  • Ends up doing the initial redraw.
• redraw(activeNote, scrollIntoView) which:
  • Uses the graphical placeholder (currently canvas) to do the drawing (if needed) with the non-highlighted style.
  • Uses the activeNote, if given, to highlight it in the drawing, dimming the style colour according with the velocity note (if any).
  • Scrolls the graphical placeholder to locate the activeNote properly if requested in scrollIntoView.

Minimum Viable Product (MVP) must add new music staff functionality through the same Visualizer interface:

• [x] Prepare a class hierarchy to allow SVG visualizers.
• [x] Provided values config.minPitch and config.maxPitch should be used to select the musical clef (at least treble clef and bass clef). Undetermined cases will default to treble clef.
• [x] Different values of time signature will be extracted from given sequence, if any. Otherwise it will be assumed 4/4 for all staff.
• [x] It will be displayed the clef signature at the beginning of the staff, and it won't change through the score.
• [x] No key signature will be displayed at the beginning of the staff. It will be assumed C key.
• [x] Initial time signature will be displayed at the beginning of the staff.
• [x] Bar separators will be displayed according with initial time signature.
• [x] There will only be a single five-line staff per NoteSequence. Multiple staff will be handled instantiating several vertically aligned ScoreVisualizer instances with different config.minPitch and config.maxPitch values.
• [x] Horizontal note disposition will be coherent with current pianoroll Visualizer in order to allow aligning heterogeneous visualizers vertically. It's out of scope in this class to visualize compact staff which would be handled in future derived visualizers.
• [x] Musical clef, time signature and key signature displayed at the beginning of the staff will be handled to allow vertical alignment with other visualizers as mentioned. Yet to be detailed.
• [x] Notes and rests will be drawn from whole note (semibreve) to sixty-fourth note (hemidemisemiquaver) including dotted-note versions (single dotted).
• [x] No ties will be displayed to join two equal-pitch different length notes, so gaps due to rounding duration would appear.
• [x] Note stems will be displayed upwards or downwards as expected (downwards if the notehead lies on or above the middle line of the staff).
• [x] Shorter than octave notes will show individual note flags.
• [x] Rests will be highlighted jointly to its former note, given they are not related to any activeNote.
• [x] It will be shown only sharp notes, no flat ones, given there's no key signature and sticking to the "all sharps or all flats" criteria.
• [x] Sharps will be shown in every note they apply, providing accidental redundancy instead skipping them when they appears previously in same note in same bar.

Nice to have functionalities out of MVP:

• [x] Different values of key signature will be extracted from given sequence, if any. Otherwise it will be assumed C key for all staff.
• [x] Initial key signature will be displayed at the beginning of the staff.
• [x] Changes in key signature will be displayed through score if any.
• [x] Changes in time signature will be displayed through score if any.
• [x] Music ties will be displayed, including multi-bar scenarios.
• [x] Accidentals won't be repeated within a bar, applying as well to the remainder of the measure where they occur.
• [ ] Shorter than eighth notes will group note flags in binary groups (2, 4, 8...) displaying homogeneous note beams. No mixed durations beam will be generated.
• [ ] Chord changes will be displayed on top of staff through score, if any. There will be no superscript text on it (7th and the like) but a straight single line text.
• [ ] Triplet signs will be shown.
• [ ] Values of qpm from sequence.tempos will be used to highlight rests.
• [ ] Values of qpm from sequence.tempos will be used to show cursor animation (starting with each redraw() call).
• [x] Extra VisualizerConfig fields will be defined to achieve finer control.

Current pianoroll Visualizer relies on html5 canvas placeholder, claming in the source code it could be improved the redrawing() performance. Benefits of using SVG instead should be evaluated.

[rogerpasky]

I propose myself to be assigned to cover this issue.

[adarob]

Looks like a great idea!

@notwaldorf @kmalta can you give any guidance?

[notwaldorf]

This is an awesome idea! I've actually been thinking of re-implementing the Visualizer with SVGs (and not canvas), and I think it works just as well. I definitely think there could be an option even for the default Visualizer to use SVG -- especially since this would allow CSS animations etc.

Are you planning on showing the notes with a font?

[rogerpasky]

Cool!

Related to SVG re-implementation, I was thinking about require in the constructor just a DIV container and let the implementation do all the magic as internals. This could allow future visualizers to use frameworks like three.js or others yet to come. Magenta.js distribution should provide a virtual base clase (BaseVisualizer following player.ts pattern) and two minimal derivatives (current Visualizer and next ScoreVisualizer) with no internal visual framework support but plain HTTP5 and CSS3. If we all agree, first step should be deprecate current constructor from:

Visualizer.constructor(sequence: INoteSequence, canvas: HTMLCanvasElement, config: VisualizerConfig = {})

to:

Visualizer.constructor(sequence: INoteSequence, div: HTMLDivElement, config: VisualizerConfig = {}), which should add two internal lines:

let canvas = document.createElement('canvas');
div.appendChild(canvas);

This change will allow us to decouple visualizers from implementation, but certainly it will impact all current usages (internals, like magenta-js/music/demos, or external ones). Internals can be easily handled, but I'm not aware about the right timings between the deprecation announcement and the actual modification.

I don't mind doing both changes (magenta-js/music/src/core/visualizer.ts and all affected files in magenta-js/music/demos) but I need some advice about the deprecation timing (maybe I'm being too polite and it should be accepted a simple overnight change :-D).

This decoupling allows maintaining current visualizer implementation vith canvas or evolve it to SVG as well. I'd be more than happy to evolve it as well once I've finished the ScoreVisualizer.

Regarding to fonts as note representation, I'm still hesitating among it and SVG <path>, because paths will be needed for staff lines, note beams and note ties anyway, due they are variable width. I'm not sure at all if different music fonts keeps equivalent dimensions (baseline, height) to be compatible with desired placement in staff, beam alignment... Probably I'll try both, because it would be great to be able to change visual style (classical, handwriting...).

Please, let me know about your thoughts on deprecation policy.

[notwaldorf]

I would prefer no breaking changes, if they're avoidable. I think with a little bit of refactoring we could have:

• a BaseVisualizer,
• Visualizer (using canvas, as it is today)
• an SVGVisualizer, using SVG instead of canvas
• a StaffVisualizer (using your implementation). I know there's not a ton of Visualizers users out there, but any change in API causes friction, and I'd like to save that for absolutely unavoidable situations.

Completely agree with you re: the font. SVGs sound better for that.

[rogerpasky]

Agreed about avoiding API change frictions. I was wrong. I've been thinking about it all day long (I'm in Europe) and there's no need to keep an uniform interface but in virtual functions implementation in derived classes (polymorphism), which excludes constructors (you can construct derivative classes with whatever parameters you want, calling super constructor with the right ones).

I'll do it as you suggest.

• BaseVisualizer (only the very common staff and some virtual functions)
  • Visualizer (extends BaseVisualizer class but offers exactly the same interface as today)
  • SVGVisualizer (extends BaseVisualizer class and introduces SVG things, receiving it through constructor)
  • StaffVisualizer (extends SVGVisualizer class and does the internal magic to draw notes on a staff)
  • (future PianoRollVisualizer)

I'll do the Pull-Request as soon as MVP gets ready and there's some demo to deal with. Given SVG is structured through HTML document, I'll try to include a proper test to the new class (another side-benefit of SVG).

[rogerpasky]

Development evolves according to plan. Current state is marked in the original issue proposal list (which has changed to a task list) and actual code can be seen in my fork repo.

There has been found another internal issue in the usage of noteSpacing configuration variable. It adds an extra space at the end of a note to separate it from the following one. It works pretty well in monophonic scores, but in polyphonic scores it adds an extra space per note, creating a slight stairs effect in chords.

It is barely noticeable in piano roll representation due it is only one pixel wide:

But it is pretty noticeable in staff representation due to vertical note stem:

(Please forgive the crappy "work in process representation", no beautiful SVG paths yet).

I'll include the note time grouping solution I'm using in staffVisualizer for vertical stems to original Visualizer in order to fix this issue and allow the claimed requirement "Horizontal note disposition will be coherent with current pianoroll Visualizer in order to allow aligning heterogeneous visualizers vertically", which would fail if I solve it only in SVG version. It won't change any interface at all.

[notwaldorf]

I think it’s totally fine if the two visualizers use slightly different config objects, or if the Staff visualizer just ignores some of the properties in the config object, even if they’re set.

Looking good!

[rogerpasky]

Thanks!

I'm trying to stick as close to the current interface as possible and I'll take the freedom degree you just gave me when needed, but not earlier. The staffVisualizer is intentionally maintained to get the desired alignment, but as you can see in the early screenshot, first note misses a bit of the extra staff line due to its extreme alignment to the left, so final version will have some option to add extra space at the left side to keep room for clef and keys. It could be used for a vertical piano reference drawing in piano roll version (canvas or SVG).

Keep on working ;-)

[notwaldorf]

@rogerpasky I've ended up having tons of performance issues recently with mm.Visualizer (all my canvases kept crashing), so I had to skip the line and implement a super quick SVGVisualizer. It's basically 90% similar to the regular one, only overwrites what redrawNote does. Let me know what you think! https://github.com/tensorflow/magenta-js/pull/260/

[rogerpasky]

@notwaldorf your approach is similar to mine, so they will end-up merging very well.

Current status is close to desired MVP as you can see:

(Screen capture appears far much pixelated than the real thing which is vectorial in all its glory)

It only misses rests (hard to think off because they are not a Protocol Buffer entity, but the absence of it, and yesternight I found out how to make them properly) and where to put the Clef, Key and Tempo signature so the vertical alignment with piano roll keeps on working (I found out a beautiful way as well).

Good news is I have a vectorial version of piano roll visualizer which took me just a couple of hours to do it. It is fully compatible with previous one and It has been tested in scrolling as well. I can do a pull request right now if you want to, removing the staff visualizer work in progress part, or I can do a merger later with your current pull request. Just let me know.

I had in mind sending the full MVP pull request on Sunday (no automatic testing yet, but fully documented), but if I have to do the extra merge it will delay a little bit.

[rogerpasky]

Just had a deeper look to your pull request and realized your class hierarchy is:

• Visualizer
  • SVGVisualizer

Which differs to the hierarchy we agreed:

• BaseVisualizer (only the very common staff and some virtual functions)
  • Visualizer (extends BaseVisualizer class but offers exactly the same interface as today)
  • SVGVisualizer (extends BaseVisualizer class and introduces SVG things, receiving it through constructor) [actually it is a virtual abstract class, non instantiable]
  • StaffVisualizer (extends SVGVisualizer class and does the internal magic to draw notes on a staff)
  • (future PianoRollVisualizer) [actually it is already developed and tested]

Given class name will remain as a stable definition (hard to evolve) it could collide with the abstract nature of SVGVisualizer class (not instantiable). If you keep on willing to go on with your pull request, I strongly suggest you to change SVGVisualizer class name to PianoRollVisualizer to ease the final transition. Otherwise I would change SVGVisualizerclass name to VectorialVisualizer, but it would lead to confusion having a derivative class named SVGVisualizer which does a particular way of displaying scores in the piano roll manner.

[notwaldorf]

Oh right, i forgot that we had talked about a BaseVisualizer. I’ll change that. I think the API for the child classes will basically require overloading the constructor and redrawNote, which is pretty good. Because of this, I don’t think that the staff visualizer and the svg visualizer need to be anything more than children. I’ll rename it to PianoRollSVGVisualizer to be consistent, though On Fri, Feb 1, 2019 at 6:22 AM Pascual de Juan notifications@github.com wrote:

Just had a deeper look to your pull request and realized your class hierarchy is:

• Visualizer
  • SVGVisualizer

Which differs to the hierarchy we agreed:

• BaseVisualizer (only the very common staff and some virtual functions)
  • Visualizer (extends BaseVisualizer class but offers exactly the same interface as today)
  • SVGVisualizer (extends BaseVisualizer class and introduces SVG things, receiving it through constructor) [actually it is a virtual abstract class, non instantiable]
    • StaffVisualizer (extends SVGVisualizer class and does the internal magic to draw notes on a staff)
    • (future PianoRollVisualizer) [actually it is already developed and tested]

Given class name will remain as a stable definition (hard to evolve) it could collide with the abstract nature of SVGVisualizer class (not instantiable). If you keep on willing to go on with your pull request, I strongly suggest you to change SVGVisualizer class name to PianoRollVisualizer to ease the final transition. Otherwise I would change SVGVisualizerclass name to VectorialVisualizer, but it would lead to confusion having a derivative class named SVGVisualizer which does a particular way of displaying scores in the piano roll manner.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tensorflow/magenta-js/issues/246#issuecomment-459738033, or mute the thread https://github.com/notifications/unsubscribe-auth/ABTkUqeAbY4cQ2aNevfrKsnqpJP9FOgfks5vJE2ZgaJpZM4Z4czw .

[notwaldorf]

So actually, your hierarchy might not be correct. What i've noticed is that 90% of the implementation in the current Visualizer is shared across all piano roll visualizers -- the area size calculation, the note positioning, etc. The only thing that changes is that redrawNote paints to a canvas or creates an svg. If that isn't the case with your visualizer, then the hierarchy might be more like

BaseVisualizer
  |__Visualizer
       |__SVGVisualizer
  |__StaffVisualizer  

(and i've made these changes in my PR)

[rogerpasky]

I understand you are in a hurry and I'm not. I can spend extra time in refactoring without pain (it is a real pleasure to me to contribute to this project). I'll just keep on going with internal advances and will do the final adaptations once your PR will get approved.

However, just to let you know, I do not redraw all the SVG elements in each redraw() call, but only when activeNote is not given. Otherwise, I highlight on and off exclusively the appropriate symbols (note heads, stems, flags, accidentals, extra-lines and dots) just changing their fill and stroke SVG attributes to optimize performance (they are grouped under a g SVG group so it is pretty efficient, leaving CPU room for TensorFlow calculations).

This is functionally compatible with current implementation and it allowed me to do the piano roll version as efficient as the staff one. That's the reason for my proposed hierarchy. If it's better for you to go on with your latest proposal not the agreed one, go for it, no worries, I'll adapt mine later keeping the naming compatibility.

[notwaldorf]

Oh interesting. I’ll give this another go with this in mind! On Fri, Feb 1, 2019 at 2:45 PM Pascual de Juan notifications@github.com wrote:

I understand you are in a hurry and I'm not. I can spend extra time in refactoring without pain (it is a real pleasure to me to contribute to this project). I'll just keep on going with internal advances and will do the final adaptations once your PR will get approved.

However, just to let you know, I do not redraw all the SVG elements in each redraw() call, but only when activeNote is not given. Otherwise, I highlight on and off exclusively the appropriate symbols (note heads, stems, flags, accidentals, extra-lines and dots) just changing their fill and stroke SVG attributes to optimize performance (they are grouped under a g SVG group so it is pretty efficient, leaving CPU room for TensorFlow calculations).

This is functionally compatible with current implementation and it allowed me to do the piano roll version as efficient as the staff one. That's the reason for my proposed hierarchy. If it's better for you to go on with your latest proposal not the agreed one, go for it, no worries, I'll adapt mine later keeping the naming compatibility.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tensorflow/magenta-js/issues/246#issuecomment-459893846, or mute the thread https://github.com/notifications/unsubscribe-auth/ABTkUs_khMw8WaCrkbbfDokYf7L5BrBAks5vJMORgaJpZM4Z4czw .

[rogerpasky]

You can have a look to my code (work in progress) in my fork repo to get inspired.

Enjoy ;-)

[notwaldorf]

Alright, cleaned this up again. I made redraw be the abstract function, and tested it on the SVG visualizer that now doesn't redraw at every step.

Still thinking that your staff visualizer should inherit from the BaseVisualizer class, however. There's nothing inherently SVG-y about PianoRollSVGVisualizer, so I don't think any of its methods will be useful to you. This will also lead to a pretty flat hierarchy, which is much easier to grok.

[rogerpasky]

Super coooool!!!!

Agreed to move towards a flat hierarchy, removing the SVG intermediate one. Love the new naming and I'll move mine to StaffSVGVisualizer.

I'll keep on advancing on my side (rests are almost done) and will start internal renaming and merging as soon as your PR gets approved. It will only be left to reach MVP goal to draw Clef, Key and Time signatures (not too hard, but tricky placement) and once it gets properly documented (according to tensorflow/src/core comments standard) and it have passed some manual testing I'd do the PR to evolve it later on (I'm still thinking of some acceptance tests given SVG structure is able to be explored).

New estimated due date: next weekend.

[rogerpasky]

MVP is operative and looking great (proud of my own 1st class minimalist SVG drawings). Still pending to do:

• some fine-grain pixel-perfect final tuning
• complete detailed documentation
• some initial test code (to validate part of the functionalities)
• final merge to current piano roll visualizer PR (just realized it's still work in process)
• the ultimate PR

Next to that I'll keep on doing formerly detailed "nice to have" features.

[rogerpasky]

Pull request done! I'm pretty proud of the result, because it has ended up being functionally more complete than I initially considered for the MVP. It was not as easy as I thought, but effort has been worthy. I hope you like it.

[rogerpasky]

Just a friendly reminder about the Pull Request regarding this very issue @notwaldorf & @adarob

Even though it copes with all requirements, offering a pretty versatile score experience, I'm planning to cover full functionality around summer with note beams (aesthetical enhancement, not functional, thanks to flagged notes current coverage) and triplets + quintuplets management (this is an actual functional enhancement, but not a dramatic one to all scores). I just want to spend some time using mm.StaffSVGVisualizator with actual Magenta AI code of mine to experiment / verify its usage in the wild.

[notwaldorf]

This was merged, so closing this! 🎉🎉🎉