Allow staff visualizers to be split into multiple lines

Cheezer1656 commented 3 months ago

Added a line parameter to the midi-visualizer element that determines how many lines the notes are split into.

Also updated some packages to fix some vulnerabilities.

cifkao commented 2 months ago

Looks like a cool contribution. Do you have a demo I could look at?

Also, please don't update yarn.lock/package-lock.json in the same PR. These are long diffs and it's hard to see what's going on there. I would prefer if you could make an issue listing what needs to be updated and why, and I can take care of it.

Cheezer1656 commented 2 months ago

Sure! I removed the yarn.lock and package.lock changes and added an example of the multi-lined visualizer to the demo (index.html). Is this ok?

cifkao commented 1 month ago

Sorry, it took me a while to get around to this! Thanks for the demo, I now see better what this is doing.

I agree it would be useful to have the staff visualizer split into multiple lines. However, I see a couple of issues with the approach:

Splitting evenly by notes (in the middle of a bar) seems inadequate:
In sheet music, breaks should always occur at downbeats.
I guess the reason why we would want this feature is for the score to fit on screen without having to scroll horizontally. So ideally it should take in a desired maximum width or just fit the container responsively, rather than being based on a specified number of lines (which would require trial and error on the user part to get it right, and it would work only for one piece of music, as can be seen from the demo). (Another possibility would be to specify the number of measures per line, but even that isn't ideal as measures within one piece can have different widths.)
The functionality makes sense for the staff visualizer, but not so much for the other visualizer types (especially the waterfall one). For this reason, I'm not a fan of having this as a property on all visualizers.

In short, it seems intricate to do this right and it would require some interaction with the actual score rendering logic, which is not accessible from here. Therefore I think it would be preferable to add this feature directly to staffrender, which is what is used to render the scores. I think it's better to keep the visualizers here as thin wrappers around the ones defined in @magenta/music (unless we fully implement a new visualizer type here, which would also be fine).

cifkao / html-midi-player

Allow staff visualizers to be split into multiple lines #79