0xfe / vexflow

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

System and Factory don't really work together #519

Closed Silverwolf90 closed 1 year ago

Silverwolf90 commented 7 years ago

I think we need to rethink the System class. It's pretty awkward to use. And doesn't really flow with the setup-stave-by-stave expectation of the Factory.

Let's take a look at this test:

var vf = VF.Test.makeFactory(options, 600, 350);
var score = vf.EasyScore();
var system = vf.System();

var voice = score.voice.bind(score);
var notes = score.notes.bind(score);

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

system.addStave({
  voices: [voice(notes('c#3/q, cn3/q, bb3/q, d##3/q', { clef: 'bass' }))],
}).addClef('bass');

system.addConnector().setType(VF.StaveConnector.type.BRACKET);

vf.draw();

If we look at the bass notes, notice that they will initially have their stave set to the treble Stave (because the bass stave hasn't been constructed by the Factory yet). So for the majority of the setup, the notes have the wrong stave set which only gets replaced at the very end when Factory#draw is called (because it calls System#format -- which is also really awkward, why is a drawing function calling a formatting function).

Silverwolf90 commented 7 years ago

Giving this more thought... a big issue is that the System shares responsibilities with the Factory. It constructs, does setup, formats, and draws. It doesn't even directly represent a graphical object. It's just a grouping+formatting mechanism; both of which should probably be represented by something more generic. Do the staves need a different formatting lifecycle than the other objects? Can we simplify by using our existing formatting lifecycle and interfaces?

Additionally, the circularity of using the Factory to create a System, but then a System holds a reference to a Factory so that it can create Staves as well -- it feels pretty wrong.

0xfe commented 7 years ago

All your points are valid, however I prefer to think of System as a prototype for a higher level helper API that should possibly be out of VexFlow. The implementation is warty because VexFlow itself is kinda clunky -- but I'd argue that the System API itself (including the code you pasted), is quite nice. If we can agree on the API, then we can work on improving the implementation (which probably also involves improving the underlying VexFlow APIs.)

Re: circular reference, I agree, but I don't think it's a problem, since it can be easily managed. Circularity is inevitable in complex systems, and so long as it is understood and managed, it can provide a lot of value. In this particular case, the circularity reduces the duplication of configuring System with the basic environment (canvas context, registry, etc.) This is, IMO, desirable from a user's perspective, even if it means more complexity for us.

Silverwolf90 commented 7 years ago

Yeah I see your point. When you frame it like that it makes more sense. But it further dilutes "the essence" of VexFlow for me. VexFlow does lots of things, and doesn't do most of them particularly well :)

So, what is VexFlow trying to be?

It seems like it's a low-level rendering framework. But sometimes has really high level stuff. Sometimes it's opinionated, sometimes it's not. It sometimes enforces notational rules and sometimes doesn't. It seems like it could work really well as a rendering layer. But then has some classes like System, EasyScore which seem tailored for writing out VexFlow by hand -- surely an atypical use case?

I guess this all relates to the clunkiness you mentioned. But in order to make sure this doesn't get worse, we should consider how we want VexFlow to grow.

Long term, have you considered breaking VexFlow up into a suite of packages, where scope is more clearly defined? Off the top of my head this seems to have a lot of advantages:

(This could entail significant redesign. For example, in my ideal world, formatting and rendering are not colocated)

0xfe commented 7 years ago

Thanks for starting this discussion. Right, so instead of asking the question, "what is VexFlow trying to be?", let's go with, "what should VexFlow be?" I think this should be (mostly) driven by the users, and based on my conversations the biggest complaints right now are, "it's too complicated", and "there's not enough documentation."

As a result, most of my recent work on VexFlow have been very user-focused (System, EasyScore, Factory, etc.) with the broader goal of hiding VexFlow's complexity behind simple APIs (which are constantly evolving.) This vastly lowers the barrier to entry for new users, for writing tests, etc. and also allows us to experiment with simpler APIs. It's also way more pleasant to use (IMO.)

Drawing the line between simplicity and flexibility is extraordinarily hard, mainly because the huge variety of ways VexFlow is used (musicxml, editing, realtime updates, etc.), and historically we've been far on the flexibility side of things.

I'm open to breaking up VexFlow into a suite, however it would be a lot of effort (as you pointed out), and probably add even more complexity to the system. If we had a much bigger team of active contributors, then this would be more attractive.

If I had to focus on a small set of goals for 2017, I'd say:

1) Vastly simplify the API for the common case. 2) Use the latest notation rendering techniques for things like formatting, collision detection, etc. 3) Add comprehensive documentation, and find a sustainable way to maintain it. 4) Have an VexFlow event.

