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 #66241: note offset not preserved for tab #2089

Merged
merged 1 commit into from
Jun 22, 2015

Conversation

MarcSabatella
Copy link
Contributor

I believe I broke this here:

fbc6761

Formerly, Note::layout2() called adjustReadPos(). I needed to move that earlier for pitched staff calculations, but in doing so, I neglected to handle the tab case. For all I know it could benefit from being moved earlier for tab as well, but simply restoring that call here for tab fixes the issue at hand.

@mgavioli : does this make sense to you? Sorry for once again failing to consider tab in one of my layout changes, even if it was over a year ago when I was young and naive :-)

@MarcSabatella MarcSabatella changed the title fix ##66241: note offset not preserved for tab fix #66241: note offset not preserved for tab Jun 22, 2015
@mgavioli
Copy link
Contributor

I assume this specific change (back) leave things not worse than they were before the incriminated commit fbc6761 , which is enough for me: am I right that nobody noticed the bug for a year or more? If so, it does not look a very frequent case.

About anticipating the call to adjustReadPos(), I do not have the code at hand right now; does Score::layoutChords3() deal with tabs too? If so, it might make sense to move ti there.

@MarcSabatella
Copy link
Contributor Author

Indeed, this sets thing back to how they were a year ago for tab, which seems sensible to me. But I didn't know if you had changed things enough since then that you would suggest an alrenative.

layoutChords3() is never called for tab. It is called within layoutChords1(), but layoutChords1() returns immediately for tab without having done anything. This is the function that handles things like offsets for seconds and overlapping voices, none of which is relevant for tab. It did occur to me I could, instead of returning immediately for tab, loop through and find all chord and all notes within those chords (including grace notes) and force the Note::layout1() there, but when I saw the comment I left within Note::layout2(), I figured why not just restore how it worked before for tab.

lasconic added a commit that referenced this pull request Jun 22, 2015
fix #66241: note offset not preserved for tab
@lasconic lasconic merged commit 0e69fa6 into musescore:master Jun 22, 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

3 participants