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 #166231: Ambitus gets reported an octave to high #2976

Merged
merged 1 commit into from Feb 1, 2017

Conversation

Jojo-Schmitz
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz commented Jan 30, 2017

in Inspector, status bar and screen reader.
Also getting rid of superfluos backslashes and an unneeded code duplication

in Inspector, status bar and screen reader.
Also getting rid of superfluos backslashes and an unneded code
duplication
@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Jan 30, 2017

Should go to 2.1 and master, regardless that in master Ambitus currently doesn't work at all

int topOctave() const { return _topPitch / 12;}
int bottomOctave() const { return _bottomPitch / 12;}
int topOctave() const { return (_topPitch / 12) - 1; }
int bottomOctave() const { return (_bottomPitch / 12) - 1; }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

using the same logic here as in libmscore/note.cpp, line 768

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is a chance to add a “pitch to octave” macro (or static inline function) in a common header, and use that both in ambitus.h and note.cpp?

Otherwise, looks good.

@lasconic lasconic merged commit 7f3b0b9 into musescore:master Feb 1, 2017
@Jojo-Schmitz Jojo-Schmitz deleted the ambitus branch February 1, 2017 13:29
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