Also, happy new year, Cyril! :-)

Silverwolf90 commented 7 years ago

We're definitely on the same page! But I have a lot of comments. When people are saying "it's too complicated" what are they talking about specifically?

There are so many aspects to VexFlow that a general "it's too complicated" is a bit too vague.

  1. Vastly simplify the API for the common case.

I completely agree about simplification. But, similar to above, I have no idea what the "common case" is. This is why I opened https://github.com/0xfe/vexflow/issues/520 -- to get a better idea around how VexFlow is being used. What do you see as the common case?

  1. Use the latest notation rendering techniques for things like formatting, collision detection, etc.

I'm not sure what "latest notation rendering techniques" means. Like the anchor points in SMuFL?

  1. Add comprehensive documentation, and find a sustainable way to maintain it.

Definitely agree here, I opened up an issue about an API reference a few days ago: https://github.com/0xfe/vexflow/issues/513

  1. Have an VexFlow event.

:+1::+1::+1::+1:

Also, happy new year to you too! :)

0xfe commented 7 years ago

We're definitely on the same page! But I have a lot of comments. When people are saying "it's too complicated" what are they talking about specifically?

The most basic stuff. Creating notes, adding accidentals and dots, justifying and rendering them onto a stave. If you look at the original VexFlow tutorial, there's a whole lot going on -- The API requires them to learn about VexFlow's internal model, which is mostly unnecessary.

I think EasyScore and System takes care of a lot of these cases, but obviously they need to be refined.

Most users (whom I've talked to) are building apps (typically phone, but also web) for education, and want to render short snippets of dynamically generated music. Very few are rendering complex scores.

  • Is is the lifecycle of the objects?
  • Is it the deviations between the different objects and their APIs?
  • Is it the sheer amount of objects?
  • Is it the semantics (eg: the awkwardness of Stave)?
  • All of the above?

I've not heard people complain about these things a lot, but there's probably a bit of all going on -- I suspect users either don't get this far, or by the time they get here have figured out how to deal with it.

Thanks for opening #520. Perhaps solicit more input from vexflow-users too, by sending them to #520?

0xfe commented 7 years ago

Also, by "latest rendering techniques", I really mean the algorithms we use for formatting. #444 is an incremental (and kinda experimental) step in that direction, setting up the infrastructure for more advanced formatting algorithms. I wish there were a good numerical computing library for JavaScript -- that would significantly reduce the work required.

You bring up a good point about SMuFL -- YES! When can I have it?! :-)

Silverwolf90 commented 7 years ago

We can have SMuFL once we overhaul formatting :P Not sure there's much point in moving to SMuFL until we can effectively use the metadata.

Most users (whom I've talked to) are building apps (typically phone, but also web) for education, and want to render short snippets of dynamically generated music. Very few are rendering complex scores.

Does that mean that dynamically rendering short snippets is the use case you want VexFlow optimized for?

0xfe commented 7 years ago

Does that mean that dynamically rendering short snippets is the use case you want VexFlow optimized for?

Um... I wouldn't go that far. Lets separate the common case from the target case:

I want to make the common case easy, and the target case possible. I think we're mostly there towards making the common case easy (with EasyScore etc.) The only remaining gap (I think) is a Page class that can render multiple Systems with numbering, titles, instrument names, etc. (modulo the incremental improvements to the existing classes.)

For the target case, we have a long way to go -- we should probably come up with a list of prioritized features towards this.

Silverwolf90 commented 7 years ago

While "short snippets" is relatively easy to define, "complex scores" has a basically-infinite scope and needs to be defined. It's impossible to define this "clearly" but we need an intuition around it. What do you consider a reasonable or unreasonable level of complexity?

I'll still contend that the most important thing for VexFlow at the moment is to continue to refactor/redesign the low-level internals. Until that kind of code "feels really good" to read/write/use we don't have a foundation to scale efficiently given the time we are able to put into development. IMO i think VexFlow has already scaled too far given it's issues. Until these kind of problems can be solved quickly (as they should be), I don't see it possible to scale VexFlow's complexity. Funny how I wouldn't have said that a few years ago :)

