Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix unnecessary shift #1372

Merged
merged 1 commit into from
Apr 11, 2022
Merged

fix unnecessary shift #1372

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/stavenote.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ export class StaveNote extends StemmableNote {
}
}
} //Very close whole notes
} else {
} else if (lineDiff < 1) {
xShift = voiceXShift + 2;
if (noteU.stemDirection === noteL.stemDirection) {
// upper voice is middle voice, so shift it right
Expand All @@ -259,6 +259,12 @@ export class StaveNote extends StemmableNote {
// shift lower voice right
noteL.note.setXShift(xShift);
}
} else if (noteU.note.hasStem()) {
noteU.stemDirection = -noteU.note.getStemDirection();
noteU.note.setStemDirection(noteU.stemDirection);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Rodrigo. We should only change the stem direction in controlled situations. Can we move this into the if (voices != 2) branch? (otherwise we'll have weird issues with stems changing direction in other situations.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Mohit, this is already, I believe, in a very controlled situation. All these conditions have to be fulfilled:

  • there are two voices
  • only one of the notes has stem (the other one is a whole)
  • they are not in the same line
  • they overlap (the stem overlaps with the whole)

There are no other visual differences in all the tests (this is the only one where the change of stem direction was applied)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Rodrigo -- I didn't realize that this was inside the voices === 2 branch.

} else if (noteL.note.hasStem()) {
noteL.stemDirection = -noteL.note.getStemDirection();
noteL.note.setStemDirection(noteL.stemDirection);
}
}
}
Expand Down