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

Prevent adding beam properties to rests #665

Closed

Conversation

MarkRS-UK
Copy link
Contributor

or trying to layout a rest with a beam property.

@MarkRS-UK
Copy link
Contributor Author

That Travis error doesn't look like anything to do with us.
How do I/we resubmit the job to Travis?

@mgavioli
Copy link
Contributor

That specific Travis failure is not related with the PR, but the build testing is currently failing anyway, since merging PR 661: PR page #661 - Travis report https://travis-ci.org/musescore/MuseScore/builds/17537479

@mgavioli
Copy link
Contributor

Building of master seems fixed now...

@wschweer
Copy link
Contributor

Whats the problem with beam properties on a rest? Actually rests have beam properties.

@Jojo-Schmitz
Copy link
Contributor

This is supposed to fix #24412, http://musescore.org/en/node/24412

@MarkRS-UK
Copy link
Contributor Author

The only problem was my lack of understanding.
However, I think the patch is valid.

@@ -331,6 +331,8 @@ void Beam::layout1()
_up = _direction == MScore::UP;
}
else {
if (!c1)
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I understand the code, this change seems OK to me: if the beamed element is a rest, there is no stem, and computing its stem length makes no sense. I would even move the addition right before line 330, skipping the stem length computation regardless of _direction value.

@Jojo-Schmitz
Copy link
Contributor

http://musescore.org/en/node/24412 has been closed as being fixed in 51d8cbd

@lasconic lasconic closed this Apr 25, 2014
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

5 participants