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 #57646: corruption on paste with segment annotation in voice > 0 #1971

Conversation

MarcSabatella
Copy link
Contributor

This definitely fixes the bug, as well as another where the pasted annotation moves to voice 1. I don't think this breaks anything else, but it would be good to have someone else think through this (see my notes in the issue report) to be sure I am not missing something.

@@ -324,9 +324,10 @@ bool Score::pasteStaff(XmlReader& e, Segment* dst, int dstStaff)

// be sure to paste the element in the destination track;
// setting track needs to be repeated, as it might have been overwritten by el->read()
el->setTrack(e.track());
// preserve *voice* from source, though
el->setTrack(e.track() + el->voice());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a chance e.track() would already have a voice offset added in? It was always the first track of the staff in my tests. I suppose safer might be to convert e.track() to staffIdx and then back to track before adding the voice? The original code here did in fact use staffIdx. That was changed here:

5ed2bc8#diff-7d7e4132f003b8764c359cfabafdf6beL303

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get all the ins and outs just yet but it feels definitely better and more understandable to use dstStaffIdx * VOICES + voice instead of relying to track(). Even if it's set at the start of the loop to dstStaffIdx * VOICES

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm inclined to agree, but I figured the change was made for a reason. I could also see taking (e.track() / VOICES) * VOICES. @wschweer, is there a reason to prefer one of these?

@lasconic
Copy link
Contributor

See 210d567

@lasconic lasconic closed this Apr 29, 2015
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.

None yet

2 participants