0xfe commented 7 years ago

Yes, it's infinite scope :-) -- my point was that rendering short snippets is not what I want VexFlow to be optimized for. The way I'd like to approach complexity is to pick a simple but complete score target and render it in VexFlow, adding features, fixing warts, etc. as they come up. tests/bach_tests.js is an example target.

After the we reach the target, pick a further one, rinse and repeat. IMO, this is a pragmatic way to move forward, but I'm open to other approaches.

I don't disagree with refactoring -- I just think of it as a continuous process. Re: redesign, we need to be careful to do it incrementally. If not, we need a really good plan on how we will avoid any kind of second system effect. (I've had this happen too many times and have become very resistant to complete redesigns without committed resources and a credible plan.) Again, I'm not saying that we shouldn't do this -- I'm saying we shouldn't start without clarity of vision and process.

Silverwolf90 commented 7 years ago

The way I'd like to approach complexity is to pick a simple but complete score target and render it in VexFlow, adding features, fixing warts, etc. as they come up. tests/bach_tests.js is an example target.

I think the full-score tests are good as a sort of "integration" test. But they shouldn't drive development.

This is how we grew the notation engine (we actually used batches of pieces at once) for our first app at Tido and it didn't go very well. We thought that the incremental difficulty per batch meant the complexity of the notation would scale in a relatively manageable manner. So we went through it level by level, catalogued features/enhancements that each batch would require. It appeared to be a reasonable progression, but in hindsight it really didn't work out. Without rigorously considering specific paths for implementing the harder stuff, our designs were too constrained to handle them.

So based on that experience, I'd suggest that we go object-by-object and consider scope very carefully with better test cases, otherwise we'll end up with rigid, unusable implementations that fall apart in the face additional complexity. Because over hundreds of years people have been mapping musical semantics to graphics in seemingly arbitrary (and certainly frustrating) ways :P

Not that every case ever needs to be supported right off the bat, but we need to be aware of what cases we will and won't support. And we need to make sure that the cases we intend to support in the long-term to support have clear paths for implementation, even if they're not supported in the short-term.

I don't disagree with refactoring -- I just think of it as a continuous process. Re: redesign, we need to be careful to do it incrementally. If not, we need a really good plan on how we will avoid any kind of second system effect. (I've had this happen too many times and have become very resistant to complete redesigns without committed resources and a credible plan.) Again, I'm not saying that we shouldn't do this -- I'm saying we shouldn't start without clarity of vision and process.

I agree that doing it incrementally is ideal. I don't think we have the capacity to do a ground-up redesign. I'm hoping that as we work on the foundation and docs that we'll get more activity.

0xfe commented 7 years ago

Okay cool, I'm open to trying other approaches too. I suspect it would be some combination of the both anyway, and I think scoping really is the hard part.

Silverwolf90 commented 7 years ago

Yeah, and "full score" tests are the only way to get a feel of how things look/feel when being used in real situations. And sometimes they highlight unconsidered cases, but that's not ideal. I definitely think they're incredibly valuable and the more the better! But they're not enough to get an idea of how a piece of notation is used. I have a decent amount of experience in this and have probably seen a lot of the 'most-common uncommon' cases. But there are lots of resources too, although some might require some digging:

mscuthbert commented 7 years ago

The Unofficial MusicXML test suite used by Lilypond is a good set of things to test for Vexflow's support for various notational features (that are also supported in MusicXML).

http://web.mit.edu/music21/doc/usersGuide/usersGuide_90_musicxmlTest.html or http://lilypond.org/doc/v2.18/input/regression/musicxml/collated-files.html

0xfe commented 7 years ago

All of these look like great test suites -- it would be great if we could piggyback of of these.

rvilarl commented 3 years ago

@0xfe I am using MusicXML in #1073 it would be interesting how to use EasyScore to go through these tests. I have downloaded the set already. Are you aware how many users are working with EasyScore?

0xfe commented 3 years ago

Note that EasyScore alone is not fully equipped for this (and isn't trying to be.) The intent is for it (along with System and Factory) to simplify the mechanics of generating Vexflow objects, which can then be fine tuned.

See bach_tests for an example.

rvilarl commented 2 years ago

@0xfe the modification in System is also related to this. I am using System as you suggested and I had a problem coming to Voices crossing Staves.