-
Notifications
You must be signed in to change notification settings - Fork 662
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Design issue: Notes in a voice cannot be placed on different staves (using System) #1373
Comments
The problem is https://jsfiddle.net/exqncky4/2/ Reference in http://lilypond.org/doc/v2.20/input/regression/musicxml/collated-files.html |
@rvilarl I went over the PR 1409 changes. I have a question and a suggestion:
addVoiceToPart(voices: Voice[], part: number) {
const ctx = this.getContext();
voices.forEach((voice) => {
voice.setContext(ctx);
});
this.parts[part].voices = this.parts[part].voices.concat(voices);
}
format(): void {
...
// Update the start position of all staves.
this.parts.forEach((part) => {
part.stave.setNoteStartX(startX);
part.voices.forEach((voice) => {
voice.getTickables().forEach((tickable) => {
const stave = tickable.getStave() ?? part.stave;
tickable.setStave(stave);
});
});
}); and one small change to formatter.ts, since some of the parts in system may have an empty voice: joinVoices(voices: Voice[]): this {
if (voices.length < 1) {
return this;
}
...
} Also, we've kind of been stymied by this code at the end of system with the bounding box. It doesn't see that it is used anywhere, and it was never correct. How about if we just remove it? I did and didn't see that it affected anything. It is not even the bounding box of the system, since that would have to include the stems and beams etc. // this.boundingBox = new BoundingBox(this.startX, this.lastY, justifyWidth, this.lastY - this.options.y);
Stave.formatBegModifiers(allStaves);
} Making just these changes, I was able to get the same output we had from the changes in the PR. Let me know what you think if this. |
You are right this a bug that I found during the change and it was not a good idea to include it. I have already started with the new PR. Issue #1427. Thanks for your feedback on the test! I will make a PR now with the formatter changes to resolve the issue.
This is right, it is possible to do cross staving without using System, but I want to make System cross stave aware in order to avoid special cases. I am using System in the XML Test Suite compliance check that I am building with @jaredjj3 in https://github.com/stringsync/vexml. There are also changes to allow the usage of System without specifying the width. I will make another PR for this one.
I will get back to you with my thoughts. |
This is not allowed by VexFlow and it seems that the solution is to make two voices out of one adding GhostNotes. However this is an impediment to have slurs or beams between staves (see #434)
I am collaborating with @jaredjj3 on https://github.com/stringsync/vexml. This is the application that I am using to produce the jsfiddle examples in the OSMD issues from musicxml files.
There I am facing currently this design issue with test_clef_end_measure.xml from OSMD
@AaronDavidNewman once vexml is more mature we will be able to generate real scores for testing. As you commented once it would be good to have more real score testing. Or as @mscuthbert did use http://lilypond.org/doc/v2.20/input/regression/musicxml/collated-files.html
@0xfe Do we want to make a design change to allow notes from one voice on different staves?
The text was updated successfully, but these errors were encountered: