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.

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

Fix MusicXML diminuendo <wedge> spread handling #3446

Merged
merged 1 commit into from
Jun 8, 2023

Conversation

paxbun
Copy link
Contributor

@paxbun paxbun commented Jun 7, 2023

The MusicXML standard specifies that the spread attribute at the start of a crescendo and the end of a diminuendo is ignored. However, the current code only reads spread at the end of a <wedge> pair, which means it handles the spread of diminuendos wrongly.

This fix changes the behavior of MusicXmlInput so it sets the opening of a hairpin at the end for diminuendos or the start for crescendos. The followings show the result of this fix. Test MusicXML files are made by modifying <wedge> elements in BeetAnGeSample.musicxml from the example set on musicxml.com.

  • test-narrow-wedge.musicxml (Made by replacing spread="15" with spread="5")
Before fix After fix
test-narrow-wedge before test-narrow-wedge after
  • test-spread-0-at-dim-stop.musicxml (Made by adding spread="0" to the end <wedge> of diminuendos)
Before fix After fix
test-spread-0-at-dim-stop before test-spread-0-at-dim-stop after

Made MusicXmlInput ignore <wedge> spread at the start of a crescendo or
or the end of a diminuendo.
@paxbun paxbun changed the title Fix MusicXML <wedge> spread handling Fix MusicXML diminuendo <wedge> spread handling Jun 7, 2023
@paxbun
Copy link
Contributor Author

paxbun commented Jun 7, 2023

BTW, I've also just found that niente must be handled similarly (ignored at the end of a crescendo and the start of a diminuendo, which is the opposite of spread), but I've never seen this kind of notation, and I don't have any MusicXML file containing this. I don't know if making a similar change for this is okay.

@lpugin lpugin merged commit 4f1a85b into rism-digital:develop Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants